Class for parsing and evaluating RFC 5280 NameConstraints.

net/cert/internal/name_constraints.h

Index: net/cert/internal/name_constraints.h
diff --git a/net/cert/internal/name_constraints.h b/net/cert/internal/name_constraints.h
new file mode 100644
index 0000000000000000000000000000000000000000..2487f34ef4a5a0773cce0427f101b05afd55db2e
--- /dev/null
+++ b/net/cert/internal/name_constraints.h
@@ -0,0 +1,111 @@
+// 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.
+
+#ifndef NET_CERT_INTERNAL_NAME_CONSTRAINTS_H_
+#define NET_CERT_INTERNAL_NAME_CONSTRAINTS_H_
+
+#include <vector>
+
+#include "net/base/ip_address_number.h"
+#include "net/der/input.h"

[eroman, 2015/08/26 19:56:44]

nit: can you forward-declare der::Input instead?

[mattm, 2015/08/29 01:37:18]

Done.

+
+namespace net {
+
+// Parses a NameConstraints extension value and allows testing whether names are
+// allowed under those constraints as defined by RFC 5280 section 4.2.1.10.
+class NET_EXPORT NameConstraints {
+ public:
+  // TODO: make this private? (requires making some currently anonymous

[eroman, 2015/08/26 19:56:44]

TODO: --> TODO(mattm):

[mattm, 2015/08/29 01:37:19]

Done.

+  // functions into private static methods.) Or maybe this will need to be split
+  // out into a public class, since GeneralNames is used other places in a

[eroman, 2015/08/26 19:56:44]

I would say not to make it private for the reasons you describe (I also prefer
being able to use anonymous functions). Can break it out later as the need
arises.

[mattm, 2015/08/29 01:37:18]

Acknowledged.

+  // certificate also...
+  struct GeneralNames {
+    GeneralNames();
+    ~GeneralNames();
+
+    // ASCII hostnames.
+    std::vector<std::string> dns_names;
+
+    // DER encoded Name values (not including the Sequence tag).

[eroman, 2015/08/26 19:56:44]

See also https://code.google.com/p/chromium/issues/detail?id=522235. Not sure
what the best approach is short of comments right now...

[mattm, 2015/08/29 01:37:18]

Acknowledged.

+    std::vector<std::vector<uint8_t>> directory_names;
+
+    // iPAddresses. For Subject Alternative Name this will be 4 bytes for IPv4
+    // or 16 bytes for IPv6. For Name Constraints, it will be ip + netmask
+    // (8 bytes for IPv4, 32 bytes for IPv6).
+    std::vector<std::vector<uint8_t>> ip_addresses;

[eroman, 2015/08/26 19:56:44]

Should we go a step further and parse this into an IPAddressNumber + mask?

[mattm, 2015/08/29 01:37:18]

I guess it could, but just the complication again due to sometimes it being only
an IP and sometimes being an ip+netmask.. dunno.

+
+    // Whether any values of the other types were present.
+    bool has_other_names;
+    bool has_rfc822_names;
+    bool has_x400_addresses;
+    bool has_edi_party_names;
+    bool has_uniform_resource_identifiers;
+    bool has_registered_ids;
+  };
+
+  ~NameConstraints();
+
+  // Parse a DER-encoded NameConstraints extension. |extension_value| should be

[eroman, 2015/08/26 19:56:44]

This comment isnt' really clear that this in fact initializes the
NameConstraints object. Also nit Parse --> Parses

[mattm, 2015/08/29 01:37:18]

Done.

+  // the extnValue octet string from the extension, |is_critical| should be true

[eroman, 2015/08/26 19:56:44]

Unclear if this is the content, or the full TLV of the octet string.

[mattm, 2015/08/29 01:37:18]

Done.

+  // if the extension was marked critical.
+  // Returns true if the extension was parsed successfully.
+  // The object lifetime is not bound to the lifetime of |extension_value| data.

[eroman, 2015/08/26 19:56:44]

Out of curiosity what made you choose this model? What are the tradeoffs?

[mattm, 2015/08/29 01:37:18]

Mainly that it just seemed safer and easier to understand, since name
constraints are something that will be used for multiple levels of checking the
cert chain, and (later) will have constraints merged together from multiple
certificates in the chain, could become confusing if a NameConstraints contained
pointers into N different cert blobs.

On the other hand, the chain validation is recursive so in practice maybe it
wouldn't be a problem to just say "this NameConstraints object depends upon the
lifetime of cert foo and possibly any of the certs in higher levels of the
verification".

+  bool Parse(const der::Input& extension_value, bool is_critical);
+
+  // Tests if a certificate is allowed by the name constraints.
+  // |subject_rdn_sequence| should be the DER-encoded value of the subject's
+  // RDNSequence field (not including Sequence tag), and may be an empty ASN.1
+  // sequence.  |subject_alt_name| should be the extnValue of the subjectAltName

[eroman, 2015/08/26 19:56:44]

Is this the OCTET STRING's value, or the full TLV? If just the value, how is
emptiness disambiguated from the not specified case?

[mattm, 2015/08/29 01:37:18]

Just the value. You're correct there could be some ambiguity there.(Although the
subjectAltName is not supposed to be empty, can't error check it here because of
the ambiguity.)

I changed it to take the TLV and added a test of empty sequence.

+  // extension, or empty if the cert did not have a subjectAltName extension.
+  // |is_leaf_cert| should be true if the certificate is the leaf of the
+  // certificate chain, in which case subject commonName hostname/ip checking is
+  // done.
+  bool IsPermittedCert(const der::Input& subject_rdn_sequence,
+                       const der::Input& subject_alt_name,
+                       bool is_leaf_cert) const;
+
+  // Returns true if the ASCII hostname |name| is permitted.
+  // |name| may be a wildcard hostname (starts with "*."). Eg, "*.bar.com" is
+  // considered would not be permitted if "bar.com" is permitted and

[eroman, 2015/08/26 19:56:44]

is considered would not be --> would not be permitted

[mattm, 2015/08/29 01:37:19]

Done.

+  // "foo.bar.com" is excluded, while "*.baz.com" would only be permitted if
+  // "baz.com" is permitted.
+  bool IsPermittedDNSName(const std::string& name) const;
+
+  // Returns true if the directoryName |name| is permitted.

[eroman, 2015/08/26 19:56:44]

what does |name| refer to?

[mattm, 2015/08/29 01:37:19]

fixed.

+  // |name_rdn_sequence| should be the DER-encoded RDNSequence value (not
+  // including the Sequence tag.)
+  bool IsPermittedDirectoryName(const der::Input& name_rdn_sequence) const;
+
+  // Returns true if the iPAddress |ip| is permitted.
+  bool IsPermittedIP(const IPAddressNumber& ip) const;
+
+  // These name types aren't supported, therefore names of these types are
+  // permitted only if they don't appear in the name constraints at all, or if
+  // the name constraint they appeared in was non-critical.
+  //
+  // RFC 5280 section 4.2.1.10 says:
+  // Applications conforming to this profile MUST be able to process name
+  // constraints that are imposed on the directoryName name form and SHOULD be
+  // able to process name constraints that are imposed on the rfc822Name,
+  // uniformResourceIdentifier, dNSName, and iPAddress name forms.
+  // 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.
+  bool IsPermittedOtherName() const;
+  bool IsPermittedRFC822Name() const;
+  bool IsPermittedX400Address() const;
+  bool IsPermittedEdiPartyName() const;
+  bool IsPermittedURI() const;
+  bool IsPermittedRegisteredId() const;
+
+ private:
+  GeneralNames permitted_subtrees_;
+  GeneralNames excluded_subtrees_;
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_INTERNAL_NAME_CONSTRAINTS_H_