Add better names validator and deprecate older one

Previous name validators have multiple issues. They do not prevent
unknown entires from passing through. They require repeating rules for
various name locations (cn, san). They also disregard wildcards when
matching only the suffix. The inflexible configuration also makes
specific validators like server_group required.

The new validator whitelist_names solves all those issues and allows to
deprecate old validators.

Implements: blueprint validator-improvement

Change-Id: Id31889f735eb34323f21a91d68a50602351f6611

anchor/X509/extension.py

@@ -323,6 +323,13 @@ class X509ExtensionSubjectAltName(X509Extension):
             ips.append(utils.asn1_to_netaddr(name.getComponent()))
         return ips
 
+    @uses_ext_value
+    def has_unknown_entries(self, ext_value=None):
+        for name in ext_value:
+            if name.getName() not in ('dNSName', 'iPAddress'):
+                return True
+        return False
+
     @modifies_ext_value
     def add_dns_id(self, dns_id, validate=True, ext_value=None):
         if validate:

anchor/X509/signing_request.py

@@ -172,6 +172,26 @@ class X509Csr(signature.SignatureMixin):
 
         ext_attr['vals'][0] = encoder.encode(exts)
 
+    def get_subject_dns_ids(self):
+        names = []
+        for ext in self.get_extensions(extension.X509ExtensionSubjectAltName):
+            for dns_id in ext.get_dns_ids():
+                names.append(dns_id)
+        return names
+
+    def get_subject_ip_ids(self):
+        names = []
+        for ext in self.get_extensions(extension.X509ExtensionSubjectAltName):
+            for ip in ext.get_ips():
+                names.append(ip)
+        return names
+
+    def has_unknown_san_entries(self):
+        for ext in self.get_extensions(extension.X509ExtensionSubjectAltName):
+            if ext.has_unknown_entries():
+                return True
+        return False
+
     def get_public_key_algo(self):
         csr_info = self._csr['certificationRequestInfo']
         key_info = csr_info['subjectPublicKeyInfo']

anchor/validators/custom.py

@@ -243,3 +243,81 @@ def public_key(csr=None, allowed_keys=None, **kwargs):
 
     if csr.get_public_key_size() < min_size:
         raise v_errors.ValidationError("Key size too small")
+
+
+def _split_names_by_type(names):
+    """Identify ips and network ranges in a list of strings."""
+    allowed_domains = []
+    allowed_ips = []
+    allowed_ranges = []
+    for name in names:
+        ip = utils.maybe_ip(name)
+        if ip:
+            allowed_ips.append(ip)
+            continue
+        net = utils.maybe_range(name)
+        if net:
+            allowed_ranges.append(net)
+            continue
+        allowed_domains.append(name)
+
+    return (allowed_domains, allowed_ips, allowed_ranges)
+
+
+def whitelist_names(csr=None, names=[], allow_cn_id=False, allow_dns_id=False,
+                    allow_ip_id=False, allow_wildcard=False, **kwargs):
+    """Ensure names match the whitelist in the allowed name slots."""
+
+    allowed_domains, allowed_ips, allowed_ranges = _split_names_by_type(names)
+
+    for dns_id in csr.get_subject_dns_ids():
+        if not allow_dns_id:
+            raise v_errors.ValidationError("IP-ID not allowed")
+        valid = False
+        for allowed_domain in allowed_domains:
+            if utils.compare_name_pattern(dns_id, allowed_domain,
+                                          allow_wildcard):
+                valid = True
+                break
+        if not valid:
+            raise v_errors.ValidationError(
+                "Value `%s` not allowed in DNS-ID" % (dns_id,))
+
+    for ip_id in csr.get_subject_ip_ids():
+        if not allow_ip_id:
+            raise v_errors.ValidationError("IP-ID not allowed")
+        if ip_id in allowed_ips:
+            continue
+        for net in allowed_ranges:
+            if ip_id in net:
+                continue
+        raise v_errors.ValidationError(
+            "Value `%s` not allowed in IP-ID" % (ip_id,))
+
+    for cn_id in csr.get_subject_cn():
+        if not allow_cn_id:
+            raise v_errors.ValidationError("CN-ID not allowed")
+        ip = utils.maybe_ip(cn_id)
+        if ip:
+            # current CN is an ip address
+            if ip in allowed_ips:
+                continue
+            if any((ip in net) for net in allowed_ranges):
+                continue
+            raise v_errors.ValidationError(
+                "Value `%s` not allowed in CN-ID" % (cn_id,))
+        else:
+            # current CN is a domain
+            valid = False
+            for allowed_domain in allowed_domains:
+                if utils.compare_name_pattern(cn_id, allowed_domain,
+                                              allow_wildcard):
+                    valid = True
+                    break
+            if valid:
+                continue
+            raise v_errors.ValidationError(
+                "Value `%s` not allowed in CN-ID" % (cn_id,))
+
+    if csr.has_unknown_san_entries():
+        raise v_errors.ValidationError("Request contains unknown SAN entries")

