Skip to content

Commit 7a56ec3

Browse files
committed
CRL PQC signing: address review feedback
- wc_SignCRL_ex: move the new ML-DSA/SLH-DSA key pointers to the end of the parameter list (after rng) so the original RSA/ECC parameter order is preserved instead of being split by the inserted PQC params. Update the prototype and all callers accordingly. - wc_SignCRL_ex: brace the single-statement key-count ifs to match file style. - Tests: size the CRL output buffer from the signing key's actual signature length (wc_MlDsaKey_GetSigLen / wc_SlhDsaKey_SigSize) plus wrapper headroom instead of a fixed 32768 magic number, so it scales to any parameter set. - Tests: add a post-quantum sigType/key mismatch negative case (a classic OID with a PQC key must return ALGO_ID_E). - Tests: assert the exact ASN_CRL_CONFIRM_E on the tampered-signature case rather than just "not success". - Tests: heap-allocate the issuer DER buffer instead of a 1 KB stack array.
1 parent 8f1d340 commit 7a56ec3

4 files changed

Lines changed: 58 additions & 27 deletions

File tree

src/crl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2903,7 +2903,7 @@ int wolfSSL_X509_CRL_sign(WOLFSSL_X509_CRL* crl, WOLFSSL_EVP_PKEY* pkey,
29032903
*/
29042904
if (ret == WOLFSSL_SUCCESS) {
29052905
totalSz = wc_SignCRL_ex(buf, tbsSz, sigType, buf, bufSz,
2906-
rsaKey, eccKey, NULL, NULL, &rng);
2906+
rsaKey, eccKey, &rng, NULL, NULL);
29072907
if (totalSz < 0) {
29082908
WOLFSSL_MSG("wc_SignCRL_ex failed");
29092909
ret = totalSz;

tests/api.c

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24338,7 +24338,7 @@ static int test_wc_MakeCRL_max_crlnum(void)
2433824338
}
2433924339
if (EXPECT_SUCCESS()) {
2434024340
crlSz = wc_SignCRL_ex(tbsBuf, tbsSz, CTC_SHA256wRSA,
24341-
crlBuf, (word32)bufSz, &rsaKey, NULL, NULL, NULL, &rng);
24341+
crlBuf, (word32)bufSz, &rsaKey, NULL, &rng, NULL, NULL);
2434224342
ExpectIntGT(crlSz, 0);
2434324343
}
2434424344

@@ -24347,7 +24347,7 @@ static int test_wc_MakeCRL_max_crlnum(void)
2434724347
* paired with an ECDSA OID must return ALGO_ID_E. --- */
2434824348
if (EXPECT_SUCCESS()) {
2434924349
ExpectIntEQ(wc_SignCRL_ex(tbsBuf, tbsSz, CTC_SHA256wECDSA,
24350-
crlBuf, (word32)bufSz, &rsaKey, NULL, NULL, NULL, &rng),
24350+
crlBuf, (word32)bufSz, &rsaKey, NULL, &rng, NULL, NULL),
2435124351
WC_NO_ERR_TRACE(ALGO_ID_E));
2435224352
}
2435324353

