{CMS,PKCS7}_verify(): use 'certs' parameter ('-certfile' option) also for chain building

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

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"},

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"),

crypto/cms/cms_smime.c

@@ -15,6 +15,7 @@
 #include <openssl/cms.h>
 #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 ((untrusted = CMS_get1_certs(cms)) == NULL)
-        if (!(flags & CMS_NOCRL))
+            goto err;
-            crls = CMS_get1_crls(cms);
+        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;

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);

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);

crypto/pkcs7/pk7_smime.c

@@ -11,6 +11,7 @@
 
 #include <stdio.h>
 #include "internal/cryptlib.h"
+#include "crypto/x509.h"
 #include <openssl/x509.h>
 #include <openssl/x509v3.h>
 #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, untrusted)) {
-                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)) {
                 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);

doc/man1/openssl-cms.pod.in

@@ -453,8 +453,9 @@ used multiple times if more than one signer is required.
 =item B<-certfile> I<file>
 
 Allows additional certificates to be specified. When signing these will
-be included with the message. When verifying these will be searched for
+be included with the message. When verifying, these will be searched for
-the signers certificates.
+signer certificates and will be used for chain building.
+
 The input can be in PEM, DER, or PKCS#12 format.
 
 =item B<-cades>

doc/man1/openssl-smime.pod.in

@@ -234,8 +234,9 @@ option is present B<CRLF> is used instead.
 =item B<-certfile> I<file>
 
 Allows additional certificates to be specified. When signing these will
-be included with the message. When verifying these will be searched for
+be included with the message. When verifying, these will be searched for
-the signers certificates.
+signer certificates and will be used for chain building.
+
 The input can be in PEM, DER, or PKCS#12 format.
 
 =item B<-signer> I<file>

doc/man3/CMS_verify.pod

@@ -26,6 +26,8 @@ B<CMS SignedData> structure contained in a structure of type B<CMS_ContentInfo>.
 I<cms> points to the B<CMS_ContentInfo> structure to verify.
 The optional I<certs> 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<cms> 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<store> may be NULL or point to

doc/man3/PKCS7_verify.pod

@@ -19,6 +19,8 @@ PKCS7_verify() is very similar to L<CMS_verify(3)>.
 It verifies a PKCS#7 signedData structure given in I<p7>.
 The optional I<certs> 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<p7> 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<store> may be NULL or point to

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";