Skip to content

Commit bc06dce

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 a known unsafe prime and known safe prime to verify the validate function. Affected function: DoKexDhGexGroup. Issue: F-1688
1 parent 543a6c2 commit bc06dce

File tree

3 files changed

+355
-3
lines changed

3 files changed

+355
-3
lines changed

‎src/internal.c‎

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

62866286

6287+
/*
6288+
* Validate a DH GEX group received from the server against RFC 4419 sec. 3
6289+
* requirements, with an additional hardening/policy check:
6290+
* - p's bit length falls within the client's requested [minBits, maxBits]
6291+
* - p is odd and probably prime
6292+
* - (additional hardening requirement) (p-1)/2 is also probably prime,
6293+
* i.e. p is a safe prime, which bounds the order of g to q or 2q and
6294+
* closes the small-subgroup attack
6295+
* Returns WS_SUCCESS if the group is acceptable.
6296+
*/
6297+
static int ValidateKexDhGexGroup(const byte* primeGroup, word32 primeGroupSz,
6298+
const byte* generator, word32 generatorSz,
6299+
word32 minBits, word32 maxBits, WC_RNG* rng)
6300+
{
6301+
mp_int p, g, q;
6302+
int pgqInit = 0;
6303+
int bits;
6304+
int isPrime = MP_NO;
6305+
int ret = WS_SUCCESS;
6306+
6307+
if (primeGroup == NULL || primeGroupSz == 0
6308+
|| generator == NULL || generatorSz == 0
6309+
|| rng == NULL)
6310+
ret = WS_BAD_ARGUMENT;
6311+
6312+
/*
6313+
* Check the bounds on the size of the flat prime group and generator
6314+
* values. The prime group shall be LE maxBits. The generator size
6315+
* shall be LE prime group size. This is a check on the sizes of values
6316+
* sent by the peer before reading them in and checking them as mp_ints.
6317+
*/
6318+
if (ret == WS_SUCCESS) {
6319+
word32 maxBytes = (maxBits / 8) + ((maxBits % 8) ? 1 : 0);
6320+
/* Adjust the sizes for signed padding. */
6321+
word32 adjPrimeGroupSz = primeGroupSz - ((primeGroup[0] == 0) ? 1 : 0);
6322+
word32 adjGeneratorSz = generatorSz - ((generator[0] == 0) ? 1 : 0);
6323+
6324+
if (adjPrimeGroupSz > maxBytes || adjGeneratorSz > adjPrimeGroupSz) {
6325+
ret = WS_DH_SIZE_E;
6326+
}
6327+
}
6328+
6329+
if (ret == WS_SUCCESS) {
6330+
if (mp_init_multi(&p, &g, &q, NULL, NULL, NULL) != MP_OKAY) {
6331+
ret = WS_CRYPTO_FAILED;
6332+
pgqInit = 1;
6333+
}
6334+
}
6335+
6336+
if (ret == WS_SUCCESS) {
6337+
if (mp_read_unsigned_bin(&p, primeGroup, primeGroupSz) != MP_OKAY)
6338+
ret = WS_CRYPTO_FAILED;
6339+
}
6340+
if (ret == WS_SUCCESS) {
6341+
if (mp_read_unsigned_bin(&g, generator, generatorSz) != MP_OKAY)
6342+
ret = WS_CRYPTO_FAILED;
6343+
}
6344+
6345+
if (ret == WS_SUCCESS) {
6346+
bits = mp_count_bits(&p);
6347+
if (bits < (int)minBits || bits > (int)maxBits) {
6348+
WLOG(WS_LOG_DEBUG,
6349+
"DH GEX: prime size %d outside requested [%u, %u]",
6350+
bits, minBits, maxBits);
6351+
ret = WS_DH_SIZE_E;
6352+
}
6353+
}
6354+
6355+
if (ret == WS_SUCCESS) {
6356+
if (!mp_isodd(&p)) {
6357+
WLOG(WS_LOG_DEBUG, "DH GEX: prime is even");
6358+
ret = WS_CRYPTO_FAILED;
6359+
}
6360+
}
6361+
6362+
/* 2 <= g: reject g == 0 and g == 1. */
6363+
if (ret == WS_SUCCESS) {
6364+
if (mp_cmp_d(&g, 1) != MP_GT) {
6365+
WLOG(WS_LOG_DEBUG, "DH GEX: generator < 2");
6366+
ret = WS_CRYPTO_FAILED;
6367+
}
6368+
}
6369+
6370+
/* g <= p - 2: reject g == p - 1 (order 2) and anything larger. */
6371+
if (ret == WS_SUCCESS) {
6372+
if (mp_sub_d(&p, 1, &q) != MP_OKAY)
6373+
ret = WS_CRYPTO_FAILED;
6374+
else if (mp_cmp(&g, &q) != MP_LT) {
6375+
WLOG(WS_LOG_DEBUG, "DH GEX: generator >= p - 1");
6376+
ret = WS_CRYPTO_FAILED;
6377+
}
6378+
}
6379+
6380+
/* Miller-Rabin: p must be prime. */
6381+
if (ret == WS_SUCCESS) {
6382+
isPrime = MP_NO;
6383+
ret = mp_prime_is_prime_ex(&p, WOLFSSH_MR_ROUNDS, &isPrime, rng);
6384+
if (ret != MP_OKAY || isPrime == MP_NO) {
6385+
WLOG(WS_LOG_DEBUG, "DH GEX: p is not prime");
6386+
ret = WS_CRYPTO_FAILED;
6387+
}
6388+
else {
6389+
ret = WS_SUCCESS;
6390+
}
6391+
}
6392+
6393+
/* Safe prime check: q = (p - 1) / 2 must also be prime. */
6394+
if (ret == WS_SUCCESS) {
6395+
if (mp_sub_d(&p, 1, &q) != MP_OKAY || mp_div_2(&q, &q) != MP_OKAY)
6396+
ret = WS_CRYPTO_FAILED;
6397+
}
6398+
if (ret == WS_SUCCESS) {
6399+
isPrime = MP_NO;
6400+
ret = mp_prime_is_prime_ex(&q, WOLFSSH_MR_ROUNDS, &isPrime, rng);
6401+
if (ret != MP_OKAY || isPrime == MP_NO) {
6402+
WLOG(WS_LOG_DEBUG, "DH GEX: (p-1)/2 is not prime, p is not safe");
6403+
ret = WS_CRYPTO_FAILED;
6404+
}
6405+
else {
6406+
ret = WS_SUCCESS;
6407+
}
6408+
}
6409+
6410+
if (pgqInit) {
6411+
mp_clear(&q);
6412+
mp_clear(&g);
6413+
mp_clear(&p);
6414+
}
6415+
6416+
return ret;
6417+
}
6418+
6419+
62876420
static int DoKexDhGexGroup(WOLFSSH* ssh,
62886421
byte* buf, word32 len, word32* idx)
62896422
{
@@ -6300,13 +6433,19 @@ static int DoKexDhGexGroup(WOLFSSH* ssh,
63006433
if (ret == WS_SUCCESS) {
63016434
begin = *idx;
63026435
ret = GetMpint(&primeGroupSz, &primeGroup, buf, len, &begin);
6303-
if (ret == WS_SUCCESS && primeGroupSz > (MAX_KEX_KEY_SZ + 1))
6304-
ret = WS_DH_SIZE_E;
63056436
}
63066437

63076438
if (ret == WS_SUCCESS)
63086439
ret = GetMpint(&generatorSz, &generator, buf, len, &begin);
63096440

6441+
if (ret == WS_SUCCESS) {
6442+
ret = ValidateKexDhGexGroup(primeGroup, primeGroupSz,
6443+
generator, generatorSz,
6444+
ssh->handshake->dhGexMinSz,
6445+
ssh->handshake->dhGexMaxSz,
6446+
ssh->rng);
6447+
}
6448+
63106449
if (ret == WS_SUCCESS) {
63116450
if (ssh->handshake->primeGroup)
63126451
WFREE(ssh->handshake->primeGroup, ssh->ctx->heap, DYNTYPE_MPINT);
@@ -6340,7 +6479,17 @@ static int DoKexDhGexGroup(WOLFSSH* ssh,
63406479

63416480
return ret;
63426481
}
6482+
6483+
#ifdef WOLFSSH_TEST_INTERNAL
6484+
int wolfSSH_TestValidateKexDhGexGroup(const byte* primeGroup,
6485+
word32 primeGroupSz, const byte* generator, word32 generatorSz,
6486+
word32 minBits, word32 maxBits, WC_RNG* rng)
6487+
{
6488+
return ValidateKexDhGexGroup(primeGroup, primeGroupSz,
6489+
generator, generatorSz, minBits, maxBits, rng);
6490+
}
63436491
#endif
6492+
#endif /* !WOLFSSH_NO_DH_GEX_SHA256 */
63446493

63456494

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

‎tests/unit.c‎

Lines changed: 195 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,191 @@ 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+
typedef struct {
448+
const char* candidate;
449+
const char* generator;
450+
word32 minBits;
451+
word32 maxBits;
452+
int expectedResult;
453+
} PrimeTestVector;
454+
455+
static const PrimeTestVector primeTestVectors[] = {
456+
{
457+
/*
458+
* For testing the ValidateKexDhGexGroup() function, we need to
459+
* verify that the function detects unsafe primes. The following
460+
* unsafe prime is the prime used with GOST-ECC. (RFC 7836) It is
461+
* prime and fine for its application. It isn't safe for DH, as
462+
* q = (p-1)/2 is not prime.
463+
*/
464+
"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
465+
"fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffdc7",
466+
"02",
467+
512, 8192, WS_CRYPTO_FAILED
468+
},
469+
{
470+
/*
471+
* We need to verify that the function detects safe primes. The
472+
* following safePrime is the MODP 2048-bit group from RFC 3526.
473+
*/
474+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
475+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
476+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
477+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
478+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
479+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
480+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
481+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68ffffffffffffffff",
482+
"02",
483+
2048, 8192, WS_SUCCESS
484+
},
485+
{
486+
/*
487+
* This checks for g = p - 1.
488+
*/
489+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
490+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
491+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
492+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
493+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
494+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
495+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
496+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68ffffffffffffffff",
497+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
498+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
499+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
500+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
501+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
502+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
503+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
504+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68fffffffffffffffe",
505+
2048, 8192, WS_CRYPTO_FAILED
506+
},
507+
{
508+
/*
509+
* This checks for g = 1.
510+
*/
511+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
512+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
513+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
514+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
515+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
516+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
517+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
518+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68ffffffffffffffff",
519+
"01",
520+
2048, 8192, WS_CRYPTO_FAILED
521+
},
522+
{
523+
/*
524+
* This checks prime size less than minBits.
525+
*/
526+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
527+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
528+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
529+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
530+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
531+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
532+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
533+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68ffffffffffffffff",
534+
"02",
535+
3072, 8192, WS_DH_SIZE_E
536+
},
537+
{
538+
/*
539+
* This checks prime size greater than maxBits.
540+
*/
541+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
542+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
543+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
544+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
545+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
546+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
547+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
548+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68ffffffffffffffff",
549+
"02",
550+
512, 1024, WS_DH_SIZE_E
551+
},
552+
{
553+
/*
554+
* This checks for even p.
555+
*/
556+
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
557+
"020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
558+
"4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
559+
"ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
560+
"98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
561+
"9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
562+
"e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
563+
"3995497cea956ae515d2261898fa051015728e5a8aacaa68fffffffffffffffe",
564+
"02",
565+
2048, 8192, WS_CRYPTO_FAILED
566+
},
567+
{
568+
/*
569+
* A well known composite number that breaks some MR implementations.
570+
* This is calculated by wolfCrypt for one of its prime tests.
571+
*/
572+
"000000000088cbf655be37a612fa535b4a9b81d394854ecbedfe1a4afbecdc7b"
573+
"a6a263549dd3c17882b054329384962576e7c5aa281e04ab5a0e7245584ad324"
574+
"9c7ac4de7caf5663bae95f6bb9e8bec4124e04d82eac54a246bda49a5c5c2a1b"
575+
"366ef8c085fc7c5f87478a55832d1b2184154c24260df67561d17c4359724403",
576+
"02",
577+
512, 8192, WS_CRYPTO_FAILED
578+
},
579+
};
580+
581+
static int test_DhGexGroupValidate(void)
582+
{
583+
WC_RNG rng;
584+
const PrimeTestVector* tv;
585+
byte* candidate;
586+
byte* generator;
587+
word32 candidateSz;
588+
word32 generatorSz;
589+
int tc = (int)(sizeof(primeTestVectors)/sizeof(primeTestVectors[0]));
590+
int result = 0, ret, i;
591+
592+
if (wc_InitRng(&rng) != 0) {
593+
printf("DhGexGroupValidate: wc_InitRng failed\n");
594+
return -110;
595+
}
596+
597+
for (i = 0, tv = primeTestVectors; i < tc; i++, tv++) {
598+
candidate = NULL;
599+
candidateSz = 0;
600+
generator = NULL;
601+
generatorSz = 0;
602+
603+
ret = ConvertHexToBin(tv->candidate, &candidate, &candidateSz,
604+
tv->generator, &generator, &generatorSz,
605+
NULL, NULL, NULL, NULL, NULL, NULL);
606+
if (ret != 0) {
607+
result = -113;
608+
break;
609+
}
610+
611+
ret = wolfSSH_TestValidateKexDhGexGroup(candidate, candidateSz,
612+
generator, generatorSz, tv->minBits, tv->maxBits, &rng);
613+
if (ret != tv->expectedResult) {
614+
printf("DhGexGroupValidate: %d validator returned %d, expected %d\n",
615+
i, ret, tv->expectedResult);
616+
result = -121;
617+
break;
618+
}
619+
620+
FreeBins(candidate, generator, NULL, NULL);
621+
}
622+
623+
wc_FreeRng(&rng);
624+
return result;
625+
}
626+
627+
#endif /* WOLFSSH_TEST_INTERNAL && !WOLFSSH_NO_DH_GEX_SHA256 */
628+
629+
442630
/* Error Code And Message Test */
443631

444632
static int test_Errors(void)
@@ -520,6 +708,13 @@ int wolfSSH_UnitTest(int argc, char** argv)
520708
testResult = testResult || unitResult;
521709
#endif
522710

711+
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_DH_GEX_SHA256)
712+
unitResult = test_DhGexGroupValidate();
713+
printf("DhGexGroupValidate: %s\n",
714+
(unitResult == 0 ? "SUCCESS" : "FAILED"));
715+
testResult = testResult || unitResult;
716+
#endif
717+
523718
#ifdef WOLFSSH_KEYGEN
524719
#ifndef WOLFSSH_NO_RSA
525720
unitResult = test_RsaKeyGen();

0 commit comments

Comments
 (0)