Skip to content

Commit a09c0db

Browse files
committed
Fix PKCS#7/CMS RSA verify of SignedData with mismatched DigestInfo params
wc_PKCS7_VerifySignedData() returned SIG_VERIFY_E for a cryptographically valid RSA SignedData whose SignerInfo digestAlgorithm parameter encoding (NULL present vs absent, a CMS field per RFC 5652/5754) differs from the parameter encoding of the AlgorithmIdentifier inside the PKCS#1 v1.5 DigestInfo that the signature actually covers (RFC 8017). The two encodings are independent, but the RSA verify rebuilt its comparison DigestInfo mirroring only the SignerInfo digestAlgorithm, so it never byte-matched the recovered DigestInfo. Go's crypto/rsa (micromdm/scep and other Go CMS/SCEP stacks) omits the SignerInfo NULL while signing a NULL-present DigestInfo, which triggers this. wc_PKCS7_BuildSignedDataDigest() now takes an explicit hashParamsAbsent argument, and the RSA verify path tries both DigestInfo encodings before failing: as-parsed, plain digest, then the flipped parameter encoding. This keeps wolfSSL's encode-and-compare approach (no parsing of attacker-controlled recovered bytes) and handles both mismatch directions. Add test_wc_PKCS7_VerifySignedData_NoDigestParams, which builds the mismatch entirely from wolfSSL's own signer (sign the same content twice, NULL-absent and NULL-present, then splice one signature onto the other message) and verifies all four direction/attribute combinations plus a tampered-signature negative control.
1 parent 9d86960 commit a09c0db

3 files changed

Lines changed: 263 additions & 13 deletions

File tree

tests/api/test_pkcs7.c

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5884,3 +5884,218 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void)
58845884
return EXPECT_RESULT();
58855885
}
58865886

