From fd01767793028d1ff5612c60c6faf63139eb9e31 Mon Sep 17 00:00:00 2001 From: beck Date: Tue, 27 Apr 2021 03:35:29 +0000 Subject: [PATCH] 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@ --- lib/libcrypto/x509/x509_constraints.c | 40 +++++++++++++----------- regress/lib/libcrypto/x509/constraints.c | 12 +++---- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c index 5fbcef304fb..fade58c620f 100644 --- 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 * @@ -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 7eef55d591c..c4dedeb1fad 100644 --- 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)) { -- 2.20.1