Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(217)

Unified Diff: net/socket/ssl_client_socket_nss.cc

Issue 11184027: net: add DANE support for DNSSEC stapled certificates. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: ... Created 8 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« net/base/dnssec_chain_verifier.cc ('K') | « net/base/dnssec_chain_verifier.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/socket/ssl_client_socket_nss.cc
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index dd3b13a16905338f08c50ec2005fa15f27bac941..acbb875b5af48fac374d8124e21cd09fc0b97396 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -278,6 +278,7 @@ enum DNSValidationResult {
// server_cert_nss: the server's leaf certificate.
// rrdatas: the CAA records for the current domain.
// port: the TCP port number that we connected to.
+// TODO(agl): remove once DANE support has been released.
DNSValidationResult VerifyCAARecords(
CERTCertificate* server_cert_nss,
const std::vector<base::StringPiece>& rrdatas,
@@ -323,6 +324,55 @@ DNSValidationResult VerifyCAARecords(
return DNSVR_FAILURE;
}
+// VerifyTLSARecords processes DNSSEC validated RRDATA for a number of DNS TLSA
+// records and checks them against the given chain.
+// server_cert_nss: the server's leaf certificate.
+// rrdatas: the TLSA records for the current domain.
+DNSValidationResult VerifyTLSARecords(
+ CERTCertificate* server_cert_nss,
+ const std::vector<base::StringPiece>& rrdatas) {
+ std::vector<DnsTLSARecord::Match> matches;
+ DnsTLSARecord::Parse(rrdatas, &matches);
Ryan Sleevi 2012/10/18 23:21:13 Should a certificate with invalid record types be
agl 2012/10/29 15:41:56 Invalid record types are future record types that
+
+ for (std::vector<DnsTLSARecord::Match>::const_iterator
+ i = matches.begin(); i != matches.end(); i++) {
Ryan Sleevi 2012/10/18 23:21:13 style nit: pre-increment http://google-styleguide
agl 2012/10/29 15:41:56 Done.
+ SECItem matched_data;
+ switch (i->target) {
+ case DnsTLSARecord::Match::CERTIFICATE:
+ matched_data = server_cert_nss->derCert;
+ break;
+ case DnsTLSARecord::Match::SUBJECT_PUBLIC_KEY_INFO:
+ matched_data = server_cert_nss->derPublicKey;
+ break;
+ default:
+ continue;
+ }
Ryan Sleevi 2012/10/18 23:21:13 style nit: indentation of cases
agl 2012/10/29 15:41:56 Done.
+
+ uint8 calculated_hash[SHA512_LENGTH]; // SHA512 is the largest.
Ryan Sleevi 2012/10/18 23:21:13 nit: HASH_LENGTH_MAX (comes from hasht.h, the same
agl 2012/10/29 15:41:56 Done.
+ SECItem processed_data;
+ if (i->algorithm == HASH_AlgNULL) {
+ processed_data = matched_data;
+ } else {
+ SECStatus rv = HASH_HashBuf(
+ static_cast<HASH_HashType>(i->algorithm),
Ryan Sleevi 2012/10/18 23:21:13 DESIGN: You seem to have tightly coupled the HASH_
agl 2012/10/29 15:41:56 The alternative would be Yet Another Hash Type enu
+ calculated_hash,
+ matched_data.data,
+ matched_data.len);
+ DCHECK(rv == SECSuccess);
Ryan Sleevi 2012/10/18 23:21:13 nit: DCHECK_EQ(SECSuccess, rv)
+ processed_data.data = calculated_hash;
+ processed_data.len = i->data.size();
+ }
+
+ if (processed_data.len == i->data.size() &&
+ memcmp(processed_data.data, i->data.data(), i->data.size()) == 0) {
+ return DNSVR_SUCCESS;
+ }
+ }
+
+ // No TLSA records matched so we reject the certificate.
+ return DNSVR_FAILURE;
+}
+
// CheckDNSSECChain tries to validate a DNSSEC chain embedded in
// |server_cert_nss|. It returns true iff a chain is found that proves the
// value of a CAA record that contains a valid public key fingerprint.
@@ -363,27 +413,50 @@ DNSValidationResult CheckDNSSECChain(
if (rv != SECSuccess)
return DNSVR_CONTINUE;
- base::StringPiece chain(
+ std::string chain(
reinterpret_cast<char*>(dnssec_embedded_chain.data),
dnssec_embedded_chain.len);
+ SECITEM_FreeItem(&dnssec_embedded_chain, PR_FALSE);
std::string dns_hostname;
if (!DNSDomainFromDot(hostname, &dns_hostname))
return DNSVR_CONTINUE;
+
DNSSECChainVerifier verifier(dns_hostname, chain);
DNSSECChainVerifier::Error err = verifier.Verify();
+ bool is_tlsa = false;
+ if (err == DNSSECChainVerifier::BAD_TARGET) {
+ // We might have failed because this is a DANE chain, not a CAA chain.
+ // DANE stores records in a subdomain of the name that includes the port
+ // and protocol, i.e. _443._tcp.www.example.com. We have to construct
+ // such a name for |hostname|.
+ const std::string port_str = base::UintToString(port);
+ char port_label_len = 1 + port_str.size();
Ryan Sleevi 2012/10/18 23:21:13 nit: I would expect a warning here from at least o
agl 2012/10/29 15:41:56 Added static_cast.
+ const std::string tlsa_domain =
+ std::string(&port_label_len, 1) + "_" + port_str +
+ "\x04_tcp" +
+ dns_hostname;
Ryan Sleevi 2012/10/18 23:21:13 world's tiniest nit: StringPrintf / StringAppend,
agl 2012/10/29 15:41:56 While I agree in general, since |dns_hostname| has
+
+ verifier = DNSSECChainVerifier(tlsa_domain, chain);
+ err = verifier.Verify();
+ is_tlsa = true;
+ }
+
if (err != DNSSECChainVerifier::OK) {
LOG(ERROR) << "DNSSEC chain verification failed: " << err;
return DNSVR_CONTINUE;
}
+ if (is_tlsa) {
+ if (verifier.rrtype() != kDNS_TLSA)
+ return DNSVR_CONTINUE;
+
+ return VerifyTLSARecords(server_cert_nss, verifier.rrdatas());
+ }
+
if (verifier.rrtype() != kDNS_CAA)
return DNSVR_CONTINUE;
- DNSValidationResult r = VerifyCAARecords(
- server_cert_nss, verifier.rrdatas(), port);
- SECITEM_FreeItem(&dnssec_embedded_chain, PR_FALSE);
-
- return r;
+ return VerifyCAARecords(server_cert_nss, verifier.rrdatas(), port);
}
void DestroyCertificates(CERTCertificate** certs, size_t len) {
« net/base/dnssec_chain_verifier.cc ('K') | « net/base/dnssec_chain_verifier.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698