Relax SAN DNSname validation and constraints to permit non leading *

wildcards. While we may choose not to support them the standards
appear to permit them optionally so we can't declare a certificate
containing them invalid. Noticed by jeremy@, and Steffan Ulrich
and others. Modify the regression tests to test these cases and
not check the SAN DNSnames as "hostnames" anymore (which don't support
wildcards).

ok jsing@, tb@

diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c
index 5fbcef3..fade58c 100644 (file)
--- a/lib/libcrypto/x509/x509_constraints.c
+++ b/lib/libcrypto/x509/x509_constraints.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_constraints.c,v 1.15 2021/03/12 15:57:30 tb Exp $ */
+/* $OpenBSD: x509_constraints.c,v 1.16 2021/04/27 03:35:29 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -169,13 +169,15 @@ x509_constraints_names_dup(struct x509_constraints_names *names)
 /*
  * Validate that the name contains only a hostname consisting of RFC
  * 5890 compliant A-labels (see RFC 6066 section 3). This is more
- * permissive to allow for a leading '*' for a SAN DNSname wildcard,
- * or a leading '.'  for a subdomain based constraint, as well as
- * allowing for '_' which is commonly accepted by nonconformant
- * DNS implementaitons.
+ * permissive to allow for a leading '.'  for a subdomain based
+ * constraint, as well as allowing for '_' which is commonly accepted
+ * by nonconformant DNS implementaitons.
+ *
+ * if "wildcards" is set it allows '*' to occur in the string at the end of a
+ * component.
  */
 static int
-x509_constraints_valid_domain_internal(uint8_t *name, size_t len)
+x509_constraints_valid_domain_internal(uint8_t *name, size_t len, int wildcards)
 {
        uint8_t prev, c = 0;
        int component = 0;
@@ -198,8 +200,8 @@ x509_constraints_valid_domain_internal(uint8_t *name, size_t len)
                if (!isalnum(c) && c != '-' && c != '.' && c != '_' && c != '*')
                        return 0;
 
-               /* '*' can only be the first thing. */
-               if (c == '*' && !first)
+               /* if it is a '*', fail if not wildcards */
+               if (!wildcards && c == '*')
                        return 0;
 
                /* '-' must not start a component or be at the end. */
@@ -221,6 +223,13 @@ x509_constraints_valid_domain_internal(uint8_t *name, size_t len)
                        component = 0;
                        continue;
                }
+               /*
+                * Wildcards can only occur at the end of a component.
+                * c*.com is valid, c*c.com is not.
+                */
+               if (prev == '*')
+                       return 0;
+
                /* Components must be 63 chars or less. */
                if (++component > 63)
                        return 0;
@@ -233,15 +242,13 @@ x509_constraints_valid_domain(uint8_t *name, size_t len)
 {
        if (len == 0)
                return 0;
-       if (name[0] == '*') /* wildcard not allowed in a domain name */
-               return 0;
        /*
         * A domain may not be less than two characters, so you can't
         * have a require subdomain name with less than that.
         */
        if (len < 3 && name[0] == '.')
                return 0;
-       return x509_constraints_valid_domain_internal(name, len);
+       return x509_constraints_valid_domain_internal(name, len, 0);
 }
 
 int
@@ -252,15 +259,13 @@ x509_constraints_valid_host(uint8_t *name, size_t len)
 
        if (len == 0)
                return 0;
-       if (name[0] == '*') /* wildcard not allowed in a host name */
-               return 0;
        if (name[0] == '.') /* leading . not allowed in a host name*/
                return 0;
        if (inet_pton(AF_INET, name, &sin4) == 1)
                return 0;
        if (inet_pton(AF_INET6, name, &sin6) == 1)
                return 0;
-       return x509_constraints_valid_domain_internal(name, len);
+       return x509_constraints_valid_domain_internal(name, len, 0);
 }
 
 int
@@ -283,7 +288,7 @@ x509_constraints_valid_sandns(uint8_t *name, size_t len)
        if (len >= 4 && name[0] == '*' && name[1] != '.')
                return 0;
 
-       return x509_constraints_valid_domain_internal(name, len);
+       return x509_constraints_valid_domain_internal(name, len, 1);
 }
 
 static inline int
@@ -431,16 +436,13 @@ x509_constraints_valid_domain_constraint(uint8_t *constraint, size_t len)
        if (len == 0)
                return 1;       /* empty constraints match */
 
-       if (constraint[0] == '*') /* wildcard not allowed in a constraint */
-               return 0;
-
        /*
         * A domain may not be less than two characters, so you
         * can't match a single domain of less than that
         */
        if (len < 3 && constraint[0] == '.')
                return 0;
-       return x509_constraints_valid_domain_internal(constraint, len);
+       return x509_constraints_valid_domain_internal(constraint, len, 0);
 }
 
 /*

diff --git a/regress/lib/libcrypto/x509/constraints.c b/regress/lib/libcrypto/x509/constraints.c
index 7eef55d..c4dedeb 100644 (file)
--- a/regress/lib/libcrypto/x509/constraints.c
+++ b/regress/lib/libcrypto/x509/constraints.c
@@ -49,6 +49,8 @@ unsigned char *valid_hostnames[] = {
 unsigned char *valid_sandns_names[] = {
        "*.ca",
        "*.op3nbsd.org",
+       "c*.openbsd.org",
+       "foo.*.d*.c*.openbsd.org",
        NULL,
 };
 
@@ -96,6 +98,7 @@ unsigned char *invalid_hostnames[] = {
        "openbsd\n.org",
        "open\178bsd.org",
        "open\255bsd.org",
+       "*.openbsd.org",
        NULL,
 };
 
@@ -110,10 +113,10 @@ unsigned char *invalid_sandns_names[] = {
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.a",
        "*.-p3nbsd.org",
-       "a*.openbsd.org",
        "*.*..openbsd.org",
        "*..openbsd.org",
        ".openbsd.org",
+       "c*c.openbsd.org",
        NULL,
 };
 
@@ -254,13 +257,6 @@ test_invalid_hostnames(void)
                        failure = 1;
                        goto done;
                }
-               if (x509_constraints_valid_sandns(invalid_hostnames[i],
-                   strlen(invalid_hostnames[i]))) {
-                       FAIL("Invalid sandns '%s' accepted\n",
-                           invalid_hostnames[i]);
-                       failure = 1;
-                       goto done;
-               }
        }
        if (x509_constraints_valid_host(nulhost,
            strlen(nulhost) + 1)) {