Improve ossl_cmp_build_cert_chain(); publish it as X509_build_chain()

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14128)

14 files changed, 112 insertions, 105 deletions

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 <cert>
- * using the optional list of intermediate CA certificates <certs>.
- * If <store> is NULL builds the chain as far down as possible, ignoring errors.
- * Else the chain must reach a trust anchor contained in <store>.
- *
- * 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<cert>
-using the optional list of intermediate CA certificates I<certs>.
-If I<store> is NULL builds the chain as far down as possible, ignoring errors.
-Else the chain must reach a trust anchor contained in I<store>.
-It internally uses a B<X509_STORE_CTX> structure associated with the library
-context I<libctx> and property query string I<propq>, 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<X509_verify_cert(3)> for details.
-
 ossl_cmp_calc_protection() calculates the protection for the given I<msg>
 according to the algorithm and parameters in the message header's protectionAlg
 using the credentials, library context, and property criteria in the I<ctx>.
@@ -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 <openssl/x509_vfy.h>
 
+ 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<target>
+using the optional list of intermediate CA certificates I<certs>.
+If I<store> is NULL it builds the chain as far down as possible, ignoring errors.
+Else the chain must reach a trust anchor contained in I<store>.
+It internally uses a B<X509_STORE_CTX> structure associated with the library
+context I<libctx> and property query string I<propq>, 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<target> and followed by all available intermediate certificates.
+A self-signed trust anchor is included only if I<target> is the trust anchor
+of I<with_self_signed> 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<ctx>.
 The verification context, of type B<X509_STORE_CTX>, can be constructed
@@ -45,7 +63,10 @@ in I<ctx> 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<X509_STORE_CTX_get_error(3)>
 
 =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: