Fix PKCS#7 verification of signature created with OpenSSL

.github/workflows/pkcs7-interop.yml

@@ -0,0 +1,96 @@
+name: PKCS7 Interoperability
+
+# START OF COMMON SECTION
+on:
+  push:
+    branches: [ 'master', 'main', 'release/**' ]
+  pull_request:
+    branches: [ '*' ]
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+# END OF COMMON SECTION
+
+jobs:
+  pkcs7_interop:
+    name: PKCS7 interop (OpenSSL, GnuTLS)
+    if: github.repository_owner == 'wolfssl'
+    runs-on: ubuntu-24.04
+    timeout-minutes: 14
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install 3rd party PKCS#7 tools
+        run: sudo apt-get install -y openssl gnutls-bin
+
+      - name: Generate keys and certificates
+        run: |
+          # RSA-2048
+          openssl req -x509 -newkey rsa:2048 -keyout $RUNNER_TEMP/rsa_key.pem \
+            -out $RUNNER_TEMP/rsa_cert.pem -days 1 -nodes \
+            -subj "/CN=wolfssl-pkcs7-interop-test"
+
+          # ECDSA P-256
+          openssl req -x509 -newkey ec -pkeyopt ec_paramgen_curve:P-256 \
+            -keyout $RUNNER_TEMP/ec_key.pem -out $RUNNER_TEMP/ec_cert.pem \
+            -days 1 -nodes -subj "/CN=wolfssl-pkcs7-interop-test-ec"
+
+          echo -n "pkcs7 interop test" > $RUNNER_TEMP/content.bin
+
+      - name: Sign with OpenSSL cms
+        run: |
+          for hash in sha1 sha256 sha384 sha512; do
+            openssl cms -sign -binary -nodetach \
+              -in $RUNNER_TEMP/content.bin \
+              -signer $RUNNER_TEMP/rsa_cert.pem -inkey $RUNNER_TEMP/rsa_key.pem \
+              -md $hash -outform DER \
+              -out $RUNNER_TEMP/openssl_cms_rsa_${hash}.der
+            openssl cms -sign -binary -nodetach \
+              -in $RUNNER_TEMP/content.bin \
+              -signer $RUNNER_TEMP/ec_cert.pem -inkey $RUNNER_TEMP/ec_key.pem \
+              -md $hash -outform DER \
+              -out $RUNNER_TEMP/openssl_cms_ecdsa_${hash}.der
+          done
+
+      - name: Sign with OpenSSL smime
+        run: |
+          for hash in sha1 sha256 sha384 sha512; do
+            openssl smime -sign -binary -nodetach \
+              -in $RUNNER_TEMP/content.bin \
+              -signer $RUNNER_TEMP/rsa_cert.pem -inkey $RUNNER_TEMP/rsa_key.pem \
+              -md $hash -outform DER \
+              -out $RUNNER_TEMP/openssl_smime_rsa_${hash}.der
+            openssl smime -sign -binary -nodetach \
+              -in $RUNNER_TEMP/content.bin \
+              -signer $RUNNER_TEMP/ec_cert.pem -inkey $RUNNER_TEMP/ec_key.pem \
+              -md $hash -outform DER \
+              -out $RUNNER_TEMP/openssl_smime_ecdsa_${hash}.der
+          done
+
+      - name: Sign with GnuTLS p7
+        run: |
+          for hash in SHA1 SHA256 SHA384 SHA512; do
+            certtool --p7-sign --hash $hash \
+              --infile $RUNNER_TEMP/content.bin \
+              --load-certificate $RUNNER_TEMP/rsa_cert.pem \
+              --load-privkey $RUNNER_TEMP/rsa_key.pem \
+              --outder --outfile $RUNNER_TEMP/gnutls_p7_rsa_${hash,,}.der
+            certtool --p7-sign --hash $hash \
+              --infile $RUNNER_TEMP/content.bin \
+              --load-certificate $RUNNER_TEMP/ec_cert.pem \
+              --load-privkey $RUNNER_TEMP/ec_key.pem \
+              --outder --outfile $RUNNER_TEMP/gnutls_p7_ecdsa_${hash,,}.der
+          done
+
+      - name: Build wolfSSL
+        run: |
+          ./autogen.sh
+          ders=($RUNNER_TEMP/{openssl_cms,openssl_smime,gnutls_p7}_{rsa,ecdsa}_{sha1,sha256,sha384,sha512}.der)
+          DER_FILES=$(IFS=,; echo "${ders[*]}")
+          ./configure --enable-pkcs7 --enable-certext --enable-examples \
+            --with-pkcs7-test-signed-data=$DER_FILES
+          make -j$(nproc)
+
+      - name: Run PKCS7 interop test
+        run: tests/unit.test -test_wc_PKCS7_VerifySignedData_interop