anchor/validators/utils.py

@@ -72,3 +72,66 @@ def check_networks(ip, allowed_networks):
         return True
 
     return False
+
+
+def maybe_ip(name):
+    try:
+        return netaddr.IPAddress(name)
+    except ValueError:
+        # happens when trying to pass a subnet prefix
+        return None
+    except netaddr.AddrFormatError:
+        return None
+
+
+def maybe_range(name):
+    try:
+        return netaddr.IPNetwork(name)
+    except netaddr.AddrFormatError:
+        return None
+
+
+def compare_name_pattern(name, pattern, allow_wildcard):
+    """Compare domain names including wildcards.
+
+    Wilcard means local Anchor wildcard which is '%'. This allows the pattern
+    to match an actual wildcard entry (*) or name which can be expanded.
+    Partial matches using % are allowed, but % matches only in one label.
+
+    In practice that means:
+    name:                    pattern:              wildard: result:
+    example.com              example.com           -        match
+    *.example.com            *.example.com         -        match
+    *.example.com            %.example.com         true     match
+    *.example.com            %.example.com         false    fail
+    abc.example.com          %.example.com         -        match
+    abc.def.example.com      %.example.com         -        fail
+    abc.def.example.com      %.%.example.com       -        match
+    host-123.example.com     host-%.example.com    -        match
+    """
+
+    name_labels = name.split('.')
+    patt_labels = pattern.split('.')
+    if len(name_labels) != len(patt_labels):
+        return False
+
+    for nl, pl in zip(name_labels, patt_labels):
+        if '%' in pl:
+            pre, _, post = pl.partition('%')
+
+            if not nl.startswith(pre):
+                return False
+            nl = nl[len(pre):]  # strip the pre part of pattern
+
+            if not nl.endswith(post):
+                return False
+            if len(post) > 0:
+                nl = nl[:-len(post)]  # strip the post part of pattern
+
+            if '*' in nl and not allow_wildcard:
+                return False
+        else:
+            if nl != pl:
+                return False
+
+    return True

doc/source/validators.rst

@@ -21,18 +21,55 @@ The following validators are implemented at the moment:
     Any requests produced using standard tooling that fail this check should be
     reported as Anchor issues.
 
+``whitelist_names``
+    Verifies: CSR. Parameters:
+
+    - ``names``: list of names/ips/ip ranges allowed in various fields
+    - ``allow_cn_id``: allow name in subject CN
+    - ``allow_dns_id``: allow name in SAN dns entry
+    - ``allow_ip_id``: allow name in SAN IP entry
+    - ``allow_wildcard``: allow wildcard certificate to match '%'
+
+    IDs available in various places in the certificate are matched against the
+    patterns in the ``names`` list. These can be:
+
+    - IP addresses: ``1.2.3.4``
+    - IP ranges: ``1.2.3.0/24``
+    - complete names: ``some.example.com``
+    - names with wildcards: ``%.example.com``, ``partial-%.example.com``
+
+    Wildcard (``%``) rules: It matches only a single name label, or part of
+    one. It can be used only in domain names, not IPs. Only one wildcard is
+    allowed in a label, but multiple in a name, so ``%.%.example.com`` is
+    valid.
+
+    Pattern wildcard (``%``) may match a domain wildcard character (``*``)
+    only if ``allow_wildcard`` is set to true.
+
+    This match will fail if the CSR contains any SAN type not included here.
+
+``blacklist_names``
+    Verifies: CSR. Parameters: ``allowed_domains``, ``allowed_networks``.
+
+    Ensures that the CN and subject alternative names do not contain anything
+    configured in the ``domains``.
+
 ``common_name``
     Verifies: CSR. Parameters: ``allowed_domains``, ``allowed_networks``.
 
     Ensures that the CN matches one of names in ``allowed_domains`` or IP
     ranges in ``allowed_networks``.
 