5887+
#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \
5888+
defined(USE_CERT_BUFFERS_2048)
5889+
/*
5890+
* Encode a minimal RSA SignedData (SHA-256) with the requested DigestInfo
5891+
* AlgorithmIdentifier parameter encoding. When signedAttribs is zero the
5892+
* signature covers the content DigestInfo directly; when non-zero it covers
5893+
* the signed-attributes DigestInfo with a deterministic attribute set
5894+
* (contentType + messageDigest, no signingTime). Either way the signed data is
5895+
* deterministic, so two encodes that differ only in hashParamsAbsent sign the
5896+
* same hash - only the DigestInfo NULL parameters differ. Returns the encoded
5897+
* size (> 0) on success, negative on failure.
5898+
*/
5899+
static int pkcs7_sign_digest_params(byte* cert, word32 certSz,
5900+
byte* key, word32 keySz,
5901+
byte hashParamsAbsent, byte signedAttribs,
5902+
byte* out, word32 outSz)
5903+
{
5904+
PKCS7* pkcs7 = NULL;
5905+
WC_RNG rng;
5906+
byte data[] = "wolfSSL PKCS#7 DigestInfo params regression content";
5907+
int ret;
5908+
5909+
XMEMSET(&rng, 0, sizeof(rng));
5910+
ret = wc_InitRng(&rng);
5911+
if (ret != 0)
5912+
return ret;
5913+
5914+
pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId);
5915+
if (pkcs7 == NULL) {
5916+
wc_FreeRng(&rng);
5917+
return MEMORY_E;
5918+
}
5919+
5920+
ret = wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID);
5921+
if (ret == 0)
5922+
ret = wc_PKCS7_InitWithCert(pkcs7, cert, certSz);
5923+
if (ret == 0) {
5924+
if (signedAttribs) {
5925+
/* Deterministic attributes only (no signingTime) so two encodes
5926+
* stay byte-identical and the cross-signature splice stays valid. */
5927+
pkcs7->defaultSignedAttribs = WOLFSSL_CONTENT_TYPE_ATTRIBUTE |
5928+
WOLFSSL_MESSAGE_DIGEST_ATTRIBUTE;
5929+
}
5930+
else {
5931+
ret = wc_PKCS7_NoDefaultSignedAttribs(pkcs7);
5932+
}
5933+
}
5934+
if (ret == 0) {
5935+
pkcs7->content = data;
5936+
pkcs7->contentSz = (word32)sizeof(data) - 1;
5937+
pkcs7->privateKey = key;
5938+
pkcs7->privateKeySz = keySz;
5939+
pkcs7->encryptOID = RSAk;
5940+
pkcs7->hashOID = SHA256h;
5941+
pkcs7->rng = &rng;
5942+
pkcs7->hashParamsAbsent = (hashParamsAbsent != 0) ? 1 : 0;
5943+
5944+
ret = wc_PKCS7_EncodeSignedData(pkcs7, out, outSz);
5945+
}
5946+
5947+
wc_PKCS7_Free(pkcs7);
5948+
wc_FreeRng(&rng);
5949+
return ret;
5950+
}
5951+
5952+
/*
5953+
* Build a SignedData whose SignerInfo digestAlgorithm parameter encoding does
5954+
* NOT match the encoding of the DigestInfo covered by the RSA signature. The
5955+
* same content is signed twice - once NULL-absent, once NULL-present - and the
5956+
* signature from one encode is spliced over the other message. RSA signatures
5957+
* are fixed length, so this is a same-length byte substitution needing no
5958+
* re-encoding. signerInfoAbsent selects the produced message's SignerInfo
5959+
* digestAlgorithm encoding; the spliced signature then carries the opposite
5960+
* encoding. Returns the message size (> 0) on success, negative on failure.
5961+
*/
5962+
static int pkcs7_build_digestparam_mismatch(byte* cert, word32 certSz,
5963+
byte* key, word32 keySz,
5964+
byte signedAttribs,
5965+
byte signerInfoAbsent,
5966+
byte* out, word32 outSz)
5967+
{
5968+
byte other[FOURK_BUF];
5969+
int keepSz, otherSz;
5970+
/* RSA-2048 signature is 256 bytes, wrapped as OCTET STRING 04 82 01 00. */
5971+
const int rsaSigSz = 256;
5972+
5973+
/* message that keeps the requested SignerInfo digestAlgorithm encoding */
5974+
keepSz = pkcs7_sign_digest_params(cert, certSz, key, keySz, signerInfoAbsent,
5975+
signedAttribs, out, outSz);
5976+
if (keepSz <= 0)
5977+
return keepSz;
5978+
5979+
/* message whose signature covers the opposite DigestInfo encoding */
5980+
XMEMSET(other, 0, sizeof(other));
5981+
otherSz = pkcs7_sign_digest_params(cert, certSz, key, keySz,
5982+
(byte)!signerInfoAbsent, signedAttribs,
5983+
other, (word32)sizeof(other));
5984+
if (otherSz <= 0)
5985+
return otherSz;
5986+
5987+
/* both messages must end with the 256-byte signature OCTET STRING */
5988+
if (keepSz <= rsaSigSz + 4 || otherSz <= rsaSigSz + 4)
5989+
return -1;
5990+
if (out[keepSz - rsaSigSz - 4] != 0x04 ||
5991+
other[otherSz - rsaSigSz - 4] != 0x04) {
5992+
return -1;
5993+
}
5994+
5995+
/* splice the opposite-encoding signature over the kept message */
5996+
XMEMCPY(out + keepSz - rsaSigSz, other + otherSz - rsaSigSz,
5997+
(size_t)rsaSigSz);
5998+
return keepSz;
5999+
}
6000+
#endif
6001+
6002+
/*
6003+
* Regression test for the DigestInfo AlgorithmIdentifier parameter mismatch.
6004+
*
6005+
* The parameter encoding (NULL present vs absent) of the SignerInfo
6006+
* digestAlgorithm - a CMS field, RFC 5652/5754 - is independent of the
6007+
* parameter encoding of the AlgorithmIdentifier inside the PKCS#1 v1.5
6008+
* DigestInfo that the RSA signature actually covers (RFC 8017). wolfSSL must
6009+
* not couple them. Go's crypto/rsa (micromdm/scep and other Go CMS/SCEP
6010+
* stacks) omits the NULL in the SignerInfo digestAlgorithm while signing a
6011+
* NULL-present DigestInfo; before the fix wc_PKCS7_VerifySignedData() returned
6012+
* SIG_VERIFY_E on such a (cryptographically valid) message.
6013+
*
6014+
* The mismatch is produced here entirely from wolfSSL's own signer, so no
6015+
* externally captured message is embedded (see pkcs7_build_digestparam_
6016+
* mismatch). The fix is symmetric, so both directions are exercised, over both
6017+
* the attribute-free and signed-attribute signing paths.
6018+
*/
6019+
int test_wc_PKCS7_VerifySignedData_NoDigestParams(void)
6020+
{
6021+
EXPECT_DECLS;
6022+
#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \
6023+
defined(USE_CERT_BUFFERS_2048)
6024+
PKCS7* pkcs7 = NULL;
6025+
byte cert[sizeof(client_cert_der_2048)];
6026+
byte key[sizeof(client_key_der_2048)];
6027+
word32 certSz = (word32)sizeof(cert);
6028+
word32 keySz = (word32)sizeof(key);
6029+
byte msg[FOURK_BUF];
6030+
int msgSz = 0;
6031+
6032+
XMEMCPY(cert, client_cert_der_2048, certSz);
6033+
XMEMCPY(key, client_key_der_2048, keySz);
6034+
6035+
/* Direction A (Go/micromdm), no signed attributes: SignerInfo
6036+
* digestAlgorithm NULL-absent, signature over a NULL-present DigestInfo. */
6037+
XMEMSET(msg, 0, sizeof(msg));
6038+
ExpectIntGT(msgSz = pkcs7_build_digestparam_mismatch(cert, certSz, key,
6039+
keySz, 0, 1, msg, (word32)sizeof(msg)), 0);
6040+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
6041+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
6042+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
6043+
ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, msg, (word32)msgSz), 0);
6044+
wc_PKCS7_Free(pkcs7);
6045+
pkcs7 = NULL;
6046+
6047+
/* Direction B (reverse), no signed attributes: SignerInfo digestAlgorithm
6048+
* NULL-present, signature over a NULL-absent DigestInfo. Exercises the
6049+
* other branch of the symmetric (!hashParamsAbsent) retry. */
6050+
XMEMSET(msg, 0, sizeof(msg));
6051+
ExpectIntGT(msgSz = pkcs7_build_digestparam_mismatch(cert, certSz, key,
6052+
keySz, 0, 0, msg, (word32)sizeof(msg)), 0);
6053+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
6054+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
6055+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
6056+
ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, msg, (word32)msgSz), 0);
6057+
wc_PKCS7_Free(pkcs7);
6058+
pkcs7 = NULL;
6059+
6060+
/* Direction A with signed attributes: same mismatch over the signed-
6061+
* attributes path, exercising the flipped rebuild's attribute-hashing
6062+
* branch. */
6063+
XMEMSET(msg, 0, sizeof(msg));
6064+
ExpectIntGT(msgSz = pkcs7_build_digestparam_mismatch(cert, certSz, key,
6065+
keySz, 1, 1, msg, (word32)sizeof(msg)), 0);
6066+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
6067+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
6068+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
6069+
ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, msg, (word32)msgSz), 0);
6070+
wc_PKCS7_Free(pkcs7);
6071+
pkcs7 = NULL;
6072+
6073+
/* Direction B with signed attributes: completes the 2x2 matrix (both flip
6074+
* directions over both the attribute-free and signed-attribute paths). */
6075+
XMEMSET(msg, 0, sizeof(msg));
6076+
ExpectIntGT(msgSz = pkcs7_build_digestparam_mismatch(cert, certSz, key,
6077+
keySz, 1, 0, msg, (word32)sizeof(msg)), 0);
6078+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
6079+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
6080+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
6081+
ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, msg, (word32)msgSz), 0);
6082+
wc_PKCS7_Free(pkcs7);
6083+
pkcs7 = NULL;
6084+
6085+
/* Negative control: a corrupted signature must still fail. The fix adds a
6086+
* third acceptance attempt (the flipped DigestInfo parameter encoding), so
6087+
* assert the added leniency did not become over-broad - a signature that
6088+
* matches under neither encoding must return non-zero. Reuse the last
6089+
* built message and flip the final signature byte. Guarded on msgSz > 0 so
6090+
* a failed build above is never passed to the (word32) size cast. */
6091+
if (msgSz > 0) {
6092+
msg[msgSz - 1] ^= 0xFF;
6093+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
6094+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
6095+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
6096+
ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, msg, (word32)msgSz), 0);
6097+
wc_PKCS7_Free(pkcs7);
6098+
}
6099+
#endif /* HAVE_PKCS7 && !NO_RSA && !NO_SHA256 && USE_CERT_BUFFERS_2048 */
6100+
return EXPECT_RESULT();
6101+
}