configure.ac

@@ -10490,6 +10490,17 @@ if test "x$ENABLED_OLD_EXTDATA_FMT" = "xyes"; then
   AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_OLD_EXTDATA_FMT"
 fi
 
+# Optional PKCS#7 SignedData interoperability test
+AC_ARG_WITH([pkcs7-test-signed-data],
+    [AS_HELP_STRING([--with-pkcs7-test-signed-data=FILE[,FILE,...]],
+        [External test data for optional PKCS#7 interoperability test])],
+    [
+        AC_DEFINE([HAVE_PKCS7_TEST_SIGNED_DATA_FILES], [1],
+            [Define to enable PKCS#7 SignedData interoperability test])
+        AC_DEFINE_UNQUOTED([PKCS7_TEST_SIGNED_DATA_FILES], ["$withval"],
+            [Comma-separated list of DER-encoded CMS SignedData files for interop test])
+    ])
+
 # determine if we have key validation mechanism
 if test "x$ENABLED_ECC" != "xno" || test "x$ENABLED_RSA" = "xyes"
 then

tests/api/test_pkcs7.c

@@ -5000,3 +5000,72 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void)
 #endif /* HAVE_PKCS7 && !NO_PKCS7_STREAM */
     return EXPECT_RESULT();
 }
+
+#if defined(HAVE_PKCS7) && defined(HAVE_PKCS7_TEST_SIGNED_DATA_FILES) && \
+    !defined(NO_FILESYSTEM)
+static int pkcs7_verify_one_signed_data_der_file(const char* path)
+{
+    EXPECT_DECLS;
+    XFILE f = XBADFILE;
+    long signedDataSz;
+    byte* signedData = NULL;
+    PKCS7* pkcs7 = NULL;
+
+    ExpectTrue((f = XFOPEN(path, "rb")) != XBADFILE);
+    ExpectIntEQ(XFSEEK(f, 0, XSEEK_END), 0);
+    ExpectIntGT(signedDataSz = XFTELL(f), 0);
+    ExpectIntEQ(XFSEEK(f, 0, XSEEK_SET), 0);
+
+    ExpectNotNull(signedData = (byte*)XMALLOC((word32)signedDataSz, NULL,
+                                              DYNAMIC_TYPE_TMP_BUFFER));
+    ExpectIntEQ((long)XFREAD(signedData, 1, (word32)signedDataSz, f),
+                signedDataSz);
+    if (f != XBADFILE)
+        XFCLOSE(f);
+
+    ExpectNotNull(pkcs7 = wc_PKCS7_New(NULL, INVALID_DEVID));
+    ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
+
+    /* test signature verification and message content */
+    ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7,
+                                          signedData,
+                                          (word32)signedDataSz), 0);
+    ExpectNotNull(pkcs7->contentDynamic);
+
+    XFREE(signedData, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    wc_PKCS7_Free(pkcs7);
+    return EXPECT_RESULT();
+}
+
+/* Verify CMS SignedData DER files created with third party tools.
+ *
+ * This test verifies signatures created with external tools can be verified by
+ * wolfSSL. Standards allow some variance when creating CMS signatures. The
+ * test is intended to catch incompatibility from subtle differences in
+ * implementation.
+ *
+ * DER files are supplied at configure time:
+ *   configure --with-pkcs7-test-signed-data=openssl_cms.der,openssl_smime.der
+ */
+int test_wc_PKCS7_VerifySignedData_interop(void)
+{
+    EXPECT_DECLS;
+    char pathsTokenBuf[FOURK_BUF];
+    char* nextPath;
+    char* tokState = NULL;
+
+    wc_static_assert2(sizeof(PKCS7_TEST_SIGNED_DATA_FILES) <= sizeof(pathsTokenBuf),
+                      "pkcs7-test-signed-data file list too long for FOURK_BUF");
+    XSTRNCPY(pathsTokenBuf, PKCS7_TEST_SIGNED_DATA_FILES, sizeof(pathsTokenBuf));
+
+    nextPath = XSTRTOK(pathsTokenBuf, ",", &tokState);
+    while (nextPath != NULL) {
+        Expect(pkcs7_verify_one_signed_data_der_file(nextPath) == TEST_SUCCESS,
+               ("%s signature verification succeeds", nextPath),
+               ("%s signature verification failed", nextPath));
+        nextPath = XSTRTOK(NULL, ",", &tokState);
+    }
+    return EXPECT_RESULT();
+}
+#endif /* HAVE_PKCS7 && HAVE_PKCS7_TEST_SIGNED_DATA_FILES && !NO_FILESYSTEM */
+

tests/api/test_pkcs7.h

@@ -59,6 +59,14 @@ int test_wc_PKCS7_DecodeCompressedData(void);
 int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void);
 int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void);
 int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
