From eeccc237239d6f2b6fbc557be7062bfe2ab836be Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Sun, 26 Apr 2020 18:30:45 +0200 Subject: [PATCH] Introduce X509_add_cert[s] simplifying various additions to cert lists Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/12615) --- 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 ++--------- ...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 01c5394344c..f0b31487140 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 558414bb5c9..3081dfcc21d 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 4e33fd339c9..84309cc1aff 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 6d6e3bd2b66..d506e7b22bd 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 880051d3dd6..0f70c29953d 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 d1128d7e66f..0ec69d0bb52 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 c702b2ec169..27dc612baf5 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 61f965c6dc6..92321dfc33d 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 9b345de553d..4fac4e6182a 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 95b16dce55a..f45bf1d6dc7 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 3ae337faeb3..d354197d4bf 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 3cfe3649ccb..d20a7148553 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 0dccb24eb50..33cd236af78 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 88925f76d94..eaf6501c1c9 100644 --- a/crypto/pkcs12/p12_kiss.c +++ b/crypto/pkcs12/p12_kiss.c @@ -10,6 +10,7 @@ #include #include "internal/cryptlib.h" #include +#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 abe3570c688..797d1d2c25f 100644 --- a/crypto/pkcs7/pk7_lib.c +++ b/crypto/pkcs7/pk7_lib.c @@ -13,7 +13,7 @@ #include #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 199a3b82e37..71664fa0919 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 25f72e057e7..0e770de11d6 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 e66cfb18258..f37e09dcdfd 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 012f932ee53..843b65f022f 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 289428878e1..97304ee40d9 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 - 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 -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 -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 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 00000000000..292559e52ce --- /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 + + 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 to the given list I. + +X509_add_certs() adds a list of certificate I to the given list I. +The I argument may be NULL, which implies no effect. + +Both these functions have a I parameter, +which is used to control details of the operation. + +The value B, which equals 0, means no special semantics. + +If B is set then +the reference counts of those certificates added successfully are increased. + +If B is set then the certifcates are prepended to I. +By default they are appended to I. +In both cases the original order of the added certificates is preserved. + +If B is set then certificates already contained in I, +which is determined using L, are ignored. + +If B is set then certificates that are marked self-signed, +which is determined using L, are ignored. + +=head1 RETURN VALUES + +Both functions return 1 for success and 0 for failure. + +=head1 SEE ALSO + +L +L + +=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. + +=cut diff --git a/include/crypto/x509.h b/include/crypto/x509.h index 712aa1cc869..87d71de4208 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 935699a55a8..d5b13ded0b6 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 297c01edb2d..fcecca25004 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 2a573cae7aa..1e50e72ffe5 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: -- 2.47.2