From b6c35519473d1d304e593201b7c5ee915c36084b Mon Sep 17 00:00:00 2001 From: tb Date: Sun, 28 May 2023 05:25:24 +0000 Subject: [PATCH] Merge X509_VERIFY_PARAM_ID into X509_VERIFY_PARAM Back in the day when essentially every struct was open to all applications, X509_VERIFY_PARAM_ID provided a modicum of opacity. This indirection is now no longer needed with X509_VERIFY_PARAM being opaque itself, so stop using X509_VERIFY_PARAM_ID and merge it into X509_VERIFY_PARAM. This is a first small step towards cleaning up the X509_VERIFY_PARAM mess. ok jsing --- lib/libcrypto/x509/x509_local.h | 22 +++--- lib/libcrypto/x509/x509_vfy.c | 25 +++--- lib/libcrypto/x509/x509_vfy.h | 4 +- lib/libcrypto/x509/x509_vpm.c | 132 ++++++++++++-------------------- 4 files changed, 73 insertions(+), 110 deletions(-) diff --git a/lib/libcrypto/x509/x509_local.h b/lib/libcrypto/x509/x509_local.h index 9ce1b58ed14..f00a55bac8b 100644 --- a/lib/libcrypto/x509/x509_local.h +++ b/lib/libcrypto/x509/x509_local.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_local.h,v 1.8 2023/05/08 14:51:00 tb Exp $ */ +/* $OpenBSD: x509_local.h,v 1.9 2023/05/28 05:25:24 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2013. */ @@ -272,7 +272,14 @@ struct X509_VERIFY_PARAM_st { int depth; /* Verify depth */ int security_level; /* 'Security level', see SP800-57. */ STACK_OF(ASN1_OBJECT) *policies; /* Permissible policies */ - X509_VERIFY_PARAM_ID *id; /* opaque ID data */ + STACK_OF(OPENSSL_STRING) *hosts; /* Set of acceptable names */ + unsigned int hostflags; /* Flags to control matching features */ + char *peername; /* Matching hostname in peer certificate */ + char *email; /* If not NULL email address to match */ + size_t emaillen; + unsigned char *ip; /* If not NULL IP address to match */ + size_t iplen; /* Length of IP address */ + int poisoned; } /* X509_VERIFY_PARAM */; /* @@ -368,17 +375,6 @@ struct x509_store_ctx_st { CRYPTO_EX_DATA ex_data; } /* X509_STORE_CTX */; -struct X509_VERIFY_PARAM_ID_st { - STACK_OF(OPENSSL_STRING) *hosts; /* Set of acceptable names */ - unsigned int hostflags; /* Flags to control matching features */ - char *peername; /* Matching hostname in peer certificate */ - char *email; /* If not NULL email address to match */ - size_t emaillen; - unsigned char *ip; /* If not NULL IP address to match */ - size_t iplen; /* Length of IP address */ - int poisoned; -}; - int x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int quiet); int name_cmp(const char *name, const char *cmp); diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c index 6bc06187e1a..0c2144752d5 100644 --- a/lib/libcrypto/x509/x509_vfy.c +++ b/lib/libcrypto/x509/x509_vfy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.c,v 1.123 2023/05/14 20:20:40 tb Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.124 2023/05/28 05:25:24 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -177,19 +177,19 @@ check_id_error(X509_STORE_CTX *ctx, int errcode) } static int -check_hosts(X509 *x, X509_VERIFY_PARAM_ID *id) +check_hosts(X509 *x, X509_VERIFY_PARAM *vpm) { int i, n; char *name; - n = sk_OPENSSL_STRING_num(id->hosts); - free(id->peername); - id->peername = NULL; + n = sk_OPENSSL_STRING_num(vpm->hosts); + free(vpm->peername); + vpm->peername = NULL; for (i = 0; i < n; ++i) { - name = sk_OPENSSL_STRING_value(id->hosts, i); - if (X509_check_host(x, name, strlen(name), id->hostflags, - &id->peername) > 0) + name = sk_OPENSSL_STRING_value(vpm->hosts, i); + if (X509_check_host(x, name, strlen(name), vpm->hostflags, + &vpm->peername) > 0) return 1; } return n == 0; @@ -199,19 +199,18 @@ static int check_id(X509_STORE_CTX *ctx) { X509_VERIFY_PARAM *vpm = ctx->param; - X509_VERIFY_PARAM_ID *id = vpm->id; X509 *x = ctx->cert; - if (id->hosts && check_hosts(x, id) <= 0) { + if (vpm->hosts && check_hosts(x, vpm) <= 0) { if (!check_id_error(ctx, X509_V_ERR_HOSTNAME_MISMATCH)) return 0; } - if (id->email != NULL && X509_check_email(x, id->email, id->emaillen, 0) + if (vpm->email != NULL && X509_check_email(x, vpm->email, vpm->emaillen, 0) <= 0) { if (!check_id_error(ctx, X509_V_ERR_EMAIL_MISMATCH)) return 0; } - if (id->ip != NULL && X509_check_ip(x, id->ip, id->iplen, 0) <= 0) { + if (vpm->ip != NULL && X509_check_ip(x, vpm->ip, vpm->iplen, 0) <= 0) { if (!check_id_error(ctx, X509_V_ERR_IP_ADDRESS_MISMATCH)) return 0; } @@ -609,7 +608,7 @@ X509_verify_cert(X509_STORE_CTX *ctx) ctx->error = X509_V_ERR_INVALID_CALL; return -1; } - if (ctx->param->id->poisoned) { + if (ctx->param->poisoned) { /* * This X509_STORE_CTX had failures setting * up verify parameters. We can not use it. diff --git a/lib/libcrypto/x509/x509_vfy.h b/lib/libcrypto/x509/x509_vfy.h index 202cf7438f1..1aa29abd3d5 100644 --- a/lib/libcrypto/x509/x509_vfy.h +++ b/lib/libcrypto/x509/x509_vfy.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.h,v 1.63 2023/04/28 16:50:16 beck Exp $ */ +/* $OpenBSD: x509_vfy.h,v 1.64 2023/05/28 05:25:24 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -105,7 +105,7 @@ DECLARE_STACK_OF(X509_LOOKUP) DECLARE_STACK_OF(X509_OBJECT) DECLARE_STACK_OF(X509_VERIFY_PARAM) -/* unused in OpenSSL */ +/* XXX - unused in OpenSSL. Can we remove this? */ typedef struct X509_VERIFY_PARAM_ID_st X509_VERIFY_PARAM_ID; diff --git a/lib/libcrypto/x509/x509_vpm.c b/lib/libcrypto/x509/x509_vpm.c index 5c8c09e9fc9..4ba697ead4e 100644 --- a/lib/libcrypto/x509/x509_vpm.c +++ b/lib/libcrypto/x509/x509_vpm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vpm.c,v 1.39 2023/05/24 09:15:14 tb Exp $ */ +/* $OpenBSD: x509_vpm.c,v 1.40 2023/05/28 05:25:24 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2004. */ @@ -122,7 +122,7 @@ sk_deep_copy(void *sk_void, void *copy_func_void, void *free_func_void) } static int -x509_param_set_hosts_internal(X509_VERIFY_PARAM_ID *id, int mode, +x509_param_set_hosts_internal(X509_VERIFY_PARAM *vpm, int mode, const char *name, size_t namelen) { char *copy; @@ -135,9 +135,9 @@ x509_param_set_hosts_internal(X509_VERIFY_PARAM_ID *id, int mode, if (name && memchr(name, '\0', namelen)) return 0; - if (mode == SET_HOST && id->hosts) { - sk_OPENSSL_STRING_pop_free(id->hosts, str_free); - id->hosts = NULL; + if (mode == SET_HOST && vpm->hosts) { + sk_OPENSSL_STRING_pop_free(vpm->hosts, str_free); + vpm->hosts = NULL; } if (name == NULL || namelen == 0) return 1; @@ -145,17 +145,17 @@ x509_param_set_hosts_internal(X509_VERIFY_PARAM_ID *id, int mode, if (copy == NULL) return 0; - if (id->hosts == NULL && - (id->hosts = sk_OPENSSL_STRING_new_null()) == NULL) { + if (vpm->hosts == NULL && + (vpm->hosts = sk_OPENSSL_STRING_new_null()) == NULL) { free(copy); return 0; } - if (!sk_OPENSSL_STRING_push(id->hosts, copy)) { + if (!sk_OPENSSL_STRING_push(vpm->hosts, copy)) { free(copy); - if (sk_OPENSSL_STRING_num(id->hosts) == 0) { - sk_OPENSSL_STRING_free(id->hosts); - id->hosts = NULL; + if (sk_OPENSSL_STRING_num(vpm->hosts) == 0) { + sk_OPENSSL_STRING_free(vpm->hosts); + vpm->hosts = NULL; } return 0; } @@ -166,9 +166,9 @@ x509_param_set_hosts_internal(X509_VERIFY_PARAM_ID *id, int mode, static void x509_verify_param_zero(X509_VERIFY_PARAM *param) { - X509_VERIFY_PARAM_ID *paramid; if (!param) return; + free(param->name); param->name = NULL; param->purpose = 0; @@ -177,40 +177,29 @@ x509_verify_param_zero(X509_VERIFY_PARAM *param) param->inh_flags = 0; param->flags = 0; param->depth = -1; - if (param->policies) { - sk_ASN1_OBJECT_pop_free(param->policies, ASN1_OBJECT_free); - param->policies = NULL; - } - paramid = param->id; - if (paramid->hosts) { - sk_OPENSSL_STRING_pop_free(paramid->hosts, str_free); - paramid->hosts = NULL; - } - free(paramid->peername); - paramid->peername = NULL; - free(paramid->email); - paramid->email = NULL; - paramid->emaillen = 0; - free(paramid->ip); - paramid->ip = NULL; - paramid->iplen = 0; - paramid->poisoned = 0; + sk_ASN1_OBJECT_pop_free(param->policies, ASN1_OBJECT_free); + param->policies = NULL; + sk_OPENSSL_STRING_pop_free(param->hosts, str_free); + param->hosts = NULL; + free(param->peername); + param->peername = NULL; + free(param->email); + param->email = NULL; + param->emaillen = 0; + free(param->ip); + param->ip = NULL; + param->iplen = 0; + param->poisoned = 0; } X509_VERIFY_PARAM * X509_VERIFY_PARAM_new(void) { X509_VERIFY_PARAM *param; - X509_VERIFY_PARAM_ID *paramid; + param = calloc(1, sizeof(X509_VERIFY_PARAM)); if (param == NULL) return NULL; - paramid = calloc(1, sizeof(X509_VERIFY_PARAM_ID)); - if (paramid == NULL) { - free(param); - return NULL; - } - param->id = paramid; x509_verify_param_zero(param); return param; } @@ -222,7 +211,6 @@ X509_VERIFY_PARAM_free(X509_VERIFY_PARAM *param) if (param == NULL) return; x509_verify_param_zero(param); - free(param->id); free(param); } LCRYPTO_ALIAS(X509_VERIFY_PARAM_free); @@ -260,18 +248,11 @@ LCRYPTO_ALIAS(X509_VERIFY_PARAM_free); */ /* Macro to test if a field should be copied from src to dest */ - #define test_x509_verify_param_copy(field, def) \ (to_overwrite || \ ((src->field != def) && (to_default || (dest->field == def)))) -/* As above but for ID fields */ - -#define test_x509_verify_param_copy_id(idf, def) \ - test_x509_verify_param_copy(id->idf, def) - /* Macro to test and copy a field if necessary */ - #define x509_verify_param_copy(field, def) \ if (test_x509_verify_param_copy(field, def)) \ dest->field = src->field @@ -281,11 +262,9 @@ X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *dest, const X509_VERIFY_PARAM *src) { unsigned long inh_flags; int to_default, to_overwrite; - X509_VERIFY_PARAM_ID *id; if (!src) return 1; - id = src->id; inh_flags = dest->inh_flags | src->inh_flags; if (inh_flags & X509_VP_FLAG_ONCE) @@ -326,30 +305,28 @@ X509_VERIFY_PARAM_inherit(X509_VERIFY_PARAM *dest, const X509_VERIFY_PARAM *src) return 0; } - if (test_x509_verify_param_copy_id(hostflags, 0)) - dest->id->hostflags = id->hostflags; + x509_verify_param_copy(hostflags, 0); - if (test_x509_verify_param_copy_id(hosts, NULL)) { - if (dest->id->hosts) { - sk_OPENSSL_STRING_pop_free(dest->id->hosts, str_free); - dest->id->hosts = NULL; + if (test_x509_verify_param_copy(hosts, NULL)) { + if (dest->hosts) { + sk_OPENSSL_STRING_pop_free(dest->hosts, str_free); + dest->hosts = NULL; } - if (id->hosts) { - dest->id->hosts = - sk_deep_copy(id->hosts, strdup, str_free); - if (dest->id->hosts == NULL) + if (src->hosts) { + dest->hosts = sk_deep_copy(src->hosts, strdup, str_free); + if (dest->hosts == NULL) return 0; } } - if (test_x509_verify_param_copy_id(email, NULL)) { - if (!X509_VERIFY_PARAM_set1_email(dest, id->email, - id->emaillen)) + if (test_x509_verify_param_copy(email, NULL)) { + if (!X509_VERIFY_PARAM_set1_email(dest, src->email, + src->emaillen)) return 0; } - if (test_x509_verify_param_copy_id(ip, NULL)) { - if (!X509_VERIFY_PARAM_set1_ip(dest, id->ip, id->iplen)) + if (test_x509_verify_param_copy(ip, NULL)) { + if (!X509_VERIFY_PARAM_set1_ip(dest, src->ip, src->iplen)) return 0; } @@ -534,9 +511,9 @@ int X509_VERIFY_PARAM_set1_host(X509_VERIFY_PARAM *param, const char *name, size_t namelen) { - if (x509_param_set_hosts_internal(param->id, SET_HOST, name, namelen)) + if (x509_param_set_hosts_internal(param, SET_HOST, name, namelen)) return 1; - param->id->poisoned = 1; + param->poisoned = 1; return 0; } LCRYPTO_ALIAS(X509_VERIFY_PARAM_set1_host); @@ -545,9 +522,9 @@ int X509_VERIFY_PARAM_add1_host(X509_VERIFY_PARAM *param, const char *name, size_t namelen) { - if (x509_param_set_hosts_internal(param->id, ADD_HOST, name, namelen)) + if (x509_param_set_hosts_internal(param, ADD_HOST, name, namelen)) return 1; - param->id->poisoned = 1; + param->poisoned = 1; return 0; } LCRYPTO_ALIAS(X509_VERIFY_PARAM_add1_host); @@ -556,20 +533,20 @@ LCRYPTO_ALIAS(X509_VERIFY_PARAM_add1_host); unsigned int X509_VERIFY_PARAM_get_hostflags(X509_VERIFY_PARAM *param) { - return param->id->hostflags; + return param->hostflags; } void X509_VERIFY_PARAM_set_hostflags(X509_VERIFY_PARAM *param, unsigned int flags) { - param->id->hostflags = flags; + param->hostflags = flags; } LCRYPTO_ALIAS(X509_VERIFY_PARAM_set_hostflags); char * X509_VERIFY_PARAM_get0_peername(X509_VERIFY_PARAM *param) { - return param->id->peername; + return param->peername; } LCRYPTO_ALIAS(X509_VERIFY_PARAM_get0_peername); @@ -577,10 +554,10 @@ int X509_VERIFY_PARAM_set1_email(X509_VERIFY_PARAM *param, const char *email, size_t emaillen) { - if (x509_param_set1_internal(¶m->id->email, ¶m->id->emaillen, + if (x509_param_set1_internal(¶m->email, ¶m->emaillen, email, emaillen, 1)) return 1; - param->id->poisoned = 1; + param->poisoned = 1; return 0; } LCRYPTO_ALIAS(X509_VERIFY_PARAM_set1_email); @@ -591,11 +568,11 @@ X509_VERIFY_PARAM_set1_ip(X509_VERIFY_PARAM *param, const unsigned char *ip, { if (iplen != 4 && iplen != 16) goto err; - if (x509_param_set1_internal((char **)¶m->id->ip, ¶m->id->iplen, + if (x509_param_set1_internal((char **)¶m->ip, ¶m->iplen, (char *)ip, iplen, 0)) return 1; err: - param->id->poisoned = 1; + param->poisoned = 1; return 0; } LCRYPTO_ALIAS(X509_VERIFY_PARAM_set1_ip); @@ -625,10 +602,6 @@ X509_VERIFY_PARAM_get0_name(const X509_VERIFY_PARAM *param) } LCRYPTO_ALIAS(X509_VERIFY_PARAM_get0_name); -static const X509_VERIFY_PARAM_ID _empty_id = { NULL }; - -#define vpm_empty_id (X509_VERIFY_PARAM_ID *)&_empty_id - /* * Default verify parameters: these are used for various applications and can * be overridden by the user specified table. @@ -640,35 +613,30 @@ static const X509_VERIFY_PARAM default_table[] = { .flags = X509_V_FLAG_TRUSTED_FIRST, .depth = 100, .trust = 0, /* XXX This is not the default trust value */ - .id = vpm_empty_id }, { .name = "pkcs7", .purpose = X509_PURPOSE_SMIME_SIGN, .trust = X509_TRUST_EMAIL, .depth = -1, - .id = vpm_empty_id }, { .name = "smime_sign", .purpose = X509_PURPOSE_SMIME_SIGN, .trust = X509_TRUST_EMAIL, .depth = -1, - .id = vpm_empty_id }, { .name = "ssl_client", .purpose = X509_PURPOSE_SSL_CLIENT, .trust = X509_TRUST_SSL_CLIENT, .depth = -1, - .id = vpm_empty_id }, { .name = "ssl_server", .purpose = X509_PURPOSE_SSL_SERVER, .trust = X509_TRUST_SSL_SERVER, .depth = -1, - .id = vpm_empty_id } }; -- 2.20.1