Implement GetSubjectAltName on all platforms

BUG=None
TEST=net_unittests

Review URL: http://codereview.chromium.org/7354017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@93049 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 147 insertions, 141 deletions

diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index 76fa4dc..5ac75b3 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -207,14 +207,14 @@ void SplitOnChar(const base::StringPiece& src,
                  char c,
                  base::StringPiece* left,
                  base::StringPiece* right) {
-   size_t pos = src.find(c);
-   if (pos == base::StringPiece::npos) {
-     *left = src;
-     right->clear();
-   } else {
-     *left = src.substr(0, pos);
-     *right = src.substr(pos);
-   }
+  size_t pos = src.find(c);
+  if (pos == base::StringPiece::npos) {
+    *left = src;
+    right->clear();
+  } else {
+    *left = src.substr(0, pos);
+    *right = src.substr(pos);
+  }
 }
 
 }  // namespace
@@ -420,6 +420,12 @@ void X509Certificate::Persist(Pickle* pickle) {
   }
 }
 
+void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
+  GetSubjectAltName(dns_names, NULL);
+  if (dns_names->empty())
+    dns_names->push_back(subject_.common_name);
+}
+
 bool X509Certificate::HasExpired() const {
   return base::Time::Now() > valid_expiry();
 }
@@ -507,6 +513,10 @@ bool X509Certificate::VerifyHostname(
   std::vector<std::string> common_name_as_vector;
   const std::vector<std::string>* presented_names = &cert_san_dns_names;
   if (common_name_fallback) {
+    // Note: there's a small possibility cert_common_name is an international
+    // domain name in non-standard encoding (e.g. UTF8String or BMPString
+    // instead of A-label). As common name fallback is deprecated we're not
+    // doing anything specific to deal with this.
     common_name_as_vector.push_back(cert_common_name);
     presented_names = &common_name_as_vector;
   }
@@ -577,10 +587,8 @@ int X509Certificate::Verify(const std::string& hostname, int flags,
 
 #if !defined(USE_NSS)
 bool X509Certificate::VerifyNameMatch(const std::string& hostname) const {
-  // TODO(joth): Define a cross platform version of ParseSubjectAltName and use
-  // that to retrieve dns names and ip addresses, independent of common name.
   std::vector<std::string> dns_names, ip_addrs;
-  GetDNSNames(&dns_names);
+  GetSubjectAltName(&dns_names, &ip_addrs);
   return VerifyHostname(hostname, subject_.common_name, dns_names, ip_addrs);
 }
 #endif
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 08aee59..b02ede6 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -215,6 +215,13 @@ class NET_API X509Certificate
   // Otherwise, it gets the common name in the subject field.
   void GetDNSNames(std::vector<std::string>* dns_names) const;
 
+  // Gets the subjectAltName extension field from the certificate, if any.
+  // For future extension; currently this only returns those name types that
+  // are required for HTTP certificate name verification - see VerifyHostname.
+  // Unrequired parameters may be passed as NULL.
+  void GetSubjectAltName(std::vector<std::string>* dns_names,
+                         std::vector<std::string>* ip_addrs) const;
+
   // Convenience method that returns whether this certificate has expired as of
   // now.
   bool HasExpired() const;
@@ -313,10 +320,6 @@ class NET_API X509Certificate
   // Does not verify that the certificate is valid, only that the certificate
   // matches this host.
   // Returns true if it matches.
-  //
-  // WARNING:  This function may return false negatives (for example, if
-  //           |hostname| is an IP address literal) on some platforms.  Only
-  //           use in cases where some false-negatives are acceptable.
   bool VerifyNameMatch(const std::string& hostname) const;
 
   // This method returns the DER encoded certificate.
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 1edcedd..3283b5f 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -176,47 +176,6 @@ OSStatus GetCertFields(X509Certificate::OSCertHandle cert_handle,
   return status;
 }
 
-void GetCertGeneralNamesForOID(X509Certificate::OSCertHandle cert_handle,
-                               CSSM_OID oid, CE_GeneralNameType name_type,
-                               std::vector<std::string>* result) {
-  // For future extension: We only support general names of types
-  // GNT_RFC822Name, GNT_DNSName or GNT_URI.
-  DCHECK(name_type == GNT_RFC822Name ||
-         name_type == GNT_DNSName ||
-         name_type == GNT_URI);
-
-  CSSMFields fields;
-  OSStatus status = GetCertFields(cert_handle, &fields);
-  if (status)
-    return;
-
-  for (size_t field = 0; field < fields.num_of_fields; ++field) {
-    if (CSSMOIDEqual(&fields.fields[field].FieldOid, &oid)) {
-      CSSM_X509_EXTENSION_PTR cssm_ext =
-          reinterpret_cast<CSSM_X509_EXTENSION_PTR>(
-              fields.fields[field].FieldValue.Data);
-      CE_GeneralNames* alt_name =
-          reinterpret_cast<CE_GeneralNames*>(cssm_ext->value.parsedValue);
-
-      for (size_t name = 0; name < alt_name->numNames; ++name) {
-        const CE_GeneralName& name_struct = alt_name->generalName[name];
-        // All of the general name types we support are encoded as
-        // IA5String. In general, we should be switching off
-        // |name_struct.nameType| and doing type-appropriate conversions. See
-        // certextensions.h and the comment immediately preceding
-        // CE_GeneralNameType for more information.
-        if (name_struct.nameType == name_type) {
-          const CSSM_DATA& name_data = name_struct.name;
-          std::string value = std::string(
-              reinterpret_cast<const char*>(name_data.Data),
-              name_data.Length);
-          result->push_back(value);
-        }
-      }
-    }
-  }
-}
-
 void GetCertDateForOID(X509Certificate::OSCertHandle cert_handle,
                        CSSM_OID oid, Time* result) {
   *result = Time::Time();
@@ -546,7 +505,7 @@ class ScopedEncodedCertResults {
     crypto::CSSMFree(results_);
   }
 
-private:
+ private:
   CSSM_TP_RESULT_SET* results_;
 };
 
@@ -644,7 +603,7 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   // Convert the map of oid/string pairs into an array of
   // CSSM_APPLE_TP_NAME_OIDs.
   std::vector<CSSM_APPLE_TP_NAME_OID> cssm_subject_names;
-  for(CSSMOIDStringVector::iterator iter = subject_name_oids.begin();
+  for (CSSMOIDStringVector::iterator iter = subject_name_oids.begin();
       iter != subject_name_oids.end(); ++iter) {
     CSSM_APPLE_TP_NAME_OID cssm_subject_name;
     cssm_subject_name.oid = iter->oid_;
@@ -666,7 +625,7 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   certReq.serialNumber = serial_number & 0x7fffffff;
   certReq.numSubjectNames = cssm_subject_names.size();
   certReq.subjectNames = &cssm_subject_names[0];
-  certReq.numIssuerNames = 0; // Root.
+  certReq.numIssuerNames = 0;  // Root.
   certReq.issuerNames = NULL;
   certReq.issuerNameX509 = NULL;
   certReq.certPublicKey = key->public_key();
@@ -700,7 +659,7 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   CSSM_RETURN crtn = CSSM_TP_SubmitCredRequest(tp_handle, NULL,
       CSSM_TP_AUTHORITY_REQUEST_CERTISSUE, &reqSet, &callerAuthContext,
        &estTime, &refId);
-  if(crtn) {
+  if (crtn) {
     DLOG(ERROR) << "CSSM_TP_SubmitCredRequest failed " << crtn;
     return NULL;
   }
@@ -745,14 +704,46 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return CreateFromHandle(scoped_cert, X509Certificate::OSCertHandles());
 }
 
-void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
-  dns_names->clear();
+void X509Certificate::GetSubjectAltName(
+    std::vector<std::string>* dns_names,
+    std::vector<std::string>* ip_addrs) const {
+  if (dns_names)
+    dns_names->clear();
+  if (ip_addrs)
+    ip_addrs->clear();
+
+  CSSMFields fields;
+  OSStatus status = GetCertFields(cert_handle_, &fields);
+  if (status)
+    return;
 
-  GetCertGeneralNamesForOID(cert_handle_, CSSMOID_SubjectAltName, GNT_DNSName,
-                            dns_names);
+  for (size_t field = 0; field < fields.num_of_fields; ++field) {
+    if (!CSSMOIDEqual(&fields.fields[field].FieldOid, &CSSMOID_SubjectAltName))
+      continue;
+    CSSM_X509_EXTENSION_PTR cssm_ext =
+        reinterpret_cast<CSSM_X509_EXTENSION_PTR>(
+            fields.fields[field].FieldValue.Data);
+    CE_GeneralNames* alt_name =
+        reinterpret_cast<CE_GeneralNames*>(cssm_ext->value.parsedValue);
+
+    for (size_t name = 0; name < alt_name->numNames; ++name) {
+      const CE_GeneralName& name_struct = alt_name->generalName[name];
+      const CSSM_DATA& name_data = name_struct.name;
+      // DNSName and IPAddress are encoded as IA5String and OCTET STRINGs
+      // respectively, both of which can be byte copied from
+      // CSSM_DATA::data into the appropriate output vector.
+      if (dns_names && name_struct.nameType == GNT_DNSName) {
+        dns_names->push_back(std::string(
+            reinterpret_cast<const char*>(name_data.Data),
+            name_data.Length));
+      } else if (ip_addrs && name_struct.nameType == GNT_IPAddress) {
+        ip_addrs->push_back(std::string(
+            reinterpret_cast<const char*>(name_data.Data),
+            name_data.Length));
 
-  if (dns_names->empty())
-    dns_names->push_back(subject_.common_name);
+      }
+    }
+  }
 }
 
 int X509Certificate::VerifyInternal(const std::string& hostname,
@@ -993,7 +984,7 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
 bool X509Certificate::GetDEREncoded(std::string* encoded) {
   encoded->clear();
   CSSM_DATA der_data;
-  if(SecCertificateGetData(cert_handle_, &der_data) == noErr) {
+  if (SecCertificateGetData(cert_handle_, &der_data) == noErr) {
     encoded->append(reinterpret_cast<char*>(der_data.Data),
                     der_data.Length);
     return true;
diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc
index db9f6a5..0162663 100644
--- a/net/base/x509_certificate_nss.cc
+++ b/net/base/x509_certificate_nss.cc
@@ -281,47 +281,6 @@ void ParseDate(SECItem* der_date, base::Time* result) {
   *result = crypto::PRTimeToBaseTime(prtime);
 }
 
-void GetCertSubjectAltNamesOfType(X509Certificate::OSCertHandle cert_handle,
-                                  CERTGeneralNameType name_type,
-                                  std::vector<std::string>* result) {
-  // For future extension: We only support general names of types
-  // RFC822Name, DNSName or URI.
-  DCHECK(name_type == certRFC822Name ||
-         name_type == certDNSName ||
-         name_type == certURI);
-
-  SECItem alt_name;
-  SECStatus rv = CERT_FindCertExtension(cert_handle,
-                                        SEC_OID_X509_SUBJECT_ALT_NAME,
-                                        &alt_name);
-  if (rv != SECSuccess)
-    return;
-
-  PRArenaPool* arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-  DCHECK(arena != NULL);
-
-  CERTGeneralName* alt_name_list;
-  alt_name_list = CERT_DecodeAltNameExtension(arena, &alt_name);
-  SECITEM_FreeItem(&alt_name, PR_FALSE);
-
-  CERTGeneralName* name = alt_name_list;
-  while (name) {
-    // All of the general name types we support are encoded as
-    // IA5String. In general, we should be switching off
-    // |name->type| and doing type-appropriate conversions.
-    if (name->type == name_type) {
-      unsigned char* p = name->name.other.data;
-      int len = name->name.other.len;
-      std::string value = std::string(reinterpret_cast<char*>(p), len);
-      result->push_back(value);
-    }
-    name = CERT_GetNextGeneralName(name);
-    if (name == alt_name_list)
-      break;
-  }
-  PORT_FreeArena(arena, PR_FALSE);
-}
-
 // Forward declarations.
 SECStatus RetryPKIXVerifyCertWithWorkarounds(
     X509Certificate::OSCertHandle cert_handle, int num_policy_oids,
@@ -755,14 +714,47 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return x509_cert;
 }
 
-void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
-  dns_names->clear();
+void X509Certificate::GetSubjectAltName(
+    std::vector<std::string>* dns_names,
+    std::vector<std::string>* ip_addrs) const {
+  if (dns_names)
+    dns_names->clear();
+  if (ip_addrs)
+    ip_addrs->clear();
+
+  SECItem alt_name;
+  SECStatus rv = CERT_FindCertExtension(cert_handle_,
+                                        SEC_OID_X509_SUBJECT_ALT_NAME,
+                                        &alt_name);
+  if (rv != SECSuccess)
+    return;
+
+  PRArenaPool* arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
+  DCHECK(arena != NULL);
 
-  // Compare with CERT_VerifyCertName().
-  GetCertSubjectAltNamesOfType(cert_handle_, certDNSName, dns_names);
+  CERTGeneralName* alt_name_list;
+  alt_name_list = CERT_DecodeAltNameExtension(arena, &alt_name);
+  SECITEM_FreeItem(&alt_name, PR_FALSE);
 
-  if (dns_names->empty())
-    dns_names->push_back(subject_.common_name);
+  CERTGeneralName* name = alt_name_list;
+  while (name) {
+    // DNSName and IPAddress are encoded as IA5String and OCTET STRINGs
+    // respectively, both of which can be byte copied from
+    // SECItemType::data into the appropriate output vector.
+    if (dns_names && name->type == certDNSName) {
+      dns_names->push_back(std::string(
+          reinterpret_cast<char*>(name->name.other.data),
+          name->name.other.len));
+    } else if (ip_addrs && name->type == certIPAddress) {
+      ip_addrs->push_back(std::string(
+          reinterpret_cast<char*>(name->name.other.data),
+          name->name.other.len));
+    }
+    name = CERT_GetNextGeneralName(name);
+    if (name == alt_name_list)
+      break;
+  }
+  PORT_FreeArena(arena, PR_FALSE);
 }
 
 int X509Certificate::VerifyInternal(const std::string& hostname,
diff --git a/net/base/x509_certificate_openssl.cc b/net/base/x509_certificate_openssl.cc
index 78ccfee..d3cad61 100644
--- a/net/base/x509_certificate_openssl.cc
+++ b/net/base/x509_certificate_openssl.cc
@@ -408,12 +408,15 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return NULL;
 }
 
-void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
-  dns_names->clear();
-
-  ParseSubjectAltName(cert_handle_, dns_names, NULL);
-  if (dns_names->empty())
-    dns_names->push_back(subject_.common_name);
+void X509Certificate::GetSubjectAltName(
+    std::vector<std::string>* dns_names,
+    std::vector<std::string>* ip_addrs) const {
+  if (dns_names)
+    dns_names->clear();
+  if (ip_addrs)
+    ip_addrs->clear();
+
+  ParseSubjectAltName(cert_handle_, dns_names, ip_addrs);
 }
 
 // static
@@ -424,10 +427,7 @@ X509_STORE* X509Certificate::cert_store() {
 int X509Certificate::VerifyInternal(const std::string& hostname,
                                     int flags,
                                     CertVerifyResult* verify_result) const {
-  std::vector<std::string> dns_names, ip_addresses;
-  ParseSubjectAltName(cert_handle_, &dns_names, &ip_addresses);
-
-  if (!VerifyHostname(hostname, subject_.common_name, dns_names, ip_addresses))
+  if (!VerifyNameMatch(hostname))
     verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
 
   crypto::ScopedOpenSSL<X509_STORE_CTX, X509_STORE_CTX_free> ctx(
@@ -541,4 +541,4 @@ bool X509Certificate::WriteOSCertHandleToPickle(OSCertHandle cert_handle,
       der_cache.data_length);
 }
 
-} // namespace net
+}  // namespace net
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 8442fdbc..57044d8 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -609,25 +609,37 @@ X509Certificate* X509Certificate::CreateSelfSigned(
   return cert;
 }
 
-void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
-  dns_names->clear();
-  if (cert_handle_) {
-    scoped_ptr_malloc<CERT_ALT_NAME_INFO> alt_name_info;
-    GetCertSubjectAltName(cert_handle_, &alt_name_info);
-    CERT_ALT_NAME_INFO* alt_name = alt_name_info.get();
-    if (alt_name) {
-      int num_entries = alt_name->cAltEntry;
-      for (int i = 0; i < num_entries; i++) {
-        // dNSName is an ASN.1 IA5String representing a string of ASCII
-        // characters, so we can use WideToASCII here.
-        if (alt_name->rgAltEntry[i].dwAltNameChoice == CERT_ALT_NAME_DNS_NAME)
-          dns_names->push_back(
-              WideToASCII(alt_name->rgAltEntry[i].pwszDNSName));
+void X509Certificate::GetSubjectAltName(
+    std::vector<std::string>* dns_names,
+    std::vector<std::string>* ip_addrs) const {
+  if (dns_names)
+    dns_names->clear();
+  if (ip_addrs)
+    ip_addrs->clear();
+
+  if (!cert_handle_)
+    return;
+
+  scoped_ptr_malloc<CERT_ALT_NAME_INFO> alt_name_info;
+  GetCertSubjectAltName(cert_handle_, &alt_name_info);
+  CERT_ALT_NAME_INFO* alt_name = alt_name_info.get();
+  if (alt_name) {
+    int num_entries = alt_name->cAltEntry;
+    for (int i = 0; i < num_entries; i++) {
+      // dNSName is an ASN.1 IA5String representing a string of ASCII
+      // characters, so we can use WideToASCII here.
+      const CERT_ALT_NAME_ENTRY& entry = alt_name->rgAltEntry[i];
+
+      if (dns_names && entry.dwAltNameChoice == CERT_ALT_NAME_DNS_NAME) {
+        dns_names->push_back(WideToASCII(entry.pwszDNSName));
+      } else if (ip_addrs &&
+                 entry.dwAltNameChoice == CERT_ALT_NAME_IP_ADDRESS) {
+        ip_addrs->push_back(std::string(
+            reinterpret_cast<const char*>(entry.IPAddress.pbData),
+            entry.IPAddress.cbData));
       }
     }
   }
-  if (dns_names->empty())
-    dns_names->push_back(subject_.common_name);
 }
 
 int X509Certificate::VerifyInternal(const std::string& hostname,