PKCS7: add notes to pkcs7.h.in and minor code cleanup in crypto/{pkcs7,cms}/

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/18915)

crypto/cms/cms_sd.c

@@ -660,7 +660,7 @@ int CMS_set1_signers_certs(CMS_ContentInfo *cms, STACK_OF(X509) *scerts,
 
         for (j = 0; j < sk_CMS_CertificateChoices_num(certs); j++) {
             cch = sk_CMS_CertificateChoices_value(certs, j);
-            if (cch->type != 0)
+            if (cch->type != CMS_CERTCHOICE_CERT)
                 continue;
             x = cch->d.certificate;
             if (CMS_SignerInfo_cert_cmp(si, x) == 0) {

crypto/cms/cms_smime.c

@@ -259,7 +259,7 @@ CMS_ContentInfo *CMS_EncryptedData_encrypt(BIO *in, const EVP_CIPHER *cipher,
 
 static int cms_signerinfo_verify_cert(CMS_SignerInfo *si,
                                       X509_STORE *store,
-                                      STACK_OF(X509) *certs,
+                                      STACK_OF(X509) *untrusted,
                                       STACK_OF(X509_CRL) *crls,
                                       STACK_OF(X509) **chain,
                                       const CMS_CTX *cms_ctx)
@@ -275,7 +275,7 @@ static int cms_signerinfo_verify_cert(CMS_SignerInfo *si,
         goto err;
     }
     CMS_SignerInfo_get0_algs(si, NULL, &signer, NULL, NULL);
-    if (!X509_STORE_CTX_init(ctx, store, signer, certs)) {
+    if (!X509_STORE_CTX_init(ctx, store, signer, untrusted)) {
         ERR_raise(ERR_LIB_CMS, CMS_R_STORE_INIT_ERROR);
         goto err;
     }
@@ -301,6 +301,7 @@ static int cms_signerinfo_verify_cert(CMS_SignerInfo *si,
 
 }
 
+/* This strongly overlaps with PKCS7_verify() */
 int CMS_verify(CMS_ContentInfo *cms, STACK_OF(X509) *certs,
                X509_STORE *store, BIO *dcont, BIO *out, unsigned int flags)
 {
@@ -336,7 +337,7 @@ int CMS_verify(CMS_ContentInfo *cms, STACK_OF(X509) *certs,
     for (i = 0; i < sk_CMS_SignerInfo_num(sinfos); i++) {
         si = sk_CMS_SignerInfo_value(sinfos, i);
         CMS_SignerInfo_get0_algs(si, NULL, &signer, NULL, NULL);
-        if (signer)
+        if (signer != NULL)
             scount++;
     }
 

crypto/pkcs7/pk7_smime.c

@@ -210,6 +210,7 @@ static int pkcs7_copy_existing_digest(PKCS7 *p7, PKCS7_SIGNER_INFO *si)
     return 0;
 }
 
+/* This strongly overlaps with CMS_verify(), partly with PKCS7_dataVerify() */
 int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
                  BIO *indata, BIO *out, int flags)
 {
@@ -235,7 +236,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
     }
 
     /* Check for no data and no content: no data to verify signature */
-    if (PKCS7_get_detached(p7) && !indata) {
+    if (PKCS7_get_detached(p7) && indata == NULL) {
         ERR_raise(ERR_LIB_PKCS7, PKCS7_R_NO_CONTENT);
         return 0;
     }
@@ -248,7 +249,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
          * tools like osslsigncode need it.  In Authenticode the verification
          * process is different, but the existing PKCs7 verification works.
          */
-        if (!PKCS7_get_detached(p7) && indata) {
+        if (!PKCS7_get_detached(p7) && indata != NULL) {
             ERR_raise(ERR_LIB_PKCS7, PKCS7_R_CONTENT_AND_DATA_PRESENT);
             return 0;
         }
@@ -350,7 +351,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
  err:
     X509_STORE_CTX_free(cert_ctx);
     OPENSSL_free(buf);
-    if (indata)
+    if (indata != NULL)
         BIO_pop(p7bio);
     BIO_free_all(p7bio);
     sk_X509_free(signers);
@@ -396,15 +397,15 @@ 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)
+        if (certs != NULL)
             signer = X509_find_by_issuer_and_serial(certs,
                                                     ias->issuer, ias->serial);
-        if (!signer && !(flags & PKCS7_NOINTERN)
+        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) {
+        if (signer == NULL) {
             ERR_raise(ERR_LIB_PKCS7, PKCS7_R_SIGNER_CERTIFICATE_NOT_FOUND);
             sk_X509_free(signers);
             return 0;

include/openssl/pkcs7.h.in

@@ -87,8 +87,8 @@ typedef struct pkcs7_recip_info_st {
 typedef struct pkcs7_signed_st {
     ASN1_INTEGER *version;      /* version 1 */
     STACK_OF(X509_ALGOR) *md_algs; /* md used */
-    STACK_OF(X509) *cert;       /* [ 0 ] */
-    STACK_OF(X509_CRL) *crl;    /* [ 1 ] */
+    STACK_OF(X509) *cert;       /* [ 0 ] */ /* name should be 'certificates' */
+    STACK_OF(X509_CRL) *crl;    /* [ 1 ] */ /* name should be 'crls' */
     STACK_OF(PKCS7_SIGNER_INFO) *signer_info;
     struct pkcs7_st *contents;
 } PKCS7_SIGNED;
@@ -114,8 +114,8 @@ typedef struct pkcs7_enveloped_st {
 typedef struct pkcs7_signedandenveloped_st {
     ASN1_INTEGER *version;      /* version 1 */
     STACK_OF(X509_ALGOR) *md_algs; /* md used */
-    STACK_OF(X509) *cert;       /* [ 0 ] */
-    STACK_OF(X509_CRL) *crl;    /* [ 1 ] */
+    STACK_OF(X509) *cert;       /* [ 0 ] */ /* name should be 'certificates' */
+    STACK_OF(X509_CRL) *crl;    /* [ 1 ] */ /* name should be 'crls' */
     STACK_OF(PKCS7_SIGNER_INFO) *signer_info;
     PKCS7_ENC_CONTENT *enc_data;
     STACK_OF(PKCS7_RECIP_INFO) *recipientinfo;
@@ -156,7 +156,7 @@ typedef struct pkcs7_st {
         /* NID_pkcs7_data */
         ASN1_OCTET_STRING *data;
         /* NID_pkcs7_signed */
-        PKCS7_SIGNED *sign;
+        PKCS7_SIGNED *sign; /* field name 'signed' would clash with C keyword */
         /* NID_pkcs7_enveloped */
         PKCS7_ENVELOPE *enveloped;
         /* NID_pkcs7_signedAndEnveloped */
@@ -273,8 +273,8 @@ int PKCS7_SIGNER_INFO_set(PKCS7_SIGNER_INFO *p7i, X509 *x509, EVP_PKEY *pkey,
                           const EVP_MD *dgst);
 int PKCS7_SIGNER_INFO_sign(PKCS7_SIGNER_INFO *si);
 int PKCS7_add_signer(PKCS7 *p7, PKCS7_SIGNER_INFO *p7i);
-int PKCS7_add_certificate(PKCS7 *p7, X509 *x509);
-int PKCS7_add_crl(PKCS7 *p7, X509_CRL *x509);
+int PKCS7_add_certificate(PKCS7 *p7, X509 *cert);
+int PKCS7_add_crl(PKCS7 *p7, X509_CRL *crl);
 int PKCS7_content_new(PKCS7 *p7, int nid);
 int PKCS7_dataVerify(X509_STORE *cert_store, X509_STORE_CTX *ctx,
                      BIO *bio, PKCS7 *p7, PKCS7_SIGNER_INFO *si);