From cd05ed3347c265940d2c4b6c7fa6804fb4f38b15 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 19 Oct 2020 08:08:04 -0700 Subject: [PATCH] iDH Fix 1. Changed the bounds of checking the key from comparisons to constants to comparisons against WOLFSSL object settings for the DH key bounds. 2. Removed redundant bounds check on the server's prime. --- src/internal.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8f8a03bde..ece674972 100644 --- a/src/internal.c +++ b/src/internal.c @@ -21178,9 +21178,6 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size, } ato16(input + args->idx, &length); - if (length < MIN_DHKEY_SZ || length > MAX_DHKEY_SZ) { - ERROR_OUT(DH_KEY_SIZE_E, exit_gdpk); - } args->idx += OPAQUE16_LEN; if ((args->idx - args->begin) + length > size) { @@ -21222,12 +21219,6 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size, } ato16(input + args->idx, &length); - if (length > MAX_DHKEY_SZ) { - XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap, - DYNAMIC_TYPE_PUBLIC_KEY); - ssl->buffers.serverDH_P.buffer = NULL; - ERROR_OUT(DH_KEY_SIZE_E, exit_gdpk); - } args->idx += OPAQUE16_LEN; if ((args->idx - args->begin) + length > size) { @@ -21237,6 +21228,12 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size, ERROR_OUT(BUFFER_ERROR, exit_gdpk); } + if (length > ssl->options.maxDhKeySz) { + WOLFSSL_MSG("Server using a DH key generator that is too big"); + SendAlert(ssl, alert_fatal, handshake_failure); + ERROR_OUT(DH_KEY_SIZE_E, exit_gdpk); + } + ssl->buffers.serverDH_G.buffer = (byte*)XMALLOC(length, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); if (ssl->buffers.serverDH_G.buffer) { @@ -21265,15 +21262,6 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size, } ato16(input + args->idx, &length); - if (length < MIN_DHKEY_SZ || length > MAX_DHKEY_SZ) { - XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap, - DYNAMIC_TYPE_PUBLIC_KEY); - ssl->buffers.serverDH_P.buffer = NULL; - XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap, - DYNAMIC_TYPE_PUBLIC_KEY); - ssl->buffers.serverDH_G.buffer = NULL; - ERROR_OUT(DH_KEY_SIZE_E, exit_gdpk); - } args->idx += OPAQUE16_LEN; if ((args->idx - args->begin) + length > size) { @@ -21286,6 +21274,17 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size, ERROR_OUT(BUFFER_ERROR, exit_gdpk); } + if (length < ssl->options.minDhKeySz) { + WOLFSSL_MSG("Server using a public DH key that is too small"); + SendAlert(ssl, alert_fatal, handshake_failure); + ERROR_OUT(DH_KEY_SIZE_E, exit_gdpk); + } + if (length > ssl->options.maxDhKeySz) { + WOLFSSL_MSG("Server using a public DH key that is too big"); + SendAlert(ssl, alert_fatal, handshake_failure); + ERROR_OUT(DH_KEY_SIZE_E, exit_gdpk); + } + ssl->buffers.serverDH_Pub.buffer = (byte*)XMALLOC(length, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); if (ssl->buffers.serverDH_Pub.buffer) {