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/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? */