Class for parsing and evaluating RFC 5280 NameConstraints.

net/cert/internal/name_constraints.cc

+// Copyright 2015 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/internal/name_constraints.h"
+
+#include "base/strings/string_util.h"
+#include "net/cert/internal/verify_name_match.h"
+#include "net/der/input.h"
+#include "net/der/parser.h"
+#include "net/der/tag.h"
+
+namespace net {
+
+namespace {
+
+const int kSupportedNameTypes = GENERAL_NAME_DNS_NAME |

[eroman, 2015/09/10 17:48:28]

Can you add a comment explaining what this represents (supported by 

[mattm, 2015/09/22 22:12:31]

Done.

+                                GENERAL_NAME_DIRECTORY_NAME |
+                                GENERAL_NAME_IP_ADDRESS;
+
+enum WildcardMatchType { WILDCARD_PARTIAL_MATCH, WILDCARD_FULL_MATCH };

[eroman, 2015/09/10 17:48:27]

Please add comments for the meaning of these.

[mattm, 2015/09/22 22:12:31]

They're described in the comment for DNSNameMatches, which is the only place
they're used. I guess I could move the description here instead, but I'm not
sure that's better.

+
+// Returns true if |name| falls in the subtree defined by |name_space|.

[eroman, 2015/09/10 17:48:28]

Did you mean raw_name / raw_name_space?

[mattm, 2015/09/22 22:12:31]

Fixed. (raw_name)

+// RFC 5280 section 4.2.1.10:
+// DNS name restrictions are expressed as host.example.com.  Any DNS
+// name that can be constructed by simply adding zero or more labels
+// to the left-hand side of the name satisfies the name constraint.  For
+// example, www.host.example.com would satisfy the constraint but
+// host1.example.com would not.
+//
+// Also handles wildcard names (|name| starts with "*.").
+// If |wildcard_matching| is WILDCARD_PARTIAL_MATCH "*.bar.com" is considered to
+// match the constraint "foo.bar.com". If it is WILDCARD_FULL_MATCH, "*.bar.com"
+// will match "bar.com" but not "foo.bar.com".
+// Wildcard handling is not specified by RFC 5280, but since certificate

[eroman, 2015/09/10 17:48:28]

What do you mean by "certificate verification allows it" ? Is this a
compatibility thing, or specified elsewhere?

[mattm, 2015/09/22 22:12:31]

Meant that Chrome's verifier allows it, so it's important to match the same
behavior here.
(https://code.google.com/p/chromium/codesearch#chromium/src/net/cert/x509_cert...)

It's described in http://tools.ietf.org/html/rfc6125#section-6.4.3

+// verification allows it, name constraints must check it similarly.
+bool DNSNameMatches(const std::string& raw_name,
+                    const std::string& raw_name_space,
+                    WildcardMatchType wildcard_matching) {
+  base::StringPiece name(raw_name);

[eroman, 2015/09/10 17:48:28]

Why not make the input parameter a StringPiece?

[mattm, 2015/09/22 22:12:31]

Hmm, dunno why it ended up like that. Done.

+  base::StringPiece name_space(raw_name_space);
+  // Normalize absolute DNS names by removing the trailing dot.

[eroman, 2015/09/10 17:48:27]

What normative requirement is this? Can you add a reference?

[mattm, 2015/09/22 22:12:31]

It's another thing to match the behavior of VerifyHostname, so mentioned that.

+  if (!name.empty() && *name.rbegin() == '.')
+    name.remove_suffix(1);
+  if (!name_space.empty() && *name_space.rbegin() == '.')
+    name_space.remove_suffix(1);
+
+  // Everything matches the empty name space.

[eroman, 2015/09/10 17:48:28]

In the case where the namespace was simply '.', shouldn't that result in no
matches?

[mattm, 2015/09/22 22:12:30]

I guess so. Changed.

+  if (name_space.empty())
+    return true;
+
+  // Wildcard partial-match handling ("*.bar.com" matching name space
+  // "foo.bar.com").
+  if (wildcard_matching == WILDCARD_PARTIAL_MATCH && name.size() > 2 &&

[eroman, 2015/09/10 17:48:28]

Under what circumstances is the searched for name containing wildcard?

[mattm, 2015/09/22 22:12:31]

When the end-entity cert has a CommonName or dNSName which specifies that the
certificate is for some wildcard hostname.

+      name[0] == '*' && name[1] == '.') {
+    size_t name_space_dot_pos = name_space.find('.');
+    if (name_space_dot_pos != std::string::npos) {
+      base::StringPiece name_space_domain(
+          name_space.begin() + name_space_dot_pos + 1,
+          name_space.size() - name_space_dot_pos - 1);
+      base::StringPiece wildcard_domain(name.begin() + 2, name.size() - 2);
+      if (base::EqualsCaseInsensitiveASCII(wildcard_domain, name_space_domain))
+        return true;
+    }
+  }
+
+  if (!base::EndsWith(name, name_space, base::CompareCase::INSENSITIVE_ASCII))
+    return false;
+  // Exact match.
+  if (name.size() == name_space.size())
+    return true;
+  // Subtree match.
+  if (name.size() >= name_space.size() + 2 &&

[eroman, 2015/09/10 17:48:27]

Why the check the check of size() +2 rather than size() +1 ? Is it because you
consider |name_| starting with a period to be invalid? (but we don't do other
validity checks on the input hostname, nor verify that it is ascii)

[mattm, 2015/09/22 22:12:31]

Yes, I suppose it is a bit extraneous to be testing that when not doing any
other validity checks on the hostname.

+      name[name.size() - name_space.size() - 1] == '.')
+    return true;
+  // Trailing text matches, but not in a subtree (e.g., "foobar.com" is not a

[eroman, 2015/09/10 17:48:27]

Does name constraints not have a way to match one and only one host, rather than
always including subdomains? (I guess it could be expressed by excluding the
subdomains in a separate rule).

[mattm, 2015/09/22 22:12:30]

Correct
There aren't wildcards in the constraints, so you could only exclude certain
subdomains..

+  // match for "bar.com").
+  return false;
+}
+
+// Returns true if |ip| matches the ip/netmask pair |ip_constraint|.
+// RFC 5280 section 4.2.1.10:
+// The syntax of iPAddress MUST be as described in Section 4.2.1.6 with
+// the following additions specifically for name constraints.  For IPv4
+// addresses, the iPAddress field of GeneralName MUST contain eight (8)
+// octets, encoded in the style of RFC 4632 (CIDR) to represent an
+// address range [RFC4632].  For IPv6 addresses, the iPAddress field
+// MUST contain 32 octets similarly encoded.  For example, a name
+// constraint for "class C" subnet 192.0.2.0 is represented as the
+// octets C0 00 02 00 FF FF FF 00, representing the CIDR notation
+// 192.0.2.0/24 (mask 255.255.255.0).
+bool VerifyIPMatchesConstraint(const IPAddressNumber& ip,
+                               const std::vector<uint8_t>& ip_constraint) {
+  if (ip.size() != kIPv4AddressSize && ip.size() != kIPv6AddressSize)

[eroman, 2015/09/10 17:48:28]

Should this include a NOTREACHED()? This looks like an input error, but I see
why you are checking for it.

[mattm, 2015/09/22 22:12:31]

It was operating on network-supplied data, so it shouldn't NOTREACHED.
But I changed it to do the check in ParseGeneralName after all, so I'll put a
DCHECK here.

+    return false;
+  if (ip_constraint.size() != ip.size() * 2)
+    return false;
+
+  std::vector<uint8_t>::const_iterator prefix_iter = ip_constraint.begin();
+  std::vector<uint8_t>::const_iterator netmask_iter =
+      ip_constraint.begin() + ip_constraint.size() / 2;
+  IPAddressNumber::const_iterator ip_iter = ip.begin();
+  for (; ip_iter != ip.end(); ++ip_iter, ++prefix_iter, ++netmask_iter) {
+    // This assumes that any non-masked bits of the prefix are 0, as required by
+    // RFC 4632 section 3.1.
+    if ((*ip_iter & *netmask_iter) != *prefix_iter)
+      return false;
+  }
+  return true;
+}
+
+enum ParseGeneralNameUnsupportedTypeBehavior {
+  RECORD_UNSUPPORTED,
+  IGNORE_UNSUPPORTED,
+};
+
+// Parses a GeneralName value and add it to |subtrees|.

[eroman, 2015/09/10 17:48:28]

nit: add it --> adds it?

[mattm, 2015/09/22 22:12:31]

Done.

+// The GeneralName values are not validated here, since failing on invalid names

[eroman, 2015/09/10 17:48:27]

Unclear why this is desirable. If it is malformed shouldn't we reject the cert?

[mattm, 2015/09/22 22:12:31]

Done (except for directoryName, checking that would be a natural consequence of
pre-normalizing once I implement that).

+// here could cause an unnecessary failure if a name of that type does not
+// actually appear in the cert chain.
+WARN_UNUSED_RESULT bool ParseGeneralName(
+    const der::Input& input,
+    ParseGeneralNameUnsupportedTypeBehavior on_unsupported_types,
+    NameConstraints::GeneralNames* subtrees) {
+  der::Parser parser(input);
+  der::Tag tag;
+  der::Input value;
+  if (!parser.ReadTagAndValue(&tag, &value))
+    return false;
+  if ((tag & der::kTagClassMask) != der::kTagContextSpecific)
+    return false;
+  int tag_class = tag & ~der::kTagClassMask;
+  int name_type = 0;
+  // GeneralName ::= CHOICE {
+  switch (tag_class) {
+    // otherName                       [0]     OtherName,
+    case 0 + der::kTagConstructed:
+      name_type = GENERAL_NAME_OTHER_NAME;
+      break;
+    // rfc822Name                      [1]     IA5String,
+    case 1:
+      name_type = GENERAL_NAME_RFC822_NAME;
+      break;
+    // dNSName                         [2]     IA5String,
+    case 2:
+      name_type = GENERAL_NAME_DNS_NAME;
+      subtrees->dns_names.push_back(value.AsString());

[eroman, 2015/09/10 17:48:28]

Won't this mean that we accept certs with malformed DER? The encoding for
IA5String is not explicitly checked anywhere (just implicitly from
case-insensitive ASCII comparisons)

[mattm, 2015/09/22 22:12:31]

That's true. Added a check.

+      break;
+    // x400Address                     [3]     ORAddress,
+    case 3 + der::kTagConstructed:
+      name_type = GENERAL_NAME_X400_ADDRESS;
+      break;
+    // directoryName                   [4]     Name,
+    case 4 + der::kTagConstructed:
+      name_type = GENERAL_NAME_DIRECTORY_NAME;
+      subtrees->directory_names.push_back(std::vector<uint8_t>(
+          value.UnsafeData(), value.UnsafeData() + value.Length()));
+      break;
+    // ediPartyName                    [5]     EDIPartyName,
+    case 5 + der::kTagConstructed:
+      name_type = GENERAL_NAME_EDI_PARTY_NAME;
+      break;
+    // uniformResourceIdentifier       [6]     IA5String,
+    case 6:
+      name_type = GENERAL_NAME_UNIFORM_RESOURCE_IDENTIFIER;
+      break;
+    // iPAddress                       [7]     OCTET STRING,
+    case 7:
+      name_type = GENERAL_NAME_IP_ADDRESS;
+      subtrees->ip_addresses.push_back(std::vector<uint8_t>(
+          value.UnsafeData(), value.UnsafeData() + value.Length()));
+      break;
+    // registeredID                    [8]     OBJECT IDENTIFIER }
+    case 8:
+      name_type = GENERAL_NAME_REGISTERED_ID;
+      break;
+    default:
+      return false;
+  }
+  DCHECK(name_type);
+  if ((name_type & kSupportedNameTypes) ||
+      on_unsupported_types == RECORD_UNSUPPORTED)
+    subtrees->present_name_types |= name_type;
+  return true;
+}
+
+// Parses a GeneralSubtrees |value| and store the contents in |subtrees|.
+// The individual values stored into |subtrees| are not validated by this
+// function.
+// NOTE: |subtrees| will be modified regardless of the return.
+WARN_UNUSED_RESULT bool ParseGeneralSubtrees(
+    const der::Input& value,
+    bool is_critical,
+    NameConstraints::GeneralNames* subtrees) {
+  // GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree
+  //
+  // GeneralSubtree ::= SEQUENCE {
+  //      base                    GeneralName,
+  //      minimum         [0]     BaseDistance DEFAULT 0,
+  //      maximum         [1]     BaseDistance OPTIONAL }
+  //
+  // BaseDistance ::= INTEGER (0..MAX)
+  der::Parser sequence_parser(value);
+  while (sequence_parser.HasMore()) {
+    der::Parser subtree_sequence;
+    if (!sequence_parser.ReadSequence(&subtree_sequence))
+      return false;
+
+    der::Input raw_general_name;
+    if (!subtree_sequence.ReadRawTLV(&raw_general_name))
+      return false;
+
+    if (!ParseGeneralName(raw_general_name,
+                          is_critical ? RECORD_UNSUPPORTED : IGNORE_UNSUPPORTED,
+                          subtrees))
+      return false;
+
+    // RFC 5280 section 4.2.1.10:
+    // Within this profile, the minimum and maximum fields are not used with any
+    // name forms, thus, the minimum MUST be zero, and maximum MUST be absent.
+    // However, if an application encounters a critical name constraints
+    // extension that specifies other values for minimum or maximum for a name
+    // form that appears in a subsequent certificate, the application MUST
+    // either process these fields or reject the certificate.
+
+    // TODO(mattm): Technically we don't need to fail here: rather we only need

[eroman, 2015/09/10 17:48:28]

This sounds like a fine approach to me. (However will be more useful when we
have specific errors to explain the parsing failures).

[mattm, 2015/09/22 22:12:31]

I've reworded it from a todo to a note.

+    // to fail if a name of this type actually appears in a subsequent cert and
+    // this extension was marked critical.
+    // TODO(mattm): should this allow for the case that minimum is present but

[eroman, 2015/09/10 17:48:28]

I would say no, since that is invalid DER

[mattm, 2015/09/22 22:12:30]

Yeah, I've removed the TODO.

+    // zero? (0 is the default, so it should not be present in DER encoding..)
+    if (subtree_sequence.HasMore())
+      return false;
+  }
+  return true;

[eroman, 2015/09/10 17:48:28]

This means an empty GeneralSubTrees sequence will be accepted, which strictly
speaking is not compliant with the given ASN.1. Is this expected?

[mattm, 2015/09/22 22:12:31]

Nope, fixed

+}
+
+}  // namespace
+
+NameConstraints::GeneralNames::GeneralNames() {}
+
+NameConstraints::GeneralNames::~GeneralNames() {}
+
+NameConstraints::~NameConstraints() {}
+
+bool NameConstraints::Parse(const der::Input& extension_value,
+                            bool is_critical) {
+  der::Parser extension_parser(extension_value);
+  der::Parser sequence_parser;
+
+  // NameConstraints ::= SEQUENCE {
+  //      permittedSubtrees       [0]     GeneralSubtrees OPTIONAL,
+  //      excludedSubtrees        [1]     GeneralSubtrees OPTIONAL }
+  if (!extension_parser.ReadSequence(&sequence_parser))
+    return false;
+  if (extension_parser.HasMore())
+    return false;
+
+  bool had_permitted_subtrees = false;
+  der::Input permitted_subtrees_value;
+  if (!sequence_parser.ReadOptionalTag(der::ContextSpecificConstructed(0),
+                                       &permitted_subtrees_value,
+                                       &had_permitted_subtrees))
+    return false;

[eroman, 2015/09/10 17:48:28]

Note: My interpretation of multi-line if statements requiring braces was that
multiple lines in the "if" also counted. Not sure any change is needed here, but
myself and davidben (based on past review feedback) would have put curlies
around this (and others in this CL)

[mattm, 2015/09/22 22:12:31]

Done.

+  if (had_permitted_subtrees) {
+    if (!ParseGeneralSubtrees(permitted_subtrees_value, is_critical,
+                              &permitted_subtrees_))
+      return false;
+  }
+
+  bool had_excluded_subtrees = false;
+  der::Input excluded_subtrees_value;
+  if (!sequence_parser.ReadOptionalTag(der::ContextSpecificConstructed(1),
+                                       &excluded_subtrees_value,
+                                       &had_excluded_subtrees))
+    return false;
+  if (had_excluded_subtrees) {
+    if (!ParseGeneralSubtrees(excluded_subtrees_value, is_critical,
+                              &excluded_subtrees_))
+      return false;
+  }
+
+  // RFC 5280 section 4.2.1.10:
+  // Conforming CAs MUST NOT issue certificates where name constraints is an
+  // empty sequence.  That is, either the permittedSubtrees field or the
+  // excludedSubtrees MUST be present.
+  if (!had_permitted_subtrees && !had_excluded_subtrees)
+    return false;
+
+  if (sequence_parser.HasMore())
+    return false;
+
+  return true;
+}
+
+bool NameConstraints::IsPermittedCert(
+    const der::Input& subject_rdn_sequence,
+    const der::Input& subject_alt_name_extnvalue_tlv,
+    bool is_leaf_cert) const {
+  // Subject Alternative Name handling:
+  //
+  // RFC 5280 section 4.2.1.6:
+  // id-ce-subjectAltName OBJECT IDENTIFIER ::=  { id-ce 17 }
+  //
+  // SubjectAltName ::= GeneralNames
+  //
+  // GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName
+
+  GeneralNames san_names;
+  if (subject_alt_name_extnvalue_tlv.Length()) {
+    der::Parser extnvalue_parser(subject_alt_name_extnvalue_tlv);
+    der::Input subject_alt_name_tlv;
+    if (!extnvalue_parser.ReadTag(der::kOctetString, &subject_alt_name_tlv))
+      return false;
+
+    der::Parser subject_alt_name_parser(subject_alt_name_tlv);
+    der::Parser san_sequence_parser;
+    if (!subject_alt_name_parser.ReadSequence(&san_sequence_parser))
+      return false;
+    // Should not have trailing data after subjectAltName sequence.
+    if (subject_alt_name_parser.HasMore())
+      return false;
+    // The subjectAltName sequence should have at least 1 element.
+    if (!san_sequence_parser.HasMore())
+      return false;
+
+    while (san_sequence_parser.HasMore()) {
+      der::Input raw_general_name;
+      if (!san_sequence_parser.ReadRawTLV(&raw_general_name))
+        return false;
+
+      if (!ParseGeneralName(raw_general_name, RECORD_UNSUPPORTED, &san_names))
+        return false;
+    }
+
+    // Check unsupported name types:
+    // ConstrainedNameTypes for the unsupported types will only be true if that
+    // type of name was present in a name constraint that was marked critical.
+    //
+    // RFC 5280 section 4.2.1.10:
+    // If a name constraints extension that is marked as critical
+    // imposes constraints on a particular name form, and an instance of
+    // that name form appears in the subject field or subjectAltName
+    // extension of a subsequent certificate, then the application MUST
+    // either process the constraint or reject the certificate.
+    if (ConstrainedNameTypes() & san_names.present_name_types &
+        ~kSupportedNameTypes)
+      return false;
+
+    // Check supported name types:
+    for (const auto& dns_name : san_names.dns_names) {
+      if (!IsPermittedDNSName(dns_name))
+        return false;
+    }
+
+    for (const auto& directory_name : san_names.directory_names) {
+      if (!IsPermittedDirectoryName(
+              der::Input(directory_name.data(), directory_name.size())))
+        return false;
+    }
+
+    for (const auto& ip_address : san_names.ip_addresses) {
+      if (!IsPermittedIP(ip_address))
+        return false;
+    }
+  }
+
+  // Subject handling:
+
+  // RFC 5280 section 4.2.1.10:
+  // Legacy implementations exist where an electronic mail address is embedded
+  // in the subject distinguished name in an attribute of type emailAddress
+  // (Section 4.1.2.6).  When constraints are imposed on the rfc822Name name
+  // form, but the certificate does not include a subject alternative name, the
+  // rfc822Name constraint MUST be applied to the attribute of type emailAddress
+  // in the subject distinguished name.
+  if (!subject_alt_name_extnvalue_tlv.Length() &&
+      (ConstrainedNameTypes() & GENERAL_NAME_RFC822_NAME)) {
+    bool contained_email_address = false;
+    if (!NameContainsEmailAddress(subject_rdn_sequence,
+                                  &contained_email_address))
+      return false;
+    if (contained_email_address)
+      return false;
+  }
+
+  // RFC 5280 does not specify checking name constraints against subject
+  // CommonName, but since certificate verification allows it, name constraints
+  // must check it similarly.
+  if (is_leaf_cert &&
+      (san_names.dns_names.empty() && san_names.ip_addresses.empty()) &&
+      (ConstrainedNameTypes() &
+       (GENERAL_NAME_DNS_NAME | GENERAL_NAME_IP_ADDRESS))) {
+    // Note that while the commonName is transcoded to UTF-8, no special
+    // handling is done of internationalized domain names. (If an
+    // internationalized hostname is specified in commonName, it must be in
+    // punycode form.)
+    std::string common_name;
+    if (!GetNormalizedCommonNameFromName(subject_rdn_sequence, &common_name))
+      return false;
+    IPAddressNumber ip_number;
+    bool was_ip = ParseIPLiteralToNumber(common_name, &ip_number);
+    // For IP addresses, Chrome only allows IPv4 in commonName (see comment in
+    // X509Certificate::VerifyHostname), otherwise interpret as a dNSName.
+    if (was_ip && ip_number.size() == kIPv4AddressSize) {
+      if (!IsPermittedIP(ip_number))
+        return false;
+    } else {
+      if (!IsPermittedDNSName(common_name))
+        return false;
+    }
+  }
+
+  // RFC 5280 4.1.2.6:
+  // If subject naming information is present only in the subjectAltName
+  // extension (e.g., a key bound only to an email address or URI), then the
+  // subject name MUST be an empty sequence and the subjectAltName extension
+  // MUST be critical.
+  if (subject_alt_name_extnvalue_tlv.Length() &&
+      subject_rdn_sequence.Length() == 0)
+    return true;
+
+  return IsPermittedDirectoryName(subject_rdn_sequence);
+}
+
+bool NameConstraints::IsPermittedDNSName(const std::string& name) const {
+  if (permitted_subtrees_.dns_names.empty() &&
+      excluded_subtrees_.dns_names.empty())
+    return true;
+
+  for (const std::string& excluded_name : excluded_subtrees_.dns_names) {
+    if (DNSNameMatches(name, excluded_name, WILDCARD_PARTIAL_MATCH))
+      return false;
+  }
+  for (const std::string& permitted_name : permitted_subtrees_.dns_names) {
+    if (DNSNameMatches(name, permitted_name, WILDCARD_FULL_MATCH))
+      return true;
+  }
+
+  return false;
+}
+
+bool NameConstraints::IsPermittedDirectoryName(
+    const der::Input& name_rdn_sequence) const {
+  if (permitted_subtrees_.directory_names.empty() &&
+      excluded_subtrees_.directory_names.empty())
+    return true;
+
+  for (const auto& excluded_name : excluded_subtrees_.directory_names) {
+    if (VerifyNameInSubtree(
+            name_rdn_sequence,
+            der::Input(excluded_name.data(), excluded_name.size()))) {
+      return false;
+    }
+  }
+  for (const auto& permitted_name : permitted_subtrees_.directory_names) {
+    if (VerifyNameInSubtree(
+            name_rdn_sequence,
+            der::Input(permitted_name.data(), permitted_name.size()))) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+bool NameConstraints::IsPermittedIP(const IPAddressNumber& ip) const {
+  if (permitted_subtrees_.ip_addresses.empty() &&
+      excluded_subtrees_.ip_addresses.empty())
+    return true;
+
+  for (const auto& excluded_ip : excluded_subtrees_.ip_addresses) {
+    if (VerifyIPMatchesConstraint(ip, excluded_ip))
+      return false;
+  }
+  for (const auto& permitted_ip : permitted_subtrees_.ip_addresses) {
+    if (VerifyIPMatchesConstraint(ip, permitted_ip))
+      return true;
+  }
+
+  return false;
+}
+
+int NameConstraints::ConstrainedNameTypes() const {
+  return (permitted_subtrees_.present_name_types |
+          excluded_subtrees_.present_name_types);
+}
+
+}  // namespace net