[openssl] master update

dev at ddvo.net dev at ddvo.net
Wed Aug 12 12:01:50 UTC 2020


The branch master has been updated
       via  eeccc237239d6f2b6fbc557be7062bfe2ab836be (commit)
      from  e3efe7a53299dff3cd2222542b6a999b1360d626 (commit)


- Log -----------------------------------------------------------------
commit eeccc237239d6f2b6fbc557be7062bfe2ab836be
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Sun Apr 26 18:30:45 2020 +0200

    Introduce X509_add_cert[s] simplifying various additions to cert lists
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/12615)

-----------------------------------------------------------------------

Summary of changes:
 apps/cmp.c                                         | 56 ++-----------------
 crypto/cmp/cmp_ctx.c                               |  3 +-
 crypto/cmp/cmp_local.h                             |  6 +-
 crypto/cmp/cmp_msg.c                               |  3 +-
 crypto/cmp/cmp_protect.c                           | 14 +++--
 crypto/cmp/cmp_util.c                              | 61 ++-------------------
 crypto/cmp/cmp_vfy.c                               | 21 +++++--
 crypto/cms/cms_lib.c                               |  9 +--
 crypto/cms/cms_sd.c                                |  9 +--
 crypto/ocsp/ocsp_cl.c                              |  9 +--
 crypto/ocsp/ocsp_local.h                           |  2 +
 crypto/ocsp/ocsp_srv.c                             |  9 +--
 crypto/ocsp/ocsp_vfy.c                             |  9 +--
 crypto/pkcs12/p12_kiss.c                           |  9 +--
 crypto/pkcs7/pk7_lib.c                             | 15 +----
 crypto/ts/ts_conf.c                                |  9 ++-
 crypto/x509/x509_cmp.c                             | 56 +++++++++++++++++++
 crypto/x509/x509_lu.c                              | 19 ++-----
 crypto/x509/x509_vfy.c                             | 39 ++-----------
 ...cert.pod => ossl_cmp_X509_STORE_add1_certs.pod} | 22 +-------
 doc/man3/X509_add_cert.pod                         | 64 ++++++++++++++++++++++
 include/crypto/x509.h                              |  3 +-
 include/openssl/x509.h                             |  8 +++
 test/cmp_vfy_test.c                                |  4 +-
 util/libcrypto.num                                 |  2 +
 25 files changed, 209 insertions(+), 252 deletions(-)
 rename doc/internal/man3/{ossl_cmp_sk_X509_add1_cert.pod => ossl_cmp_X509_STORE_add1_certs.pod} (53%)
 create mode 100644 doc/man3/X509_add_cert.pod

diff --git a/apps/cmp.c b/apps/cmp.c
index 01c5394344..f0b3148714 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -603,54 +603,6 @@ static int print_to_bio_out(const char *func, const char *file, int line,
     return OSSL_CMP_print_to_bio(bio_out, func, file, line, level, msg);
 }
 
