From 48659a1f9ce9f64440ac6f3cb84d23e4ee8ed9b5 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 15 Jan 2025 14:22:28 +0100 Subject: [PATCH] ssl_tls: remove usage of DHE-PSK Signed-off-by: Valerio Setti --- include/mbedtls/ssl.h | 4 --- library/ssl_tls.c | 29 ++---------------- library/ssl_tls12_client.c | 61 ++++---------------------------------- library/ssl_tls12_server.c | 59 ++++-------------------------------- 4 files changed, 13 insertions(+), 140 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index fff53399b7..21dafd9dc6 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -669,10 +669,6 @@ union mbedtls_ssl_premaster_secret { #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) unsigned char _pms_psk[4 + 2 * MBEDTLS_PSK_MAX_LEN]; /* RFC 4279 2 */ #endif -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - unsigned char _pms_dhe_psk[4 + MBEDTLS_MPI_MAX_SIZE - + MBEDTLS_PSK_MAX_LEN]; /* RFC 4279 3 */ -#endif #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) unsigned char _pms_ecdhe_psk[4 + MBEDTLS_ECP_MAX_BYTES + MBEDTLS_PSK_MAX_LEN]; /* RFC 5489 2 */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5031c77a56..0d07a855fc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7025,7 +7025,6 @@ static int ssl_compute_master(mbedtls_ssl_handshake_params *handshake, * length of the other key. */ case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: - case MBEDTLS_KEY_EXCHANGE_DHE_PSK: other_secret_len = MBEDTLS_GET_UINT16_BE(handshake->premaster, 0); other_secret = handshake->premaster + 2; break; @@ -7326,14 +7325,9 @@ int mbedtls_ssl_psk_derive_premaster(mbedtls_ssl_context *ssl, mbedtls_key_excha /* * This should never happen because the existence of a PSK is always * checked before calling this function. - * - * The exception is opaque DHE-PSK. For DHE-PSK fill premaster with - * the shared secret without PSK. */ - if (key_ex != MBEDTLS_KEY_EXCHANGE_DHE_PSK) { - MBEDTLS_SSL_DEBUG_MSG(1, ("should never happen")); - return MBEDTLS_ERR_SSL_INTERNAL_ERROR; - } + MBEDTLS_SSL_DEBUG_MSG(1, ("should never happen")); + return MBEDTLS_ERR_SSL_INTERNAL_ERROR; } /* @@ -7360,24 +7354,6 @@ int mbedtls_ssl_psk_derive_premaster(mbedtls_ssl_context *ssl, mbedtls_key_excha p += psk_len; } else #endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if (key_ex == MBEDTLS_KEY_EXCHANGE_DHE_PSK) { - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t len; - - /* Write length only when we know the actual value */ - if ((ret = mbedtls_dhm_calc_secret(&ssl->handshake->dhm_ctx, - p + 2, (size_t) (end - (p + 2)), &len, - ssl->conf->f_rng, ssl->conf->p_rng)) != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_dhm_calc_secret", ret); - return ret; - } - MBEDTLS_PUT_UINT16_BE(len, p, 0); - p += 2 + len; - - MBEDTLS_SSL_DEBUG_MPI(3, "DHM: K ", &ssl->handshake->dhm_ctx.K); - } else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) if (key_ex == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -9686,7 +9662,6 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, /* Don't use default: we want warnings when adding new values */ case MBEDTLS_KEY_EXCHANGE_NONE: case MBEDTLS_KEY_EXCHANGE_PSK: - case MBEDTLS_KEY_EXCHANGE_DHE_PSK: case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: case MBEDTLS_KEY_EXCHANGE_ECJPAKE: usage = 0; diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 14ce37757e..63f4240f21 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -1660,8 +1660,7 @@ static int ssl_parse_server_hello(mbedtls_ssl_context *ssl) return 0; } -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_dh_params(mbedtls_ssl_context *ssl, unsigned char **p, @@ -1699,8 +1698,7 @@ static int ssl_parse_server_dh_params(mbedtls_ssl_context *ssl, return ret; } -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ #if defined(MBEDTLS_USE_PSA_CRYPTO) #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ @@ -2171,7 +2169,6 @@ start_processing: #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) { if (ssl_parse_server_psk_hint(ssl, &p, end) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad server key exchange message")); @@ -2189,10 +2186,8 @@ start_processing: ; /* nothing more to do */ } else #endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_RSA || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK) { +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) + if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_RSA) { if (ssl_parse_server_dh_params(ssl, &p, end) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad server key exchange message")); mbedtls_ssl_send_alert_message( @@ -2202,8 +2197,7 @@ start_processing: return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER; } } else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) @@ -3043,51 +3037,6 @@ ecdh_calc_secret: content_len = 0; } else #endif -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK) { - /* - * ClientDiffieHellmanPublic public (DHM send G^X mod P) - */ - content_len = mbedtls_dhm_get_len(&ssl->handshake->dhm_ctx); - - if (header_len + 2 + content_len > - MBEDTLS_SSL_OUT_CONTENT_LEN) { - MBEDTLS_SSL_DEBUG_MSG(1, - ("psk identity or DHM size too long or SSL buffer too short")); - return MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL; - } - - ssl->out_msg[header_len++] = MBEDTLS_BYTE_1(content_len); - ssl->out_msg[header_len++] = MBEDTLS_BYTE_0(content_len); - - ret = mbedtls_dhm_make_public(&ssl->handshake->dhm_ctx, - (int) mbedtls_dhm_get_len(&ssl->handshake->dhm_ctx), - &ssl->out_msg[header_len], content_len, - ssl->conf->f_rng, ssl->conf->p_rng); - if (ret != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_dhm_make_public", ret); - return ret; - } - -#if defined(MBEDTLS_USE_PSA_CRYPTO) - unsigned char *pms = ssl->handshake->premaster; - unsigned char *pms_end = pms + sizeof(ssl->handshake->premaster); - size_t pms_len; - - /* Write length only when we know the actual value */ - if ((ret = mbedtls_dhm_calc_secret(&ssl->handshake->dhm_ctx, - pms + 2, pms_end - (pms + 2), &pms_len, - ssl->conf->f_rng, ssl->conf->p_rng)) != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_dhm_calc_secret", ret); - return ret; - } - MBEDTLS_PUT_UINT16_BE(pms_len, pms, 0); - pms += 2 + pms_len; - - MBEDTLS_SSL_DEBUG_MPI(3, "DHM: K ", &ssl->handshake->dhm_ctx.K); -#endif - } else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ #if !defined(MBEDTLS_USE_PSA_CRYPTO) && \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) { diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 9e7c52c5e6..7b013f9cc2 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -2887,19 +2887,16 @@ static int ssl_prepare_server_key_exchange(mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ /* - * For (EC)DHE key exchanges with PSK, parameters are prefixed by support + * For ECDHE key exchanges with PSK, parameters are prefixed by support * identity hint (RFC 4279, Sec. 3). Until someone needs this feature, * we use empty support identity hints here. **/ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) - if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK || - ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) { +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) + if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) { ssl->out_msg[ssl->out_msglen++] = 0x00; ssl->out_msg[ssl->out_msglen++] = 0x00; } -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED || - MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ /* * - DHE key exchanges @@ -3375,8 +3372,7 @@ static int ssl_write_server_hello_done(mbedtls_ssl_context *ssl) return 0; } -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ - defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +#if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_dh_public(mbedtls_ssl_context *ssl, unsigned char **p, const unsigned char *end) @@ -3411,8 +3407,7 @@ static int ssl_parse_client_dh_public(mbedtls_ssl_context *ssl, unsigned char ** return ret; } -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED || - MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) @@ -3838,48 +3833,6 @@ static int ssl_parse_client_key_exchange(mbedtls_ssl_context *ssl) #endif /* !MBEDTLS_USE_PSA_CRYPTO */ } else #endif /* MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) - if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_DHE_PSK) { - if ((ret = ssl_parse_client_psk_identity(ssl, &p, end)) != 0) { - MBEDTLS_SSL_DEBUG_RET(1, ("ssl_parse_client_psk_identity"), ret); - return ret; - } - if ((ret = ssl_parse_client_dh_public(ssl, &p, end)) != 0) { - MBEDTLS_SSL_DEBUG_RET(1, ("ssl_parse_client_dh_public"), ret); - return ret; - } - - if (p != end) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad client key exchange")); - return MBEDTLS_ERR_SSL_DECODE_ERROR; - } - -#if defined(MBEDTLS_USE_PSA_CRYPTO) - unsigned char *pms = ssl->handshake->premaster; - unsigned char *pms_end = pms + sizeof(ssl->handshake->premaster); - size_t pms_len; - - /* Write length only when we know the actual value */ - if ((ret = mbedtls_dhm_calc_secret(&ssl->handshake->dhm_ctx, - pms + 2, pms_end - (pms + 2), &pms_len, - ssl->conf->f_rng, ssl->conf->p_rng)) != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_dhm_calc_secret", ret); - return ret; - } - MBEDTLS_PUT_UINT16_BE(pms_len, pms, 0); - pms += 2 + pms_len; - - MBEDTLS_SSL_DEBUG_MPI(3, "DHM: K ", &ssl->handshake->dhm_ctx.K); -#else - if ((ret = mbedtls_ssl_psk_derive_premaster(ssl, - (mbedtls_key_exchange_type_t) ciphersuite_info-> - key_exchange)) != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_psk_derive_premaster", ret); - return ret; - } -#endif /* MBEDTLS_USE_PSA_CRYPTO */ - } else -#endif /* MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) if (ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK) { #if defined(MBEDTLS_USE_PSA_CRYPTO)