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@

2 files changed, 25 insertions, 27 deletions

diff --git a/src/lib/libcrypto/x509/x509_constraints.c b/src/lib/libcrypto/x509/x509_constraints.c
index 5fbcef304f..fade58c620 100644
--- a/src/lib/libcrypto/x509/x509_constraints.c
+++ b/src/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/src/regress/lib/libcrypto/x509/constraints.c b/src/regress/lib/libcrypto/x509/constraints.c
index 7eef55d591..c4dedeb1fa 100644
--- a/src/regress/lib/libcrypto/x509/constraints.c
+++ b/src/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)) {