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)

diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c
index 26274611a8996e4f103d8665978fbd7d39e0edf9..e65dabe3239666bcca73cd60260f4324346e5114 100644 (file)
--- a/crypto/cmp/cmp_ctx.c
+++ b/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)
-        return 0;
-    if (X509_add_certs(untrusted, certs,
-                       X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP) != 1)
+    if (!ossl_x509_add_certs_new(&untrusted, certs,
+                                 X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         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 ?
-        !X509_add_certs(ctx->untrusted, candidates,
-                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP) :
-        !OSSL_CMP_CTX_set1_untrusted(ctx, candidates))
+    if (!ossl_x509_add_certs_new(&ctx->untrusted, candidates,
+                                 X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         return 0;
 
     ossl_cmp_debug(ctx, "trying to build chain for own CMP signer cert");

diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index c615865864e048f6dac62243c8bdbbdcaa06cd81..a4d3cf9ea4a510c3db03a491d4e26525c202e692 100644 (file)
--- a/crypto/cmp/cmp_local.h
+++ b/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

diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index 4e94d5c1fdb4562c26cbb1c812e72f8df4f88587..36256b3d1d9372b4654ab77826a5a9b59e0adb71 100644 (file)
--- a/crypto/cmp/cmp_msg.c
+++ b/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) {
-        msg->extraCerts = sk_X509_new_reserve(NULL, sk_X509_num(chain));
-        if (msg->extraCerts == NULL
-                || !X509_add_certs(msg->extraCerts, chain,
-                                   X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
-            goto err;
-    }
+    if (sk_X509_num(chain) > 0
+        && !ossl_x509_add_certs_new(&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)

diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c
index fce2ebc468b76163ee76456884763c1c973d615c..aa51bbaa77e707a850e879efe64cb5082aa6b9b8 100644 (file)
--- a/crypto/cmp/cmp_protect.c
+++ b/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,20 +158,19 @@ 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,
-                           "fallback: adding just own CMP signer cert");
+            ossl_cmp_debug(ctx, "fallback: adding just own CMP signer cert");
         }
     }
 
     /* add any additional certificates from ctx->extraCertsOut */
-    if (!X509_add_certs(msg->extraCerts, ctx->extraCertsOut,
-                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
+    if (!ossl_x509_add_certs_new(&msg->extraCerts, ctx->extraCertsOut,
+                                 X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         return 0;
 
     /* in case extraCerts are empty list avoid empty ASN.1 sequence */

diff --git a/crypto/cmp/cmp_util.c b/crypto/cmp/cmp_util.c
index 4f9714a64a2951fa09a18a8e882f21575c1d169d..d246047943621d1c8ef6739adb985067c2c34ecb 100644 (file)
--- a/crypto/cmp/cmp_util.c
+++ b/crypto/cmp/cmp_util.c
@@ -248,11 +248,9 @@ 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)
-        goto err;
-    if (!X509_add_certs(result, chain,
-                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
-                        | X509_ADD_FLAG_NO_SS)) {
+    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;
     }

diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c
index 8b6e856d1aa9f66743465844c6d5b2191eda3d4c..f525c691dece0132db53a6965c9eb8d00a592e6f 100644 (file)
--- a/crypto/cmp/cmp_vfy.c
+++ b/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,

diff --git a/crypto/ocsp/ocsp_vfy.c b/crypto/ocsp/ocsp_vfy.c
index cd9274dd317faa2c1880bfa1387689ca52ffc9b2..544748851ff3a4fa1f97b794b6474ebf8ed36028 100644 (file)
--- a/crypto/ocsp/ocsp_vfy.c
+++ b/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)

diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index a74311e92d4997626b8bebf78cab2320a21feaf9..9cf02be0cb25c86a7876bfe107177c308cab3516 100644 (file)
--- a/crypto/x509/x509_cmp.c
+++ b/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
-            && (*sk = sk_X509_new_null()) == NULL) {
+    if (*p_sk == 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;

diff --git a/include/crypto/x509.h b/include/crypto/x509.h
index 93cb814017bff36658a3dbd6c1f709f74146dd3b..4898eeed61321b7a0fce8d1dec2a433cfe6cb093 100644 (file)
--- a/include/crypto/x509.h
+++ b/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);

diff --git a/test/helpers/cmp_testlib.h b/test/helpers/cmp_testlib.h
index cb881465bc951d7cbd9866fb7e4c6621acd548ad..681b06ae229019557392cdef3ad0cfd5ce933fde 100644 (file)
--- a/test/helpers/cmp_testlib.h
+++ b/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"