Add a DevTools warning for a missing subjectAltName

net/cert/x509_certificate_mac.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 <CommonCrypto/CommonDigest.h>
 #include <CoreServices/CoreServices.h>
 #include <Security/Security.h>
 
@@ skipping 198 matching lines @@
     return true;
 
   for (OSCertHandles::iterator it = intermediate_ca_certs_.begin();
        it != intermediate_ca_certs_.end(); ++it) {
     if (IsCertIssuerInEncodedList(*it, valid_issuers))
       return true;
   }
   return false;
 }
 
-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();

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

Note the openssl implementations differ, and do a straight up append rather than
assignment (as the rest seem to do). Please clarify the expectation for
implementations in the common header

[Ryan Sleevi, 2017/03/21 21:48:43]

No, they reset in the GetSubjectAltName and then append in the Parse

   if (ip_addrs)
     ip_addrs->clear();
 
   x509_util::CSSMCachedCertificate cached_cert;
   OSStatus status = cached_cert.Init(cert_handle_);
   if (status)
-    return;
+    return false;
+
   x509_util::CSSMFieldValue subject_alt_name;
   status = cached_cert.GetField(&CSSMOID_SubjectAltName, &subject_alt_name);
   if (status || !subject_alt_name.field())
-    return;
+    return false;
+
   const CSSM_X509_EXTENSION* cssm_ext =
       subject_alt_name.GetAs<CSSM_X509_EXTENSION>();
   if (!cssm_ext || !cssm_ext->value.parsedValue)
-    return;
+    return false;
   const CE_GeneralNames* alt_name =
       reinterpret_cast<const CE_GeneralNames*>(cssm_ext->value.parsedValue);
 
+  bool has_san = false;
   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 (name_struct.nameType == GNT_DNSName) {
+      has_san = true;
+      if (dns_names) {
+        dns_names->push_back(std::string(
+            reinterpret_cast<const char*>(name_data.Data), name_data.Length));
+      }
+    } else if (name_struct.nameType == GNT_IPAddress) {
+      has_san = true;
+      if (ip_addrs) {
+        ip_addrs->push_back(std::string(
+            reinterpret_cast<const char*>(name_data.Data), name_data.Length));
+      }
     }
+    // Fast path: Found at least one subjectAltName and the caller doesn't
+    // need the actual values.
+    if (has_san && !ip_addrs && !dns_names)
+      return true;
   }
+
+  return has_san;
 }
 
 // static
 bool X509Certificate::GetDEREncoded(X509Certificate::OSCertHandle cert_handle,
                                     std::string* encoded) {
   CSSM_DATA der_data;
   if (!cert_handle || SecCertificateGetData(cert_handle, &der_data) != noErr)
     return false;
   encoded->assign(reinterpret_cast<char*>(der_data.Data),
                   der_data.Length);
@@ skipping 225 matching lines @@
     return false;
 
   if (CSSM_CL_CertVerify(cl_handle, 0, &cert_data, &cert_data, NULL, 0))
     return false;
   return true;
 }
 
 #pragma clang diagnostic pop  // "-Wdeprecated-declarations"
 
 }  // namespace net