From ee023c6bf4c689189f1f3857fae131ae46c6ae05 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 25 Jun 2019 09:53:09 +1000 Subject: [PATCH] Simple checks of DH public value from peer. Add test for wc_DhCheckPubValue --- src/internal.c | 16 ++++++++-- wolfcrypt/src/dh.c | 59 +++++++++++++++++++++++++++++------- wolfcrypt/test/test.c | 69 ++++++++++++++++++++++++++++++++++++++++++ wolfssl/wolfcrypt/dh.h | 2 ++ 4 files changed, 133 insertions(+), 13 deletions(-) diff --git a/src/internal.c b/src/internal.c index 668767b66..da398abba 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4196,8 +4196,20 @@ int DhAgree(WOLFSSL* ssl, DhKey* dhKey, else #endif { - ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub, - otherPubSz); +#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST) + ret = wc_DhCheckPubValue(ssl->buffers.serverDH_P.buffer, + ssl->buffers.serverDH_P.length, otherPub, otherPubSz); + if (ret != 0) { + #ifdef OPENSSL_EXTRA + SendAlert(ssl, alert_fatal, illegal_parameter); + #endif + } + else +#endif + { + ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub, + otherPubSz); + } } /* Handle async pending response */ diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index 795086e97..622d2f984 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -1522,6 +1522,46 @@ int wc_DhCheckPubKey(DhKey* key, const byte* pub, word32 pubSz) } +/** + * Quick validity check of public key value agaist prime. + * Checks are: + * - Public key not 0 or 1 + * - Public key not equal to prime or prime - 1 + * - Public key not bigger than prime. + * + * prime Big-endian encoding of prime in bytes. + * primeSz Size of prime in bytes. + * pub Big-endian encoding of public key in bytes. + * pubSz Size of public key in bytes. + */ +int wc_DhCheckPubValue(const byte* prime, word32 primeSz, const byte* pub, + word32 pubSz) +{ + int ret = 0; + word32 i; + + for (i = 0; i < pubSz && pub[i] == 0; i++) { + } + pubSz -= i; + pub += i; + + if (pubSz == 0 || (pubSz == 1 && pub[0] == 1)) + ret = MP_VAL; + else if (pubSz == primeSz) { + for (i = 0; i < pubSz-1 && pub[i] == prime[i]; i++) { + } + if (i == pubSz-1 && (pub[i] == prime[i] || pub[i] == prime[i] - 1)) + ret = MP_VAL; + else if (pub[i] > prime[i]) + ret = MP_VAL; + } + else if (pubSz > primeSz) + ret = MP_VAL; + + return ret; +} + + /* Check DH Private Key for invalid numbers, optionally allowing * the private key to be checked against the large prime (q). * Check per process in SP 800-56Ar3, section 5.6.2.1.2. @@ -1987,6 +2027,7 @@ static int _DhSetKey(DhKey* key, const byte* p, word32 pSz, const byte* g, int ret = 0; mp_int* keyP = NULL; mp_int* keyG = NULL; + mp_int* keyQ = NULL; if (key == NULL || p == NULL || g == NULL || pSz == 0 || gSz == 0) { ret = BAD_FUNC_ARG; @@ -2051,9 +2092,13 @@ static int _DhSetKey(DhKey* key, const byte* p, word32 pSz, const byte* g, if (ret == 0 && q != NULL) { if (mp_read_unsigned_bin(&key->q, q, qSz) != MP_OKAY) ret = MP_INIT_E; + else + keyQ = &key->q; } if (ret != 0 && key != NULL) { + if (keyQ) + mp_clear(keyQ); if (keyG) mp_clear(keyG); if (keyP) @@ -2096,8 +2141,7 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh) int groupSz = 0, bufSz = 0, primeCheckCount = 0, primeCheck = MP_NO, - ret = 0, - tmp_valid = 0; + ret = 0; unsigned char *buf = NULL; if (rng == NULL || dh == NULL) @@ -2147,10 +2191,6 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh) != MP_OKAY) { ret = MP_INIT_E; } - else { - /* tmp and tmp2 are initialized */ - tmp_valid = 1; - } } if (ret == 0) { @@ -2237,11 +2277,8 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh) XFREE(buf, dh->heap, DYNAMIC_TYPE_TMP_BUFFER); } } - - if (tmp_valid) { - mp_clear(&tmp); - mp_clear(&tmp2); - } + mp_clear(&tmp); + mp_clear(&tmp2); return ret; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 572c71beb..7139c505e 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -12830,6 +12830,71 @@ exit_gen_test: return ret; } +#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST) +typedef struct dh_pubvalue_test { + const byte* data; + word32 len; +} dh_pubvalue_test; + +static int dh_test_check_pubvalue(void) +{ + int ret; + word32 i; + const byte prime[] = {0x01, 0x00, 0x01}; + const byte pubValZero[] = { 0x00 }; + const byte pubValZeroLong[] = { 0x00, 0x00, 0x00 }; + const byte pubValOne[] = { 0x01 }; + const byte pubValOneLong[] = { 0x00, 0x00, 0x01 }; + const byte pubValPrimeMinusOne[] = { 0x01, 0x00, 0x00 }; + const byte pubValPrimeLong[] = {0x00, 0x01, 0x00, 0x01}; + const byte pubValPrimePlusOne[] = { 0x01, 0x00, 0x02 }; + const byte pubValTooBig0[] = { 0x02, 0x00, 0x01 }; + const byte pubValTooBig1[] = { 0x01, 0x01, 0x01 }; + const byte pubValTooLong[] = { 0x01, 0x00, 0x00, 0x01 }; + const dh_pubvalue_test dh_pubval_fail[] = { + { prime, sizeof(prime) }, + { pubValZero, sizeof(pubValZero) }, + { pubValZeroLong, sizeof(pubValZeroLong) }, + { pubValOne, sizeof(pubValOne) }, + { pubValOneLong, sizeof(pubValOneLong) }, + { pubValPrimeMinusOne, sizeof(pubValPrimeMinusOne) }, + { pubValPrimeLong, sizeof(pubValPrimeLong) }, + { pubValPrimePlusOne, sizeof(pubValPrimePlusOne) }, + { pubValTooBig0, sizeof(pubValTooBig0) }, + { pubValTooBig1, sizeof(pubValTooBig1) }, + { pubValTooLong, sizeof(pubValTooLong) }, + }; + const byte pubValTwo[] = { 0x02 }; + const byte pubValTwoLong[] = { 0x00, 0x00, 0x02 }; + const byte pubValGood[] = { 0x12, 0x34 }; + const byte pubValGoodLen[] = { 0x00, 0x12, 0x34 }; + const byte pubValGoodLong[] = { 0x00, 0x00, 0x12, 0x34 }; + const dh_pubvalue_test dh_pubval_pass[] = { + { pubValTwo, sizeof(pubValTwo) }, + { pubValTwoLong, sizeof(pubValTwoLong) }, + { pubValGood, sizeof(pubValGood) }, + { pubValGoodLen, sizeof(pubValGoodLen) }, + { pubValGoodLong, sizeof(pubValGoodLong) }, + }; + + for (i = 0; i < sizeof(dh_pubval_fail) / sizeof(*dh_pubval_fail); i++) { + ret = wc_DhCheckPubValue(prime, sizeof(prime), dh_pubval_fail[i].data, + dh_pubval_fail[i].len); + if (ret != MP_VAL) + return -7150 - i; + } + + for (i = 0; i < sizeof(dh_pubval_pass) / sizeof(*dh_pubval_pass); i++) { + ret = wc_DhCheckPubValue(prime, sizeof(prime), dh_pubval_pass[i].data, + dh_pubval_pass[i].len); + if (ret != 0) + return -7160 - i; + } + + return 0; +} +#endif + int dh_test(void) { int ret; @@ -12977,6 +13042,10 @@ int dh_test(void) ret = dh_generate_test(&rng); if (ret == 0) ret = dh_fips_generate_test(&rng); +#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST) + if (ret == 0) + ret = dh_test_check_pubvalue(); +#endif wc_FreeDhKey(&key); diff --git a/wolfssl/wolfcrypt/dh.h b/wolfssl/wolfcrypt/dh.h index 497b8be8e..1f51c8dc0 100644 --- a/wolfssl/wolfcrypt/dh.h +++ b/wolfssl/wolfcrypt/dh.h @@ -106,6 +106,8 @@ WOLFSSL_API int wc_DhParamsLoad(const byte* input, word32 inSz, byte* p, WOLFSSL_API int wc_DhCheckPubKey(DhKey* key, const byte* pub, word32 pubSz); WOLFSSL_API int wc_DhCheckPubKey_ex(DhKey* key, const byte* pub, word32 pubSz, const byte* prime, word32 primeSz); +WOLFSSL_API int wc_DhCheckPubValue(const byte* prime, word32 primeSz, + const byte* pub, word32 pubSz); WOLFSSL_API int wc_DhCheckPrivKey(DhKey* key, const byte* priv, word32 pubSz); WOLFSSL_API int wc_DhCheckPrivKey_ex(DhKey* key, const byte* priv, word32 pubSz, const byte* prime, word32 primeSz);