tests/api/test_pkcs7.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void);
7777
int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
7878
int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void);
7979
int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void);
80+
int test_wc_PKCS7_VerifySignedData_NoDigestParams(void);
8081

8182

8283
#define TEST_PKCS7_DECLS \
@@ -130,7 +131,8 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void);
130131
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq), \
131132
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_IndefLenOOB), \
132133
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncEContentTag), \
133-
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncCertSetTag)
134+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncCertSetTag), \
135+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoDigestParams)
134136

135137
#define TEST_PKCS7_ENCRYPTED_DATA_DECLS \
136138
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_stream), \

wolfcrypt/src/pkcs7.c

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5639,20 +5639,25 @@ static int wc_PKCS7_MlDsaVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz,
56395639
/* build SignedData digest, both in PKCS#7 DigestInfo format and
56405640
* as plain digest for CMS.
56415641
*
5642-
* pkcs7 - pointer to initialized PKCS7 struct
5643-
* signedAttrib - signed attributes
5644-
* signedAttribSz - size of signedAttrib, octets
5645-
* pkcs7Digest - [OUT] PKCS#7 DigestInfo
5646-
* pkcs7DigestSz - [IN/OUT] size of pkcs7Digest
5647-
* plainDigest - [OUT] pointer to plain digest, offset into pkcs7Digest
5648-
* plainDigestSz - [OUT] size of digest at plainDigest
5642+
* pkcs7 - pointer to initialized PKCS7 struct
5643+
* signedAttrib - signed attributes
5644+
* signedAttribSz - size of signedAttrib, octets
5645+
* pkcs7Digest - [OUT] PKCS#7 DigestInfo
5646+
* pkcs7DigestSz - [IN/OUT] size of pkcs7Digest
5647+
* plainDigest - [OUT] pointer to plain digest, offset into pkcs7Digest
5648+
* plainDigestSz - [OUT] size of digest at plainDigest
5649+
* hashParamsAbsent - if non-zero, omit the NULL parameters in the DigestInfo
5650+
* AlgorithmIdentifier; if zero, include them. This is the
5651+
* DigestInfo encoding (RFC 8017), which is independent of
5652+
* the SignerInfo digestAlgorithm parameters (RFC 5652/5754).
56495653
*
56505654
* returns 0 on success, negative on error */
56515655
static int wc_PKCS7_BuildSignedDataDigest(wc_PKCS7* pkcs7, byte* signedAttrib,
56525656
word32 signedAttribSz, byte* pkcs7Digest,
56535657
word32* pkcs7DigestSz, byte** plainDigest,
56545658
word32* plainDigestSz,
5655-
const byte* hashBuf, word32 hashBufSz)
5659+
const byte* hashBuf, word32 hashBufSz,
5660+
byte hashParamsAbsent)
56565661
{
56575662
int ret = 0, digIdx = 0;
56585663
word32 attribSetSz = 0, hashSz = 0;
@@ -5737,9 +5742,10 @@ static int wc_PKCS7_BuildSignedDataDigest(wc_PKCS7* pkcs7, byte* signedAttrib,
57375742
}
57385743
}
57395744

