Class for parsing and evaluating RFC 5280 NameConstraints.

net/cert/internal/name_constraints.cc

Index: net/cert/internal/name_constraints.cc
diff --git a/net/cert/internal/name_constraints.cc b/net/cert/internal/name_constraints.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d966e409ca2a85ffe3b888a6d0f6a26e504a8308
--- /dev/null
+++ b/net/cert/internal/name_constraints.cc
@@ -0,0 +1,538 @@
+// 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 {
+
+// The name types of GeneralName that are fully supported in name constraints.
+//
+// (The other types will have the minimal checking described by 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.)
+const int kSupportedNameTypes = GENERAL_NAME_DNS_NAME |
+                                GENERAL_NAME_DIRECTORY_NAME |
+                                GENERAL_NAME_IP_ADDRESS;
+
+enum WildcardMatchType { WILDCARD_PARTIAL_MATCH, WILDCARD_FULL_MATCH };

[Ryan Sleevi, 2015/09/24 22:06:18]

Document

[mattm, 2015/09/30 04:52:31]

Done.

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

[Ryan Sleevi, 2015/09/24 22:06:19]

From reading this comment alone, I'm confused as to what |name_space| represents
(and why is it two words, rather than namespace?)

[mattm, 2015/09/30 04:52:30]

renamed to dns_constraint

+// 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 Chrome's
+// certificate verification allows it, name constraints must check it similarly.
+bool DNSNameMatches(base::StringPiece name,
+                    base::StringPiece name_space,
+                    WildcardMatchType wildcard_matching) {
+  // Everything matches the empty name space.
+  if (name_space.empty())
+    return true;
+
+  // Normalize absolute DNS names by removing the trailing dot to match Chrome's
+  // certificate verification behavior.

[Ryan Sleevi, 2015/09/24 22:06:19]

Documentation bug: You shouldn't be talking about "Chrome's certificate
validation code" in //net.

Simply "Normalize absolute DNS names by removing the trailing dot, if any" is
sufficient.

[mattm, 2015/09/30 04:52:31]

Done.

+  if (!name.empty() && *name.rbegin() == '.')
+    name.remove_suffix(1);
+  if (!name_space.empty() && *name_space.rbegin() == '.')
+    name_space.remove_suffix(1);
+
+  // Wildcard partial-match handling ("*.bar.com" matching name space
+  // "foo.bar.com").
+  if (wildcard_matching == WILDCARD_PARTIAL_MATCH && name.size() > 2 &&
+      name[0] == '*' && name[1] == '.') {
+    size_t name_space_dot_pos = name_space.find('.');

[Ryan Sleevi, 2015/09/24 22:06:18]

If there is no wildcard match, what's supposed to happen then? It's unclear
whether 71 -> 72 should have a return false, or, if not, why it's acceptable to
fall through.

[mattm, 2015/09/30 04:52:30]

expanded on the comment.

+    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() &&
+      name[name.size() - name_space.size() - 1] == '.') {
+    return true;
+  }
+  // Trailing text matches, but not in a subtree (e.g., "foobar.com" is not a
+  // 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) {
+  DCHECK(ip.size() == kIPv4AddressSize || ip.size() == kIPv6AddressSize);
+  DCHECK(ip_constraint.size() == kIPv4AddressSize * 2 ||
+         ip_constraint.size() == kIPv6AddressSize * 2);
+
+  if (ip_constraint.size() != ip.size() * 2)

[Ryan Sleevi, 2015/09/24 22:06:19]

No need for the DCHECK on lines 103-104 then, especially with this being
'attacker' supplied data (from the cert)

[mattm, 2015/09/30 04:52:31]

The intent was that this if condition is for not trying to do ipv4 vs ipv6
matches, whereas the DCHECK was for wrong lengths in general (and it's a dcheck
since ParseGeneralName should already have enforced it). I could still remove it
though since it should never be possible to hit..

+    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;
+  }

