Add a DevTools warning for a missing subjectAltName

net/cert/x509_certificate_ios.cc

Index: net/cert/x509_certificate_ios.cc
diff --git a/net/cert/x509_certificate_ios.cc b/net/cert/x509_certificate_ios.cc
index ce93f90dd7918a90a12d62904f468bf5cbb70484..60b43f3272cd513b0036dc6d9e711fb720c0c67e 100644
--- a/net/cert/x509_certificate_ios.cc
+++ b/net/cert/x509_certificate_ios.cc
@@ -100,49 +100,62 @@ void ParsePrincipal(X509Certificate::OSCertHandle os_cert,
                                       &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));
+      }
+    } 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));
       }
-      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
@@ -271,7 +284,7 @@ X509Certificate::OSCertHandles X509Certificate::CreateOSCertHandlesFromBytes(
   return results;
 }
 
-void X509Certificate::GetSubjectAltName(
+bool X509Certificate::GetSubjectAltName(
     std::vector<std::string>* dns_names,
     std::vector<std::string>* ip_addrs) const {
   if (dns_names)
@@ -279,7 +292,7 @@ void X509Certificate::GetSubjectAltName(
   if (ip_addrs)
     ip_addrs->clear();
 
-  ParseSubjectAltName(cert_handle_, dns_names, ip_addrs);
+  return ParseSubjectAltName(cert_handle_, dns_names, ip_addrs);
 }
 
 // static