From c94030cadf3ff98b8099f19a46dc9329e84cd6c1 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 29 Oct 2026 16:03:55 -0500 Subject: [PATCH 1/3] Bound the DH shared secret output in wc_DhAgree --- wolfcrypt/src/dh.c | 6 ++++++ wolfcrypt/test/test.c | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index d0099b82ead..0217335f626 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -2119,6 +2119,12 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz, return MP_VAL; } + /* The secret can be as long as the prime and is written unbounded. */ + if (*agreeSz < (word32)mp_unsigned_bin_size(&key->p)) { + WOLFSSL_MSG("DH agree buffer smaller than prime"); + return BUFFER_E; + } + /* Non-blocking re-entry: the same wc_DhAgree call repeats until the * SP state machine completes, so cache the per-op key validation * results instead of re-running them each yield. The cache is diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 8f9cd7acda9..92caa5e441a 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -44724,6 +44724,17 @@ static wc_test_ret_t dh_ffdhe_test(WC_RNG *rng, int name) ERROR_OUT(WC_TEST_RET_ENC_EC(nb_ret), done); #endif /* WC_DH_NONBLOCK + SP nonblock */ +#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST) && \ + !defined(WOLFSSL_ASYNC_CRYPT) && !defined(WOLFSSL_KCAPI_DH) + /* An output buffer shorter than the prime must be refused. pubSz is at + * most the prime size. */ + agreeSz2 = pubSz - 1; + ret = wc_DhAgree(key2, agree2, &agreeSz2, priv2, privSz2, pub, pubSz); + if (ret != WC_NO_ERR_TRACE(BUFFER_E)) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done); + ret = 0; +#endif + /* wc_DhGeneratePublic_fips() was added in 5.2.3, but some customers are * building with configure scripts that set version to 5.2.1, but with 5.2.3 * wolfCrypt sources. 5.3.0 is used for both fips-v5-ready and v5-kcapi, From 17caf6f28850ba05d2f5b1f5d4d72623888e659b Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 29 Oct 2026 16:15:34 -0500 Subject: [PATCH 2/3] Check FFDHE size against defaulted math sizes --- wolfssl/wolfcrypt/settings.h | 1 + wolfssl/wolfcrypt/sp_int.h | 5 +++++ wolfssl/wolfcrypt/tfm.h | 5 +++++ 3 files changed, 11 insertions(+) diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index 0400501fcb6..7b319bcd5af 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -4051,6 +4051,7 @@ #define MIN_FFDHE_BITS 0 #endif #define MIN_FFDHE_FP_MAX_BITS (MIN_FFDHE_BITS * 2) +/* Only explicit sizes are checked here; tfm.h and sp_int.h check defaults. */ #if defined(HAVE_FFDHE) && defined(FP_MAX_BITS) #if MIN_FFDHE_FP_MAX_BITS > FP_MAX_BITS #error "FFDHE parameters are too large for FP_MAX_BIT as set" diff --git a/wolfssl/wolfcrypt/sp_int.h b/wolfssl/wolfcrypt/sp_int.h index 585d703e5e7..8713dc5f327 100644 --- a/wolfssl/wolfcrypt/sp_int.h +++ b/wolfssl/wolfcrypt/sp_int.h @@ -532,6 +532,11 @@ typedef struct sp_dh_ctx { #endif #endif #endif +/* settings.h cannot check a default chosen here. */ +#if defined(HAVE_FFDHE) && defined(MIN_FFDHE_BITS) && \ + (MIN_FFDHE_BITS > SP_INT_BITS) + #error "FFDHE parameters are too large for SP_INT_BITS" +#endif #ifndef SP_INT_DIGITS /* Calculate number of digits to have in an sp_int based on maximum size of diff --git a/wolfssl/wolfcrypt/tfm.h b/wolfssl/wolfcrypt/tfm.h index 0a89b049e30..a17a13dcfc6 100644 --- a/wolfssl/wolfcrypt/tfm.h +++ b/wolfssl/wolfcrypt/tfm.h @@ -278,6 +278,11 @@ #undef FP_MAX_BITS #define FP_MAX_BITS 16384 #endif +/* settings.h cannot check a default chosen here. */ +#if defined(HAVE_FFDHE) && defined(MIN_FFDHE_FP_MAX_BITS) && \ + (MIN_FFDHE_FP_MAX_BITS > FP_MAX_BITS) + #error "FFDHE parameters are too large for FP_MAX_BITS" +#endif #define FP_MAX_SIZE (FP_MAX_BITS+(8*DIGIT_BIT)) /* will this lib work? */ From c411cc71b7e6c8b8050ff98c6ffdb6c91632cc6e Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Fri, 30 Oct 2026 08:25:04 -0500 Subject: [PATCH 3/3] Fixes from review --- doc/dox_comments/header_files/dh.h | 5 ++++ src/internal.c | 18 ++++++++++--- tests/api/test_tls13_bounds.c | 41 ++++++++++++++++++++++++++++++ tests/api/test_tls13_bounds.h | 4 ++- wolfcrypt/src/dh.c | 6 ----- wolfcrypt/test/test.c | 11 -------- 6 files changed, 63 insertions(+), 22 deletions(-) diff --git a/doc/dox_comments/header_files/dh.h b/doc/dox_comments/header_files/dh.h index ba2afd24d8e..9516b384ebe 100644 --- a/doc/dox_comments/header_files/dh.h +++ b/doc/dox_comments/header_files/dh.h @@ -103,6 +103,10 @@ int wc_DhGenerateKeyPair(DhKey* key, WC_RNG* rng, byte* priv, communication. On successfully generating a shared secret key, the size of the secret key written will be stored in agreeSz. + The value of agreeSz on input is not checked. The agree buffer must be at + least as large as the prime, in bytes. Use wc_DhAgree_ct() to have the + buffer size checked. + \return 0 Returned on successfully generating an agreed upon secret key \return MP_INIT_E May be returned if there is an error while generating the shared secret key @@ -141,6 +145,7 @@ int wc_DhGenerateKeyPair(DhKey* key, WC_RNG* rng, byte* priv, \endcode \sa wc_DhGenerateKeyPair + \sa wc_DhAgree_ct */ int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz, const byte* priv, word32 privSz, const byte* otherPub, diff --git a/src/internal.c b/src/internal.c index 1f62c435246..98eaa7542af 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7833,10 +7833,20 @@ int DhAgree(WOLFSSL* ssl, DhKey* dhKey, else #endif { - PRIVATE_KEY_UNLOCK(); - ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub, - otherPubSz); - PRIVATE_KEY_LOCK(); + /* wc_DhAgree() treats agreeSz as output only and writes up to the + * prime size, so bound the buffer here. */ + if (dhKey != NULL && agreeSz != NULL && + *agreeSz < (word32)mp_unsigned_bin_size(&dhKey->p)) { + WOLFSSL_MSG("DH agree buffer smaller than prime"); + ret = BUFFER_E; + WOLFSSL_ERROR_VERBOSE(ret); + } + else { + PRIVATE_KEY_UNLOCK(); + ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub, + otherPubSz); + PRIVATE_KEY_LOCK(); + } } } diff --git a/tests/api/test_tls13_bounds.c b/tests/api/test_tls13_bounds.c index ba503e82aba..3336f0b85c2 100644 --- a/tests/api/test_tls13_bounds.c +++ b/tests/api/test_tls13_bounds.c @@ -927,3 +927,44 @@ int test_tls13_mutual_auth_rsa(void) return TEST_SKIPPED; #endif } + +/* A pre-master secret buffer shorter than the FFDHE prime must fail the + * handshake with BUFFER_E rather than let wc_DhAgree() write past it. */ +int test_tls13_ffdhe_pms_too_small(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_TLS13) && !defined(NO_DH) && defined(HAVE_FFDHE_2048) && \ + defined(HAVE_SUPPORTED_CURVES) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_CERTS) && !defined(NO_FILESYSTEM) + WOLFSSL_CTX* ctx_c = NULL; + WOLFSSL_CTX* ctx_s = NULL; + WOLFSSL* ssl_c = NULL; + WOLFSSL* ssl_s = NULL; + struct test_memio_ctx test_ctx; + int groups[1] = { WOLFSSL_FFDHE_2048 }; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0); + ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups, 1), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_groups(ssl_s, groups, 1), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_FFDHE_2048), + WOLFSSL_SUCCESS); + + /* One byte short of the 2048-bit prime. */ + if (ssl_s != NULL && ssl_s->arrays != NULL) + ssl_s->arrays->preMasterSz = (2048 / 8) - 1; + + ExpectIntNE(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(BUFFER_E)); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_tls13_bounds.h b/tests/api/test_tls13_bounds.h index 8f086607fa5..19ff49e669a 100644 --- a/tests/api/test_tls13_bounds.h +++ b/tests/api/test_tls13_bounds.h @@ -39,6 +39,7 @@ int test_tls13_mutual_auth_ecdsa(void); int test_tls13_mutual_auth_ed25519(void); int test_tls13_mutual_auth_ed448(void); int test_tls13_mutual_auth_rsa(void); +int test_tls13_ffdhe_pms_too_small(void); #define TEST_TLS13_BOUNDS_DECLS \ TEST_DECL_GROUP("tls13", test_tls13_ch_legacy_version_is_tls13), \ @@ -55,6 +56,7 @@ int test_tls13_mutual_auth_rsa(void); TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_ecdsa), \ TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_ed25519), \ TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_ed448), \ - TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_rsa) + TEST_DECL_GROUP("tls13", test_tls13_mutual_auth_rsa), \ + TEST_DECL_GROUP("tls13", test_tls13_ffdhe_pms_too_small) #endif /* WOLFCRYPT_TEST_TLS13_BOUNDS_H */ diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index 0217335f626..d0099b82ead 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -2119,12 +2119,6 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz, return MP_VAL; } - /* The secret can be as long as the prime and is written unbounded. */ - if (*agreeSz < (word32)mp_unsigned_bin_size(&key->p)) { - WOLFSSL_MSG("DH agree buffer smaller than prime"); - return BUFFER_E; - } - /* Non-blocking re-entry: the same wc_DhAgree call repeats until the * SP state machine completes, so cache the per-op key validation * results instead of re-running them each yield. The cache is diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 92caa5e441a..8f9cd7acda9 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -44724,17 +44724,6 @@ static wc_test_ret_t dh_ffdhe_test(WC_RNG *rng, int name) ERROR_OUT(WC_TEST_RET_ENC_EC(nb_ret), done); #endif /* WC_DH_NONBLOCK + SP nonblock */ -#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST) && \ - !defined(WOLFSSL_ASYNC_CRYPT) && !defined(WOLFSSL_KCAPI_DH) - /* An output buffer shorter than the prime must be refused. pubSz is at - * most the prime size. */ - agreeSz2 = pubSz - 1; - ret = wc_DhAgree(key2, agree2, &agreeSz2, priv2, privSz2, pub, pubSz); - if (ret != WC_NO_ERR_TRACE(BUFFER_E)) - ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done); - ret = 0; -#endif - /* wc_DhGeneratePublic_fips() was added in 5.2.3, but some customers are * building with configure scripts that set version to 5.2.1, but with 5.2.3 * wolfCrypt sources. 5.3.0 is used for both fips-v5-ready and v5-kcapi,