crypt/wolfssl
1
0
Fork 0
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-10-24 04:09:07 +03:00
Code Activity

Merge pull request #11482 from gasbytes/dsa-sig-fix

Browse Source 
Fix DSA signature length handling in certificate verification
This commit is contained in:
Sean Parkinson
2026-10-07 10:46:56 +10:00
committed by GitHub
parent 8a93f0f78f ab1b141a19
commit 8719ecb6b7
6 changed files with 121 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
tests/api/test_dsa.c
View File

@@ -1039,3 +1039,66 @@ int test_wc_DsaExportKeyRaw_individual_args(void)
#endif /* !NO_DSA && WOLFSSL_KEY_GEN */
return EXPECT_RESULT();
} /* END test_wc_DsaExportKeyRaw_individual_args */
/*
* Testing that a certificate signature field holding fewer bytes than the
* verifying DSA key's 2 * |q| is rejected without reading past the copy
* ConfirmSignature() makes of it.
*/
int test_wc_DsaVerify_cert_sig_narrower_than_q(void)
{
EXPECT_DECLS;
#if !defined(NO_DSA) && !defined(HAVE_SELFTEST) && !defined(NO_CERTS) && \
!defined(NO_SHA256) && !defined(NO_FILESYSTEM) && \
(defined(OPENSSL_EXTRA) || defined(WOLFSSL_SMALL_CERT_VERIFY))
/* v1 certificate signed with dsa-with-SHA256 whose signature field is
* DSA_MIN_SIG_SIZE bytes, as produced by splicing a 40 byte signature into
* a DSA certificate. The key it is checked against is supplied separately,
* so only the signature algorithm and the signature length matter here. */
static const byte cert[] = {
0x30, 0x81, 0x9f,
0x30, 0x65,
0x02, 0x01, 0x01,
0x30, 0x0b,
0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03,
0x02,
0x30, 0x0e,
0x31, 0x0c, 0x30, 0x0a, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c,
0x03, 0x64, 0x73, 0x61,
0x30, 0x1e,
0x17, 0x0d, 0x32, 0x35, 0x30, 0x31, 0x30, 0x31, 0x30, 0x30,
0x30, 0x30, 0x30, 0x30, 0x5a,
0x17, 0x0d, 0x33, 0x35, 0x30, 0x31, 0x30, 0x31, 0x30, 0x30,
0x30, 0x30, 0x30, 0x30, 0x5a,
0x30, 0x0e,
0x31, 0x0c, 0x30, 0x0a, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c,
0x03, 0x64, 0x73, 0x61,
0x30, 0x13,
0x30, 0x0b,
0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x38, 0x04, 0x01,
0x05, 0x00,
0x03, 0x04, 0x00, 0x02, 0x01, 0x01,
0x30, 0x0b,
0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03,
0x02,
0x03, 0x29, 0x00,
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01
};
/* 256 bit q, so the verifier reads r and s as 32 bytes each. */
byte key[TWOK_BUF];
word32 keySz = 0;
XFILE fp = XBADFILE;
ExpectTrue((fp = XFOPEN("./certs/dsa-pubkey-2048.der", "rb")) != XBADFILE);
ExpectIntGT(keySz = (word32)XFREAD(key, 1, sizeof(key), fp), 0);
if (fp != XBADFILE)
XFCLOSE(fp);
ExpectIntNE(wc_CheckCertSigPubKey(cert, (word32)sizeof(cert), NULL, key,
keySz, DSAk), 0);
#endif
return EXPECT_RESULT();
} /* END test_wc_DsaVerify_cert_sig_narrower_than_q */

4
tests/api/test_dsa.h
View File

@@ -39,6 +39,7 @@ int test_wc_DsaSign_bad_digestSz(void);
int test_wc_DsaImportParamsRaw_individual_args(void);
int test_wc_DsaExportParamsRaw_individual_args(void);
int test_wc_DsaExportKeyRaw_individual_args(void);
int test_wc_DsaVerify_cert_sig_narrower_than_q(void);
#define TEST_DSA_DECLS \
TEST_DECL_GROUP("dsa", test_wc_InitDsaKey), \
@@ -55,6 +56,7 @@ int test_wc_DsaExportKeyRaw_individual_args(void);
TEST_DECL_GROUP("dsa", test_wc_DsaSign_bad_digestSz), \
TEST_DECL_GROUP("dsa", test_wc_DsaImportParamsRaw_individual_args), \
TEST_DECL_GROUP("dsa", test_wc_DsaExportParamsRaw_individual_args), \
TEST_DECL_GROUP("dsa", test_wc_DsaExportKeyRaw_individual_args)
TEST_DECL_GROUP("dsa", test_wc_DsaExportKeyRaw_individual_args), \
TEST_DECL_GROUP("dsa", test_wc_DsaVerify_cert_sig_narrower_than_q)
#endif /* WOLFCRYPT_TEST_DSA_H */

5
tests/unit-mcdc/test_asn_cert_whitebox.c
View File

@@ -1733,9 +1733,10 @@ static void wb_decode_dsa_asn1_sig(void)
contentSz += wb_tlv(content + contentSz, ASN_INTEGER, &sVal, 1);
sigSz = WB_SEQ(sig, content, contentSz);
ret = DecodeDsaAsn1Sig(sig, sigSz, sigCpy, NULL);
/* qSz 1: r and s are one byte each, so sigCpy holds them unpadded. */
ret = DecodeDsaAsn1Sig(sig, sigSz, sigCpy, 1, NULL);
WB_CHECK(ret == 0 && sigCpy[0] == rVal && sigCpy[1] == sVal,
"valid r/s -> mp_to_unsigned_bin() succeeds both times "
"valid r/s -> mp_to_unsigned_bin_len() succeeds both times "
"(:17294 false side)");
}
#else

47
tests/unit-mcdc/test_asn_fault_whitebox.c
View File

@@ -109,9 +109,9 @@
* :24921/:24925 leading operands ARE reachable and are driven by the
* allocation sweep in section 24.
* - DecodeDsaAsn1Sig() :17348 both operands: the block is guarded by
* :17342, which has already rejected rSz + sSz > sigSz, and every
* caller passes a sigCpy of at least sigSz bytes, so both
* mp_to_unsigned_bin() calls write inside the buffer and cannot fail.
* :17342, which has already rejected rSz > qSz || sSz > qSz, and every
* caller passes a sigCpy of 2 * qSz bytes, so both
* mp_to_unsigned_bin_len() calls write inside the buffer and cannot fail.
*
* the uncovered-condition report rows in the deep certificate chain-verification internals
* (name-constraint enforcement, X.509 extension decoding/verification,
@@ -2575,25 +2575,25 @@ static byte* wb_read_pem_file(const char* path, long* outLen)
/* ------------------------------------------------------------------------- *
* Section 24b: ConfirmSignature() DSA signature-size dispatch (:17703).
* if (sigSz != DSA_160_SIG_SIZE && sigSz != DSA_256_SIG_SIZE)
* ret = DecodeDsaAsn1Sig(...);
* else
* if (sigSz == 2 * qSz)
* XMEMCPY(sigCtx->sigCpy, sig, sigSz);
* else
* ret = DecodeDsaAsn1Sig(...);
* A DSA-signed certificate carries an ASN.1 DSA-Sig-Value, so the raw-copy
* arm (a signature that is exactly 40 or 64 bytes) is never taken from a
* arm (a signature that is exactly 2 * |q| bytes) is never taken from a
* certificate parse. Calling ConfirmSignature() directly with a DSA public
* key and three signature lengths drives all three rows. The signature bytes
* key and two signature lengths drives both rows. The signature bytes
* themselves are irrelevant -- the call always ends in a verification
* failure; the point is which arm the size dispatch picks.
* ------------------------------------------------------------------------- */
#if !defined(NO_DSA) && !defined(HAVE_SELFTEST) && !defined(NO_ASN_CRYPT)
static void wb_confirm_signature_dsa_sigsz(void)
{
static const word32 sizes[3] = { DSA_160_SIG_SIZE, DSA_256_SIG_SIZE, 50 };
static const char* names[3] = {
"sigSz == DSA_160_SIG_SIZE (1st operand false)",
"sigSz == DSA_256_SIG_SIZE (1st operand true, 2nd false)",
"sigSz neither (both operands true, ASN.1 decode path)"
/* certs/dsa2048.der has a 160-bit q, so 2 * qSz is DSA_160_SIG_SIZE. */
static const word32 sizes[2] = { DSA_160_SIG_SIZE, 50 };
static const char* names[2] = {
"sigSz == 2 * qSz (true, raw copy)",
"sigSz != 2 * qSz (false, ASN.1 decode path)"
};
byte* pem = NULL;
long pemSz = 0;
@@ -2637,7 +2637,7 @@ static void wb_confirm_signature_dsa_sigsz(void)
XMEMSET(sig, 0x5A, sizeof(sig));
XMEMSET(tbs, 0x11, sizeof(tbs));
for (i = 0; i < 3; i++) {
for (i = 0; i < 2; i++) {
SignatureCtx sigCtx;
InitSignatureCtx(&sigCtx, NULL, INVALID_DEVID);
ret = ConfirmSignature(&sigCtx, tbs, (word32)sizeof(tbs), pubDer,
@@ -2992,12 +2992,12 @@ static void wb_encoder_size_guards(void)
* are two consecutive XMALLOCs, so failing from the first allocation drives
* the 1st operand true and failing only the second drives the 2nd.
*
* RESIDUAL -- :17348 (`mp_to_unsigned_bin(r, sigCpy) != MP_OKAY ||
* mp_to_unsigned_bin(s, sigCpy + rSz) != MP_OKAY`) has no reachable true
* side: it is guarded by :17342, which has already rejected rSz + sSz >
* sigSz, and every caller passes a sigCpy of at least sigSz bytes, so both
* conversions write inside the buffer. mp_to_unsigned_bin() on an
* initialised mp_int with a large enough output cannot fail.
* RESIDUAL -- :17348 (`mp_to_unsigned_bin_len(r, sigCpy, qSz) != MP_OKAY ||
* mp_to_unsigned_bin_len(s, sigCpy + qSz, qSz) != MP_OKAY`) has no reachable
* true side: it is guarded by :17342, which has already rejected rSz > qSz ||
* sSz > qSz, and every caller passes a sigCpy of 2 * qSz bytes, so both
* conversions write inside the buffer. mp_to_unsigned_bin_len() on an
* initialised mp_int that fits the requested length cannot fail.
* ------------------------------------------------------------------------- */
#if !defined(NO_DSA) && !defined(HAVE_SELFTEST)
static void wb_decode_dsa_asn1_sig_alloc(void)
@@ -3010,15 +3010,16 @@ static void wb_decode_dsa_asn1_sig_alloc(void)
WB_NOTE("DecodeDsaAsn1Sig(): r/s allocation guard [:17324]");
XMEMSET(sigCpy, 0, sizeof(sigCpy));
ret = DecodeDsaAsn1Sig(sig, (word32)sizeof(sig), sigCpy, NULL);
/* qSz 1: r and s are one byte each, so sigCpy holds them unpadded. */
ret = DecodeDsaAsn1Sig(sig, (word32)sizeof(sig), sigCpy, 1, NULL);
WB_CHECK(ret == 0, "well-formed DSA-Sig-Value (both operands false)");
mcdc_fa_install();
mcdc_fa_arm(1);
(void)DecodeDsaAsn1Sig(sig, (word32)sizeof(sig), sigCpy, NULL);
(void)DecodeDsaAsn1Sig(sig, (word32)sizeof(sig), sigCpy, 1, NULL);
mcdc_fa_disarm();
mcdc_fa_arm_only(2);
(void)DecodeDsaAsn1Sig(sig, (word32)sizeof(sig), sigCpy, NULL);
(void)DecodeDsaAsn1Sig(sig, (word32)sizeof(sig), sigCpy, 1, NULL);
mcdc_fa_disarm();
mcdc_fa_restore();
}

42
wolfcrypt/src/asn.c
View File

@@ -17695,9 +17695,9 @@ static int HashForSignature(const byte* buf, word32 bufSz, word32 sigOID,
#endif /* !NO_ASN_CRYPT && !NO_HASH_WRAPPER */
#if !defined(NO_DSA) && !defined(HAVE_SELFTEST)
/* Try to parse as ASN.1 bitstring */
/* Parse as an ASN.1 bitstring into sigCpy as r||s, each zero padded to qSz. */
static int DecodeDsaAsn1Sig(const byte* sig, word32 sigSz, byte* sigCpy,
void* heap)
int qSz, void* heap)
{
int ret = 0;
int rSz = 0, sSz = 0, mpinit = 0;
@@ -17730,14 +17730,14 @@ static int DecodeDsaAsn1Sig(const byte* sig, word32 sigSz, byte* sigCpy,
if (ret == 0) {
rSz = mp_unsigned_bin_size(r);
sSz = mp_unsigned_bin_size(s);
if (rSz + sSz > (int)sigSz) {
if (rSz > qSz || sSz > qSz) {
WOLFSSL_MSG("DSA sig size invalid");
ret = ASN_SIG_CONFIRM_E;
}
}
if (ret == 0) {
if (mp_to_unsigned_bin(r, sigCpy) != MP_OKAY ||
mp_to_unsigned_bin(s, sigCpy + rSz) != MP_OKAY) {
if (mp_to_unsigned_bin_len(r, sigCpy, qSz) != MP_OKAY ||
mp_to_unsigned_bin_len(s, sigCpy + qSz, qSz) != MP_OKAY) {
WOLFSSL_MSG("DSA sig to unsigned bin failed!");
ret = ASN_SIG_CONFIRM_E;
}
@@ -18058,6 +18058,7 @@ int ConfirmSignature(SignatureCtx* sigCtx,
case DSAk:
{
word32 idx = 0;
int qSz;
if (sigSz < DSA_MIN_SIG_SIZE) {
WOLFSSL_MSG("Verify Signature is too small");
@@ -18078,26 +18079,33 @@ int ConfirmSignature(SignatureCtx* sigCtx,
WOLFSSL_MSG("wc_InitDsaKey_h error");
goto exit_cs;
}
#ifndef WOLFSSL_NO_MALLOC
sigCtx->sigCpy = (byte*)XMALLOC(sigSz,
sigCtx->heap, DYNAMIC_TYPE_SIGNATURE);
if (sigCtx->sigCpy == NULL) {
ERROR_OUT(MEMORY_E, exit_cs);
}
#endif
if ((ret = wc_DsaPublicKeyDecode(key, &idx, sigCtx->key.dsa,
keySz)) != 0) {
WOLFSSL_MSG("ASN Key decode error DSA");
WOLFSSL_ERROR_VERBOSE(ret);
goto exit_cs;
}
if (sigSz != DSA_160_SIG_SIZE &&
sigSz != DSA_256_SIG_SIZE) {
ret = DecodeDsaAsn1Sig(sig, sigSz, sigCtx->sigCpy,
sigCtx->heap);
/* wc_DsaVerify() reads 2 * |q| bytes from sigCpy, so size
* the copy from the key and reject any |q| the DSA code
* cannot produce a signature for. */
qSz = mp_unsigned_bin_size(&sigCtx->key.dsa->q);
if (qSz < DSA_MIN_HALF_SIZE || qSz > DSA_MAX_HALF_SIZE) {
WOLFSSL_MSG("Verify DSA key q size invalid");
ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs);
}
#ifndef WOLFSSL_NO_MALLOC
sigCtx->sigCpy = (byte*)XMALLOC((word32)(2 * qSz),
sigCtx->heap, DYNAMIC_TYPE_SIGNATURE);
if (sigCtx->sigCpy == NULL) {
ERROR_OUT(MEMORY_E, exit_cs);
}
#endif
if (sigSz == (word32)(2 * qSz)) {
XMEMCPY(sigCtx->sigCpy, sig, sigSz);
}
else {
XMEMCPY(sigCtx->sigCpy, sig, sigSz);
ret = DecodeDsaAsn1Sig(sig, sigSz, sigCtx->sigCpy, qSz,
sigCtx->heap);
}
break;
}

4
wolfcrypt/src/dsa.c
View File

@@ -1216,8 +1216,10 @@ int wc_DsaVerify_ex(const byte* digest, word32 digestSz, const byte* sig,
break;
}
/* sig holds r||s as 2 * |q| bytes and carries no length, so bound |q|
* to keep the read within DSA_MAX_SIG_SIZE. */
qSz = mp_unsigned_bin_size(&key->q);
if (qSz <= 0) {
if (qSz <= 0 || qSz > DSA_MAX_HALF_SIZE) {
ret = BAD_FUNC_ARG;
break;
}