diff options
Diffstat (limited to '')
| -rw-r--r-- | tls_verify.c | 101 |
1 files changed, 74 insertions, 27 deletions
diff --git a/tls_verify.c b/tls_verify.c index 0cb86f6..c588f02 100644 --- a/tls_verify.c +++ b/tls_verify.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls_verify.c,v 1.23 2023/05/11 07:35:27 tb Exp $ */ +/* $OpenBSD: tls_verify.c,v 1.28 2023/06/01 07:32:25 tb Exp $ */ /* * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org> * @@ -92,15 +92,21 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name, union tls_addr addrbuf; int addrlen, type; int count, i; - int rv = 0; + int critical = 0; + int rv = -1; *alt_match = 0; *alt_exists = 0; - altname_stack = X509_get_ext_d2i(cert, NID_subject_alt_name, - NULL, NULL); - if (altname_stack == NULL) - return 0; + altname_stack = X509_get_ext_d2i(cert, NID_subject_alt_name, &critical, + NULL); + if (altname_stack == NULL) { + if (critical != -1) { + tls_set_errorx(ctx, "error decoding subjectAltName"); + goto err; + } + goto done; + } if (inet_pton(AF_INET, name, &addrbuf) == 1) { type = GEN_IPADD; @@ -140,8 +146,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name, "NUL byte in subjectAltName, " "probably a malicious certificate", name); - rv = -1; - break; + goto err; } /* @@ -154,13 +159,12 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name, "error verifying name '%s': " "a dNSName of \" \" must not be " "used", name); - rv = -1; - break; + goto err; } if (tls_match_name(data, name) == 0) { *alt_match = 1; - break; + goto done; } } else { #ifdef DEBUG @@ -181,8 +185,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name, tls_set_errorx(ctx, "Unexpected negative length for an " "IP address: %d", datalen); - rv = -1; - break; + goto err; } /* @@ -192,11 +195,15 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name, if (datalen == addrlen && memcmp(data, &addrbuf, addrlen) == 0) { *alt_match = 1; - break; + goto done; } } } + done: + rv = 0; + + err: sk_GENERAL_NAME_pop_free(altname_stack, GENERAL_NAME_free); return rv; } @@ -205,10 +212,13 @@ static int tls_check_common_name(struct tls *ctx, X509 *cert, const char *name, int *cn_match) { + unsigned char *utf8_bytes = NULL; X509_NAME *subject_name; char *common_name = NULL; union tls_addr addrbuf; int common_name_len; + ASN1_STRING *data; + int lastpos = -1; int rv = -1; *cn_match = 0; @@ -217,29 +227,65 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name, if (subject_name == NULL) goto done; - common_name_len = X509_NAME_get_text_by_NID(subject_name, - NID_commonName, NULL, 0); - if (common_name_len < 0) + lastpos = X509_NAME_get_index_by_NID(subject_name, + NID_commonName, lastpos); + if (lastpos == -1) goto done; - - common_name = calloc(common_name_len + 1, 1); - if (common_name == NULL) { - tls_set_error(ctx, "out of memory"); + if (lastpos < 0) + goto err; + if (X509_NAME_get_index_by_NID(subject_name, NID_commonName, lastpos) + != -1) { + /* + * Having multiple CN's is possible, and even happened back in + * the glory days of mullets and Hammer pants. In anything like + * a modern TLS cert, CN is as close to deprecated as it gets, + * and having more than one is bad. We therefore fail if we have + * more than one CN fed to us in the subject, treating the + * certificate as hostile. + */ + tls_set_errorx(ctx, "error verifying name '%s': " + "Certificate subject contains mutiple Common Name fields, " + "probably a malicious or malformed certificate", name); goto err; } - X509_NAME_get_text_by_NID(subject_name, NID_commonName, common_name, - common_name_len + 1); - - /* NUL bytes in CN? */ - if (common_name_len < 0 || - (size_t)common_name_len != strlen(common_name)) { + data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(subject_name, + lastpos)); + /* + * Fail if we cannot encode the CN bytes as UTF-8. + */ + if ((common_name_len = ASN1_STRING_to_UTF8(&utf8_bytes, data)) < 0) { + tls_set_errorx(ctx, "error verifying name '%s': " + "Common Name field cannot be encoded as a UTF-8 string, " + "probably a malicious certificate", name); + goto err; + } + /* + * Fail if the CN is of invalid length. RFC 5280 specifies that a CN + * must be between 1 and 64 bytes long. + */ + if (common_name_len < 1 || common_name_len > 64) { + tls_set_errorx(ctx, "error verifying name '%s': " + "Common Name field has invalid length, " + "probably a malicious certificate", name); + goto err; + } + /* + * Fail if the resulting text contains a NUL byte. + */ + if (memchr(utf8_bytes, 0, common_name_len) != NULL) { tls_set_errorx(ctx, "error verifying name '%s': " "NUL byte in Common Name field, " "probably a malicious certificate", name); goto err; } + common_name = strndup(utf8_bytes, common_name_len); + if (common_name == NULL) { + tls_set_error(ctx, "out of memory"); + goto err; + } + /* * We don't want to attempt wildcard matching against IP addresses, * so perform a simple comparison here. @@ -258,6 +304,7 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name, rv = 0; err: + free(utf8_bytes); free(common_name); return rv; } |