From 1e84d24c2055c164d984641b098e80ceea2ab88f Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 17 Oct 2023 16:23:54 +1000 Subject: [PATCH] SM2 named curve disabled: value outside of supported values SM2 named curve value is specified in specification. Values 0-14 aren't used, so, those bits in disabledCurves are used for values over 31. Add range checks. --- src/ssl.c | 33 ++++++++++++++++++++++++--------- src/tls.c | 6 ++---- wolfssl/ssl.h | 3 +++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index bd7f167ea..b9fbe40cf 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -32491,9 +32491,20 @@ void wolfSSL_get0_next_proto_negotiated(const WOLFSSL *s, const unsigned char ** #if defined(OPENSSL_EXTRA) || defined(HAVE_CURL) int wolfSSL_curve_is_disabled(const WOLFSSL* ssl, word16 curve_id) { - return (curve_id <= WOLFSSL_ECC_MAX && - ssl->disabledCurves && - ssl->disabledCurves & (1 << curve_id)); + if (curve_id >= WOLFSSL_FFDHE_START) { + /* DH parameters are never disabled. */ + return 0; + } + if (curve_id > WOLFSSL_ECC_MAX_AVAIL) { + WOLFSSL_MSG("Curve id out of supported range"); + /* Disabled if not in valid range. */ + return 1; + } + if (curve_id >= 32) { + /* 0 is for invalid and 1-14 aren't used otherwise. */ + return (ssl->disabledCurves & (1 << (curve_id - 32))) != 0; + } + return (ssl->disabledCurves & (1 << curve_id)) != 0; } #if (defined(HAVE_ECC) || \ @@ -32553,7 +32564,7 @@ static int set_curves_list(WOLFSSL* ssl, WOLFSSL_CTX *ctx, const char* names) else if ((XSTRNCMP(name, "sm2p256v1", len) == 0) || (XSTRNCMP(name, "SM2", len) == 0)) { - curve = WOLFSSL_ECC_SECP521R1; + curve = WOLFSSL_ECC_SM2P256V1; } #endif #ifdef HAVE_CURVE25519 @@ -32592,10 +32603,8 @@ static int set_curves_list(WOLFSSL* ssl, WOLFSSL_CTX *ctx, const char* names) #endif } - if (curve >= (sizeof(word32) * WOLFSSL_BIT_SIZE)) { - /* shift left more than size of ctx->disabledCurves causes static - * analysis report */ - WOLFSSL_MSG("curve value is too large for upcoming shift"); + if (curve >= WOLFSSL_ECC_MAX_AVAIL) { + WOLFSSL_MSG("curve value is not supported"); goto leave; } @@ -32622,7 +32631,13 @@ static int set_curves_list(WOLFSSL* ssl, WOLFSSL_CTX *ctx, const char* names) for (i = 0; i < groups_len; ++i) { /* Switch the bit to off and therefore is enabled. */ curve = (word16)groups[i]; - disabled &= ~(1U << curve); + if (curve >= 32) { + /* 0 is for invalid and 1-14 aren't used otherwise. */ + disabled &= ~(1U << (curve - 32)); + } + else { + disabled &= ~(1U << curve); + } #ifdef HAVE_SUPPORTED_CURVES #if defined(WOLFSSL_TLS13) && !defined(WOLFSSL_OLD_SET_CURVES_LIST) /* using the wolfSSL API to set the groups, this will populate diff --git a/src/tls.c b/src/tls.c index 70fd8f740..b4bd9faa3 100644 --- a/src/tls.c +++ b/src/tls.c @@ -4705,8 +4705,7 @@ int TLSX_ValidateSupportedCurves(const WOLFSSL* ssl, byte first, byte second, #ifdef OPENSSL_EXTRA /* skip if name is not in supported ECC range * or disabled by user */ - if (curve->name > WOLFSSL_ECC_MAX || - wolfSSL_curve_is_disabled(ssl, curve->name)) + if (wolfSSL_curve_is_disabled(ssl, curve->name)) continue; #endif @@ -8651,8 +8650,7 @@ static int TLSX_SupportedGroups_Find(const WOLFSSL* ssl, word16 name, TLSX* extension; SupportedCurve* curve = NULL; - if ((extension = TLSX_Find(extensions, - TLSX_SUPPORTED_GROUPS)) == NULL) { + if ((extension = TLSX_Find(extensions, TLSX_SUPPORTED_GROUPS)) == NULL) { if ((extension = TLSX_Find(ssl->ctx->extensions, TLSX_SUPPORTED_GROUPS)) == NULL) { return 0; diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index c50c8d5c2..1dc12e7d8 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -3926,7 +3926,10 @@ enum { WOLFSSL_ECC_X448 = 30, WOLFSSL_ECC_SM2P256V1 = 41, WOLFSSL_ECC_MAX = 41, + WOLFSSL_ECC_MAX_AVAIL = 46, + /* Update use of disabled curves when adding value greater than 46. */ + WOLFSSL_FFDHE_START = 256, WOLFSSL_FFDHE_2048 = 256, WOLFSSL_FFDHE_3072 = 257, WOLFSSL_FFDHE_4096 = 258,