Add support for certificate name checking

net/base/x509_openssl_util.cc

-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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/base/x509_openssl_util.h"
 
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/string_piece.h"
+#include "base/string_util.h"
 #include "base/time.h"
+#include "net/base/registry_controlled_domain.h"
 
 namespace net {
 
 namespace x509_openssl_util {
 
 namespace {
 
 // Helper for ParseDate. |*field| must contain at least |field_len| characters.
 // |*field| will be advanced by |field_len| on exit. |*ok| is set to false if
 // there is an error in parsing the number, but left untouched otherwise.
@@ skipping 80 matching lines @@
 
   if (!valid) {
     NOTREACHED() << "can't parse x509 date " << str_date;
     return false;
   }
 
   *time = base::Time::FromUTCExploded(exploded);
   return true;
 }
 
+// Implemented as per http://tools.ietf.org/html/draft-saintandre-tls-server-id-check-09#section-4.4.3
+bool VerifyHostname(const std::string& hostname,
+                    const std::vector<std::string>& cert_names) {
+  DCHECK(!hostname.empty());
+
+  // Simple host name validation. A valid domain name must only contain
+  // alpha, digits, hyphens, and dots. An IP address may have digits and dots,
+  // and also square braces and colons for IPv6 addresses.

[wtc, 2010/11/03 00:29:49]

IPv6 address literals passed to this function should not contain square braces
[].

[Ryan Sleevi, 2010/11/03 03:42:21]

BUG: I'm not sure that this function should consider IP address matches as
valid. RFC 2818, and by transitive properties,
draft-saintandre-tls-server-id-check, stated

"   In some cases, the URI is specified as an IP address rather than a
   hostname. In this case, the iPAddress subjectAltName must be present
   in the certificate and must exactly match the IP in the URI."

So if |hostname| is supplied as an IP address, then it should only match if a
subjectAltName is present. Since you've TODO'd the SAN support, I think it'd be
appropriate here to return FALSE if |hostname| is an IP.

I'm wondering if there is a "blessed" IP canonicalization, since net_util's
ParseIPLiteralToNumber depends on googleurl (which agl didn't want in the
security hot path).

For NSS, it also compares if the IPv4-as-IPv6 (::ffff:192.168.0.1) matches: if
|hostname| is an IPv4 address and the subjectAltName is an IPv6 address, or the
inverse. 

For Apple, it lacks any support for IPv6 matching (I'd mention the Radar # but
Apple's bugreporter hasn't been working for me tonight)

[joth, 2010/11/12 18:55:23]

Done.

[joth, 2010/11/12 18:55:23]

Done.
Removed all attempt at IP support, for now.

+  bool found_alpha = false;
+  bool found_ip6_chars = false;
+  bool found_hyphen = false;
+
+  std::string reference_name;
+  size_t first_dot_index = std::string::npos;
+  reference_name.reserve(hostname.length());
+  for (std::string::const_iterator it = hostname.begin();
+      it != hostname.end(); ++it) {
+    char c = *it;
+    if (IsAsciiAlpha(c)) {
+      found_alpha = true;
+      c = ToLowerASCII(c);
+    } else if (c == '.') {
+      if (first_dot_index == std::string::npos)
+        first_dot_index = reference_name.length();
+    } else if (c == '[' || c == ']' || c == ':') {
+      found_ip6_chars = true;
+    } else if (c == '-') {
+      found_hyphen = true;
+    } else if (!IsAsciiDigit(c)) {
+      LOG(WARNING) << "Invalid char " << c << " in hostname " << hostname;
+      return false;
+    }
+    reference_name.push_back(c);
+  }
+  DCHECK(!reference_name.empty());
+  if (found_hyphen && found_ip6_chars) {
+    LOG(WARNING) << "Mixed IPv6 domain name " << hostname;
+    return false;
+  }
+
+  // |host_domain| is the remainder of |host| after the leading host component
+  // is stripped off, but includes the leading dot e.g. "www.f.com" -> ".f.com".
+  // If there is no meaningful domain part to |host| (e.g. it is an IP address
+  // or contains no dots) then |host_domain| will be empty.
+  base::StringPiece host_domain;
+  if (found_alpha && !found_ip6_chars && first_dot_index != std::string::npos) {
+    host_domain = reference_name;
+    host_domain.remove_prefix(first_dot_index);
+    DCHECK(host_domain.starts_with("."));
+
+    // Make sure the domain is not a registry domain, e.g. to avoid a dodgy cert
+    // claiming to be for *.org matching a hostname of chromium.org.
+    size_t registry_length = RegistryControlledDomainService::GetRegistryLength(
+        reference_name, true);
+    // The registry length does not include the leading dot, so add 1 for it.
+    if (host_domain.length() == registry_length + 1) {

[wtc, 2010/11/03 00:29:49]

Nit: omit curly braces {} for one-liner "if" bodies.

[joth, 2010/11/12 18:55:23]

Done.

+      host_domain.clear();
+    }
+  }
+
+  for (std::vector<std::string>::const_iterator it = cert_names.begin();
+       it != cert_names.end(); ++it) {
+    // Catch badly corrupt cert names up front.
+    if (it->empty() || it->find('\0') != std::string::npos) {
+      LOG(WARNING) << "Bad name in cert: " << *it;
+      continue;
+    }
+    const std::string cert_name_string(StringToLowerASCII(*it));
+    base::StringPiece cert_match(cert_name_string);
+
+    // Remove trailing dot, if any.
+    if (cert_match.ends_with("."))
+      cert_match.remove_suffix(1);
+
+    // The hostname must be at least as long as the cert name it is matching,
+    // as we require the wildcard (if present) to match at least one character.
+    if (cert_match.length() > reference_name.length())
+      continue;
+
+    if (cert_match == reference_name)
+      return true;
+
+    // Next see if this cert name starts with a wildcard, so long as the
+    // hostname we're matching against has a valid 'domain' part to match.
+    // TODO(joth): The "-10" version of draft-saintandre-tls-server-id-check
+    // allows the wildcard to appear anywhere in the leftmost  label. Once this
+    // is finalized, we may need to update this check to use MatchPattern.

[Ryan Sleevi, 2010/11/03 03:42:21]

@wtc: here

[joth, 2010/11/12 18:55:23]

Done.

+    if (host_domain.empty() || !cert_match.starts_with("*"))
+      continue;
+
+    // Erase the * but not the . from the domain, as we need to include the dot
+    // in the comparison.
+    cert_match.remove_prefix(1);
+
+    // Do character by character comparison on the remainder to see
+    // if we have a wildcard match. This intentionally does no special handling
+    // for any other wildcard characters in |domain|; alternatively it could
+    // detect these and skip those candidate cert names.
+    if (cert_match == host_domain)
+      return true;
+  }
+  DVLOG(1) << "Could not find any match for " << hostname
+           << " (canonicalized as " << reference_name
+           << ") in cert names " << JoinString(cert_names, '|');
+  return false;
+}
+
 } // namespace x509_openssl_util
 
 } // namespace net