Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(471)

Unified Diff: net/cert/internal/name_constraints.cc

Issue 1214933009: Class for parsing and evaluating RFC 5280 NameConstraints. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@compare_DN2
Patch Set: review changes for comment #6 Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..62d39e577bbe25ba3293db9eceda3ee5b297847d
--- /dev/null
+++ b/net/cert/internal/name_constraints.cc
@@ -0,0 +1,488 @@
+// 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 represe
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 DNSNameMatche
+
+// 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 allo
mattm 2015/09/22 22:12:31 Meant that Chrome's verifier allows it, so it's im
+// 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
mattm 2015/09/22 22:12:31 It's another thing to match the behavior of Verify
+ 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 '.', sh
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
mattm 2015/09/22 22:12:31 When the end-entity cert has a CommonName or dNSNa
+ 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 s
mattm 2015/09/22 22:12:31 Yes, I suppose it is a bit extraneous to be testin
+ 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
mattm 2015/09/22 22:12:30 Correct
+ // 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 lik
mattm 2015/09/22 22:12:31 It was operating on network-supplied data, so it s
+ 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
mattm 2015/09/22 22:12:31 Done (except for directoryName, checking that woul
+// 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 malforme
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 w
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
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 statement
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

Powered by Google App Engine
This is Rietveld 408576698