Skip to content

Commit 0de8587

Browse files
ejohnstownejohnstown
committed
Validate server's group
The client wasn't validating the DH group parameters in the KEX DH GEX Group message. This adds a function to perform the validation of the prime `p` to verify it is safe. (Prime and that ((p - 1) / 2) is prime.) Also adds a test to generate an unsafe prime and verify the function rejects it appropriately. Affected function: DoKexDhGexGroup. Issue: F-1688
1 parent 543a6c2 commit 0de8587

File tree

3 files changed

+290
-1
lines changed

3 files changed

+290
-1
lines changed

‎src/internal.c‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6284,6 +6284,126 @@ static int DoKexDhGexRequest(WOLFSSH* ssh,
62846284
}
62856285

62866286

6287+
/*
6288+
* Validate a DH GEX group received from the server per RFC 4419 sec. 3:
6289+
* - p's bit length falls within the client's requested [minBits, maxBits]
6290+
* - p is odd and probably prime
6291+
* - (p-1)/2 is also probably prime, i.e. p is a safe prime, which bounds
6292+
* the order of g to q or 2q and closes the small-subgroup attack
6293+
* - 2 <= g <= p - 2
6294+
* Returns WS_SUCCESS if the group is acceptable.
6295+
*/
6296+
static int ValidateKexDhGexGroup(const byte* primeGroup, word32 primeGroupSz,
6297+
const byte* generator, word32 generatorSz,
6298+
word32 minBits, word32 maxBits)
6299+
{
6300+
mp_int p;
6301+
mp_int g;
6302+
mp_int q;
6303+
int pInit = 0, gInit = 0, qInit = 0;
6304+
int bits;
6305+
int isPrime = MP_NO;
6306+
int ret = WS_SUCCESS;
6307+
6308+
if (primeGroup == NULL || primeGroupSz == 0
6309+
|| generator == NULL || generatorSz == 0)
6310+
ret = WS_BAD_ARGUMENT;
6311+
6312+
if (ret == WS_SUCCESS) {
6313+
if (mp_init(&p) != MP_OKAY)
6314+
ret = WS_CRYPTO_FAILED;
6315+
else
6316+
pInit = 1;
6317+
}
6318+
if (ret == WS_SUCCESS) {
6319+
if (mp_init(&g) != MP_OKAY)
6320+
ret = WS_CRYPTO_FAILED;
6321+
else
6322+
gInit = 1;
6323+
}
6324+
if (ret == WS_SUCCESS) {
6325+
if (mp_init(&q) != MP_OKAY)
6326+
ret = WS_CRYPTO_FAILED;
6327+
else
6328+
qInit = 1;
6329+
}
6330+
6331+
if (ret == WS_SUCCESS) {
6332+
if (mp_read_unsigned_bin(&p, primeGroup, primeGroupSz) != MP_OKAY)
6333+
ret = WS_CRYPTO_FAILED;
6334+
}
6335+
if (ret == WS_SUCCESS) {
6336+
if (mp_read_unsigned_bin(&g, generator, generatorSz) != MP_OKAY)
6337+
ret = WS_CRYPTO_FAILED;
6338+
}
6339+
6340+
if (ret == WS_SUCCESS) {
6341+
bits = mp_count_bits(&p);
6342+
if (bits < (int)minBits || bits > (int)maxBits) {
6343+
WLOG(WS_LOG_DEBUG,
6344+
"DH GEX: prime size %d outside requested [%u, %u]",
6345+
bits, minBits, maxBits);
6346+
ret = WS_DH_SIZE_E;
6347+
}
6348+
}
6349+
6350+
if (ret == WS_SUCCESS) {
6351+
if (!mp_isodd(&p)) {
6352+
WLOG(WS_LOG_DEBUG, "DH GEX: prime is even");
6353+
ret = WS_CRYPTO_FAILED;
6354+
}
6355+
}
6356+
6357+
/* 2 <= g: reject g == 0 and g == 1. */
6358+
if (ret == WS_SUCCESS) {
6359+
if (mp_cmp_d(&g, 1) != MP_GT) {
6360+
WLOG(WS_LOG_DEBUG, "DH GEX: generator < 2");
6361+
ret = WS_CRYPTO_FAILED;
6362+
}
6363+
}
6364+
6365+
/* g <= p - 2: reject g == p - 1 (order 2) and anything larger. */
6366+
if (ret == WS_SUCCESS) {
6367+
if (mp_sub_d(&p, 1, &q) != MP_OKAY)
6368+
ret = WS_CRYPTO_FAILED;
6369+
else if (mp_cmp(&g, &q) != MP_LT) {
6370+
WLOG(WS_LOG_DEBUG, "DH GEX: generator >= p - 1");
6371+
ret = WS_CRYPTO_FAILED;
6372+
}
6373+
}
6374+
6375+
/* Miller-Rabin: p must be prime. */
6376+
if (ret == WS_SUCCESS) {
6377+
isPrime = MP_NO;
6378+
if (mp_prime_is_prime(&p, 8, &isPrime) != MP_OKAY
6379+
|| isPrime == MP_NO) {
6380+
WLOG(WS_LOG_DEBUG, "DH GEX: p is not prime");
6381+
ret = WS_CRYPTO_FAILED;
6382+
}
6383+
}
6384+
6385+
/* Safe prime check: q = (p - 1) / 2 must also be prime. */
6386+
if (ret == WS_SUCCESS) {
6387+
if (mp_sub_d(&p, 1, &q) != MP_OKAY || mp_div_2(&q, &q) != MP_OKAY)
6388+
ret = WS_CRYPTO_FAILED;
6389+
}
6390+
if (ret == WS_SUCCESS) {
6391+
isPrime = MP_NO;
6392+
if (mp_prime_is_prime(&q, 8, &isPrime) != MP_OKAY
6393+
|| isPrime == MP_NO) {
6394+
WLOG(WS_LOG_DEBUG, "DH GEX: (p-1)/2 is not prime, p is not safe");
6395+
ret = WS_CRYPTO_FAILED;
6396+
}
6397+
}
6398+
6399+
if (qInit) mp_clear(&q);
6400+
if (gInit) mp_clear(&g);
6401+
if (pInit) mp_clear(&p);
6402+
6403+
return ret;
6404+
}
6405+
6406+
62876407
static int DoKexDhGexGroup(WOLFSSH* ssh,
62886408
byte* buf, word32 len, word32* idx)
62896409
{
@@ -6307,6 +6427,13 @@ static int DoKexDhGexGroup(WOLFSSH* ssh,
63076427
if (ret == WS_SUCCESS)
63086428
ret = GetMpint(&generatorSz, &generator, buf, len, &begin);
63096429

6430+
if (ret == WS_SUCCESS) {
6431+
ret = ValidateKexDhGexGroup(primeGroup, primeGroupSz,
6432+
generator, generatorSz,
6433+
ssh->handshake->dhGexMinSz,
6434+
ssh->handshake->dhGexMaxSz);
6435+
}
6436+
63106437
if (ret == WS_SUCCESS) {
63116438
if (ssh->handshake->primeGroup)
63126439
WFREE(ssh->handshake->primeGroup, ssh->ctx->heap, DYNTYPE_MPINT);
@@ -6340,7 +6467,17 @@ static int DoKexDhGexGroup(WOLFSSH* ssh,
63406467

63416468
return ret;
63426469
}
6470+
6471+
#ifdef WOLFSSH_TEST_INTERNAL
6472+
int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup,
6473+
word32 primeGroupSz, const byte* generator, word32 generatorSz,
6474+
word32 minBits, word32 maxBits)
6475+
{
6476+
return ValidateKexDhGexGroup(primeGroup, primeGroupSz,
6477+
generator, generatorSz, minBits, maxBits);
6478+
}
63436479
#endif
6480+
#endif /* !WOLFSSH_NO_DH_GEX_SHA256 */
63446481

63456482

63466483
static int DoIgnore(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)

‎tests/unit.c‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@
3131
#include <stdio.h>
3232
#include <wolfssh/ssh.h>
3333
#include <wolfssh/keygen.h>
34+
#include <wolfssh/error.h>
3435
#include <wolfssh/internal.h>
36+
#include <wolfssl/wolfcrypt/random.h>
37+
#include <wolfssl/wolfcrypt/integer.h>
3538
#include <wolfssl/wolfcrypt/hmac.h>
3639

3740
#define WOLFSSH_TEST_HEX2BIN
@@ -439,6 +442,143 @@ static int test_DoReceive_VerifyMacFailure(void)
439442
#endif /* WOLFSSH_TEST_INTERNAL && any HMAC SHA variant enabled */
440443

441444

445+
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256)
446+
447+
/*
448+
* DH GEX Group Validation Unit Test
449+
*
450+
* Generate a prime p of bitSz bits where p is prime but (p - 1) / 2
451+
* is even, so ValidateKexDhGexGroup must reject it at the safe-prime
452+
* check. Any prime p congruent to 1 (mod 4) satisfies this: p - 1 is
453+
* then divisible by 4, so (p - 1) / 2 is even, and for p > 5 it is
454+
* composite.
455+
*
456+
* Strategy: draw a random candidate with the top bit forced (to fix the
457+
* bit length) and the low two bits forced to 01 (odd, congruent to 1
458+
* mod 4), then Miller-Rabin test and step by 4 on failure to preserve
459+
* the mod-4 invariant.
460+
*/
461+
static int GenerateNonSafePrime(mp_int* p, int bitSz, WC_RNG* rng)
462+
{
463+
byte buf[512];
464+
int bytes = (bitSz + 7) / 8;
465+
int extraBits = bytes * 8 - bitSz;
466+
int isPrime = MP_NO;
467+
int ret;
468+
469+
if (p == NULL || rng == NULL || bitSz < 8
470+
|| bytes > (int)sizeof(buf))
471+
return WS_BAD_ARGUMENT;
472+
473+
ret = wc_RNG_GenerateBlock(rng, buf, (word32)bytes);
474+
if (ret != 0)
475+
return ret;
476+
477+
buf[0] &= (byte)(0xFF >> extraBits);
478+
buf[0] |= (byte)(0x80 >> extraBits);
479+
buf[bytes - 1] = (byte)((buf[bytes - 1] & 0xFC) | 0x01);
480+
481+
ret = mp_read_unsigned_bin(p, buf, (word32)bytes);
482+
if (ret != MP_OKAY)
483+
return ret;
484+
485+
for (;;) {
486+
ret = mp_prime_is_prime(p, 8, &isPrime);
487+
if (ret != MP_OKAY)
488+
return ret;
489+
if (isPrime == MP_YES)
490+
break;
491+
ret = mp_add_d(p, 4, p);
492+
if (ret != MP_OKAY)
493+
return ret;
494+
}
495+
496+
return MP_OKAY;
497+
}
498+
499+
500+
static int test_DhGexGroupValidate(void)
501+
{
502+
WC_RNG rng;
503+
mp_int p;
504+
mp_int q;
505+
byte pBuf[256];
506+
byte gBuf[1] = { 2 };
507+
word32 pSz;
508+
int isPrime = MP_NO;
509+
int result = 0;
510+
int ret;
511+
512+
if (wc_InitRng(&rng) != 0) {
513+
printf("DhGexGroupValidate: wc_InitRng failed\n");
514+
return -110;
515+
}
516+
if (mp_init(&p) != MP_OKAY) {
517+
wc_FreeRng(&rng);
518+
return -111;
519+
}
520+
if (mp_init(&q) != MP_OKAY) {
521+
mp_clear(&p);
522+
wc_FreeRng(&rng);
523+
return -112;
524+
}
525+
526+
/* 1024 bits keeps the Miller-Rabin loop quick while still landing
527+
* inside the default [dhGexMinSz, dhGexMaxSz] window. */
528+
if (GenerateNonSafePrime(&p, 1024, &rng) != MP_OKAY) {
529+
printf("DhGexGroupValidate: failed to generate candidate prime\n");
530+
result = -113;
531+
goto done;
532+
}
533+
534+
/* Sanity check: p is prime. */
535+
if (mp_prime_is_prime(&p, 20, &isPrime) != MP_OKAY || isPrime != MP_YES) {
536+
printf("DhGexGroupValidate: candidate p is not prime\n");
537+
result = -114;
538+
goto done;
539+
}
540+
541+
/* Sanity check: (p - 1) / 2 is even (i.e. p is not a safe prime). */
542+
if (mp_sub_d(&p, 1, &q) != MP_OKAY || mp_div_2(&q, &q) != MP_OKAY) {
543+
result = -115;
544+
goto done;
545+
}
546+
if (mp_isodd(&q)) {
547+
printf("DhGexGroupValidate: (p-1)/2 was odd, retry needed\n");
548+
result = -116;
549+
goto done;
550+
}
551+
552+
pSz = (word32)mp_unsigned_bin_size(&p);
553+
if (pSz > sizeof(pBuf)) {
554+
result = -117;
555+
goto done;
556+
}
557+
if (mp_to_unsigned_bin(&p, pBuf) != MP_OKAY) {
558+
result = -118;
559+
goto done;
560+
}
561+
562+
/* The validator must reject the group at the safe-prime check. */
563+
ret = wolfSSH_TestValidateKexDhGexGroup(pBuf, pSz, gBuf,
564+
(word32)sizeof(gBuf), 1024, 8192);
565+
if (ret != WS_CRYPTO_FAILED) {
566+
printf("DhGexGroupValidate: validator returned %d, expected %d\n",
567+
ret, WS_CRYPTO_FAILED);
568+
result = -119;
569+
goto done;
570+
}
571+
572+
done:
573+
mp_clear(&q);
574+
mp_clear(&p);
575+
wc_FreeRng(&rng);
576+
return result;
577+
}
578+
579+
#endif /* WOLFSSH_TEST_INTERNAL && !WOLFSSH_NO_DH_GEX_SHA256 */
580+
581+
442582
/* Error Code And Message Test */
443583

444584
static int test_Errors(void)
@@ -520,6 +660,13 @@ int wolfSSH_UnitTest(int argc, char** argv)
520660
testResult = testResult || unitResult;
521661
#endif
522662

663+
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256)
664+
unitResult = test_DhGexGroupValidate();
665+
printf("DhGexGroupValidate: %s\n",
666+
(unitResult == 0 ? "SUCCESS" : "FAILED"));
667+
testResult = testResult || unitResult;
668+
#endif
669+
523670
#ifdef WOLFSSH_KEYGEN
524671
#ifndef WOLFSSH_NO_RSA
525672
unitResult = test_RsaKeyGen();

‎wolfssh/internal.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1324,7 +1324,12 @@ enum WS_MessageIdLimits {
13241324
WOLFSSH_API int wolfSSH_TestIsMessageAllowed(WOLFSSH* ssh, byte msg,
13251325
byte state);
13261326
WOLFSSH_API int wolfSSH_TestDoReceive(WOLFSSH* ssh);
1327-
#endif
1327+
#ifndef WOLFSSH_NO_DH_GEX_SHA256
1328+
WOLFSSH_API int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup,
1329+
word32 primeGroupSz, const byte* generator, word32 generatorSz,
1330+
word32 minBits, word32 maxBits);
1331+
#endif /* WOFLSSH_NO_DH_GEX_SHA256 */
1332+
#endif /* WOLFSSH_TEST_INTERNAL */
13281333

13291334
/* dynamic memory types */
13301335
enum WS_DynamicTypes {

0 commit comments

Comments
 (0)