[Ryan Sleevi, 2015/09/24 22:06:18]

We have net::IPNumberMatchesPrefix() and net::MaskPrefixLength [although I have
no idea while we have the latter]

[mattm, 2015/09/30 04:52:31]

Is there any guarantee the constraint netmask will be representable as a prefix
length as opposed to an arbitrary bit pattern?

[Ryan Sleevi, 2015/10/01 23:52:23]

Define guarantee ;)

RFC 1878 covers the v4 subnetting rules, and v6 always follows prefix rules. So
from a networking standpoint, "yes"

Now, in the gross and grotty world of CAs, I don't know if this is a field
people have invalidly encoded. However, if it fails to parse, you're free to
reject it :)

[mattm, 2015/10/06 21:55:07]

Done.

+  return true;
+}
+
+enum ParseGeneralNameUnsupportedTypeBehavior {
+  RECORD_UNSUPPORTED,
+  IGNORE_UNSUPPORTED,
+};

[Ryan Sleevi, 2015/09/24 22:06:19]

Document

[mattm, 2015/09/30 04:52:31]

Done.

+enum ParseGeneralNameIPAddressType {

[Ryan Sleevi, 2015/09/24 22:06:19]

newline

[mattm, 2015/09/30 04:52:30]

Done.

+  IP_ADDRESS_ONLY,
+  IP_ADDRESS_AND_NETMASK,
+};
+

[Ryan Sleevi, 2015/09/24 22:06:18]

Document

[mattm, 2015/09/30 04:52:30]

Done.

+// Parses a GeneralName value and adds it to |subtrees|.
+WARN_UNUSED_RESULT bool ParseGeneralName(
+    const der::Input& input,
+    ParseGeneralNameUnsupportedTypeBehavior on_unsupported_types,

[Ryan Sleevi, 2015/09/24 22:06:18]

naming: "on_unsupported_types" doesn't seem like it fits our naming
pattern/idiom.

I'm not sure if it helps readability over a bool, given the lengths.

[mattm, 2015/09/30 04:52:30]

renamed to "unsupported_type_behavior"
It's more intended to help the readability of the call sites.

+    ParseGeneralNameIPAddressType ip_address_type,
+    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)

[Ryan Sleevi, 2015/09/24 22:06:19]

Another place where a helper function might help (IsContextSpecific) ?

[mattm, 2015/09/30 04:52:30]

Done.

+    return false;
+  int tag_class = tag & ~der::kTagClassMask;

[Ryan Sleevi, 2015/09/24 22:06:19]

BUG: Why are you using a signed-type, when der::Tag is uint8_t?

BUG 2: Given that a der::Tag could conceivably occupy multiple bytes, it
suggests that perhaps in the same way we have net::der::IsConstructed, we might
have net::der::GetTagClass(der::Tag tag) that returns the tag class?

[mattm, 2015/09/30 04:52:31]

fixed.
Done (though I realized my naming was a bit weird. I called it GetTagNumber
instead.)

+  int name_type = 0;

[Ryan Sleevi, 2015/09/24 22:06:19]

Is int the right type, since you're using the enum?

[mattm, 2015/09/30 04:52:31]

done (though still with present_name_types as an int, since otherwise requires
ugly casting)

