From a7f2167b9666a92d0ae8ad6930786d18d2ce8cc7 Mon Sep 17 00:00:00 2001 From: tb Date: Mon, 14 Mar 2022 21:15:49 +0000 Subject: [PATCH] Rework ownership handling in x509_constraints_validate() Instead of having the caller allocate and pass in a new x509_constraints_name struct, handle allocation inside x509_constraints_validate(). Also make the error optional. All this is done to simplify the call sites and to make it more obvious that there are no leaks. ok jsing --- lib/libcrypto/x509/x509_alt.c | 11 ++-- lib/libcrypto/x509/x509_constraints.c | 72 ++++++++++++++++----------- lib/libcrypto/x509/x509_internal.h | 5 +- 3 files changed, 49 insertions(+), 39 deletions(-) diff --git a/lib/libcrypto/x509/x509_alt.c b/lib/libcrypto/x509/x509_alt.c index 35aae6f185f..845ab1364f7 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.10 2022/03/13 16:48:49 tb Exp $ */ +/* $OpenBSD: x509_alt.c,v 1.11 2022/03/14 21:15:49 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project. */ @@ -657,17 +657,14 @@ v2i_GENERAL_NAME_ex(GENERAL_NAME *out, const X509V3_EXT_METHOD *method, */ if (is_nc) { - struct x509_constraints_name constraints_name; - int error = 0; + struct x509_constraints_name *constraints_name = NULL; - memset(&constraints_name, 0, sizeof(constraints_name)); - type = x509_constraints_validate(ret, &constraints_name, &error); - if (type == 0 || error != 0) { + if (!x509_constraints_validate(ret, &constraints_name, NULL)) { X509V3error(X509V3_R_BAD_OBJECT); ERR_asprintf_error_data("name=%s", name); goto err; } - x509_constraints_name_clear(&constraints_name); + x509_constraints_name_free(constraints_name); return ret; } diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c index 27d87d4c11e..6e88a941892 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.23 2022/03/13 17:23:02 tb Exp $ */ +/* $OpenBSD: x509_constraints.c,v 1.24 2022/03/14 21:15:49 tb Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -896,21 +896,34 @@ x509_constraints_extract_names(struct x509_constraints_names *names, */ int x509_constraints_validate(GENERAL_NAME *constraint, - struct x509_constraints_name *name, int *error) + struct x509_constraints_name **out_name, int *out_error) { uint8_t *bytes = NULL; size_t len = 0; + struct x509_constraints_name *name; + int error = X509_V_ERR_UNSUPPORTED_CONSTRAINT_SYNTAX; int name_type; + if (out_name == NULL || *out_name != NULL) + return 0; + + if (out_error != NULL) + *out_error = 0; + + if ((name = x509_constraints_name_new()) == NULL) { + error = X509_V_ERR_OUT_OF_MEM; + goto err; + } + name_type = x509_constraints_general_to_bytes(constraint, &bytes, &len); switch (name_type) { case GEN_DIRNAME: - if (bytes == NULL || (name->der = malloc(len)) == NULL) { - *error = X509_V_ERR_OUT_OF_MEM; - return 0; - } if (len == 0) goto err; /* XXX The RFCs are delightfully vague */ + if (bytes == NULL || (name->der = malloc(len)) == NULL) { + error = X509_V_ERR_OUT_OF_MEM; + goto err; + } memcpy(name->der, bytes, len); name->der_len = len; name->type = GEN_DIRNAME; @@ -919,8 +932,8 @@ x509_constraints_validate(GENERAL_NAME *constraint, if (!x509_constraints_valid_domain_constraint(bytes, len)) goto err; if ((name->name = strdup(bytes)) == NULL) { - *error = X509_V_ERR_OUT_OF_MEM; - return 0; + error = X509_V_ERR_OUT_OF_MEM; + goto err; } name->type = GEN_DNS; break; @@ -933,8 +946,8 @@ x509_constraints_validate(GENERAL_NAME *constraint, len)) goto err; if ((name->name = strdup(bytes)) == NULL) { - *error = X509_V_ERR_OUT_OF_MEM; - return 0; + error = X509_V_ERR_OUT_OF_MEM; + goto err; } } name->type = GEN_EMAIL; @@ -954,17 +967,24 @@ x509_constraints_validate(GENERAL_NAME *constraint, if (!x509_constraints_valid_domain_constraint(bytes, len)) goto err; if ((name->name = strdup(bytes)) == NULL) { - *error = X509_V_ERR_OUT_OF_MEM; - return 0; + error = X509_V_ERR_OUT_OF_MEM; + goto err; } name->type = GEN_URI; break; default: break; } + + *out_name = name; + return 1; + err: - *error = X509_V_ERR_UNSUPPORTED_CONSTRAINT_SYNTAX; + x509_constraints_name_free(name); + if (out_error != NULL) + *out_error = error; + return 0; } @@ -974,7 +994,7 @@ x509_constraints_extract_constraints(X509 *cert, struct x509_constraints_names *excluded, int *error) { - struct x509_constraints_name *vname; + struct x509_constraints_name *vname = NULL; NAME_CONSTRAINTS *nc = cert->nc; GENERAL_SUBTREE *subtree; int i; @@ -989,24 +1009,20 @@ x509_constraints_extract_constraints(X509 *cert, *error = X509_V_ERR_SUBTREE_MINMAX; return 0; } - if ((vname = x509_constraints_name_new()) == NULL) { - *error = X509_V_ERR_OUT_OF_MEM; - return 0; - } - if (x509_constraints_validate(subtree->base, vname, error) == - 0) { - x509_constraints_name_free(vname); + if (!x509_constraints_validate(subtree->base, &vname, error)) return 0; - } if (vname->type == 0) { x509_constraints_name_free(vname); + vname = NULL; continue; } if (!x509_constraints_names_add(permitted, vname)) { x509_constraints_name_free(vname); + vname = NULL; *error = X509_V_ERR_OUT_OF_MEM; return 0; } + vname = NULL; } for (i = 0; i < sk_GENERAL_SUBTREE_num(nc->excludedSubtrees); i++) { @@ -1015,24 +1031,20 @@ x509_constraints_extract_constraints(X509 *cert, *error = X509_V_ERR_SUBTREE_MINMAX; return 0; } - if ((vname = x509_constraints_name_new()) == NULL) { - *error = X509_V_ERR_OUT_OF_MEM; + if (!x509_constraints_validate(subtree->base, &vname, error)) return 0; - } - if (x509_constraints_validate(subtree->base, vname, error) == - 0) { - x509_constraints_name_free(vname); - return 0; - } if (vname->type == 0) { x509_constraints_name_free(vname); + vname = NULL; continue; } if (!x509_constraints_names_add(excluded, vname)) { x509_constraints_name_free(vname); + vname = NULL; *error = X509_V_ERR_OUT_OF_MEM; return 0; } + vname = NULL; } return 1; diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h index c64992a9ee7..c6ce5229ade 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.17 2022/03/13 17:08:04 tb Exp $ */ +/* $OpenBSD: x509_internal.h,v 1.18 2022/03/14 21:15:49 tb Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -101,6 +101,7 @@ time_t x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notafter); struct x509_verify_ctx *x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc); void x509_constraints_name_clear(struct x509_constraints_name *name); +void x509_constraints_name_free(struct x509_constraints_name *name); int x509_constraints_names_add(struct x509_constraints_names *names, struct x509_constraints_name *name); struct x509_constraints_names *x509_constraints_names_dup( @@ -127,7 +128,7 @@ int x509_constraints_extract_constraints(X509 *cert, struct x509_constraints_names *permitted, struct x509_constraints_names *excluded, int *error); int x509_constraints_validate(GENERAL_NAME *constraint, - struct x509_constraints_name *name, int *error); + struct x509_constraints_name **out_name, int *error); int x509_constraints_check(struct x509_constraints_names *names, struct x509_constraints_names *permitted, struct x509_constraints_names *excluded, int *error); -- 2.20.1