Add internal X509_add_certs_new(), which simplifies matters

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

crypto/cmp/cmp_ctx.c

@@ -12,7 +12,6 @@
 #include <openssl/trace.h>
 #include <openssl/bio.h>
 #include <openssl/ocsp.h> /* for OCSP_REVOKED_STATUS_* */
-#include "crypto/x509.h" /* for x509v3_cache_extensions() */
 
 #include "cmp_local.h"
 
@@ -65,15 +64,14 @@ STACK_OF(X509) *OSSL_CMP_CTX_get0_untrusted(const OSSL_CMP_CTX *ctx)
  */
 int OSSL_CMP_CTX_set1_untrusted(OSSL_CMP_CTX *ctx, STACK_OF(X509) *certs)
 {
-    STACK_OF(X509) *untrusted;
+    STACK_OF(X509) *untrusted = NULL;
+
     if (ctx == NULL) {
         ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
         return 0;
     }
-    if ((untrusted = sk_X509_new_null()) == NULL)
+    if (!ossl_x509_add_certs_new(&untrusted, certs,
-        return 0;
+                                 X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
-    if (X509_add_certs(untrusted, certs,
-                       X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP) != 1)
         goto err;
     sk_X509_pop_free(ctx->untrusted, X509_free);
     ctx->untrusted = untrusted;
@@ -731,10 +729,8 @@ int OSSL_CMP_CTX_build_cert_chain(OSSL_CMP_CTX *ctx, X509_STORE *own_trusted,
         return 0;
     }
 
-    if (ctx->untrusted != NULL ?
+    if (!ossl_x509_add_certs_new(&ctx->untrusted, candidates,
-        !X509_add_certs(ctx->untrusted, candidates,
+                                 X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
-                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP) :
-        !OSSL_CMP_CTX_set1_untrusted(ctx, candidates))
         return 0;
 
     ossl_cmp_debug(ctx, "trying to build chain for own CMP signer cert");

crypto/cmp/cmp_local.h

@@ -23,6 +23,7 @@
 # include <openssl/safestack.h>
 # include <openssl/x509.h>
 # include <openssl/x509v3.h>
+# include "crypto/x509.h"
 
 /*
  * this structure is used to store the context for CMP sessions

crypto/cmp/cmp_msg.c

@@ -19,7 +19,6 @@
 #include <openssl/crmf.h>
 #include <openssl/err.h>
 #include <openssl/x509.h>
-#include "crypto/x509.h" /* for x509_set0_libctx() */
 
 OSSL_CMP_PKIHEADER *OSSL_CMP_MSG_get0_header(const OSSL_CMP_MSG *msg)
 {
@@ -466,13 +465,10 @@ OSSL_CMP_MSG *ossl_cmp_certrep_new(OSSL_CMP_CTX *ctx, int bodytype,
     if (bodytype == OSSL_CMP_PKIBODY_IP && caPubs != NULL
             && (repMsg->caPubs = X509_chain_up_ref(caPubs)) == NULL)
         goto err;
-    if (sk_X509_num(chain) > 0) {
+    if (sk_X509_num(chain) > 0
-        msg->extraCerts = sk_X509_new_reserve(NULL, sk_X509_num(chain));
+        && !ossl_x509_add_certs_new(&msg->extraCerts, chain,
-        if (msg->extraCerts == NULL
-                || !X509_add_certs(msg->extraCerts, chain,
                                     X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         goto err;
-    }
 
     if (!unprotectedErrors
             || ossl_cmp_pkisi_get_status(si) != OSSL_CMP_PKISTATUS_rejection)

crypto/cmp/cmp_protect.c

@@ -134,14 +134,10 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
     if (!ossl_assert(ctx != NULL && msg != NULL))
         return 0;
 
-    if (msg->extraCerts == NULL
-            && (msg->extraCerts = sk_X509_new_null()) == NULL)
-        return 0;
-
     /* Add first ctx->cert and its chain if using signature-based protection */
     if (!ctx->unprotectedSend && ctx->secretValue == NULL
             && ctx->cert != NULL && ctx->pkey != NULL) {
-        int flags_prepend = X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
+        int prepend = X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
             | X509_ADD_FLAG_PREPEND | X509_ADD_FLAG_NO_SS;
 
         /* if not yet done try to build chain using available untrusted certs */
@@ -162,19 +158,18 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
             }
         }
         if (ctx->chain != NULL) {
-            if (!X509_add_certs(msg->extraCerts, ctx->chain, flags_prepend))
+            if (!ossl_x509_add_certs_new(&msg->extraCerts, ctx->chain, prepend))
                 return 0;
         } else {
             /* make sure that at least our own signer cert is included first */
-            if (!X509_add_cert(msg->extraCerts, ctx->cert, flags_prepend))
+            if (!X509_add_cert_new(&msg->extraCerts, ctx->cert, prepend))
                 return 0;
-            ossl_cmp_debug(ctx,
+            ossl_cmp_debug(ctx, "fallback: adding just own CMP signer cert");
-                           "fallback: adding just own CMP signer cert");
         }
     }
 
     /* add any additional certificates from ctx->extraCertsOut */
-    if (!X509_add_certs(msg->extraCerts, ctx->extraCertsOut,
+    if (!ossl_x509_add_certs_new(&msg->extraCerts, ctx->extraCertsOut,
                                  X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         return 0;
 

crypto/cmp/cmp_util.c

@@ -248,9 +248,7 @@ STACK_OF(X509)
     chain = X509_STORE_CTX_get0_chain(csc);
 
     /* result list to store the up_ref'ed not self-signed certificates */
-    if ((result = sk_X509_new_null()) == NULL)
+    if (!ossl_x509_add_certs_new(&result, chain,
-        goto err;
-    if (!X509_add_certs(result, chain,
                                  X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
                                  | X509_ADD_FLAG_NO_SS)) {
         sk_X509_free(result);

crypto/cmp/cmp_vfy.c

@@ -20,7 +20,6 @@
 #include <openssl/crmf.h>
 #include <openssl/err.h>
 #include <openssl/x509.h>
-#include "crypto/x509.h"
 
 /* Verify a message protected by signature according to RFC section 5.1.3.3 */
 static int verify_signature(const OSSL_CMP_CTX *cmp_ctx,

crypto/ocsp/ocsp_vfy.c

@@ -118,10 +118,6 @@ int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs,
                 goto end;
             if (!X509_add_certs(untrusted, certs, X509_ADD_FLAG_DEFAULT))
                 goto end;
-        } else if (certs != NULL) {
-            untrusted = certs;
-        } else {
-            untrusted = bs->certs;
         }
         ret = ocsp_verify_signer(signer, 1, st, flags, untrusted, &chain);
         if (ret <= 0)

crypto/x509/x509_cmp.c

@@ -175,14 +175,14 @@ int X509_cmp(const X509 *a, const X509 *b)
     return rv < 0 ? -1 : rv > 0;
 }
 
-int X509_add_cert_new(STACK_OF(X509) **sk, X509 *cert, int flags)
+int X509_add_cert_new(STACK_OF(X509) **p_sk, X509 *cert, int flags)
 {
-    if (*sk == NULL
+    if (*p_sk == NULL
-            && (*sk = sk_X509_new_null()) == NULL) {
+            && (*p_sk = sk_X509_new_null()) == NULL) {
         ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
         return 0;
     }
-    return X509_add_cert(*sk, cert, flags);
+    return X509_add_cert(*p_sk, cert, flags);
 }
 
 int X509_add_cert(STACK_OF(X509) *sk, X509 *cert, int flags)
@@ -218,14 +218,25 @@ int X509_add_cert(STACK_OF(X509) *sk, X509 *cert, int flags)
 int X509_add_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs, int flags)
 /* compiler would allow 'const' for the list of certs, yet they are up-ref'ed */
 {
-    int n = sk_X509_num(certs); /* certs may be NULL */
+    if (sk == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
+        return 0;
+    }
+    return ossl_x509_add_certs_new(&sk, certs, flags);
+}
+
+int ossl_x509_add_certs_new(STACK_OF(X509) **p_sk, STACK_OF(X509) *certs,
+                            int flags)
+/* compiler would allow 'const' for the list of certs, yet they are up-ref'ed */
+{
+    int n = sk_X509_num(certs /* may be NULL */);
     int i;
 
     for (i = 0; i < n; i++) {
         int j = (flags & X509_ADD_FLAG_PREPEND) == 0 ? i : n - 1 - i;
         /* if prepend, add certs in reverse order to keep original order */
 
-        if (!X509_add_cert(sk, sk_X509_value(certs, j), flags))
+        if (!X509_add_cert_new(p_sk, sk_X509_value(certs, j), flags))
             return 0;
     }
     return 1;

include/crypto/x509.h

@@ -319,6 +319,8 @@ int asn1_item_digest_ex(const ASN1_ITEM *it, const EVP_MD *type, void *data,
                         unsigned char *md, unsigned int *len,
                         OSSL_LIB_CTX *libctx, const char *propq);
 int X509_add_cert_new(STACK_OF(X509) **sk, X509 *cert, int flags);
+int ossl_x509_add_certs_new(STACK_OF(X509) **p_sk, STACK_OF(X509) *certs,
+                            int flags);
 
 int X509_PUBKEY_get0_libctx(OSSL_LIB_CTX **plibctx, const char **ppropq,
                             const X509_PUBKEY *key);

test/helpers/cmp_testlib.h

@@ -15,7 +15,6 @@
 # include <openssl/cmp.h>
 # include <openssl/pem.h>
 # include <openssl/rand.h>
-# include "crypto/x509.h" /* for x509_set0_libctx() and x509_dup_ex() */
 
 # include "../../crypto/cmp/cmp_local.h"
 # include "../testutil.h"