@@ -24430,13 +24430,14 @@ static int pqc_crl_sign_verify(const byte* caCertDer, word32 caCertDerSz,
2443024430
int caCertInit = 0;
2443124431
WC_RNG rng;
2443224432
int rngInit = 0;
24433-
byte issuerDer[1024];
24433+
byte* issuerDer = NULL;
2443424434
word32 issuerDerSz = 0;
2443524435
byte* tbsBuf = NULL;
2443624436
byte* crlBuf = NULL;
2443724437
int tbsSz = 0;
2443824438
int crlSz = 0;
2443924439
int bufSz = 0;
24440+
int sigSz = 0;
2444024441

2444124442
/* thisUpdate in the past, nextUpdate far in the future so the CRL is
2444224443
* current whenever the test runs. */
@@ -24451,15 +24452,16 @@ static int pqc_crl_sign_verify(const byte* caCertDer, word32 caCertDerSz,
2445124452
wc_InitDecodedCert(&caCert, caCertDer, caCertDerSz, NULL);
2445224453
caCertInit = 1;
2445324454
ExpectIntEQ(wc_ParseCert(&caCert, CERT_TYPE, 0, NULL), 0);
24455+
if (EXPECT_SUCCESS()) {
24456+
ExpectNotNull(issuerDer = (byte*)XMALLOC(
24457+
(size_t)caCert.subjectRawLen + MAX_SEQ_SZ, NULL,
24458+
DYNAMIC_TYPE_TMP_BUFFER));
24459+
}
2445424460
if (EXPECT_SUCCESS()) {
2445524461
word32 seqHdrSz = SetSequence((word32)caCert.subjectRawLen, issuerDer);
24456-
ExpectIntLE((int)(seqHdrSz + (word32)caCert.subjectRawLen),
24457-
(int)sizeof(issuerDer));
24458-
if (EXPECT_SUCCESS()) {
24459-
XMEMCPY(issuerDer + seqHdrSz, caCert.subjectRaw,
24460-
(size_t)caCert.subjectRawLen);
24461-
issuerDerSz = seqHdrSz + (word32)caCert.subjectRawLen;
24462-
}
24462+
XMEMCPY(issuerDer + seqHdrSz, caCert.subjectRaw,
24463+
(size_t)caCert.subjectRawLen);
24464+
issuerDerSz = seqHdrSz + (word32)caCert.subjectRawLen;
2446324465
}
2446424466

2446524467
ExpectIntEQ(wc_InitRng(&rng), 0);
@@ -24473,10 +24475,25 @@ static int pqc_crl_sign_verify(const byte* caCertDer, word32 caCertDerSz,
2447324475
NULL, crlNum, (word32)sizeof(crlNum), sigType, 2, NULL, 0);
2447424476
ExpectIntGT(tbsSz, 0);
2447524477
}
24478+
/* Size the output from the key's actual signature length (PQC signatures
24479+
* range from a few KB for ML-DSA to tens of KB for large SLH-DSA sets)
24480+
* plus headroom for the AlgorithmIdentifier, BIT STRING and SEQUENCE
24481+
* wrappers, rather than a fixed magic number. */
24482+
#ifdef WOLFSSL_HAVE_MLDSA
24483+
if (mldsaKey != NULL) {
24484+
int l = 0;
24485+
ExpectIntEQ(wc_MlDsaKey_GetSigLen(mldsaKey, &l), 0);
24486+
sigSz = l;
24487+
}
24488+
#endif
24489+
#ifdef WOLFSSL_HAVE_SLHDSA
24490+
if (slhDsaKey != NULL) {
24491+
sigSz = wc_SlhDsaKey_SigSize(slhDsaKey);
24492+
}
24493+
#endif
24494+
ExpectIntGT(sigSz, 0);
2447624495
if (EXPECT_SUCCESS()) {
24477-
/* Generous room for the (large) PQC signature and ASN.1 wrappers;
24478-
* SLH-DSA signatures alone are several KB. */
24479-
bufSz = tbsSz + 32768;
24496+
bufSz = tbsSz + sigSz + 512;
2448024497
ExpectNotNull(tbsBuf = (byte*)XMALLOC(bufSz, NULL,
2448124498
DYNAMIC_TYPE_TMP_BUFFER));
2448224499
ExpectNotNull(crlBuf = (byte*)XMALLOC(bufSz, NULL,
@@ -24493,10 +24510,19 @@ static int pqc_crl_sign_verify(const byte* caCertDer, word32 caCertDerSz,
2449324510
/* Sign the CRL with the post-quantum key. */
2449424511
if (EXPECT_SUCCESS()) {
2449524512
crlSz = wc_SignCRL_ex(tbsBuf, tbsSz, sigType, crlBuf, (word32)bufSz,
24496-
NULL, NULL, mldsaKey, slhDsaKey, &rng);
24513+
NULL, NULL, &rng, mldsaKey, slhDsaKey);
2449724514
ExpectIntGT(crlSz, 0);
2449824515
}
2449924516

24517+
/* Negative: a classic signatureAlgorithm OID must be rejected for a PQC
24518+
* key before any signature is produced. CheckSigTypeForKey runs before the
24519+
* TBS is copied into the output, so crlBuf still holds the valid CRL. */
24520+
if (EXPECT_SUCCESS()) {
24521+
ExpectIntEQ(wc_SignCRL_ex(tbsBuf, tbsSz, CTC_SHA256wRSA, crlBuf,
24522+
(word32)bufSz, NULL, NULL, &rng, mldsaKey, slhDsaKey),
24523+
WC_NO_ERR_TRACE(ALGO_ID_E));
24524+
}
24525+
2450024526
/* Load the issuing CA and verify the freshly signed CRL. */
2450124527
ExpectNotNull(cm = wolfSSL_CertManagerNew());
2450224528
ExpectIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, caCertDer, caCertDerSz,
@@ -24506,7 +24532,9 @@ static int pqc_crl_sign_verify(const byte* caCertDer, word32 caCertDerSz,
2450624532
ExpectIntEQ(wolfSSL_CertManagerLoadCRLBuffer(cm, crlBuf, crlSz,
2450724533
WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
2450824534

24509-
/* Negative: corrupt the last signature byte; verification must now fail. */
24535+
/* Negative: flip a byte of the signature *value*. The DER lengths are
24536+
* unchanged so the CRL still parses; only the signature check can reject
24537+
* it, which must surface as ASN_CRL_CONFIRM_E. */
2451024538
if (EXPECT_SUCCESS()) {
2451124539
WOLFSSL_CERT_MANAGER* cm2 = NULL;
2451224540
crlBuf[crlSz - 1] ^= 0xFF;
@@ -24515,12 +24543,13 @@ static int pqc_crl_sign_verify(const byte* caCertDer, word32 caCertDerSz,
2451524543
caCertDerSz, WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
2451624544
ExpectIntEQ(wolfSSL_CertManagerEnableCRL(cm2, WOLFSSL_CRL_CHECKALL),
2451724545
WOLFSSL_SUCCESS);
24518-
ExpectIntNE(wolfSSL_CertManagerLoadCRLBuffer(cm2, crlBuf, crlSz,
24519-
WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
24546+
ExpectIntEQ(wolfSSL_CertManagerLoadCRLBuffer(cm2, crlBuf, crlSz,
24547+
WOLFSSL_FILETYPE_ASN1), WC_NO_ERR_TRACE(ASN_CRL_CONFIRM_E));
2452024548
wolfSSL_CertManagerFree(cm2);
2452124549
}
2452224550

2452324551
wolfSSL_CertManagerFree(cm);
24552+
XFREE(issuerDer, NULL, DYNAMIC_TYPE_TMP_BUFFER);
2452424553
XFREE(crlBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
2452524554
XFREE(tbsBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
2452624555
if (rngInit)

wolfcrypt/src/asn.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36920,15 +36920,17 @@ int wc_MakeCRL_ex(const byte* issuerDer, word32 issuerSz,
3692036920
* bufSz: size of output buffer
3692136921
* rsaKey/eccKey/mldsaKey/slhDsaKey: signing key (exactly one must be non-NULL).
3692236922
* ML-DSA and SLH-DSA produce post-quantum signatures; their (much larger)
36923-
* signature buffer is sized from the key rather than assumed classic.
36923+
* signature buffer is sized from the key rather than assumed classic. The
36924+
* PQC key pointers are last so the original RSA/ECC parameter order is
36925+
* preserved.
3692436926
* rng: random number generator
3692536927
*
3692636928
* Returns: size of complete CRL on success, negative error on failure
3692736929
*/
3692836930
int wc_SignCRL_ex(const byte* tbsBuf, int tbsSz, int sType,
3692936931
byte* buf, word32 bufSz,
36930-
RsaKey* rsaKey, ecc_key* eccKey,
36931-
wc_MlDsaKey* mldsaKey, SlhDsaKey* slhDsaKey, WC_RNG* rng)
36932+
RsaKey* rsaKey, ecc_key* eccKey, WC_RNG* rng,
36933+
wc_MlDsaKey* mldsaKey, SlhDsaKey* slhDsaKey)
3693236934
{
3693336935
int ret;
3693436936
int sigSz;
@@ -36942,10 +36944,10 @@ int wc_SignCRL_ex(const byte* tbsBuf, int tbsSz, int sType,
3694236944
return BAD_FUNC_ARG;
3694336945

3694436946
/* Exactly one signing key must be supplied. */
36945-
if (rsaKey != NULL) nKeys++;
36946-
if (eccKey != NULL) nKeys++;
36947-
if (mldsaKey != NULL) nKeys++;
36948-
if (slhDsaKey != NULL) nKeys++;
36947+
if (rsaKey != NULL) { nKeys++; }
36948+
if (eccKey != NULL) { nKeys++; }
36949+
if (mldsaKey != NULL) { nKeys++; }
36950+
if (slhDsaKey != NULL) { nKeys++; }
3694936951
if (nKeys != 1)
3695036952
return BAD_FUNC_ARG;
3695136953

wolfssl/wolfcrypt/asn_public.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -716,8 +716,8 @@ WOLFSSL_API int wc_MakeCRL_ex(const byte* issuerDer, word32 issuerSz,
716716
byte* output, word32 outputSz);
717717
WOLFSSL_API int wc_SignCRL_ex(const byte* tbsBuf, int tbsSz, int sType,
718718
byte* buf, word32 bufSz,
719-
RsaKey* rsaKey, ecc_key* eccKey,
720-
wc_MlDsaKey* mldsaKey, SlhDsaKey* slhDsaKey, WC_RNG* rng);
719+
RsaKey* rsaKey, ecc_key* eccKey, WC_RNG* rng,
720+
wc_MlDsaKey* mldsaKey, SlhDsaKey* slhDsaKey);
721721
#endif /* WOLFSSL_CERT_GEN && HAVE_CRL */
722722

723723
WOLFSSL_API int wc_GetDateInfo(const byte* certDate, int certDateSz,

0 commit comments

Comments
 (0)