-/* code duplicated from crypto/cmp/cmp_util.c */
-static int sk_X509_add1_cert(STACK_OF(X509) *sk, X509 *cert,
-                             int no_dup, int prepend)
-{
-    if (no_dup) {
-        /*
-         * not using sk_X509_set_cmp_func() and sk_X509_find()
-         * because this re-orders the certs on the stack
-         */
-        int i;
-
-        for (i = 0; i < sk_X509_num(sk); i++) {
-            if (X509_cmp(sk_X509_value(sk, i), cert) == 0)
-                return 1;
-        }
-    }
-    if (!X509_up_ref(cert))
-        return 0;
-    if (!sk_X509_insert(sk, cert, prepend ? 0 : -1)) {
-        X509_free(cert);
-        return 0;
-    }
-    return 1;
-}
-
-/* code duplicated from crypto/cmp/cmp_util.c */
-static int sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs,
-                              int no_self_signed, int no_dups, int prepend)
-/* compiler would allow 'const' for the list of certs, yet they are up-ref'ed */
-{
-    int i;
-
-    if (sk == NULL)
-        return 0;
-    if (certs == NULL)
-        return 1;
-    for (i = 0; i < sk_X509_num(certs); i++) {
-        X509 *cert = sk_X509_value(certs, i);
-
-        if (!no_self_signed || X509_check_issued(cert, cert) != X509_V_OK) {
-            if (!sk_X509_add1_cert(sk, cert, no_dups, prepend))
-                return 0;
-        }
-    }
-    return 1;
-}
-
-/* TODO potentially move to apps/lib/apps.c */
 static char *next_item(char *opt) /* in list separated by comma and/or space */
 {
     /* advance to separator (comma or whitespace), if any */
@@ -1210,7 +1162,8 @@ static STACK_OF(X509) *load_certs_multifile(char *files,
 
         if (!load_certs_autofmt(files, &certs, 0, pass, desc))
             goto err;
-        if (!sk_X509_add1_certs(result, certs, 0, 1 /* no dups */, 0))
+        if (!X509_add_certs(result, certs,
+                            X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
             goto oom;
         sk_X509_pop_free(certs, X509_free);
         certs = NULL;
@@ -1787,8 +1740,9 @@ static int setup_protection_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine)
             /* add any remaining certs to the list of untrusted certs */
             STACK_OF(X509) *untrusted = OSSL_CMP_CTX_get0_untrusted_certs(ctx);
             ok = untrusted != NULL ?
-                sk_X509_add1_certs(untrusted, certs, 0, 1 /* no dups */, 0) :
-                OSSL_CMP_CTX_set1_untrusted_certs(ctx, certs);
+                X509_add_certs(untrusted, certs,
+                               X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP)
+                : OSSL_CMP_CTX_set1_untrusted_certs(ctx, certs);
         }
         sk_X509_pop_free(certs, X509_free);
         if (!ok)
diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c
index 558414bb5c..3081dfcc21 100644
--- a/crypto/cmp/cmp_ctx.c
+++ b/crypto/cmp/cmp_ctx.c
@@ -78,7 +78,8 @@ int OSSL_CMP_CTX_set1_untrusted_certs(OSSL_CMP_CTX *ctx, STACK_OF(X509) *certs)
     }
     if ((untrusted_certs = sk_X509_new_null()) == NULL)
         return 0;
-    if (ossl_cmp_sk_X509_add1_certs(untrusted_certs, certs, 0, 1, 0) != 1)
+    if (X509_add_certs(untrusted_certs, certs,
+                       X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP) != 1)
         goto err;
     sk_X509_pop_free(ctx->untrusted_certs, X509_free);
     ctx->untrusted_certs = untrusted_certs;
diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index 4e33fd339c..84309cc1af 100644
--- a/crypto/cmp/cmp_local.h
+++ b/crypto/cmp/cmp_local.h
@@ -732,11 +732,7 @@ const char *ossl_cmp_log_parse_metadata(const char *buf,
                                         char **file, int *line);
 # define ossl_cmp_add_error_data(txt) ERR_add_error_txt(" : ", txt)
 # define ossl_cmp_add_error_line(txt) ERR_add_error_txt("\n", txt)
-/* functions manipulating lists of certificates etc could be generally useful */
-int ossl_cmp_sk_X509_add1_cert(STACK_OF(X509) *sk, X509 *cert,
-                               int no_dup, int prepend);
-int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs,
-                                int no_self_issued, int no_dups, int prepend);
+/* The two functions manipulating X509_STORE could be generally useful */
 int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs,
                                    int only_self_issued);
 STACK_OF(X509) *ossl_cmp_X509_STORE_get1_certs(X509_STORE *store);
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index 6d6e3bd2b6..d506e7b22b 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -440,7 +440,8 @@ OSSL_CMP_MSG *ossl_cmp_certrep_new(OSSL_CMP_CTX *ctx, int bodytype,
     if (sk_X509_num(chain) > 0) {
         msg->extraCerts = sk_X509_new_reserve(NULL, sk_X509_num(chain));
         if (msg->extraCerts == NULL
-            || !ossl_cmp_sk_X509_add1_certs(msg->extraCerts, chain, 0, 1, 0))
+                || !X509_add_certs(msg->extraCerts, chain,
+                                   X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         goto err;
     }
 
diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c
index 880051d3dd..0f70c29953 100644
--- a/crypto/cmp/cmp_protect.c
+++ b/crypto/cmp/cmp_protect.c
@@ -147,15 +147,17 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
 
     if (ctx->cert != NULL && ctx->pkey != NULL) {
         /* make sure that our own cert is included in the first position */
-        if (!ossl_cmp_sk_X509_add1_cert(msg->extraCerts, ctx->cert, 1, 1))
+        if (!X509_add_cert(msg->extraCerts, ctx->cert,
+                           X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
+                           | X509_ADD_FLAG_PREPEND))
             return 0;
         /* if we have untrusted certs, try to add intermediate certs */
         if (ctx->untrusted_certs != NULL) {
             STACK_OF(X509) *chain =
                 ossl_cmp_build_cert_chain(ctx->untrusted_certs, ctx->cert);
-            int res = ossl_cmp_sk_X509_add1_certs(msg->extraCerts, chain,
-                                                  1 /* no self-issued */,
-                                                  1 /* no duplicates */, 0);
+            int res = X509_add_certs(msg->extraCerts, chain,
+                                     X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
+                                     | X509_ADD_FLAG_NO_SS);
 
             sk_X509_pop_free(chain, X509_free);
             if (res == 0)
@@ -164,8 +166,8 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
     }
 
     /* add any additional certificates from ctx->extraCertsOut */
-    if (!ossl_cmp_sk_X509_add1_certs(msg->extraCerts, ctx->extraCertsOut, 0,
-                                     1 /* no duplicates */, 0))
+    if (!X509_add_certs(msg->extraCerts, ctx->extraCertsOut,
+                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         return 0;
 
     /* if none was found avoid empty ASN.1 sequence */
diff --git a/crypto/cmp/cmp_util.c b/crypto/cmp/cmp_util.c
index d1128d7e66..0ec69d0bb5 100644
--- a/crypto/cmp/cmp_util.c
+++ b/crypto/cmp/cmp_util.c
@@ -184,60 +184,6 @@ void OSSL_CMP_print_errors_cb(OSSL_CMP_log_cb_t log_fn)
     }
 }
 
-/*
- * functions manipulating lists of certificates etc.
- * these functions could be generally useful.
- */
-
-int ossl_cmp_sk_X509_add1_cert(STACK_OF(X509) *sk, X509 *cert,
-                               int no_dup, int prepend)
-{
-    if (sk == NULL) {
-        CMPerr(0, CMP_R_NULL_ARGUMENT);
-        return 0;
-    }
-    if (no_dup) {
-        /*
-         * not using sk_X509_set_cmp_func() and sk_X509_find()
-         * because this re-orders the certs on the stack
-         */
-        int i;
-
-        for (i = 0; i < sk_X509_num(sk); i++) {
-            if (X509_cmp(sk_X509_value(sk, i), cert) == 0)
-                return 1;
-        }
-    }
-    if (!X509_up_ref(cert))
-        return 0;
-    if (!sk_X509_insert(sk, cert, prepend ? 0 : -1)) {
-        X509_free(cert);
-        return 0;
-    }
-    return 1;
-}
-
-int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs,
-                                int no_self_signed, int no_dups, int prepend)
-/* compiler would allow 'const' for the list of certs, yet they are up-ref'ed */
-{
-    int i;
-
-    if (sk == NULL) {
-        CMPerr(0, CMP_R_NULL_ARGUMENT);
-        return 0;
-    }
-    for (i = 0; i < sk_X509_num(certs); i++) { /* certs may be NULL */
-        X509 *cert = sk_X509_value(certs, i);
-
-        if (!no_self_signed || X509_self_signed(cert, 0) != 1) {
-            if (!ossl_cmp_sk_X509_add1_cert(sk, cert, no_dups, prepend))
-                return 0;
-        }
-    }
-    return 1;
-}
-
 int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs,
                                    int only_self_signed)
 {
@@ -310,11 +256,12 @@ STACK_OF(X509) *ossl_cmp_build_cert_chain(STACK_OF(X509) *certs, X509 *cert)
 
     chain = X509_STORE_CTX_get0_chain(csc);
 
-    /* result list to store the up_ref'ed not self-issued certificates */
+    /* result list to store the up_ref'ed not self-signed certificates */
     if ((result = sk_X509_new_null()) == NULL)
         goto err;
-    if (!ossl_cmp_sk_X509_add1_certs(result, chain, 1 /* no self-issued */,
-                                     1 /* no duplicates */, 0)) {
+    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);
         result = NULL;
     }
diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c
index c702b2ec16..27dc612baf 100644
--- a/crypto/cmp/cmp_vfy.c
+++ b/crypto/cmp/cmp_vfy.c
@@ -695,9 +695,10 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
      * extraCerts because they do not belong to the protected msg part anyway.
      * For efficiency, the extraCerts are prepended so they get used first.
      */
-    if (!ossl_cmp_sk_X509_add1_certs(ctx->untrusted_certs, msg->extraCerts,
-                                     0 /* this allows self-issued certs */,
-                                     1 /* no_dups */, 1 /* prepend */))
+    if (!X509_add_certs(ctx->untrusted_certs, msg->extraCerts,
+                        /* this allows self-signed certs */
+                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
+                        | X509_ADD_FLAG_PREPEND))
         return 0;
 
     /* validate message protection */
@@ -768,7 +769,19 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
     /* if not yet present, learn transactionID */
     if (ctx->transactionID == NULL
         && !OSSL_CMP_CTX_set1_transactionID(ctx, hdr->transactionID))
-        return 0;
+        return -1;
+
+    /*
+     * Store any provided extraCerts in ctx for future use,
+     * such that they are available to ctx->certConf_cb and
+     * the peer does not need to send them again in the same transaction.
+     * For efficiency, the extraCerts are prepended so they get used first.
+     */
+    if (!X509_add_certs(ctx->untrusted_certs, msg->extraCerts,
+                        /* this allows self-signed certs */
+                        X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
+                        | X509_ADD_FLAG_PREPEND))
+        return -1;
 
     if (ossl_cmp_hdr_get_protection_nid(hdr) == NID_id_PasswordBasedMAC) {
         /*
diff --git a/crypto/cms/cms_lib.c b/crypto/cms/cms_lib.c
index 61f965c6dc..92321dfc33 100644
--- a/crypto/cms/cms_lib.c
+++ b/crypto/cms/cms_lib.c
@@ -603,16 +603,11 @@ STACK_OF(X509) *CMS_get1_certs(CMS_ContentInfo *cms)
     for (i = 0; i < sk_CMS_CertificateChoices_num(*pcerts); i++) {
         cch = sk_CMS_CertificateChoices_value(*pcerts, i);
         if (cch->type == 0) {
-            if (!certs) {
-                certs = sk_X509_new_null();
-                if (!certs)
-                    return NULL;
-            }
-            if (!sk_X509_push(certs, cch->d.certificate)) {
+            if (!X509_add_cert_new(&certs, cch->d.certificate,
+                                   X509_ADD_FLAG_UP_REF)) {
                 sk_X509_pop_free(certs, X509_free);
                 return NULL;
             }
-            X509_up_ref(cch->d.certificate);
         }
     }
     return certs;
diff --git a/crypto/cms/cms_sd.c b/crypto/cms/cms_sd.c
index 9b345de553..4fac4e6182 100644
--- a/crypto/cms/cms_sd.c
+++ b/crypto/cms/cms_sd.c
@@ -20,6 +20,7 @@
 #include "crypto/evp.h"
 #include "crypto/cms.h"
 #include "crypto/ess.h"
+#include "crypto/x509.h" /* for X509_add_cert_new() */
 
 DEFINE_STACK_OF(CMS_RevocationInfoChoice)
 DEFINE_STACK_OF(CMS_SignerInfo)
@@ -510,12 +511,8 @@ STACK_OF(X509) *CMS_get0_signers(CMS_ContentInfo *cms)
     for (i = 0; i < sk_CMS_SignerInfo_num(sinfos); i++) {
         si = sk_CMS_SignerInfo_value(sinfos, i);
         if (si->signer != NULL) {
-            if (signers == NULL) {
-                signers = sk_X509_new_null();
-                if (signers == NULL)
-                    return NULL;
-            }
-            if (!sk_X509_push(signers, si->signer)) {
+            if (!X509_add_cert_new(&signers, si->signer,
+                                   X509_ADD_FLAG_DEFAULT)) {
                 sk_X509_free(signers);
                 return NULL;
             }
diff --git a/crypto/ocsp/ocsp_cl.c b/crypto/ocsp/ocsp_cl.c
index 95b16dce55..f45bf1d6dc 100644
--- a/crypto/ocsp/ocsp_cl.c
+++ b/crypto/ocsp/ocsp_cl.c
@@ -81,14 +81,7 @@ int OCSP_request_add1_cert(OCSP_REQUEST *req, X509 *cert)
         return 0;
     if (cert == NULL)
         return 1;
-    if (sig->certs == NULL
-        && (sig->certs = sk_X509_new_null()) == NULL)
-        return 0;
-
-    if (!sk_X509_push(sig->certs, cert))
-        return 0;
-    X509_up_ref(cert);
-    return 1;
+    return X509_add_cert_new(&sig->certs, cert, X509_ADD_FLAG_UP_REF);
 }
 
 /*
diff --git a/crypto/ocsp/ocsp_local.h b/crypto/ocsp/ocsp_local.h
index 3ae337faeb..d354197d4b 100644
--- a/crypto/ocsp/ocsp_local.h
+++ b/crypto/ocsp/ocsp_local.h
@@ -7,6 +7,8 @@
  * https://www.openssl.org/source/license.html
  */
 
+#include "crypto/x509.h" /* for X509_add_cert_new() */
+
 /*-  CertID ::= SEQUENCE {
  *       hashAlgorithm            AlgorithmIdentifier,
  *       issuerNameHash     OCTET STRING, -- Hash of Issuer's DN
diff --git a/crypto/ocsp/ocsp_srv.c b/crypto/ocsp/ocsp_srv.c
index 3cfe3649cc..d20a714855 100644
--- a/crypto/ocsp/ocsp_srv.c
+++ b/crypto/ocsp/ocsp_srv.c
@@ -162,14 +162,7 @@ OCSP_SINGLERESP *OCSP_basic_add1_status(OCSP_BASICRESP *rsp,
 
 int OCSP_basic_add1_cert(OCSP_BASICRESP *resp, X509 *cert)
 {
-    if (resp->certs == NULL
-        && (resp->certs = sk_X509_new_null()) == NULL)
-        return 0;
-
-    if (!sk_X509_push(resp->certs, cert))
-        return 0;
-    X509_up_ref(cert);
-    return 1;
+    return X509_add_cert_new(&resp->certs, cert, X509_ADD_FLAG_UP_REF);
 }
 
 /*
diff --git a/crypto/ocsp/ocsp_vfy.c b/crypto/ocsp/ocsp_vfy.c
index 0dccb24eb5..33cd236af7 100644
--- a/crypto/ocsp/ocsp_vfy.c
+++ b/crypto/ocsp/ocsp_vfy.c
@@ -67,16 +67,13 @@ int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs,
     }
     if (!(flags & OCSP_NOVERIFY)) {
         int init_res;
+
         if (flags & OCSP_NOCHAIN) {
             untrusted = NULL;
         } else if (bs->certs && certs) {
             untrusted = sk_X509_dup(bs->certs);
-            for (i = 0; i < sk_X509_num(certs); i++) {
-                if (!sk_X509_push(untrusted, sk_X509_value(certs, i))) {
-                    OCSPerr(OCSP_F_OCSP_BASIC_VERIFY, ERR_R_MALLOC_FAILURE);
-                    goto f_err;
-                }
-            }
+            if (!X509_add_certs(untrusted, certs, X509_ADD_FLAG_DEFAULT))
+                goto f_err;
         } else if (certs != NULL) {
             untrusted = certs;
         } else {
diff --git a/crypto/pkcs12/p12_kiss.c b/crypto/pkcs12/p12_kiss.c
index 88925f76d9..eaf6501c1c 100644
--- a/crypto/pkcs12/p12_kiss.c
+++ b/crypto/pkcs12/p12_kiss.c
@@ -10,6 +10,7 @@
 #include <stdio.h>
 #include "internal/cryptlib.h"
 #include <openssl/pkcs12.h>
+#include "crypto/x509.h" /* for X509_add_cert_new() */
 
 DEFINE_STACK_OF(X509)
 DEFINE_STACK_OF(PKCS7)
@@ -99,12 +100,8 @@ int PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
             ERR_pop_to_mark();
         }
 
-        if (ca && x) {
-            if (*ca == NULL)
-                *ca = sk_X509_new_null();
-            if (*ca == NULL)
-                goto err;
-            if (!sk_X509_push(*ca, x))
+        if (ca != NULL && x != NULL) {
+            if (!X509_add_cert_new(ca, x, X509_ADD_FLAG_DEFAULT))
                 goto err;
             x = NULL;
         }
diff --git a/crypto/pkcs7/pk7_lib.c b/crypto/pkcs7/pk7_lib.c
index abe3570c68..797d1d2c25 100644
--- a/crypto/pkcs7/pk7_lib.c
+++ b/crypto/pkcs7/pk7_lib.c
@@ -13,7 +13,7 @@
 #include <openssl/x509.h>
 #include "crypto/asn1.h"
 #include "crypto/evp.h"
-#include "crypto/x509.h"
+#include "crypto/x509.h" /* for sk_X509_add1_cert() */
 #include "pk7_local.h"
 
 DEFINE_STACK_OF(X509)
@@ -262,18 +262,7 @@ int PKCS7_add_certificate(PKCS7 *p7, X509 *x509)
         return 0;
     }
 
-    if (*sk == NULL)
-        *sk = sk_X509_new_null();
-    if (*sk == NULL) {
-        PKCS7err(PKCS7_F_PKCS7_ADD_CERTIFICATE, ERR_R_MALLOC_FAILURE);
-        return 0;
-    }
-    X509_up_ref(x509);
-    if (!sk_X509_push(*sk, x509)) {
-        X509_free(x509);
-        return 0;
-    }
-    return 1;
+    return X509_add_cert_new(sk, x509, X509_ADD_FLAG_UP_REF);
 }
 
 int PKCS7_add_crl(PKCS7 *p7, X509_CRL *crl)
diff --git a/crypto/ts/ts_conf.c b/crypto/ts/ts_conf.c
index 199a3b82e3..71664fa091 100644
--- a/crypto/ts/ts_conf.c
+++ b/crypto/ts/ts_conf.c
@@ -78,8 +78,13 @@ STACK_OF(X509) *TS_CONF_load_certs(const char *file)
     allcerts = PEM_X509_INFO_read_bio(certs, NULL, NULL, NULL);
     for (i = 0; i < sk_X509_INFO_num(allcerts); i++) {
         X509_INFO *xi = sk_X509_INFO_value(allcerts, i);
-        if (xi->x509) {
-            sk_X509_push(othercerts, xi->x509);
+
+        if (xi->x509 != NULL) {
+            if (!X509_add_cert(othercerts, xi->x509, X509_ADD_FLAG_DEFAULT)) {
+                sk_X509_pop_free(othercerts, X509_free);
+                othercerts = NULL;
+                goto end;
+            }
             xi->x509 = NULL;
         }
     }
diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index 25f72e057e..0e770de11d 100644
--- a/crypto/x509/x509_cmp.c
+++ b/crypto/x509/x509_cmp.c
@@ -163,6 +163,62 @@ int X509_cmp(const X509 *a, const X509 *b)
     return rv;
 }
 
+int X509_add_cert_new(STACK_OF(X509) **sk, X509 *cert, int flags)
+{
+    if (*sk == NULL
+            && (*sk = sk_X509_new_null()) == NULL) {
+        X509err(0, ERR_R_MALLOC_FAILURE);
+        return 0;
+    }
+    return X509_add_cert(*sk, cert, flags);
+}
+
+int X509_add_cert(STACK_OF(X509) *sk, X509 *cert, int flags)
+{
+    if (sk == NULL) {
+        X509err(0, ERR_R_PASSED_NULL_PARAMETER);
+        return 0;
+    }
+    if ((flags & X509_ADD_FLAG_NO_DUP) != 0) {
+        /*
+         * not using sk_X509_set_cmp_func() and sk_X509_find()
+         * because this re-orders the certs on the stack
+         */
+        int i;
+
+        for (i = 0; i < sk_X509_num(sk); i++) {
+            if (X509_cmp(sk_X509_value(sk, i), cert) == 0)
+                return 1;
+        }
+    }
+    if ((flags & X509_ADD_FLAG_NO_SS) != 0 && X509_self_signed(cert, 0))
+        return 1;
+    if (!sk_X509_insert(sk, cert,
+                        (flags & X509_ADD_FLAG_PREPEND) != 0 ? 0 : -1)) {
+        X509err(0, ERR_R_MALLOC_FAILURE);
+        return 0;
+    }
+    if ((flags & X509_ADD_FLAG_UP_REF) != 0)
+        (void)X509_up_ref(cert);
+    return 1;
+}
+
+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 */
+    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))
+            return 0;
+    }
+    return 1;
+}
+
 int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b)
 {
     int ret;
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index e66cfb1825..f37e09dcdf 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -578,14 +578,9 @@ STACK_OF(X509) *X509_STORE_get1_all_certs(X509_STORE *store)
     for (i = 0; i < sk_X509_OBJECT_num(objs); i++) {
         X509 *cert = X509_OBJECT_get0_X509(sk_X509_OBJECT_value(objs, i));
 
-        if (cert != NULL) {
-            if (!X509_up_ref(cert))
-                goto err;
-            if (!sk_X509_push(sk, cert)) {
-                X509_free(cert);
-                goto err;
-            }
-        }
+        if (cert != NULL
+            && !X509_add_cert(sk, cert, X509_ADD_FLAG_UP_REF))
+            goto err;
     }
     X509_STORE_unlock(store);
     return sk;
@@ -638,14 +633,8 @@ STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
     for (i = 0; i < cnt; i++, idx++) {
         obj = sk_X509_OBJECT_value(store->objs, idx);
         x = obj->data.x509;
-        if (!X509_up_ref(x)) {
-            X509_STORE_unlock(store);
-            sk_X509_pop_free(sk, X509_free);
-            return NULL;
-        }
-        if (!sk_X509_push(sk, x)) {
+        if (!X509_add_cert(sk, x, X509_ADD_FLAG_UP_REF)) {
             X509_STORE_unlock(store);
-            X509_free(x);
             sk_X509_pop_free(sk, X509_free);
             return NULL;
         }
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 012f932ee5..843b65f022 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -285,24 +285,10 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
         return -1;
     }
 
-    if (!X509_up_ref(ctx->cert)) {
-        X509err(X509_F_X509_VERIFY_CERT, ERR_R_INTERNAL_ERROR);
-        ctx->error = X509_V_ERR_UNSPECIFIED;
-        return -1;
-    }
-
-    /*
-     * first we make sure the chain we are going to build is present and that
-     * the first entry is in place
-     */
-    if ((ctx->chain = sk_X509_new_null()) == NULL
-            || !sk_X509_push(ctx->chain, ctx->cert)) {
-        X509_free(ctx->cert);
-        X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
+    if (!X509_add_cert_new(&ctx->chain, ctx->cert, X509_ADD_FLAG_UP_REF)) {
         ctx->error = X509_V_ERR_OUT_OF_MEM;
         return -1;
     }
-
     ctx->num_untrusted = 1;
 
     /* If the peer's public key is too weak, we can stop early. */
@@ -395,18 +381,8 @@ static STACK_OF(X509) *lookup_certs_sk(X509_STORE_CTX *ctx,
     for (i = 0; i < sk_X509_num(ctx->other_ctx); i++) {
         x = sk_X509_value(ctx->other_ctx, i);
         if (X509_NAME_cmp(nm, X509_get_subject_name(x)) == 0) {
-            if (!X509_up_ref(x)) {
-                sk_X509_pop_free(sk, X509_free);
-                X509err(X509_F_LOOKUP_CERTS_SK, ERR_R_INTERNAL_ERROR);
-                ctx->error = X509_V_ERR_UNSPECIFIED;
-                return NULL;
-            }
-            if (sk == NULL)
-                sk = sk_X509_new_null();
-            if (sk == NULL || !sk_X509_push(sk, x)) {
-                X509_free(x);
+            if (!X509_add_cert_new(&sk, x, X509_ADD_FLAG_UP_REF)) {
                 sk_X509_pop_free(sk, X509_free);
-                X509err(X509_F_LOOKUP_CERTS_SK, ERR_R_MALLOC_FAILURE);
                 ctx->error = X509_V_ERR_OUT_OF_MEM;
                 return NULL;
             }
@@ -3053,13 +3029,10 @@ static int build_chain(X509_STORE_CTX *ctx)
             ctx->error = X509_V_ERR_OUT_OF_MEM;
             return 0;
         }
-        for (i = 0; i < sk_X509_num(dane->certs); ++i) {
-            if (!sk_X509_push(sktmp, sk_X509_value(dane->certs, i))) {
-                sk_X509_free(sktmp);
-                X509err(X509_F_BUILD_CHAIN, ERR_R_MALLOC_FAILURE);
-                ctx->error = X509_V_ERR_OUT_OF_MEM;
-                return 0;
-            }
+        if (!X509_add_certs(sktmp, dane->certs, X509_ADD_FLAG_DEFAULT)) {
+            sk_X509_free(sktmp);
+            ctx->error = X509_V_ERR_OUT_OF_MEM;
+            return 0;
         }
     }
 
diff --git a/doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod b/doc/internal/man3/ossl_cmp_X509_STORE_add1_certs.pod
similarity index 53%
rename from doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod
rename to doc/internal/man3/ossl_cmp_X509_STORE_add1_certs.pod
index 289428878e..97304ee40d 100644
--- a/doc/internal/man3/ossl_cmp_sk_X509_add1_cert.pod
+++ b/doc/internal/man3/ossl_cmp_X509_STORE_add1_certs.pod
@@ -2,36 +2,20 @@
 
 =head1 NAME
 
-ossl_cmp_sk_X509_add1_cert,
-ossl_cmp_sk_X509_add1_certs,
 ossl_cmp_X509_STORE_add1_certs,
 ossl_cmp_X509_STORE_get1_certs
-- functions manipulating lists of certificates
+- functions manipulating stores of certificates
 
 =head1 SYNOPSIS
 
   #include <openssl/cmp_util.h>
 
-  int ossl_cmp_sk_X509_add1_cert(STACK_OF(X509) *sk, X509 *cert,
-                                 int no_dup, int prepend);
-  int ossl_cmp_sk_X509_add1_certs(STACK_OF(X509) *sk, STACK_OF(X509) *certs,
-                                  int no_self_signed, int no_dups, int prepend);
   int ossl_cmp_X509_STORE_add1_certs(X509_STORE *store, STACK_OF(X509) *certs,
                                      int only_self_signed);
   STACK_OF(X509) *ossl_cmp_X509_STORE_get1_certs(X509_STORE *store);
 
 =head1 DESCRIPTION
 
-ossl_cmp_sk_X509_add1_cert() appends or prepends (depending on the I<prepend>
-argument) a certificate to the given list,
-optionally only if it is not already contained.
-On success the reference count of the certificate is increased.
-
-ossl_cmp_sk_X509_add1_certs() appends or prepends (depending on the I<prepend>
-argument) a list of certificates to the given list,
-optionally only if not self-signed and optionally only if not already contained.
-The reference counts of those certificates appended successfully are increased.
-
 ossl_cmp_X509_STORE_add1_certs() adds all or only self-signed certificates from
 the given stack to given store. The I<certs> parameter may be NULL.
 
@@ -40,9 +24,9 @@ given store.
 
 =head1 RETURN VALUES
 
-ossl_cmp_X509_STORE_get1_certs() returns a list of certificates, NULL on error.
+ossl_cmp_X509_STORE_add1_certs() returns 1 on success, 0 on error.
 
-All other functions return 1 on success, 0 on error.
+ossl_cmp_X509_STORE_get1_certs() returns a list of certificates, NULL on error.
 
 =head1 HISTORY
 
diff --git a/doc/man3/X509_add_cert.pod b/doc/man3/X509_add_cert.pod
new file mode 100644
index 0000000000..292559e52c
--- /dev/null
+++ b/doc/man3/X509_add_cert.pod
@@ -0,0 +1,64 @@
+=pod
+
+=head1 NAME
+
+X509_add_cert,
+X509_add_certs -
+X509 certificate list addition functions
+
+=head1 SYNOPSIS
+
+ #include <openssl/x509.h>
+
+ 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);
+
+=head1 DESCRIPTION
+
+X509_add_cert() adds a certificate I<cert> to the given list I<sk>.
+
+X509_add_certs() adds a list of certificate I<certs> to the given list I<sk>.
+The I<certs> argument may be NULL, which implies no effect.
+
+Both these functions have a I<flags> parameter,
+which is used to control details of the operation.
+
+The value B<X509_ADD_FLAG_DEFAULT>, which equals 0, means no special semantics.
+
+If B<X509_ADD_FLAG_UP_REF> is set then
+the reference counts of those certificates added successfully are increased.
+
+If B<X509_ADD_FLAG_PREPEND> is set then the certifcates are prepended to I<sk>.
+By default they are appended to I<sk>.
+In both cases the original order of the added certificates is preserved.
+
+If B<X509_ADD_FLAG_NO_DUP> is set then certificates already contained in I<sk>,
+which is determined using L<X509_cmp(3)>, are ignored.
+
+If B<X509_ADD_FLAG_NO_SS> is set then certificates that are marked self-signed,
+which is determined using L<X509_self_signed(3)>, are ignored.
+
+=head1 RETURN VALUES
+
+Both functions return 1 for success and 0 for failure.
+
+=head1 SEE ALSO
+
+L<X509_cmp(3)>
+L<X509_self_signed(3)>
+
+=head1 HISTORY
+
+The functions X509_add_cert() and X509_add_certs()
+were added in OpenSSL 3.0.
+
+=head1 COPYRIGHT
+
+Copyright 2019-2020 The OpenSSL Project Authors. All Rights Reserved.
+
+Licensed under the Apache License 2.0 (the "License").  You may not use
+this file except in compliance with the License.  You can obtain a copy
+in the file LICENSE in the source distribution or at
+L<https://www.openssl.org/source/license.html>.
+
+=cut
diff --git a/include/crypto/x509.h b/include/crypto/x509.h
index 712aa1cc86..87d71de420 100644
--- a/include/crypto/x509.h
+++ b/include/crypto/x509.h
@@ -300,10 +300,9 @@ int x509_set1_time(ASN1_TIME **ptm, const ASN1_TIME *tm);
 int x509_print_ex_brief(BIO *bio, X509 *cert, unsigned long neg_cflags);
 int x509v3_cache_extensions(X509 *x);
 int x509_set0_libctx(X509 *x, OPENSSL_CTX *libctx, const char *propq);
-
 void x509_init_sig_info(X509 *x);
-
 int asn1_item_digest_with_libctx(const ASN1_ITEM *it, const EVP_MD *type,
                                  void *data, unsigned char *md,
                                  unsigned int *len, OPENSSL_CTX *libctx,
                                  const char *propq);
+int X509_add_cert_new(STACK_OF(X509) **sk, X509 *cert, int flags);
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 935699a55a..d5b13ded0b 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -787,6 +787,14 @@ unsigned long X509_issuer_name_hash_old(X509 *a);
 unsigned long X509_subject_name_hash_old(X509 *x);
 # endif
 
+# define X509_ADD_FLAG_DEFAULT  0
+# define X509_ADD_FLAG_UP_REF   0x1
+# define X509_ADD_FLAG_PREPEND  0x2
+# define X509_ADD_FLAG_NO_DUP   0x4
+# define X509_ADD_FLAG_NO_SS    0x8
+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);
+
 int X509_cmp(const X509 *a, const X509 *b);
 int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b);
 unsigned long X509_NAME_hash(const X509_NAME *x);
diff --git a/test/cmp_vfy_test.c b/test/cmp_vfy_test.c
index 297c01edb2..fcecca2500 100644
--- a/test/cmp_vfy_test.c
+++ b/test/cmp_vfy_test.c
@@ -186,8 +186,8 @@ static int add_trusted(OSSL_CMP_CTX *ctx, X509 *cert)
 
 static int add_untrusted(OSSL_CMP_CTX *ctx, X509 *cert)
 {
-    return ossl_cmp_sk_X509_add1_cert(OSSL_CMP_CTX_get0_untrusted_certs(ctx),
-                                      cert, 0, 0);
+    return X509_add_cert(OSSL_CMP_CTX_get0_untrusted_certs(ctx), cert,
+                         X509_ADD_FLAG_UP_REF);
 }
 
 static int test_validate_msg_signature_partial_chain(int expired)
diff --git a/util/libcrypto.num b/util/libcrypto.num
index 2a573cae7a..1e50e72ffe 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -4837,6 +4837,8 @@ OSSL_CMP_CTX_snprint_PKIStatus          ?	3_0_0	EXIST::FUNCTION:CMP
 OSSL_CMP_HDR_get0_transactionID         ?	3_0_0	EXIST::FUNCTION:CMP
 OSSL_CMP_HDR_get0_recipNonce            ?	3_0_0	EXIST::FUNCTION:CMP
 X509_LOOKUP_store                       ?	3_0_0	EXIST::FUNCTION:
+X509_add_cert                           ?	3_0_0	EXIST::FUNCTION:
+X509_add_certs                          ?	3_0_0	EXIST::FUNCTION:
 X509_STORE_load_file                    ?	3_0_0	EXIST::FUNCTION:
 X509_STORE_load_path                    ?	3_0_0	EXIST::FUNCTION:
 X509_STORE_load_store                   ?	3_0_0	EXIST::FUNCTION:


More information about the openssl-commits mailing list