+#if defined(HAVE_PKCS7) && defined(HAVE_PKCS7_TEST_SIGNED_DATA_FILES) && \
+    !defined(NO_FILESYSTEM)
+int test_wc_PKCS7_VerifySignedData_interop(void);
+#define TEST_PKCS7_INTEROP_VERIFY_SD_DECL \
+    TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_interop),
+#else
+#define TEST_PKCS7_INTEROP_VERIFY_SD_DECL
+#endif
 
 
 #define TEST_PKCS7_DECLS                                        \
@@ -94,6 +102,7 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
     TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_BER),                     \
     TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_NoDefaultSignedAttribs),  \
     TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq), \
+    TEST_PKCS7_INTEROP_VERIFY_SD_DECL                                            \
     TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_IndefLenOOB)
 
 #define TEST_PKCS7_ENCRYPTED_DATA_DECLS                                     \

wolfcrypt/src/pkcs7.c

@@ -4682,8 +4682,10 @@ static int wc_PKCS7_EcdsaVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz,
 #endif /* HAVE_ECC */
 
 
-/* build SignedData digest, both in PKCS#7 DigestInfo format and
- * as plain digest for CMS.
+/* Build CMS SignedData digest (RFC 5652 sec 5.4) wrapped in a DigestInfo
+ * structure (RFC 8017 sec 9.2) as for example used by RSA PKCS#1 v1.5
+ * verification. Also returns a pointer to the inner plain digest for
+ * algorithms that do not wrap digist in DigestInfo, e.g. ECDSA and RSA-PSS.
  *
  * pkcs7          - pointer to initialized PKCS7 struct
  * signedAttrib   - signed attributes
@@ -4779,9 +4781,8 @@ static int wc_PKCS7_BuildSignedDataDigest(wc_PKCS7* pkcs7, byte* signedAttrib,
         }
     }
 
-    /* Set algoID, match whatever was input to match either NULL or absent */
-    algoIdSz = SetAlgoIDEx(pkcs7->hashOID, algoId, oidHashType,
-                            0, pkcs7->hashParamsAbsent);
+    /* parameters SHALL be NULL per RFC 8017 section 9.2, not absent */
+    algoIdSz = SetAlgoID(pkcs7->hashOID, algoId, oidHashType, 0);
 
     digestStrSz = SetOctetString(hashSz, digestStr);
     digestInfoSeqSz = SetSequence(algoIdSz + digestStrSz + hashSz,