Merge X509_VERIFY_PARAM_ID into X509_VERIFY_PARAM
authortb <tb@openbsd.org>
Sun, 28 May 2023 05:25:24 +0000 (05:25 +0000)
committertb <tb@openbsd.org>
Sun, 28 May 2023 05:25:24 +0000 (05:25 +0000)
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
lib/libcrypto/x509/x509_vfy.c
lib/libcrypto/x509/x509_vfy.h
lib/libcrypto/x509/x509_vpm.c

index 9ce1b58..f00a55b 100644 (file)
@@ -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);
index 6bc0618..0c21447 100644 (file)
@@ -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.
index 202cf74..1aa29ab 100644 (file)
@@ -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;
 
 
index 5c8c09e..4ba697e 100644 (file)
@@ -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(&param->id->email, &param->id->emaillen,
+       if (x509_param_set1_internal(&param->email, &param->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 **)&param->id->ip, &param->id->iplen,
+       if (x509_param_set1_internal((char **)&param->ip, &param->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
        }
 };