Add a DevTools warning for a missing subjectAltName

net/cert/x509_certificate_ios.cc

 // Copyright (c) 2016 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 <CommonCrypto/CommonDigest.h>
 #include <Security/Security.h>
 
 #include "base/mac/scoped_cftyperef.h"
@@ skipping 82 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 os_cert,
+bool ParseSubjectAltName(X509Certificate::OSCertHandle os_cert,
                          std::vector<std::string>* dns_names,
                          std::vector<std::string>* ip_addresses) {
-  DCHECK(dns_names || ip_addresses);
   bssl::UniquePtr<X509> cert = OSCertHandleToOpenSSL(os_cert);
   if (!cert.get())
-    return;
+    return false;
   int index = X509_get_ext_by_NID(cert.get(), NID_subject_alt_name, -1);
   X509_EXTENSION* alt_name_ext = X509_get_ext(cert.get(), 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]

Is it expected that this code-path sets |has_san = true|, however does not
correspondingly fill |dns_names| ? This is a deviation in meaning of |has_san|
from patchset 1, which tested non-emptiness of the out-parameters.

+        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_addrs) {
+        const unsigned char* ip_addr = name->d.iPAddress->data;
+        if (!ip_addr)
+          continue;

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

Same here and other 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;
 }
 
 }  // namespace
 
 // static
 X509Certificate::OSCertHandle X509Certificate::DupOSCertHandle(
     OSCertHandle handle) {
   if (!handle)
     return nullptr;
   return reinterpret_cast<OSCertHandle>(const_cast<void*>(CFRetain(handle)));
@@ skipping 108 matching lines @@
     }
     default: {
       NOTREACHED() << "Certificate format " << format << " unimplemented";
       break;
     }
   }
 
   return results;
 }
 
-void X509Certificate::GetSubjectAltName(
+bool 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
 bool X509Certificate::GetDEREncoded(X509Certificate::OSCertHandle cert_handle,
                                     std::string* encoded) {
   base::StringPiece der;
   if (!cert_handle)
     return false;
   ScopedCFTypeRef<CFDataRef> der_data(SecCertificateCopyData(cert_handle));
   if (!der_data)
@@ skipping 140 matching lines @@
     return false;
   bssl::UniquePtr<EVP_PKEY> scoped_key(X509_get_pubkey(cert.get()));
   if (!scoped_key)
     return false;
   if (!X509_verify(cert.get(), scoped_key.get()))
     return false;
   return X509_check_issued(cert.get(), cert.get()) == X509_V_OK;
 }
 
 }  // namespace net