+    Deprecated: use ``whitelist_names`` / ``blacklist_names`` instead.
+
 ``alternative_names``
     Verifies: CSR. Parameters: ``allowed_domains``.
 
     Ensures that names specified in the subject alternative names extension
     match one of the names in ``allowed_domains``.
 
+    Deprecated: use ``whitelist_names`` / ``blacklist_names`` instead.
+
 ``alternative_names_ip``
     Verifies: CSR. Parameters: ``allowed_domains``, ``allowed_networks``.
 
@@ -40,11 +77,7 @@ The following validators are implemented at the moment:
     match one of the names in ``allowed_domains`` or IP ranges in
     ``allowed_networks``.
 
-``blacklist_names``
-    Verifies: CSR. Parameters: ``allowed_domains``, ``allowed_networks``.
-
-    Ensures that the CN and subject alternative names do not contain anything
-    configured in the ``domains``.
+    Deprecated: use ``whitelist_names`` / ``blacklist_names`` instead.
 
 ``server_group``
     Verifies: Auth, CSR. Parameters: ``group_prefixes``.
@@ -59,8 +92,7 @@ The following validators are implemented at the moment:
     Only CN is checked and if there are no dashes in the CN, validation
     succeeds.
 
-    This is not a well designed validator and may not be safe to use! A better
-    version is on the TODO list.
+    Deprecated: use ``whitelist_names`` / ``blacklist_names`` instead.
 
 ``extensions``
     Verifies: CSR. Parameters: ``allowed_extensions``.

setup.cfg

@@ -42,6 +42,7 @@ anchor.validators =
     extensions = anchor.validators.custom:extensions
     key_usage = anchor.validators.custom:key_usage
     source_cidrs = anchor.validators.custom:source_cidrs
+    whitelist_names = anchor.validators.custom:whitelist_names
     standards_compliance = anchor.validators.standards:standards_compliance
 
 anchor.authentication =

tests/validators/test_base_validation_functions.py

@@ -63,3 +63,20 @@ class TestBaseValidators(tests.DefaultRequestMixin, unittest.TestCase):
     def test_check_networks_passthrough(self):
         good_ip = netaddr.IPAddress('10.2.3.4')
         self.assertTrue(utils.check_networks(good_ip, []))
+
+    def test_check_compare_name_pattern(self):
+        cases = [
+            ("example.com", "example.com", False, True),
+            ("*.example.com", "*.example.com", False, True),
+            ("*.example.com", "%.example.com", True, True),
+            ("*.example.com", "%.example.com", False, False),
+            ("abc.example.com", "%.example.com", False, True),
+            ("abc.def.example.com", "%.example.com", False, False),
+            ("abc.def.example.com", "%.%.example.com", False, True),
+            ("host-123.example.com", "host-%.example.com", False, True),
+        ]
+        for value, pattern, wildcard, result in cases:
+            self.assertEqual(
+                result,
+                utils.compare_name_pattern(value, pattern, wildcard),
+                "checking %s against %s failed" % (value, pattern))

tests/validators/test_callable_validators.py

@@ -39,6 +39,19 @@ class TestValidators(tests.DefaultRequestMixin, unittest.TestCase):
     def tearDown(self):
         super(TestValidators, self).tearDown()
 