5740-
/* Set algoID, match whatever was input to match either NULL or absent */
5745+
/* Set algoID, using the requested DigestInfo parameter encoding (NULL
5746+
* present or absent) so the caller can try both against the signature. */
57415747
algoIdSz = SetAlgoIDEx(pkcs7->hashOID, algoId, oidHashType,
5742-
0, pkcs7->hashParamsAbsent);
5748+
0, hashParamsAbsent);
57435749

57445750
digestStrSz = SetOctetString(hashSz, digestStr);
57455751
digestInfoSeqSz = SetSequence(algoIdSz + digestStrSz + hashSz,
@@ -5944,11 +5950,13 @@ static int wc_PKCS7_SignedDataVerifySignature(wc_PKCS7* pkcs7, byte* sig,
59445950
}
59455951
}
59465952

5947-
/* build hash to verify against */
5953+
/* build hash to verify against, mirroring the SignerInfo digestAlgorithm
5954+
* parameter encoding as parsed */
59485955
ret = wc_PKCS7_BuildSignedDataDigest(pkcs7, signedAttrib,
59495956
signedAttribSz, pkcs7Digest,
59505957
&pkcs7DigestSz, &plainDigest,
5951-
&plainDigestSz, hashBuf, hashSz);
5958+
&plainDigestSz, hashBuf, hashSz,
5959+
pkcs7->hashParamsAbsent);
59525960
if (ret < 0) {
59535961
WC_FREE_VAR_EX(pkcs7Digest, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
59545962
return ret;
@@ -6041,13 +6049,38 @@ static int wc_PKCS7_SignedDataVerifySignature(wc_PKCS7* pkcs7, byte* sig,
60416049

60426050
#ifndef NO_RSA
60436051
case RSAk:
6052+
/* Try the DigestInfo built with the SignerInfo digestAlgorithm
6053+
* parameter encoding as parsed. */
60446054
ret = wc_PKCS7_RsaVerify(pkcs7, sig, (int)sigSz, pkcs7Digest,
60456055
pkcs7DigestSz);
60466056
if (ret < 0) {
6057+
/* Some signers place the raw digest (no DigestInfo) in the
6058+
* signature. */
60476059
WOLFSSL_MSG("PKCS#7 verification failed, trying CMS");
60486060
ret = wc_PKCS7_RsaVerify(pkcs7, sig, (int)sigSz, plainDigest,
60496061
plainDigestSz);
60506062
}
6063+
if (ret < 0) {
6064+
/* The DigestInfo AlgorithmIdentifier inside a PKCS#1 v1.5
6065+
* signature (RFC 8017) is encoded independently of the
6066+
* SignerInfo digestAlgorithm parameters (RFC 5652/5754): a
6067+
* signer may omit the NULL in one and include it in the other
6068+
* (e.g. Go's crypto/rsa signs a NULL-present DigestInfo while
6069+
* omitting the NULL in the SignerInfo). Rebuild the DigestInfo
6070+
* with the opposite parameter encoding and try once more. This
6071+
* overwrites pkcs7Digest/plainDigest, so it must run after the
6072+
* plain-digest attempt above. */
6073+
WOLFSSL_MSG("PKCS#7 verification failed, trying flipped params");
6074+
ret = wc_PKCS7_BuildSignedDataDigest(pkcs7, signedAttrib,
6075+
signedAttribSz, pkcs7Digest,
6076+
&pkcs7DigestSz, &plainDigest,
6077+
&plainDigestSz, hashBuf, hashSz,
6078+
(byte)(!pkcs7->hashParamsAbsent));
6079+
if (ret == 0) {
6080+
ret = wc_PKCS7_RsaVerify(pkcs7, sig, (int)sigSz, pkcs7Digest,
6081+
pkcs7DigestSz);
6082+
}
6083+
}
60516084
break;
60526085

60536086
#ifdef WC_RSA_PSS

0 commit comments

Comments
 (0)