+  // GeneralName ::= CHOICE {
+  switch (tag_class) {
+    // otherName                       [0]     OtherName,
+    case 0 + der::kTagConstructed:

[Ryan Sleevi, 2015/09/24 22:06:19]

? This is a bit surprising/confusing. COuld you explain?

[mattm, 2015/09/30 04:52:30]

Since I was only excluding the kTagClassMask (kTagContextSpecific |
kTagApplication), I get the tag number + the constructed tag for constructed
types. So the simple string types are just the tag number, while the structured
types have the constructed bit in addition to the implicit tag value.

But now I switched to using GetTagNumber, this is now changed. (Makes the switch
cleaner, but then I have to test for IsConstructed separately.)

+      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;
+      const std::string s = value.AsString();
+      if (!base::IsStringASCII(s))
+        return false;
+      subtrees->dns_names.push_back(s);
+      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;
+      if ((ip_address_type == IP_ADDRESS_ONLY &&
+           (value.Length() != kIPv4AddressSize &&
+            value.Length() != kIPv6AddressSize)) ||
+          (ip_address_type == IP_ADDRESS_AND_NETMASK &&
+           (value.Length() != kIPv4AddressSize * 2 &&
+            value.Length() != kIPv6AddressSize * 2))) {
+        return false;
+      }
+      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);
+  // The GeneralSubtrees sequence should have at least 1 element.
+  if (!sequence_parser.HasMore())
+    return false;
+  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,
+                          IP_ADDRESS_AND_NETMASK, 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.
+
+    // Note that technically failing here isn't required: rather only need to
+    // fail if a name of this type actually appears in a subsequent cert and
+    // this extension was marked critical. However the minimum and maximum
+    // fields appear uncommon enough that implementing that isn't useful.
+    if (subtree_sequence.HasMore())
+      return false;
+  }
+  return true;
+}
+
+}  // namespace
+
+NameConstraints::GeneralNames::GeneralNames() {}
+
+NameConstraints::GeneralNames::~GeneralNames() {}
+
+NameConstraints::~NameConstraints() {}
+
+// static
+scoped_ptr<NameConstraints> NameConstraints::CreateFromDer(
+    const der::Input& extension_value,
+    bool is_critical) {
+  scoped_ptr<NameConstraints> name_constraints(new NameConstraints());
+  if (!name_constraints->Parse(extension_value, is_critical))
+    return nullptr;
+  return name_constraints;
+}
+
+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;
+  }
+  if (had_permitted_subtrees) {
+    if (!ParseGeneralSubtrees(permitted_subtrees_value, is_critical,

[Ryan Sleevi, 2015/09/24 22:06:19]

Why not combine this conditional with the one on 303?

[mattm, 2015/09/30 04:52:31]

Done.

+                              &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,

[Ryan Sleevi, 2015/09/24 22:06:18]

Same here - why not combine with the conditional on 317?

[mattm, 2015/09/30 04:52:31]

Done.

+                              &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,
+                            IP_ADDRESS_ONLY, &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.

[Ryan Sleevi, 2015/09/24 22:06:18]

This seems tightly coupled in the comment; same layering concerns here.

[mattm, 2015/09/30 04:52:30]

Well, I'd say that it is actually coupled, unfortunately. I removed the
reference to Chrome though. Referencing X509Certificate should be ok since
that's also in net?

+    if (was_ip && ip_number.size() == kIPv4AddressSize) {
+      if (!IsPermittedIP(ip_number))
+        return false;
+    } else {
+      if (!IsPermittedDNSName(common_name))

[Ryan Sleevi, 2015/09/24 22:06:19]

why not combine this with 453 to be an else-if?

[mattm, 2015/09/30 04:52:31]

I kinda liked how it made the IsPermittedDNSName line up with the IsPermittedIP
check. But it's not that important, changed.

+        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() &&

[Ryan Sleevi, 2015/09/24 22:06:19]

It doesn't hurt to be extra verbose in the comments here.

// If there are no name constraints for DNS names, all names are accepted.

[mattm, 2015/09/30 04:52:30]

Done.

+      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))

[Ryan Sleevi, 2015/09/24 22:06:19]

Comments explaining nuance? :)

[mattm, 2015/09/30 04:52:30]

Done.

+      return false;
+  }
+  for (const std::string& permitted_name : permitted_subtrees_.dns_names) {
+    if (DNSNameMatches(name, permitted_name, WILDCARD_FULL_MATCH))

[Ryan Sleevi, 2015/09/24 22:06:19]

Ditto

[mattm, 2015/09/30 04:52:30]

Done.

+      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