diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c index 728ec21968..54c8f5094b 100644 --- a/crypto/cmp/cmp_client.c +++ b/crypto/cmp/cmp_client.c @@ -496,9 +496,8 @@ int OSSL_CMP_certConf_cb(OSSL_CMP_CTX *ctx, X509 *cert, int fail_info, return fail_info; ossl_cmp_debug(ctx, "trying to build chain for newly enrolled cert"); - chain = ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, - out_trusted /* may be NULL */, - ctx->untrusted, cert); + chain = X509_build_chain(cert, ctx->untrusted, out_trusted /* maybe NULL */, + 0, ctx->libctx, ctx->propq); if (sk_X509_num(chain) > 0) X509_free(sk_X509_shift(chain)); /* remove leaf (EE) cert */ if (out_trusted != NULL) { diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c index 110361320d..7e7af63b4a 100644 --- a/crypto/cmp/cmp_ctx.c +++ b/crypto/cmp/cmp_ctx.c @@ -735,8 +735,8 @@ int OSSL_CMP_CTX_build_cert_chain(OSSL_CMP_CTX *ctx, X509_STORE *own_trusted, return 0; ossl_cmp_debug(ctx, "trying to build chain for own CMP signer cert"); - chain = ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, own_trusted, - ctx->untrusted, ctx->cert); + chain = X509_build_chain(ctx->cert, ctx->untrusted, own_trusted, 0, + ctx->libctx, ctx->propq); if (chain == NULL) { ERR_raise(ERR_LIB_CMP, CMP_R_FAILED_BUILDING_OWN_CHAIN); return 0; diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h index 1ec16d4b2b..b2a3382079 100644 --- a/crypto/cmp/cmp_local.h +++ b/crypto/cmp/cmp_local.h @@ -749,10 +749,6 @@ int ossl_cmp_asn1_octet_string_set1(ASN1_OCTET_STRING **tgt, const ASN1_OCTET_STRING *src); int ossl_cmp_asn1_octet_string_set1_bytes(ASN1_OCTET_STRING **tgt, const unsigned char *bytes, int len); -STACK_OF(X509) - *ossl_cmp_build_cert_chain(OSSL_LIB_CTX *libctx, const char *propq, - X509_STORE *store, - STACK_OF(X509) *certs, X509 *cert); /* from cmp_ctx.c */ int ossl_cmp_print_log(OSSL_CMP_severity level, const OSSL_CMP_CTX *ctx, diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c index 45bea73d13..36a6597145 100644 --- a/crypto/cmp/cmp_protect.c +++ b/crypto/cmp/cmp_protect.c @@ -144,9 +144,8 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg) if (ctx->chain == NULL) { ossl_cmp_debug(ctx, "trying to build chain for own CMP signer cert"); - ctx->chain = - ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, NULL, - ctx->untrusted, ctx->cert); + ctx->chain = X509_build_chain(ctx->cert, ctx->untrusted, NULL, 0, + ctx->libctx, ctx->propq); if (ctx->chain != NULL) { ossl_cmp_debug(ctx, "success building chain for own CMP signer cert"); diff --git a/crypto/cmp/cmp_util.c b/crypto/cmp/cmp_util.c index 56f2b0eeb8..fbb8d1e249 100644 --- a/crypto/cmp/cmp_util.c +++ b/crypto/cmp/cmp_util.c @@ -220,67 +220,6 @@ int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs, return 1; } -/*- - * Builds a certificate chain starting from - * using the optional list of intermediate CA certificates . - * If is NULL builds the chain as far down as possible, ignoring errors. - * Else the chain must reach a trust anchor contained in . - * - * Returns NULL on error, else a pointer to a stack of (up_ref'ed) certificates - * starting with given EE certificate and followed by all available intermediate - * certificates down towards any trust anchor but without including the latter. - * - * NOTE: If a non-NULL stack is returned the caller is responsible for freeing. - * NOTE: In case there is more than one possibility for the chain, - * OpenSSL seems to take the first one; check X509_verify_cert() for details. - */ -/* TODO this should be of more general interest and thus be exported. */ -STACK_OF(X509) - *ossl_cmp_build_cert_chain(OSSL_LIB_CTX *libctx, const char *propq, - X509_STORE *store, - STACK_OF(X509) *certs, X509 *cert) -{ - STACK_OF(X509) *chain = NULL, *result = NULL; - X509_STORE *ts = store == NULL ? X509_STORE_new() : store; - X509_STORE_CTX *csc = NULL; - - if (ts == NULL || cert == NULL) { - ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT); - goto err; - } - - if ((csc = X509_STORE_CTX_new_ex(libctx, propq)) == NULL) - goto err; - if (store == NULL && certs != NULL - && !ossl_cmp_X509_STORE_add1_certs(ts, certs, 0)) - goto err; - if (!X509_STORE_CTX_init(csc, ts, cert, - store == NULL ? NULL : certs)) - goto err; - /* disable any cert status/revocation checking etc. */ - X509_VERIFY_PARAM_clear_flags(X509_STORE_CTX_get0_param(csc), - ~(X509_V_FLAG_USE_CHECK_TIME - | X509_V_FLAG_NO_CHECK_TIME)); - - if (X509_verify_cert(csc) <= 0 && store != NULL) - goto err; - chain = X509_STORE_CTX_get0_chain(csc); - - /* result list to store the up_ref'ed not self-signed certificates */ - if (!ossl_x509_add_certs_new(&result, chain, - X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP - | X509_ADD_FLAG_NO_SS)) { - sk_X509_free(result); - result = NULL; - } - - err: - if (store == NULL) - X509_STORE_free(ts); - X509_STORE_CTX_free(csc); - return result; -} - int ossl_cmp_sk_ASN1_UTF8STRING_push_str(STACK_OF(ASN1_UTF8STRING) *sk, const char *text) { diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 01871b9090..cb541084df 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -3322,6 +3322,48 @@ static int build_chain(X509_STORE_CTX *ctx) return -1; } +STACK_OF(X509) *X509_build_chain(X509 *target, STACK_OF(X509) *certs, + X509_STORE *store, int with_self_signed, + OSSL_LIB_CTX *libctx, const char *propq) +{ + int finish_chain = store != NULL; + X509_STORE_CTX *ctx; + int flags = X509_ADD_FLAG_UP_REF; + STACK_OF(X509) *result = NULL; + + if (target == NULL) { + ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER); + return NULL; + } + + if ((ctx = X509_STORE_CTX_new_ex(libctx, propq)) == NULL) + return NULL; + if (!X509_STORE_CTX_init(ctx, store, target, finish_chain ? certs : NULL)) + goto err; + if (!finish_chain) + X509_STORE_CTX_set0_trusted_stack(ctx, certs); + if (!ossl_x509_add_cert_new(&ctx->chain, target, X509_ADD_FLAG_UP_REF)) { + ctx->error = X509_V_ERR_OUT_OF_MEM; + goto err; + } + ctx->num_untrusted = 1; + + if (!build_chain(ctx) && finish_chain) + goto err; + + /* result list to store the up_ref'ed certificates */ + if (sk_X509_num(ctx->chain) > 1 && !with_self_signed) + flags |= X509_ADD_FLAG_NO_SS; + if (!ossl_x509_add_certs_new(&result, ctx->chain, flags)) { + sk_X509_free(result); + result = NULL; + } + + err: + X509_STORE_CTX_free(ctx); + return result; +} + static const int minbits_table[] = { 80, 112, 128, 192, 256 }; static const int NUM_AUTH_LEVELS = OSSL_NELEM(minbits_table); diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c index a4a169a97e..529d701bbb 100644 --- a/crypto/x509/x_x509.c +++ b/crypto/x509/x_x509.c @@ -129,7 +129,7 @@ X509 *d2i_X509(X509 **a, const unsigned char **in, long len) cert = (X509 *)ASN1_item_d2i((ASN1_VALUE **)a, in, len, (X509_it())); /* Only cache the extensions if the cert object was passed in */ - if (cert != NULL && a != NULL) { + if (cert != NULL && a != NULL) { /* then cert == *a */ if (!ossl_x509v3_cache_extensions(cert)) { if (free_on_error) X509_free(cert); diff --git a/doc/internal/man3/ossl_cmp_msg_protect.pod b/doc/internal/man3/ossl_cmp_msg_protect.pod index 681d572af5..11a7a02901 100644 --- a/doc/internal/man3/ossl_cmp_msg_protect.pod +++ b/doc/internal/man3/ossl_cmp_msg_protect.pod @@ -2,7 +2,6 @@ =head1 NAME -ossl_cmp_build_cert_chain, ossl_cmp_calc_protection, ossl_cmp_msg_protect, ossl_cmp_msg_add_extraCerts @@ -12,10 +11,6 @@ ossl_cmp_msg_add_extraCerts #include "cmp_local.h" - STACK_OF(X509) - *ossl_cmp_build_cert_chain(OSSL_LIB_CTX *libctx, const char *propq, - X509_STORE *store, - STACK_OF(X509) *certs, X509 *cert); ASN1_BIT_STRING *ossl_cmp_calc_protection(const OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg); int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg); @@ -23,16 +18,6 @@ ossl_cmp_msg_add_extraCerts =head1 DESCRIPTION -ossl_cmp_build_cert_chain() builds a certificate chain starting from I -using the optional list of intermediate CA certificates I. -If I is NULL builds the chain as far down as possible, ignoring errors. -Else the chain must reach a trust anchor contained in I. -It internally uses a B structure associated with the library -context I and property query string I, both of which may be NULL. -If a non-NULL stack is returned the caller is responsible for freeing it. -In case there is more than one possibility for the chain, -OpenSSL seems to take the first one; check L for details. - ossl_cmp_calc_protection() calculates the protection for the given I according to the algorithm and parameters in the message header's protectionAlg using the credentials, library context, and property criteria in the I. @@ -57,11 +42,6 @@ CMP is defined in RFC 4210 (and CRMF in RFC 4211). =head1 RETURN VALUES -ossl_cmp_build_cert_chain() returns NULL on error, -else a pointer to a stack of (up_ref'ed) certificates -starting with given EE certificate and followed by all available intermediate -certificates down towards (but excluding) any trusted root certificate. - ossl_cmp_calc_protection() returns the protection on success, else NULL. All other functions return 1 on success, 0 on error. diff --git a/doc/man3/X509_verify_cert.pod b/doc/man3/X509_verify_cert.pod index deb6b15869..a14a0b25c4 100644 --- a/doc/man3/X509_verify_cert.pod +++ b/doc/man3/X509_verify_cert.pod @@ -2,18 +2,36 @@ =head1 NAME +X509_build_chain, X509_verify_cert, -X509_STORE_CTX_verify - discover and verify X509 certificate chain +X509_STORE_CTX_verify - build and verify X509 certificate chain =head1 SYNOPSIS #include + STACK_OF(X509) *X509_build_chain(X509 *target, STACK_OF(X509) *certs, + X509_STORE *store, int with_self_signed, + OSSL_LIB_CTX *libctx, const char *propq); int X509_verify_cert(X509_STORE_CTX *ctx); int X509_STORE_CTX_verify(X509_STORE_CTX *ctx); =head1 DESCRIPTION +X509_build_chain() builds a certificate chain starting from I +using the optional list of intermediate CA certificates I. +If I is NULL it builds the chain as far down as possible, ignoring errors. +Else the chain must reach a trust anchor contained in I. +It internally uses a B structure associated with the library +context I and property query string I, both of which may be NULL. +In case there is more than one possibility for the chain, only one is taken. + +On success it returns a pointer to a new stack of (up_ref'ed) certificates +starting with I and followed by all available intermediate certificates. +A self-signed trust anchor is included only if I is the trust anchor +of I is 1. +If a non-NULL stack is returned the caller is responsible for freeing it. + The X509_verify_cert() function attempts to discover and validate a certificate chain based on parameters in I. The verification context, of type B, can be constructed @@ -45,7 +63,10 @@ in I unless a target certificate is set explicitly. =head1 RETURN VALUES -Both functions return 1 if a complete chain can be built and validated, +X509_build_chain() returns NULL on error, else a stack of certificates. + +Both X509_verify_cert() and X509_STORE_CTX_verify() +return 1 if a complete chain can be built and validated, otherwise they return 0, and in exceptional circumstances (such as malloc failure and internal errors) they can also return a negative code. @@ -66,7 +87,7 @@ L =head1 HISTORY -X509_STORE_CTX_verify() was added in OpenSSL 3.0. +X509_build_chain() and X509_STORE_CTX_verify() were added in OpenSSL 3.0. =head1 COPYRIGHT diff --git a/include/openssl/x509_vfy.h.in b/include/openssl/x509_vfy.h.in index 662a8b6ab8..126c1d658a 100644 --- a/include/openssl/x509_vfy.h.in +++ b/include/openssl/x509_vfy.h.in @@ -72,6 +72,9 @@ typedef enum { .generate_stack_macros("X509_VERIFY_PARAM"); -} +STACK_OF(X509) *X509_build_chain(X509 *target, STACK_OF(X509) *certs, + X509_STORE *store, int with_self_signed, + OSSL_LIB_CTX *libctx, const char *propq); int X509_verify_cert(X509_STORE_CTX *ctx); int X509_STORE_CTX_verify(X509_STORE_CTX *ctx); diff --git a/test/cmp_ctx_test.c b/test/cmp_ctx_test.c index 96b0f1b511..2ca2c26dd5 100644 --- a/test/cmp_ctx_test.c +++ b/test/cmp_ctx_test.c @@ -874,6 +874,5 @@ int setup_tests(void) ADD_TEST(test_CTX_set1_get0_transactionID); ADD_TEST(test_CTX_set1_get0_senderNonce); ADD_TEST(test_CTX_set1_get0_recipNonce); - /* ossl_cmp_build_cert_chain() is tested in cmp_protect.c */ return 1; } diff --git a/test/cmp_protect_test.c b/test/cmp_protect_test.c index 543ff10dca..5fafb69475 100644 --- a/test/cmp_protect_test.c +++ b/test/cmp_protect_test.c @@ -27,6 +27,7 @@ typedef struct test_fixture { X509 *cert; STACK_OF(X509) *certs; STACK_OF(X509) *chain; + int with_ss; int callback_arg; int expected; } CMP_PROTECT_TEST_FIXTURE; @@ -333,8 +334,8 @@ static int execute_cmp_build_cert_chain_test(CMP_PROTECT_TEST_FIXTURE *fixture) OSSL_CMP_CTX *ctx = fixture->cmp_ctx; X509_STORE *store; STACK_OF(X509) *chain = - ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, NULL, - fixture->certs, fixture->cert); + X509_build_chain(fixture->cert, fixture->certs, NULL, + fixture->with_ss, ctx->libctx, ctx->propq); if (TEST_ptr(chain)) { /* Check whether chain built is equal to the expected one */ @@ -348,8 +349,8 @@ static int execute_cmp_build_cert_chain_test(CMP_PROTECT_TEST_FIXTURE *fixture) && TEST_true(X509_STORE_add_cert(store, root))) { X509_VERIFY_PARAM_set_flags(X509_STORE_get0_param(store), X509_V_FLAG_NO_CHECK_TIME); - chain = ossl_cmp_build_cert_chain(ctx->libctx, ctx->propq, - store, fixture->certs, fixture->cert); + chain = X509_build_chain(fixture->cert, fixture->certs, store, + fixture->with_ss, ctx->libctx, ctx->propq); ret = TEST_int_eq(fixture->expected, chain != NULL); if (ret && chain != NULL) { /* Check whether chain built is equal to the expected one */ @@ -365,6 +366,7 @@ static int test_cmp_build_cert_chain(void) { SETUP_TEST_FIXTURE(CMP_PROTECT_TEST_FIXTURE, set_up); fixture->expected = 1; + fixture->with_ss = 0; fixture->cert = endentity2; if (!TEST_ptr(fixture->certs = sk_X509_new_null()) || !TEST_ptr(fixture->chain = sk_X509_new_null()) @@ -376,7 +378,13 @@ static int test_cmp_build_cert_chain(void) tear_down(fixture); fixture = NULL; } - EXECUTE_TEST(execute_cmp_build_cert_chain_test, tear_down); + if (fixture != NULL) { + result = execute_cmp_build_cert_chain_test(fixture); + fixture->with_ss = 1; + if (result && TEST_true(sk_X509_push(fixture->chain, root))) + result = execute_cmp_build_cert_chain_test(fixture); + } + tear_down(fixture); return result; } @@ -384,6 +392,7 @@ static int test_cmp_build_cert_chain_missing_intermediate(void) { SETUP_TEST_FIXTURE(CMP_PROTECT_TEST_FIXTURE, set_up); fixture->expected = 0; + fixture->with_ss = 0; fixture->cert = endentity2; if (!TEST_ptr(fixture->certs = sk_X509_new_null()) || !TEST_ptr(fixture->chain = sk_X509_new_null()) @@ -401,6 +410,7 @@ static int test_cmp_build_cert_chain_no_root(void) { SETUP_TEST_FIXTURE(CMP_PROTECT_TEST_FIXTURE, set_up); fixture->expected = 1; + fixture->with_ss = 0; fixture->cert = endentity2; if (!TEST_ptr(fixture->certs = sk_X509_new_null()) || !TEST_ptr(fixture->chain = sk_X509_new_null()) @@ -415,10 +425,28 @@ static int test_cmp_build_cert_chain_no_root(void) return result; } +static int test_cmp_build_cert_chain_only_root(void) +{ + SETUP_TEST_FIXTURE(CMP_PROTECT_TEST_FIXTURE, set_up); + fixture->expected = 1; + fixture->with_ss = 0; /* still chain must include the only cert (root) */ + fixture->cert = root; + if (!TEST_ptr(fixture->certs = sk_X509_new_null()) + || !TEST_ptr(fixture->chain = sk_X509_new_null()) + || !TEST_true(sk_X509_push(fixture->certs, root)) + || !TEST_true(sk_X509_push(fixture->chain, root))) { + tear_down(fixture); + fixture = NULL; + } + EXECUTE_TEST(execute_cmp_build_cert_chain_test, tear_down); + return result; +} + static int test_cmp_build_cert_chain_no_certs(void) { SETUP_TEST_FIXTURE(CMP_PROTECT_TEST_FIXTURE, set_up); fixture->expected = 0; + fixture->with_ss = 0; fixture->cert = endentity2; if (!TEST_ptr(fixture->certs = sk_X509_new_null()) || !TEST_ptr(fixture->chain = sk_X509_new_null()) @@ -576,6 +604,7 @@ int setup_tests(void) #ifndef OPENSSL_NO_EC ADD_TEST(test_cmp_build_cert_chain); + ADD_TEST(test_cmp_build_cert_chain_only_root); ADD_TEST(test_cmp_build_cert_chain_no_root); ADD_TEST(test_cmp_build_cert_chain_missing_intermediate); ADD_TEST(test_cmp_build_cert_chain_no_certs); diff --git a/test/verify_extra_test.c b/test/verify_extra_test.c index 758a1a4971..6ab779671f 100644 --- a/test/verify_extra_test.c +++ b/test/verify_extra_test.c @@ -177,12 +177,11 @@ static int test_req_distinguishing_id(void) static int test_self_signed(const char *filename, int use_trusted, int expected) { - X509 *cert; + X509 *cert = load_cert_from_file(filename); /* may result in NULL */ STACK_OF(X509) *trusted = sk_X509_new_null(); X509_STORE_CTX *ctx = X509_STORE_CTX_new(); int ret; - cert = load_cert_from_file(filename); /* may result in NULL */ ret = TEST_int_eq(X509_self_signed(cert, 1), expected); if (cert != NULL) { diff --git a/util/libcrypto.num b/util/libcrypto.num index a059aecd5e..9237bdcd4b 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -5097,6 +5097,7 @@ X509_STORE_load_file_ex ? 3_0_0 EXIST::FUNCTION: X509_STORE_load_store_ex ? 3_0_0 EXIST::FUNCTION: X509_STORE_load_locations_ex ? 3_0_0 EXIST::FUNCTION: X509_STORE_set_default_paths_ex ? 3_0_0 EXIST::FUNCTION: +X509_build_chain ? 3_0_0 EXIST::FUNCTION: X509V3_set_issuer_pkey ? 3_0_0 EXIST::FUNCTION: i2s_ASN1_UTF8STRING ? 3_0_0 EXIST::FUNCTION: s2i_ASN1_UTF8STRING ? 3_0_0 EXIST::FUNCTION: