Fix DSA signature length

The length of the DSA signature is 40 bytes for N=160 but 64 bytes for N=256. New enum values are added for better clarity.
This commit is contained in:
Juliusz Sosinowicz
2021-06-21 19:57:25 +02:00
parent 142ff6d885
commit c7d6e26437
5 changed files with 128 additions and 89 deletions

127
src/ssl.c
View File

@ -31579,7 +31579,7 @@ int wolfSSL_DSA_SIG_set0(WOLFSSL_DSA_SIG *sig, WOLFSSL_BIGNUM *r,
int wolfSSL_i2d_DSA_SIG(const WOLFSSL_DSA_SIG *sig, byte **out) int wolfSSL_i2d_DSA_SIG(const WOLFSSL_DSA_SIG *sig, byte **out)
{ {
/* Space for sequence + two asn ints */ /* Space for sequence + two asn ints */
byte buf[MAX_SEQ_SZ + 2*(ASN_TAG_SZ + MAX_LENGTH_SZ + DSA_HALF_SIZE)]; byte buf[MAX_SEQ_SZ + 2*(ASN_TAG_SZ + MAX_LENGTH_SZ + DSA_MAX_HALF_SIZE)];
word32 bufLen = sizeof(buf); word32 bufLen = sizeof(buf);
WOLFSSL_ENTER("wolfSSL_i2d_DSA_SIG"); WOLFSSL_ENTER("wolfSSL_i2d_DSA_SIG");
@ -31615,8 +31615,8 @@ int wolfSSL_i2d_DSA_SIG(const WOLFSSL_DSA_SIG *sig, byte **out)
* ASN1_SEQUENCE * ASN1_SEQUENCE
* ASN1_INTEGER (DSA r) * ASN1_INTEGER (DSA r)
* ASN1_INTEGER (DSA s) * ASN1_INTEGER (DSA s)
* Alternatively, if the input is DSA_SIG_SIZE in length then this * Alternatively, if the input is DSA_160_SIG_SIZE or DSA_256_SIG_SIZE in
* API interprets this as two unsigned binary numbers. * length then this API interprets this as two unsigned binary numbers.
* @param sig If non-null then free'd first and then newly created * @param sig If non-null then free'd first and then newly created
* WOLFSSL_DSA_SIG is assigned * WOLFSSL_DSA_SIG is assigned
* @param pp Input buffer that is moved forward on success * @param pp Input buffer that is moved forward on success
@ -31645,28 +31645,31 @@ WOLFSSL_DSA_SIG* wolfSSL_d2i_DSA_SIG(WOLFSSL_DSA_SIG **sig,
r = (mp_int*)ret->r->internal; r = (mp_int*)ret->r->internal;
s = (mp_int*)ret->s->internal; s = (mp_int*)ret->s->internal;
if (length == DSA_SIG_SIZE) { if (DecodeECC_DSA_Sig(*pp, length, r, s) != 0) {
/* Two raw numbers of DSA_HALF_SIZE size each */ if (length == DSA_160_SIG_SIZE || length == DSA_256_SIG_SIZE) {
if (mp_read_unsigned_bin(r, *pp, DSA_HALF_SIZE) != 0) { /* Two raw numbers of length/2 size each */
WOLFSSL_MSG("r mp_read_unsigned_bin error"); if (mp_read_unsigned_bin(r, *pp, length/2) != 0) {
wolfSSL_DSA_SIG_free(ret); WOLFSSL_MSG("r mp_read_unsigned_bin error");
return NULL; wolfSSL_DSA_SIG_free(ret);
} return NULL;
}
if (mp_read_unsigned_bin(s, *pp + DSA_HALF_SIZE, DSA_HALF_SIZE) != 0) { if (mp_read_unsigned_bin(s, *pp + (length/2), length/2) != 0) {
WOLFSSL_MSG("s mp_read_unsigned_bin error"); WOLFSSL_MSG("s mp_read_unsigned_bin error");
wolfSSL_DSA_SIG_free(ret); wolfSSL_DSA_SIG_free(ret);
return NULL; return NULL;
} }
*pp += DSA_SIG_SIZE; *pp += length;
} }
else { else {
if (DecodeECC_DSA_Sig(*pp, length, r, s) != 0) {
WOLFSSL_MSG("DecodeECC_DSA_Sig error"); WOLFSSL_MSG("DecodeECC_DSA_Sig error");
wolfSSL_DSA_SIG_free(ret); wolfSSL_DSA_SIG_free(ret);
return NULL; return NULL;
} }
}
else {
/* DecodeECC_DSA_Sig success move pointer forward */
#ifndef NO_STRICT_ECDSA_LEN #ifndef NO_STRICT_ECDSA_LEN
*pp += length; *pp += length;
#else #else
@ -31684,7 +31687,6 @@ WOLFSSL_DSA_SIG* wolfSSL_d2i_DSA_SIG(WOLFSSL_DSA_SIG **sig,
#endif #endif
} }
if (sig != NULL) { if (sig != NULL) {
if (*sig != NULL) if (*sig != NULL)
wolfSSL_DSA_SIG_free(*sig); wolfSSL_DSA_SIG_free(*sig);
@ -31714,10 +31716,8 @@ int wolfSSL_DSA_do_sign(const unsigned char* d, unsigned char* sigRet,
return ret; return ret;
} }
if (dsa->inSet == 0) if (dsa->inSet == 0) {
{
WOLFSSL_MSG("No DSA internal set, do it"); WOLFSSL_MSG("No DSA internal set, do it");
if (SetDsaInternal(dsa) != WOLFSSL_SUCCESS) { if (SetDsaInternal(dsa) != WOLFSSL_SUCCESS) {
WOLFSSL_MSG("SetDsaInternal failed"); WOLFSSL_MSG("SetDsaInternal failed");
return ret; return ret;
@ -31762,8 +31762,9 @@ int wolfSSL_DSA_do_sign(const unsigned char* d, unsigned char* sigRet,
WOLFSSL_DSA_SIG* wolfSSL_DSA_do_sign_ex(const unsigned char* digest, WOLFSSL_DSA_SIG* wolfSSL_DSA_do_sign_ex(const unsigned char* digest,
int inLen, WOLFSSL_DSA* dsa) int inLen, WOLFSSL_DSA* dsa)
{ {
WOLFSSL_DSA_SIG* sig = NULL; byte sigBin[DSA_MAX_SIG_SIZE];
byte sigBin[DSA_SIG_SIZE]; const byte *tmp = sigBin;
int sigLen;
WOLFSSL_ENTER("wolfSSL_DSA_do_sign_ex"); WOLFSSL_ENTER("wolfSSL_DSA_do_sign_ex");
@ -31773,27 +31774,23 @@ WOLFSSL_DSA_SIG* wolfSSL_DSA_do_sign_ex(const unsigned char* digest,
} }
if (wolfSSL_DSA_do_sign(digest, sigBin, dsa) != WOLFSSL_SUCCESS) { if (wolfSSL_DSA_do_sign(digest, sigBin, dsa) != WOLFSSL_SUCCESS) {
WOLFSSL_MSG("wolfSSL_DSA_do_sign error");
return NULL; return NULL;
} }
if (!(sig = wolfSSL_DSA_SIG_new())) { if (dsa->internal == NULL) {
goto error; WOLFSSL_MSG("dsa->internal is null");
return NULL;
} }
if (!(sig->r = wolfSSL_BN_bin2bn(sigBin, DSA_HALF_SIZE, NULL))) { sigLen = mp_unsigned_bin_size(&((DsaKey*)dsa->internal)->q);
goto error; if (sigLen <= 0) {
WOLFSSL_MSG("mp_unsigned_bin_size error");
return NULL;
} }
if (!(sig->s = wolfSSL_BN_bin2bn(sigBin + DSA_HALF_SIZE, DSA_HALF_SIZE, NULL))) { /* 2 * sigLen for the two points r and s */
goto error; return wolfSSL_d2i_DSA_SIG(NULL, &tmp, 2 * sigLen);
}
return sig;
error:
if (sig) {
wolfSSL_DSA_SIG_free(sig);
}
return NULL;
} }
#endif /* !HAVE_SELFTEST && !HAVE_FIPS */ #endif /* !HAVE_SELFTEST && !HAVE_FIPS */
@ -31842,8 +31839,10 @@ int wolfSSL_DSA_do_verify_ex(const unsigned char* digest, int digest_len,
WOLFSSL_DSA_SIG* sig, WOLFSSL_DSA* dsa) WOLFSSL_DSA_SIG* sig, WOLFSSL_DSA* dsa)
{ {
int dsacheck, sz; int dsacheck, sz;
byte sigBin[DSA_SIG_SIZE]; byte sigBin[DSA_MAX_SIG_SIZE];
byte* sigBinPtr = sigBin; byte* sigBinPtr = sigBin;
DsaKey* key;
int qSz;
WOLFSSL_ENTER("wolfSSL_DSA_do_verify_ex"); WOLFSSL_ENTER("wolfSSL_DSA_do_verify_ex");
@ -31857,37 +31856,51 @@ int wolfSSL_DSA_do_verify_ex(const unsigned char* digest, int digest_len,
return WOLFSSL_FAILURE; return WOLFSSL_FAILURE;
} }
/* front pad with zeros */ if (dsa->inSet == 0) {
if (!(sz = wolfSSL_BN_num_bytes(sig->r))) { WOLFSSL_MSG("No DSA internal set, do it");
return WOLFSSL_FAILURE; if (SetDsaInternal(dsa) != WOLFSSL_SUCCESS) {
} WOLFSSL_MSG("SetDsaInternal failed");
while (sz++ < DSA_HALF_SIZE) { return WOLFSSL_FAILURE;
*sigBinPtr++ = 0; }
} }
if (wolfSSL_BN_bn2bin(sig->r, sigBinPtr) == WOLFSSL_FATAL_ERROR) { key = dsa->internal;
if (key == NULL) {
WOLFSSL_MSG("dsa->internal is null");
return WOLFSSL_FAILURE; return WOLFSSL_FAILURE;
} }
qSz = mp_unsigned_bin_size(&key->q);
if (qSz < 0 || qSz > DSA_MAX_HALF_SIZE) {
WOLFSSL_MSG("mp_unsigned_bin_size error");
return WOLFSSL_FAILURE;
}
/* read r */
/* front pad with zeros */
if ((sz = wolfSSL_BN_num_bytes(sig->r)) < 0 || sz > DSA_MAX_HALF_SIZE)
return WOLFSSL_FAILURE;
while (sz++ < qSz)
*sigBinPtr++ = 0;
if (wolfSSL_BN_bn2bin(sig->r, sigBinPtr) == WOLFSSL_FATAL_ERROR)
return WOLFSSL_FAILURE;
/* Move to s */ /* Move to s */
sigBinPtr = sigBin + DSA_HALF_SIZE; sigBinPtr = sigBin + qSz;
/* read s */
/* front pad with zeros */ /* front pad with zeros */
if (!(sz = wolfSSL_BN_num_bytes(sig->s))) { if ((sz = wolfSSL_BN_num_bytes(sig->s)) < 0 || sz > DSA_MAX_HALF_SIZE)
return WOLFSSL_FAILURE; return WOLFSSL_FAILURE;
} while (sz++ < qSz)
while (sz++ < DSA_HALF_SIZE) {
*sigBinPtr++ = 0; *sigBinPtr++ = 0;
} if (wolfSSL_BN_bn2bin(sig->s, sigBinPtr) == WOLFSSL_FATAL_ERROR)
if (wolfSSL_BN_bn2bin(sig->s, sigBinPtr) == WOLFSSL_FATAL_ERROR) {
return WOLFSSL_FAILURE; return WOLFSSL_FAILURE;
}
if (wolfSSL_DSA_do_verify(digest, sigBin, dsa, &dsacheck) != WOLFSSL_SUCCESS || if (wolfSSL_DSA_do_verify(digest, sigBin, dsa, &dsacheck) != WOLFSSL_SUCCESS ||
dsacheck != 1) { dsacheck != 1)
return WOLFSSL_FAILURE; return WOLFSSL_FAILURE;
}
return WOLFSSL_SUCCESS; return WOLFSSL_SUCCESS;
} }

View File

@ -7709,7 +7709,7 @@ static int ConfirmSignature(SignatureCtx* sigCtx,
{ {
word32 idx = 0; word32 idx = 0;
if (sigSz < DSA_SIG_SIZE) { if (sigSz < DSA_MIN_SIG_SIZE) {
WOLFSSL_MSG("Verify Signature is too small"); WOLFSSL_MSG("Verify Signature is too small");
ERROR_OUT(BUFFER_E, exit_cs); ERROR_OUT(BUFFER_E, exit_cs);
} }
@ -7729,10 +7729,12 @@ static int ConfirmSignature(SignatureCtx* sigCtx,
WOLFSSL_MSG("ASN Key decode error DSA"); WOLFSSL_MSG("ASN Key decode error DSA");
goto exit_cs; goto exit_cs;
} }
if (sigSz != DSA_SIG_SIZE) { if (sigSz != DSA_160_SIG_SIZE &&
#ifdef HAVE_ECC sigSz != DSA_256_SIG_SIZE) {
/* Try to parse it as the contents of a bitstring */ /* Try to parse it as the contents of a bitstring */
mp_int r, s; mp_int r, s;
int rSz;
int sSz;
idx = 0; idx = 0;
if (DecodeECC_DSA_Sig(sig + idx, sigSz - idx, if (DecodeECC_DSA_Sig(sig + idx, sigSz - idx,
&r, &s) != 0) { &r, &s) != 0) {
@ -7740,25 +7742,25 @@ static int ConfirmSignature(SignatureCtx* sigCtx,
"incorrect format"); "incorrect format");
ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs); ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs);
} }
if (mp_to_unsigned_bin_len(&r, sigCtx->sigCpy, rSz = mp_unsigned_bin_size(&r);
DSA_HALF_SIZE) != MP_OKAY || sSz = mp_unsigned_bin_size(&s);
mp_to_unsigned_bin_len(&s, if (rSz + sSz > (int)sigSz) {
sigCtx->sigCpy + DSA_HALF_SIZE, WOLFSSL_MSG("DSA Sig is in unrecognized or "
DSA_HALF_SIZE) != MP_OKAY) { "incorrect format");
ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs);
}
if (mp_to_unsigned_bin(&r, sigCtx->sigCpy) != MP_OKAY ||
mp_to_unsigned_bin(&s,
sigCtx->sigCpy + rSz) != MP_OKAY) {
WOLFSSL_MSG("DSA Sig is in unrecognized or " WOLFSSL_MSG("DSA Sig is in unrecognized or "
"incorrect format"); "incorrect format");
ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs); ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs);
} }
mp_free(&r); mp_free(&r);
mp_free(&s); mp_free(&s);
#else
WOLFSSL_MSG("DSA Sig is in unrecognized or "
"incorrect format");
ERROR_OUT(ASN_SIG_CONFIRM_E, exit_cs);
#endif
} }
else { else {
XMEMCPY(sigCtx->sigCpy, sig, DSA_SIG_SIZE); XMEMCPY(sigCtx->sigCpy, sig, sigSz);
} }
break; break;
} }

View File

@ -241,6 +241,7 @@ int wc_MakeDsaParameters(WC_RNG *rng, int modulus_size, DsaKey *dsa)
*/ */
switch (modulus_size) { switch (modulus_size) {
#ifdef WOLFSSL_DSA_768_MODULUS #ifdef WOLFSSL_DSA_768_MODULUS
/* This key length is unsecure and only included for bind 9 testing */
case 768: case 768:
#endif #endif
case 1024: case 1024:
@ -685,10 +686,10 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
#ifndef WOLFSSL_MP_INVMOD_CONSTANT_TIME #ifndef WOLFSSL_MP_INVMOD_CONSTANT_TIME
mp_int b[1]; mp_int b[1];
#endif #endif
byte buffer[DSA_HALF_SIZE]; byte buffer[DSA_MAX_HALF_SIZE];
#endif #endif
mp_int* qMinus1; mp_int* qMinus1;
int ret = 0, sz = 0; int ret = 0, halfSz = 0;
byte* tmp; /* initial output pointer */ byte* tmp; /* initial output pointer */
do { do {
@ -707,7 +708,7 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
#ifndef WOLFSSL_MP_INVMOD_CONSTANT_TIME #ifndef WOLFSSL_MP_INVMOD_CONSTANT_TIME
b = (mp_int *)XMALLOC(sizeof *b, key->heap, DYNAMIC_TYPE_TMP_BUFFER); b = (mp_int *)XMALLOC(sizeof *b, key->heap, DYNAMIC_TYPE_TMP_BUFFER);
#endif #endif
buffer = (byte *)XMALLOC(DSA_HALF_SIZE, key->heap, buffer = (byte *)XMALLOC(DSA_MAX_HALF_SIZE, key->heap,
DYNAMIC_TYPE_TMP_BUFFER); DYNAMIC_TYPE_TMP_BUFFER);
if ((k == NULL) || if ((k == NULL) ||
@ -734,7 +735,7 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
break; break;
} }
sz = min(DSA_HALF_SIZE, mp_unsigned_bin_size(&key->q)); halfSz = min(DSA_MAX_HALF_SIZE, mp_unsigned_bin_size(&key->q));
tmp = out; tmp = out;
qMinus1 = kInv; qMinus1 = kInv;
@ -751,12 +752,12 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
do { do {
/* Step 4: generate k */ /* Step 4: generate k */
if ((ret = wc_RNG_GenerateBlock(rng, buffer, sz))) { if ((ret = wc_RNG_GenerateBlock(rng, buffer, halfSz))) {
break; break;
} }
/* Step 5 */ /* Step 5 */
if (mp_read_unsigned_bin(k, buffer, sz) != MP_OKAY) { if (mp_read_unsigned_bin(k, buffer, halfSz) != MP_OKAY) {
ret = MP_READ_E; ret = MP_READ_E;
break; break;
} }
@ -820,10 +821,10 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
* Generate b in range [1, q-1]. * Generate b in range [1, q-1].
*/ */
do { do {
if ((ret = wc_RNG_GenerateBlock(rng, buffer, sz))) { if ((ret = wc_RNG_GenerateBlock(rng, buffer, halfSz))) {
break; break;
} }
if (mp_read_unsigned_bin(b, buffer, sz) != MP_OKAY) { if (mp_read_unsigned_bin(b, buffer, halfSz) != MP_OKAY) {
ret = MP_READ_E; ret = MP_READ_E;
break; break;
} }
@ -916,15 +917,15 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
int rSz = mp_unsigned_bin_size(r); int rSz = mp_unsigned_bin_size(r);
int sSz = mp_unsigned_bin_size(s); int sSz = mp_unsigned_bin_size(s);
while (rSz++ < DSA_HALF_SIZE) { while (rSz++ < halfSz) {
*out++ = 0x00; /* pad front with zeros */ *out++ = 0x00; /* pad front with zeros */
} }
if (mp_to_unsigned_bin(r, out) != MP_OKAY) if (mp_to_unsigned_bin(r, out) != MP_OKAY)
ret = MP_TO_E; ret = MP_TO_E;
else { else {
out = tmp + DSA_HALF_SIZE; /* advance to s in output */ out = tmp + halfSz; /* advance to s in output */
while (sSz++ < DSA_HALF_SIZE) { while (sSz++ < halfSz) {
*out++ = 0x00; /* pad front with zeros */ *out++ = 0x00; /* pad front with zeros */
} }
ret = mp_to_unsigned_bin(s, out); ret = mp_to_unsigned_bin(s, out);
@ -971,7 +972,7 @@ int wc_DsaSign(const byte* digest, byte* out, DsaKey* key, WC_RNG* rng)
} }
#else /* !WOLFSSL_SMALL_STACK */ #else /* !WOLFSSL_SMALL_STACK */
if (ret != MP_INIT_E) { if (ret != MP_INIT_E) {
ForceZero(buffer, sz); ForceZero(buffer, halfSz);
mp_forcezero(kInv); mp_forcezero(kInv);
mp_forcezero(k); mp_forcezero(k);
#ifndef WOLFSSL_MP_INVMOD_CONSTANT_TIME #ifndef WOLFSSL_MP_INVMOD_CONSTANT_TIME
@ -1000,6 +1001,7 @@ int wc_DsaVerify(const byte* digest, const byte* sig, DsaKey* key, int* answer)
mp_int w[1], u1[1], u2[1], v[1], r[1], s[1]; mp_int w[1], u1[1], u2[1], v[1], r[1], s[1];
#endif #endif
int ret = 0; int ret = 0;
int qSz;
do { do {
if (digest == NULL || sig == NULL || key == NULL || answer == NULL) { if (digest == NULL || sig == NULL || key == NULL || answer == NULL) {
@ -1031,9 +1033,15 @@ int wc_DsaVerify(const byte* digest, const byte* sig, DsaKey* key, int* answer)
break; break;
} }
qSz = mp_unsigned_bin_size(&key->q);
if (qSz <= 0) {
ret = BAD_FUNC_ARG;
break;
}
/* set r and s from signature */ /* set r and s from signature */
if (mp_read_unsigned_bin(r, sig, DSA_HALF_SIZE) != MP_OKAY || if (mp_read_unsigned_bin(r, sig, qSz) != MP_OKAY ||
mp_read_unsigned_bin(s, sig + DSA_HALF_SIZE, DSA_HALF_SIZE) != MP_OKAY) { mp_read_unsigned_bin(s, sig + qSz, qSz) != MP_OKAY) {
ret = MP_READ_E; ret = MP_READ_E;
break; break;
} }

View File

@ -1940,7 +1940,11 @@ int wolfSSL_EVP_PKEY_size(WOLFSSL_EVP_PKEY *pkey)
#ifndef NO_DSA #ifndef NO_DSA
case EVP_PKEY_DSA: case EVP_PKEY_DSA:
return DSA_SIG_SIZE; if (pkey->dsa == NULL ||
(!pkey->dsa->exSet &&
SetDsaExternal(pkey->dsa) != WOLFSSL_SUCCESS))
return WOLFSSL_FAILURE;
return wolfSSL_BN_num_bytes(pkey->dsa->p);
#endif #endif
#ifdef HAVE_ECC #ifdef HAVE_ECC
@ -2417,7 +2421,7 @@ int wolfSSL_EVP_SignFinal(WOLFSSL_EVP_MD_CTX *ctx, unsigned char *sigret,
#ifndef NO_DSA #ifndef NO_DSA
case EVP_PKEY_DSA: case EVP_PKEY_DSA:
if (wolfSSL_DSA_do_sign(md, sigret, pkey->dsa) == WOLFSSL_SUCCESS) { if (wolfSSL_DSA_do_sign(md, sigret, pkey->dsa) == WOLFSSL_SUCCESS) {
*siglen = DSA_SIG_SIZE; *siglen = wolfSSL_BN_num_bytes(pkey->dsa->q);
return WOLFSSL_SUCCESS; return WOLFSSL_SUCCESS;
} }
else { else {

View File

@ -53,8 +53,20 @@ enum {
}; };
enum { enum {
DSA_HALF_SIZE = 20, /* r and s size */ /* 160 bit q length */
DSA_SIG_SIZE = 40 /* signature size */ DSA_160_HALF_SIZE = 20, /* r and s size */
DSA_160_SIG_SIZE = 40, /* signature size */
DSA_HALF_SIZE = DSA_160_HALF_SIZE, /* kept for compatiblity */
DSA_SIG_SIZE = DSA_160_SIG_SIZE, /* kept for compatiblity */
/* 256 bit q length */
DSA_256_HALF_SIZE = 32, /* r and s size */
DSA_256_SIG_SIZE = 64, /* signature size */
DSA_MIN_HALF_SIZE = DSA_160_HALF_SIZE,
DSA_MIN_SIG_SIZE = DSA_160_SIG_SIZE,
DSA_MAX_HALF_SIZE = DSA_256_HALF_SIZE,
DSA_MAX_SIG_SIZE = DSA_256_SIG_SIZE,
}; };
/* DSA */ /* DSA */