From: Dr. David von Oheimb Date: Wed, 4 Oct 2023 19:32:00 +0000 (+0200) Subject: {CMS,PKCS7}_verify(): use 'certs' parameter ('-certfile' option) also for chain building X-Git-Tag: openssl-3.4.0-alpha1~326 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=29bbe7d0086aec1f0fec1ffc03d05aa4610c4a12;p=thirdparty%2Fopenssl.git {CMS,PKCS7}_verify(): use 'certs' parameter ('-certfile' option) also for chain building Reviewed-by: Viktor Dukhovni Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18916) --- diff --git a/apps/cms.c b/apps/cms.c index f93c98ac92c..daa1d7c1a6b 100644 --- a/apps/cms.c +++ b/apps/cms.c @@ -175,7 +175,10 @@ const OPTIONS cms_options[] = { OPT_SECTION("Signing"), {"md", OPT_MD, 's', "Digest algorithm to use"}, {"signer", OPT_SIGNER, 's', "Signer certificate input file"}, - {"certfile", OPT_CERTFILE, '<', "Other certificates file"}, + {"certfile", OPT_CERTFILE, '<', + "Extra signer and intermediate CA certificates to include when signing"}, + {OPT_MORE_STR, 0, 0, + "or to use as preferred signer certs and for chain building when verifying"}, {"cades", OPT_CADES, '-', "Include signingCertificate attribute (CAdES-BES)"}, {"nodetach", OPT_NODETACH, '-', "Use opaque signing"}, diff --git a/apps/smime.c b/apps/smime.c index b59e14b0b5e..88ce8fd98d9 100644 --- a/apps/smime.c +++ b/apps/smime.c @@ -96,7 +96,10 @@ const OPTIONS smime_options[] = { {"nosigs", OPT_NOSIGS, '-', "Don't verify message signature"}, {"noverify", OPT_NOVERIFY, '-', "Don't verify signers certificate"}, - {"certfile", OPT_CERTFILE, '<', "Other certificates file"}, + {"certfile", OPT_CERTFILE, '<', + "Extra signer and intermediate CA certificates to include when signing"}, + {OPT_MORE_STR, 0, 0, + "or to use as preferred signer certs and for chain building when verifying"}, {"recip", OPT_RECIP, '<', "Recipient certificate file for decryption"}, OPT_SECTION("Email"), diff --git a/crypto/cms/cms_smime.c b/crypto/cms/cms_smime.c index 4df4487cbc2..3bc70a7b30b 100644 --- a/crypto/cms/cms_smime.c +++ b/crypto/cms/cms_smime.c @@ -15,6 +15,7 @@ #include #include "cms_local.h" #include "crypto/asn1.h" +#include "crypto/x509.h" static BIO *cms_get_text_bio(BIO *out, unsigned int flags) { @@ -308,7 +309,7 @@ int CMS_verify(CMS_ContentInfo *cms, STACK_OF(X509) *certs, { CMS_SignerInfo *si; STACK_OF(CMS_SignerInfo) *sinfos; - STACK_OF(X509) *cms_certs = NULL; + STACK_OF(X509) *untrusted = NULL; STACK_OF(X509_CRL) *crls = NULL; STACK_OF(X509) **si_chains = NULL; X509 *signer; @@ -360,13 +361,21 @@ int CMS_verify(CMS_ContentInfo *cms, STACK_OF(X509) *certs, if (si_chains == NULL) goto err; } - cms_certs = CMS_get1_certs(cms); - if (!(flags & CMS_NOCRL)) - crls = CMS_get1_crls(cms); + if ((untrusted = CMS_get1_certs(cms)) == NULL) + goto err; + if (sk_X509_num(certs) > 0 + && !ossl_x509_add_certs_new(&untrusted, certs, + X509_ADD_FLAG_UP_REF | + X509_ADD_FLAG_NO_DUP)) + goto err; + + if ((flags & CMS_NOCRL) == 0 + && (crls = CMS_get1_crls(cms)) == NULL) + goto err; for (i = 0; i < scount; i++) { si = sk_CMS_SignerInfo_value(sinfos, i); - if (!cms_signerinfo_verify_cert(si, store, cms_certs, crls, + if (!cms_signerinfo_verify_cert(si, store, untrusted, crls, si_chains ? &si_chains[i] : NULL, ctx)) goto err; @@ -482,7 +491,7 @@ int CMS_verify(CMS_ContentInfo *cms, STACK_OF(X509) *certs, OSSL_STACK_OF_X509_free(si_chains[i]); OPENSSL_free(si_chains); } - OSSL_STACK_OF_X509_free(cms_certs); + sk_X509_pop_free(untrusted, X509_free); sk_X509_CRL_pop_free(crls, X509_CRL_free); return ret; diff --git a/crypto/pkcs7/pk7_lib.c b/crypto/pkcs7/pk7_lib.c index 7be29285429..05ca70ae24c 100644 --- a/crypto/pkcs7/pk7_lib.c +++ b/crypto/pkcs7/pk7_lib.c @@ -413,7 +413,7 @@ PKCS7_SIGNER_INFO *PKCS7_add_signature(PKCS7 *p7, X509 *x509, EVP_PKEY *pkey, return NULL; } -static STACK_OF(X509) *pkcs7_get_signer_certs(const PKCS7 *p7) +STACK_OF(X509) *pkcs7_get0_certificates(const PKCS7 *p7) { if (p7->d.ptr == NULL) return NULL; @@ -454,7 +454,7 @@ void ossl_pkcs7_resolve_libctx(PKCS7 *p7) rinfos = pkcs7_get_recipient_info(p7); sinfos = PKCS7_get_signer_info(p7); - certs = pkcs7_get_signer_certs(p7); + certs = pkcs7_get0_certificates(p7); for (i = 0; i < sk_X509_num(certs); i++) ossl_x509_set0_libctx(sk_X509_value(certs, i), libctx, propq); diff --git a/crypto/pkcs7/pk7_local.h b/crypto/pkcs7/pk7_local.h index 8deb342b791..7e9335f6017 100644 --- a/crypto/pkcs7/pk7_local.h +++ b/crypto/pkcs7/pk7_local.h @@ -9,6 +9,7 @@ #include "crypto/pkcs7.h" +STACK_OF(X509) *pkcs7_get0_certificates(const PKCS7 *p7); const PKCS7_CTX *ossl_pkcs7_get0_ctx(const PKCS7 *p7); OSSL_LIB_CTX *ossl_pkcs7_ctx_get0_libctx(const PKCS7_CTX *ctx); const char *ossl_pkcs7_ctx_get0_propq(const PKCS7_CTX *ctx); diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c index 747c4177186..3f9ba3b7d6d 100644 --- a/crypto/pkcs7/pk7_smime.c +++ b/crypto/pkcs7/pk7_smime.c @@ -11,6 +11,7 @@ #include #include "internal/cryptlib.h" +#include "crypto/x509.h" #include #include #include "pk7_local.h" @@ -215,6 +216,8 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, BIO *indata, BIO *out, int flags) { STACK_OF(X509) *signers; + STACK_OF(X509) *included_certs; + STACK_OF(X509) *untrusted = NULL; X509 *signer; STACK_OF(PKCS7_SIGNER_INFO) *sinfos; PKCS7_SIGNER_INFO *si; @@ -272,21 +275,24 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, ossl_pkcs7_ctx_get0_propq(p7_ctx)); if (cert_ctx == NULL) goto err; - if (!(flags & PKCS7_NOVERIFY)) + if ((flags & PKCS7_NOVERIFY) == 0) { + if (!ossl_x509_add_certs_new(&untrusted, certs, X509_ADD_FLAG_NO_DUP)) + goto err; + included_certs = pkcs7_get0_certificates(p7); + if ((flags & PKCS7_NOCHAIN) == 0 + && !ossl_x509_add_certs_new(&untrusted, included_certs, + X509_ADD_FLAG_NO_DUP)) + goto err; + for (k = 0; k < sk_X509_num(signers); k++) { signer = sk_X509_value(signers, k); - if (!(flags & PKCS7_NOCHAIN)) { - if (!X509_STORE_CTX_init(cert_ctx, store, signer, - p7->d.sign->cert)) { - ERR_raise(ERR_LIB_PKCS7, ERR_R_X509_LIB); - goto err; - } - if (!X509_STORE_CTX_set_default(cert_ctx, "smime_sign")) - goto err; - } else if (!X509_STORE_CTX_init(cert_ctx, store, signer, NULL)) { + if (!X509_STORE_CTX_init(cert_ctx, store, signer, untrusted)) { ERR_raise(ERR_LIB_PKCS7, ERR_R_X509_LIB); goto err; } + if ((flags & PKCS7_NOCHAIN) == 0 + && !X509_STORE_CTX_set_default(cert_ctx, "smime_sign")) + goto err; if (!(flags & PKCS7_NOCRL)) X509_STORE_CTX_set0_crls(cert_ctx, p7->d.sign->crl); i = X509_verify_cert(cert_ctx); @@ -299,6 +305,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, } /* Check for revocation status here */ } + } if ((p7bio = PKCS7_dataInit(p7, indata)) == NULL) goto err; @@ -353,13 +360,14 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, BIO_pop(p7bio); BIO_free_all(p7bio); sk_X509_free(signers); + sk_X509_free(untrusted); return ret; } STACK_OF(X509) *PKCS7_get0_signers(PKCS7 *p7, STACK_OF(X509) *certs, int flags) { - STACK_OF(X509) *signers; + STACK_OF(X509) *signers, *included_certs; STACK_OF(PKCS7_SIGNER_INFO) *sinfos; PKCS7_SIGNER_INFO *si; PKCS7_ISSUER_AND_SERIAL *ias; @@ -375,6 +383,7 @@ STACK_OF(X509) *PKCS7_get0_signers(PKCS7 *p7, STACK_OF(X509) *certs, ERR_raise(ERR_LIB_PKCS7, PKCS7_R_WRONG_CONTENT_TYPE); return NULL; } + included_certs = pkcs7_get0_certificates(p7); /* Collect all the signers together */ @@ -395,14 +404,11 @@ STACK_OF(X509) *PKCS7_get0_signers(PKCS7 *p7, STACK_OF(X509) *certs, ias = si->issuer_and_serial; signer = NULL; /* If any certificates passed they take priority */ - if (certs != NULL) - signer = X509_find_by_issuer_and_serial(certs, + signer = X509_find_by_issuer_and_serial(certs, + ias->issuer, ias->serial); + if (signer == NULL && (flags & PKCS7_NOINTERN) == 0) + signer = X509_find_by_issuer_and_serial(included_certs, ias->issuer, ias->serial); - if (signer == NULL && !(flags & PKCS7_NOINTERN) - && p7->d.sign->cert) - signer = - X509_find_by_issuer_and_serial(p7->d.sign->cert, - ias->issuer, ias->serial); if (signer == NULL) { ERR_raise(ERR_LIB_PKCS7, PKCS7_R_SIGNER_CERTIFICATE_NOT_FOUND); sk_X509_free(signers); diff --git a/doc/man1/openssl-cms.pod.in b/doc/man1/openssl-cms.pod.in index 78be2e6c090..43a9a149794 100644 --- a/doc/man1/openssl-cms.pod.in +++ b/doc/man1/openssl-cms.pod.in @@ -453,8 +453,9 @@ used multiple times if more than one signer is required. =item B<-certfile> I Allows additional certificates to be specified. When signing these will -be included with the message. When verifying these will be searched for -the signers certificates. +be included with the message. When verifying, these will be searched for +signer certificates and will be used for chain building. + The input can be in PEM, DER, or PKCS#12 format. =item B<-cades> diff --git a/doc/man1/openssl-smime.pod.in b/doc/man1/openssl-smime.pod.in index 4d8d6f52cb4..36527641530 100644 --- a/doc/man1/openssl-smime.pod.in +++ b/doc/man1/openssl-smime.pod.in @@ -234,8 +234,9 @@ option is present B is used instead. =item B<-certfile> I Allows additional certificates to be specified. When signing these will -be included with the message. When verifying these will be searched for -the signers certificates. +be included with the message. When verifying, these will be searched for +signer certificates and will be used for chain building. + The input can be in PEM, DER, or PKCS#12 format. =item B<-signer> I diff --git a/doc/man3/CMS_verify.pod b/doc/man3/CMS_verify.pod index bd46a1262cf..3f3488b2f68 100644 --- a/doc/man3/CMS_verify.pod +++ b/doc/man3/CMS_verify.pod @@ -26,6 +26,8 @@ B structure contained in a structure of type B. I points to the B structure to verify. The optional I parameter refers to a set of certificates in which to search for signing certificates. +It is also used +as a source of untrusted intermediate CA certificates for chain building. I may contain extra untrusted CA certificates that may be used for chain building as well as CRLs that may be used for certificate validation. I may be NULL or point to diff --git a/doc/man3/PKCS7_verify.pod b/doc/man3/PKCS7_verify.pod index 5d4f6ad9e1d..99afa0ea158 100644 --- a/doc/man3/PKCS7_verify.pod +++ b/doc/man3/PKCS7_verify.pod @@ -19,6 +19,8 @@ PKCS7_verify() is very similar to L. It verifies a PKCS#7 signedData structure given in I. The optional I parameter refers to a set of certificates in which to search for signer's certificates. +It is also used +as a source of untrusted intermediate CA certificates for chain building. I may contain extra untrusted CA certificates that may be used for chain building as well as CRLs that may be used for certificate validation. I may be NULL or point to diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t index 07014945a7d..59756cbd861 100644 --- a/test/recipes/80-test_cms.t +++ b/test/recipes/80-test_cms.t @@ -1044,6 +1044,53 @@ subtest "CMS signed digest, S/MIME format" => sub { "Verify CMS signed digest, S/MIME format"); }; +sub path_tests { + our $app = shift; + our @path = qw(test certs); + our $key = srctop_file(@path, "ee-key.pem"); + our $ee = srctop_file(@path, "ee-cert.pem"); + our $ca = srctop_file(@path, "ca-cert.pem"); + our $root = srctop_file(@path, "root-cert.pem"); + our $sig_file = "signature.p7s"; + + sub sign { + my $inter = shift; + my @inter = $inter ? ("-certfile", $inter) : (); + my $msg = shift; + ok(run(app(["openssl", $app, @prov, "-sign", "-in", $smcont, + "-inkey", $key, "-signer", $ee, @inter, + "-out", $sig_file], + "accept $app sign with EE $msg". + " intermediate CA certificates"))); + } + sub verify { + my $inter = shift; + my @inter = $inter ? ("-certfile", $inter) : (); + my $msg = shift; + my $res = shift; + ok($res == run(app(["openssl", $app, @prov, "-verify", "-in", $sig_file, + "-purpose", "sslserver", "-CAfile", $root, @inter, + "-content", $smcont], + "accept $app verify with EE ". + "$msg intermediate CA certificates"))); + } + sign($ca, "and"); + verify(0, "with included", 1); + sign(0, "without"); + verify(0, "without", 0); + verify($ca, "with added", 1); +}; +subtest "CMS sign+verify cert path tests" => sub { + plan tests => 5; + + path_tests("cms"); +}; +subtest "PKCS7 sign+verify cert path tests" => sub { + plan tests => 5; + + path_tests("smime"); +}; + subtest "CMS code signing test" => sub { plan tests => 7; my $sig_file = "signature.p7s";