From c57a19656a888a40c309a73d0264dad3fab27857 Mon Sep 17 00:00:00 2001 From: beck Date: Fri, 29 Sep 2023 15:53:59 +0000 Subject: [PATCH] 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@ --- lib/libcrypto/x509/x509_constraints.c | 31 ++++++++------ lib/libcrypto/x509/x509_internal.h | 4 +- regress/lib/libcrypto/x509/constraints.c | 54 ++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c index 346cab0a403..0773d2ba719 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.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 * @@ -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/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h index c4222bcfe55..15efff60975 100644 --- a/lib/libcrypto/x509/x509_internal.h +++ b/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 * @@ -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/regress/lib/libcrypto/x509/constraints.c b/regress/lib/libcrypto/x509/constraints.c index 8771367bd6c..90b7ffbaebc 100644 --- a/regress/lib/libcrypto/x509/constraints.c +++ b/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 * @@ -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); -- 2.20.1