Allow IP addresses to be specified in a URI.

Our checking here was a bit too aggressive, and did not permit an
IP address in a URI. IP's in a URI are allowed for things like CRLdp's
AIA, SAN URI's etc.). The check for this was also slightly flawed as
we would permit an IP if memory allocation failed while checking for
an IP.

Correct both issues.

ok tb@

3 files changed, 70 insertions, 19 deletions

diff --git a/src/lib/libcrypto/x509/x509_constraints.c b/src/lib/libcrypto/x509/x509_constraints.c
index 346cab0a40..0773d2ba71 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.31 2022/12/26 07:18:53 jmc Exp $ */
+/* $OpenBSD: x509_constraints.c,v 1.32 2023/09/29 15:53:59 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -38,23 +38,23 @@
 #define MAX_IP_ADDRESS_LENGTH (size_t)46
 
 static int
-cbs_is_ip_address(CBS *cbs)
+cbs_is_ip_address(CBS *cbs, int *is_ip)
 {
         struct sockaddr_in6 sin6;
         struct sockaddr_in sin4;
         char *name = NULL;
-        int ret = 0;
 
+        *is_ip = 0;
         if (CBS_len(cbs) > MAX_IP_ADDRESS_LENGTH)
-                return 0;
+                return 1;
         if (!CBS_strdup(cbs, &name))
                 return 0;
         if (inet_pton(AF_INET, name, &sin4) == 1 ||
             inet_pton(AF_INET6, name, &sin6) == 1)
-                ret = 1;
+                *is_ip = 1;
 
         free(name);
-        return ret;
+        return 1;
 }
 
 struct x509_constraints_name *
@@ -264,16 +264,21 @@ x509_constraints_valid_domain_internal(CBS *cbs, int wildcards)
 }
 
 int
-x509_constraints_valid_host(CBS *cbs)
+x509_constraints_valid_host(CBS *cbs, int permit_ip)
 {
         uint8_t first;
+        int is_ip;
 
         if (!CBS_peek_u8(cbs, &first))
                 return 0;
         if (first == '.')
-                return 0; /* leading . not allowed in a host name */
-        if (cbs_is_ip_address(cbs))
-                return 0;
+                return 0; /* leading . not allowed in a host name or IP */
+        if (!permit_ip) {
+                if (!cbs_is_ip_address(cbs, &is_ip))
+                        return 0;
+                if (is_ip)
+                        return 0;
+        }
 
         return x509_constraints_valid_domain_internal(cbs, 0);
 }
@@ -441,7 +446,7 @@ x509_constraints_parse_mailbox(CBS *candidate,
         if (candidate_local == NULL || candidate_domain == NULL)
                 goto bad;
         CBS_init(&domain_cbs, candidate_domain, strlen(candidate_domain));
-        if (!x509_constraints_valid_host(&domain_cbs))
+        if (!x509_constraints_valid_host(&domain_cbs, 0))
                 goto bad;
 
         if (name != NULL) {
@@ -558,7 +563,7 @@ x509_constraints_uri_host(uint8_t *uri, size_t len, char **hostpart)
         if (host == NULL)
                 host = authority;
         CBS_init(&host_cbs, host, hostlen);
-        if (!x509_constraints_valid_host(&host_cbs))
+        if (!x509_constraints_valid_host(&host_cbs, 1))
                 return 0;
         if (hostpart != NULL && !CBS_strdup(&host_cbs, hostpart))
                 return 0;
@@ -924,7 +929,7 @@ x509_constraints_extract_names(struct x509_constraints_names *names,
                                 goto err;
                         }
                         CBS_init(&cbs, aname->data, aname->length);
-                        if (!x509_constraints_valid_host(&cbs))
+                        if (!x509_constraints_valid_host(&cbs, 0))
                                 continue; /* ignore it if not a hostname */
                         if ((vname = x509_constraints_name_new()) == NULL) {
                                 *error = X509_V_ERR_OUT_OF_MEM;
diff --git a/src/lib/libcrypto/x509/x509_internal.h b/src/lib/libcrypto/x509/x509_internal.h
index c4222bcfe5..15efff6097 100644
--- a/src/lib/libcrypto/x509/x509_internal.h
+++ b/src/lib/libcrypto/x509/x509_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_internal.h,v 1.25 2023/01/28 19:08:09 tb Exp $ */
+/* $OpenBSD: x509_internal.h,v 1.26 2023/09/29 15:53:59 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -111,7 +111,7 @@ struct x509_constraints_names *x509_constraints_names_new(size_t names_max);
 int x509_constraints_general_to_bytes(GENERAL_NAME *name, uint8_t **bytes,
     size_t *len);
 void x509_constraints_names_free(struct x509_constraints_names *names);
-int x509_constraints_valid_host(CBS *cbs);
+int x509_constraints_valid_host(CBS *cbs, int permit_ip);
 int x509_constraints_valid_sandns(CBS *cbs);
 int x509_constraints_domain(char *domain, size_t dlen, char *constraint,
     size_t len);
diff --git a/src/regress/lib/libcrypto/x509/constraints.c b/src/regress/lib/libcrypto/x509/constraints.c
index 8771367bd6..90b7ffbaeb 100644
--- a/src/regress/lib/libcrypto/x509/constraints.c
+++ b/src/regress/lib/libcrypto/x509/constraints.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: constraints.c,v 1.15 2022/11/28 07:24:03 tb Exp $     */
+/*      $OpenBSD: constraints.c,v 1.16 2023/09/29 15:53:59 beck Exp $   */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -154,6 +154,12 @@ unsigned char *invaliduri[] = {
         "https://.www.openbsd.org/",
         "https://www.ope|nbsd.org%",
         "https://www.openbsd.org.#",
+        "https://192.168.1.1./",
+        "https://192.168.1.1|/",
+        "https://.192.168.1.1/",
+        "https://192.168..1.1/",
+        "https://.2001:0DB8:AC10:FE01::/",
+        "https://.2001:0DB8:AC10:FE01::|/",
         "///",
         "//",
         "/",
@@ -161,6 +167,15 @@ unsigned char *invaliduri[] = {
         NULL,
 };
 
+unsigned char *validuri[] = {
+        "https://www.openbsd.org/meep/meep/meep/",
+        "https://192.168.1.1/",
+        "https://2001:0DB8:AC10:FE01::/",
+        "https://192.168.1/",  /* Not an IP, but valid component */
+        "https://999.999.999.999/", /* Not an IP, but valid component */
+        NULL,
+};
+
 static int
 test_valid_hostnames(void)
 {
@@ -169,7 +184,7 @@ test_valid_hostnames(void)
         for (i = 0; valid_hostnames[i] != NULL; i++) {
                 CBS cbs;
                 CBS_init(&cbs, valid_hostnames[i], strlen(valid_hostnames[i]));
-                if (!x509_constraints_valid_host(&cbs)) {
+                if (!x509_constraints_valid_host(&cbs, 0)) {
                         FAIL("Valid hostname '%s' rejected\n",
                             valid_hostnames[i]);
                         failure = 1;
@@ -183,6 +198,7 @@ test_valid_hostnames(void)
                         goto done;
                 }
         }
+
  done:
         return failure;
 }
@@ -202,6 +218,7 @@ test_valid_sandns_names(void)
                         goto done;
                 }
         }
+
  done:
         return failure;
 }
@@ -221,6 +238,7 @@ test_valid_domain_constraints(void)
                         goto done;
                 }
         }
