From a8034bb6cd6cc380cba72ddd462c8f0a876e1642 Mon Sep 17 00:00:00 2001 From: tb Date: Sat, 26 Mar 2022 16:34:21 +0000 Subject: [PATCH] name constraints: be more careful with NULs An IA5STRING is a Pascal string that can have embedded NULs and is not NUL terminated (except that for legacy reasons it happens to be). Instead of taking the strlen(), use the already known ASN.1 length and use strndup() instead of strdup() to generate NUL terminated strings after some existing code has checked that there are no embedded NULs. In v2i_GENERAL_NAME_ex() use %.*s to print the bytes. This is not optimal and might be switched to using strvis() later. ok beck inoguchi jsing --- lib/libcrypto/x509/x509_alt.c | 11 +++++++---- lib/libcrypto/x509/x509_constraints.c | 26 ++++++++++++++++++-------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/lib/libcrypto/x509/x509_alt.c b/lib/libcrypto/x509/x509_alt.c index 845ab1364f7..8656df82b3f 100644 --- a/lib/libcrypto/x509/x509_alt.c +++ b/lib/libcrypto/x509/x509_alt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_alt.c,v 1.11 2022/03/14 21:15:49 tb Exp $ */ +/* $OpenBSD: x509_alt.c,v 1.12 2022/03/26 16:34:21 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project. */ @@ -673,21 +673,24 @@ v2i_GENERAL_NAME_ex(GENERAL_NAME *out, const X509V3_EXT_METHOD *method, case GEN_DNS: if (!x509_constraints_valid_sandns(bytes, len)) { X509V3error(X509V3_R_BAD_OBJECT); - ERR_asprintf_error_data("name=%s value='%s'", name, bytes); + ERR_asprintf_error_data("name=%s value='%.*s'", name, + (int)len, bytes); goto err; } break; case GEN_URI: if (!x509_constraints_uri_host(bytes, len, NULL)) { X509V3error(X509V3_R_BAD_OBJECT); - ERR_asprintf_error_data("name=%s value='%s'", name, bytes); + ERR_asprintf_error_data("name=%s value='%.*s'", name, + (int)len, bytes); goto err; } break; case GEN_EMAIL: if (!x509_constraints_parse_mailbox(bytes, len, NULL)) { X509V3error(X509V3_R_BAD_OBJECT); - ERR_asprintf_error_data("name=%s value='%s'", name, bytes); + ERR_asprintf_error_data("name=%s value='%.*s'", name, + (int)len, bytes); goto err; } break; diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c index 4f24277918f..533bbbf4cad 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.25 2022/03/14 21:29:46 tb Exp $ */ +/* $OpenBSD: x509_constraints.c,v 1.26 2022/03/26 16:34:21 tb Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -657,35 +657,45 @@ x509_constraints_general_to_bytes(GENERAL_NAME *name, uint8_t **bytes, if (name->type == GEN_DNS) { ASN1_IA5STRING *aname = name->d.dNSName; + *bytes = aname->data; - *len = strlen(aname->data); + *len = aname->length; + return name->type; } if (name->type == GEN_EMAIL) { ASN1_IA5STRING *aname = name->d.rfc822Name; + *bytes = aname->data; - *len = strlen(aname->data); + *len = aname->length; + return name->type; } if (name->type == GEN_URI) { ASN1_IA5STRING *aname = name->d.uniformResourceIdentifier; + *bytes = aname->data; - *len = strlen(aname->data); + *len = aname->length; + return name->type; } if (name->type == GEN_DIRNAME) { X509_NAME *dname = name->d.directoryName; + if (!dname->modified || i2d_X509_NAME(dname, NULL) >= 0) { *bytes = dname->canon_enc; *len = dname->canon_enclen; + return name->type; } } if (name->type == GEN_IPADD) { *bytes = name->d.ip->data; *len = name->d.ip->length; + return name->type; } + return 0; } @@ -723,7 +733,7 @@ x509_constraints_extract_names(struct x509_constraints_names *names, *error = X509_V_ERR_UNSUPPORTED_NAME_SYNTAX; goto err; } - if ((vname->name = strdup(bytes)) == NULL) { + if ((vname->name = strndup(bytes, len)) == NULL) { *error = X509_V_ERR_OUT_OF_MEM; goto err; } @@ -931,7 +941,7 @@ x509_constraints_validate(GENERAL_NAME *constraint, case GEN_DNS: if (!x509_constraints_valid_domain_constraint(bytes, len)) goto err; - if ((name->name = strdup(bytes)) == NULL) { + if ((name->name = strndup(bytes, len)) == NULL) { error = X509_V_ERR_OUT_OF_MEM; goto err; } @@ -953,7 +963,7 @@ x509_constraints_validate(GENERAL_NAME *constraint, } if (!x509_constraints_valid_domain_constraint(bytes, len)) goto err; - if ((name->name = strdup(bytes)) == NULL) { + if ((name->name = strndup(bytes, len)) == NULL) { error = X509_V_ERR_OUT_OF_MEM; goto err; } @@ -973,7 +983,7 @@ x509_constraints_validate(GENERAL_NAME *constraint, case GEN_URI: if (!x509_constraints_valid_domain_constraint(bytes, len)) goto err; - if ((name->name = strdup(bytes)) == NULL) { + if ((name->name = strndup(bytes, len)) == NULL) { error = X509_V_ERR_OUT_OF_MEM; goto err; } -- 2.20.1