+    def _csr_with_cn(self, cn):
+        csr = x509_csr.X509Csr()
+        name = csr.get_subject()
+        name.add_name_entry(x509_name.OID_commonName, cn)
+        return csr
+
+    def _csr_with_san_dns(self, dns):
+        csr = x509_csr.X509Csr()
+        ext = x509_ext.X509ExtensionSubjectAltName()
+        ext.add_dns_id(dns)
+        csr.add_extension(ext)
+        return csr
+
     def test_check_networks_good(self):
         allowed_networks = ['15/8', '74.125/16']
         self.assertTrue(utils.check_networks(
@@ -595,3 +608,90 @@ class TestValidators(tests.DefaultRequestMixin, unittest.TestCase):
         csr = x509_csr.X509Csr.from_buffer(self.csr_sample)
         with self.assertRaises(errors.ValidationError):
             custom.public_key(csr=csr, allowed_keys={'XXX': 0})
+
+    def test_whitelist_names_empty_list(self):
+        # empty whitelist should block everything
+        csr = self._csr_with_san_dns('example.com')
+
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, domains=[],)
+
+    def test_whitelist_names_full_dnsid_match(self):
+        csr = self._csr_with_san_dns('example.com')
+        custom.whitelist_names(csr=csr, allow_dns_id=True,
+                               names=['example.com'])
+
+    def test_whitelist_names_partial_dnsid_match(self):
+        csr = self._csr_with_san_dns('good-123.example.com')
+        custom.whitelist_names(csr=csr, allow_dns_id=True,
+                               names=['good-%.example.com'])
+
+    def test_whitelist_names_full_dnsid_fail(self):
+        csr = self._csr_with_san_dns('bad.example.com')
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, allow_dns_id=True,
+                                   names=['good.example.com'])
+
+    def test_whitelist_names_full_ipid_match(self):
+        csr = x509_csr.X509Csr()
+        ext = x509_ext.X509ExtensionSubjectAltName()
+        ext.add_ip(netaddr.IPAddress('1.2.3.4'))
+        csr.add_extension(ext)
+
+        custom.whitelist_names(csr=csr, allow_ip_id=True, names=['1.2.3.4'])
+
+    def test_whitelist_names_full_ipid_fail(self):
+        csr = x509_csr.X509Csr()
+        ext = x509_ext.X509ExtensionSubjectAltName()
+        ext.add_ip(netaddr.IPAddress('4.3.2.1'))
+        csr.add_extension(ext)
+
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, allow_ip_id=True,
+                                   names=['1.2.3.4'])
+
+    def test_whitelist_names_cn_not_allowed(self):
+        csr = self._csr_with_cn("bad.example.com")
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, names=[],)
+
+    def test_whitelist_names_cn_ip_fail(self):
+        csr = self._csr_with_cn("4.3.2.1")
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, allow_cn_id=True,
+                                   names=["1.2.3.4"])
+
+    def test_whitelist_names_cn_ip_match(self):
+        csr = self._csr_with_cn("1.2.3.4")
+        custom.whitelist_names(csr=csr, allow_cn_id=True, names=["1.2.3.4"])
+
+    def test_whitelist_names_cn_ip_net_fail(self):
+        csr = self._csr_with_cn("4.3.2.1")
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, allow_cn_id=True, names=["1/8"])
+
+    def test_whitelist_names_cn_ip_net_match(self):
+        csr = self._csr_with_cn("1.2.3.4")
+        custom.whitelist_names(csr=csr, allow_cn_id=True, names=["1/8"])
+
+    def test_whitelist_names_cn_name_fail(self):
+        csr = self._csr_with_cn("bad.example.com")
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, allow_cn_id=True,
+                                   names=["good.example.com"])
+
+    def test_whitelist_names_cn_name_match(self):
+        csr = self._csr_with_cn("good.example.com")
+        custom.whitelist_names(csr=csr, allow_cn_id=True,
+                               names=["good.example.com"])
+
+    def test_whitelist_names_cn_partial_name_fail(self):
+        csr = self._csr_with_cn("bad.example.com")
+        with self.assertRaises(errors.ValidationError):
+            custom.whitelist_names(csr=csr, allow_cn_id=True,
+                                   names=[".good.example.com"])
+
+    def test_whitelist_names_cn_partial_name_match(self):
+        csr = self._csr_with_cn("good.example.com")
+        custom.whitelist_names(csr=csr, allow_cn_id=True,
+                               names=["%.example.com"])