+
  done:
         return failure;
 }
@@ -245,6 +263,7 @@ test_valid_mbox_names(void)
                 free(name.local);
                 name.local = NULL;
         }
+
  done:
         return failure;
 }
@@ -259,7 +278,7 @@ test_invalid_hostnames(void)
         for (i = 0; invalid_hostnames[i] != NULL; i++) {
                 CBS_init(&cbs, invalid_hostnames[i],
                     strlen(invalid_hostnames[i]));
-                if (x509_constraints_valid_host(&cbs)) {
+                if (x509_constraints_valid_host(&cbs, 0)) {
                         FAIL("Invalid hostname '%s' accepted\n",
                             invalid_hostnames[i]);
                         failure = 1;
@@ -267,7 +286,7 @@ test_invalid_hostnames(void)
                 }
         }
         CBS_init(&cbs, nulhost, strlen(nulhost) + 1);
-        if (x509_constraints_valid_host(&cbs)) {
+        if (x509_constraints_valid_host(&cbs, 0)) {
                 FAIL("hostname with NUL byte accepted\n");
                 failure = 1;
                 goto done;
@@ -278,6 +297,7 @@ test_invalid_hostnames(void)
                 failure = 1;
                 goto done;
         }
+
  done:
         return failure;
 }
@@ -297,6 +317,7 @@ test_invalid_sandns_names(void)
                         goto done;
                 }
         }
+
  done:
         return failure;
 }
@@ -321,6 +342,7 @@ test_invalid_mbox_names(void)
                 free(name.local);
                 name.local = NULL;
         }
+
  done:
         return failure;
 }
@@ -340,6 +362,7 @@ test_invalid_domain_constraints(void)
                         goto done;
                 }
         }
+
  done:
         return failure;
 }
@@ -365,6 +388,27 @@ test_invalid_uri(void)
  done:
         return failure;
 }
+static int
+test_valid_uri(void)
+{
+        int j, failure = 0;
+        char *hostpart = NULL;
+
+        for (j = 0; validuri[j] != NULL; j++) {
+                if (x509_constraints_uri_host(validuri[j],
+                    strlen(invaliduri[j]), &hostpart) == 0) {
+                        FAIL("Valid URI '%s' NOT accepted\n",
+                            validuri[j]);
+                        failure = 1;
+                        goto done;
+                }
+                free(hostpart);
+                hostpart = NULL;
+        }
+
+ done:
+        return failure;
+}
 
 static int
 test_constraints1(void)
@@ -513,6 +557,7 @@ test_constraints1(void)
                 failure = 1;
                 goto done;
         }
+
  done:
         return failure;
 }
@@ -531,6 +576,7 @@ main(int argc, char **argv)
         failed |= test_valid_domain_constraints();
         failed |= test_invalid_domain_constraints();
         failed |= test_invalid_uri();
+        failed |= test_valid_uri();
         failed |= test_constraints1();
 
         return (failed);