Add a DevTools warning for a missing subjectAltName

net/cert/x509_certificate_openssl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/x509_certificate.h"
 
 #include "base/macros.h"
 #include "base/memory/singleton.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/pickle.h"
@@ skipping 75 matching lines @@
   x509_util::ParsePrincipalValueByNID(x509_name, NID_commonName,
                                       &principal->common_name);
   x509_util::ParsePrincipalValueByNID(x509_name, NID_localityName,
                                       &principal->locality_name);
   x509_util::ParsePrincipalValueByNID(x509_name, NID_stateOrProvinceName,
                                       &principal->state_or_province_name);
   x509_util::ParsePrincipalValueByNID(x509_name, NID_countryName,
                                       &principal->country_name);
 }
 
-void ParseSubjectAltName(X509Certificate::OSCertHandle cert,
+bool ParseSubjectAltName(X509Certificate::OSCertHandle cert,
                          std::vector<std::string>* dns_names,
                          std::vector<std::string>* ip_addresses) {
-  DCHECK(dns_names || ip_addresses);
   int index = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1);
   X509_EXTENSION* alt_name_ext = X509_get_ext(cert, index);
   if (!alt_name_ext)
-    return;
+    return false;
 
   bssl::UniquePtr<GENERAL_NAMES> alt_names(
       reinterpret_cast<GENERAL_NAMES*>(X509V3_EXT_d2i(alt_name_ext)));
   if (!alt_names.get())
-    return;
+    return false;
 
+  bool has_san = false;
   for (size_t i = 0; i < sk_GENERAL_NAME_num(alt_names.get()); ++i) {
     const GENERAL_NAME* name = sk_GENERAL_NAME_value(alt_names.get(), i);
-    if (name->type == GEN_DNS && dns_names) {
-      const unsigned char* dns_name = ASN1_STRING_data(name->d.dNSName);
-      if (!dns_name)
-        continue;
-      int dns_name_len = ASN1_STRING_length(name->d.dNSName);
-      dns_names->push_back(
-          std::string(reinterpret_cast<const char*>(dns_name), dns_name_len));
-    } else if (name->type == GEN_IPADD && ip_addresses) {
-      const unsigned char* ip_addr = name->d.iPAddress->data;
-      if (!ip_addr)
-        continue;
-      int ip_addr_len = name->d.iPAddress->length;
-      if (ip_addr_len != static_cast<int>(IPAddress::kIPv4AddressSize) &&
-          ip_addr_len != static_cast<int>(IPAddress::kIPv6AddressSize)) {
-        // http://www.ietf.org/rfc/rfc3280.txt requires subjectAltName iPAddress
-        // to have 4 or 16 bytes, whereas in a name constraint it includes a
-        // net mask hence 8 or 32 bytes. Logging to help diagnose any mixup.
-        LOG(WARNING) << "Bad sized IP Address in cert: " << ip_addr_len;
-        continue;
+    if (name->type == GEN_DNS) {
+      has_san = true;
+      if (dns_names) {
+        const unsigned char* dns_name = ASN1_STRING_data(name->d.dNSName);
+        if (!dns_name)
+          continue;

[eroman, 2017/03/21 21:09:54]

Same comments as with the ios one (which this version duplicates)

+        int dns_name_len = ASN1_STRING_length(name->d.dNSName);
+        dns_names->push_back(
+            std::string(reinterpret_cast<const char*>(dns_name), dns_name_len));
       }
-      ip_addresses->push_back(
-          std::string(reinterpret_cast<const char*>(ip_addr), ip_addr_len));
+    } else if (name->type == GEN_IPADD) {
+      has_san = true;
+      if (ip_addresses) {
+        const unsigned char* ip_addr = name->d.iPAddress->data;
+        if (!ip_addr)
+          continue;
+        int ip_addr_len = name->d.iPAddress->length;
+        if (ip_addr_len != static_cast<int>(IPAddress::kIPv4AddressSize) &&
+            ip_addr_len != static_cast<int>(IPAddress::kIPv6AddressSize)) {
+          // http://www.ietf.org/rfc/rfc3280.txt requires subjectAltName
+          // iPAddress to have 4 or 16 bytes, whereas in a name constraint it
+          // includes a net mask hence 8 or 32 bytes. Logging to help diagnose
+          // any mixup.
+          LOG(WARNING) << "Bad sized IP Address in cert: " << ip_addr_len;
+          continue;
+        }
+        ip_addresses->push_back(
+            std::string(reinterpret_cast<const char*>(ip_addr), ip_addr_len));
+      }
     }
+    // Fast path: Found at least one subjectAltName and the caller doesn't
+    // need the actual values.
+    if (has_san && !ip_addresses && !dns_names)
+      return true;
   }
+  return has_san;
 }
 
 class X509InitSingleton {
  public:
   static X509InitSingleton* GetInstance() {
     // We allow the X509 store to leak, because it is used from a non-joinable
     // worker that is not stopped on shutdown, hence may still be using
     // OpenSSL library after the AtExit runner has completed.
     return base::Singleton<X509InitSingleton, base::LeakySingletonTraits<
                                                   X509InitSingleton>>::get();
@@ skipping 133 matching lines @@
 }
 
 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);
+  return ParseSubjectAltName(cert_handle_, dns_names, ip_addrs);
 }
 
 // static
 X509_STORE* X509Certificate::cert_store() {
   return X509InitSingleton::GetInstance()->store();
 }
 
 // static
 bool X509Certificate::GetDEREncoded(X509Certificate::OSCertHandle cert_handle,
                                     std::string* encoded) {
@@ skipping 130 matching lines @@
 bool X509Certificate::IsSelfSigned(OSCertHandle cert_handle) {
   bssl::UniquePtr<EVP_PKEY> scoped_key(X509_get_pubkey(cert_handle));
   if (!scoped_key)
     return false;
   if (!X509_verify(cert_handle, scoped_key.get()))
     return false;
   return X509_check_issued(cert_handle, cert_handle) == X509_V_OK;
 }
 
 }  // namespace net