mirror of
https://github.com/Mbed-TLS/mbedtls.git
synced 2026-10-30 19:07:48 +08:00
Validate the TLS 1.3 server supported_groups extension
The TLS 1.3 server accepted an empty named_group_list, and data trailing the list, in a ClientHello supported_groups extension and continued the handshake. RFC 8446 section 4.2.7 defines the list as NamedGroup<2..2^16-1>, so it must hold at least one group and be the whole content of the extension. Reject an empty list and trailing data in ssl_tls13_parse_supported_groups_ext() with a decode_error alert, which is what OpenSSL does. Expose the parser with MBEDTLS_STATIC_TESTABLE and add unit tests. Fixes #10897. Fixes #10733. Signed-off-by: Joachim Bauernberger <joachim.bauernberger@protonmail.com>
This commit is contained in:
parent
e462a2ea4e
commit
f958c05fa7
4
ChangeLog.d/tls13-supported-groups-validation.txt
Normal file
4
ChangeLog.d/tls13-supported-groups-validation.txt
Normal file
@ -0,0 +1,4 @@
|
||||
Bugfix
|
||||
* On the TLS 1.3 server, reject an empty supported_groups list, and
|
||||
trailing data after the list, in the ClientHello with a decode_error
|
||||
alert, as required by RFC 8446. Fixes #10897. Fixes #10733.
|
||||
@ -17,6 +17,13 @@ int mbedtls_ssl_tls13_parse_certificate(mbedtls_ssl_context *ssl,
|
||||
const unsigned char *buf,
|
||||
const unsigned char *end);
|
||||
|
||||
#if defined(MBEDTLS_SSL_SRV_C) && \
|
||||
(defined(PSA_WANT_ALG_ECDH) || defined(PSA_WANT_ALG_FFDH))
|
||||
int mbedtls_ssl_tls13_parse_supported_groups_ext(mbedtls_ssl_context *ssl,
|
||||
const unsigned char *buf,
|
||||
const unsigned char *end);
|
||||
#endif
|
||||
|
||||
#if defined(MBEDTLS_SSL_SRV_C) && \
|
||||
defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_EPHEMERAL_ENABLED)
|
||||
int mbedtls_ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
|
||||
|
||||
@ -830,9 +830,10 @@ static int ssl_tls13_parse_supported_versions_ext(mbedtls_ssl_context *ssl,
|
||||
* } NamedGroupList;
|
||||
*/
|
||||
MBEDTLS_CHECK_RETURN_CRITICAL
|
||||
static int ssl_tls13_parse_supported_groups_ext(mbedtls_ssl_context *ssl,
|
||||
const unsigned char *buf,
|
||||
const unsigned char *end)
|
||||
MBEDTLS_STATIC_TESTABLE
|
||||
int mbedtls_ssl_tls13_parse_supported_groups_ext(mbedtls_ssl_context *ssl,
|
||||
const unsigned char *buf,
|
||||
const unsigned char *end)
|
||||
{
|
||||
const unsigned char *p = buf;
|
||||
size_t named_group_list_len;
|
||||
@ -842,8 +843,24 @@ static int ssl_tls13_parse_supported_groups_ext(mbedtls_ssl_context *ssl,
|
||||
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, 2);
|
||||
named_group_list_len = MBEDTLS_GET_UINT16_BE(p, 0);
|
||||
p += 2;
|
||||
|
||||
/* named_group_list is NamedGroup<2..2^16-1>: at least one group (RFC 8446 4.2.7). */
|
||||
if (named_group_list_len < 2) {
|
||||
MBEDTLS_SSL_DEBUG_MSG(1, ("supported_groups list is empty"));
|
||||
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR,
|
||||
MBEDTLS_ERR_SSL_DECODE_ERROR);
|
||||
return MBEDTLS_ERR_SSL_DECODE_ERROR;
|
||||
}
|
||||
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, named_group_list_len);
|
||||
named_group_list_end = p + named_group_list_len;
|
||||
|
||||
/* named_group_list is the whole extension; no trailing data. */
|
||||
if (named_group_list_end != end) {
|
||||
MBEDTLS_SSL_DEBUG_MSG(1, ("supported_groups extension has trailing data"));
|
||||
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR,
|
||||
MBEDTLS_ERR_SSL_DECODE_ERROR);
|
||||
return MBEDTLS_ERR_SSL_DECODE_ERROR;
|
||||
}
|
||||
ssl->handshake->hrr_selected_group = 0;
|
||||
|
||||
while (p < named_group_list_end) {
|
||||
@ -1551,11 +1568,11 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl,
|
||||
* indicates the named groups which the client supports,
|
||||
* ordered from most preferred to least preferred.
|
||||
*/
|
||||
ret = ssl_tls13_parse_supported_groups_ext(
|
||||
ret = mbedtls_ssl_tls13_parse_supported_groups_ext(
|
||||
ssl, p, extension_data_end);
|
||||
if (ret != 0) {
|
||||
MBEDTLS_SSL_DEBUG_RET(
|
||||
1, "ssl_tls13_parse_supported_groups_ext", ret);
|
||||
1, "mbedtls_ssl_tls13_parse_supported_groups_ext", ret);
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
||||
@ -3673,3 +3673,14 @@ tls13_server_parse_key_shares_ext:"0103010000ff111111111111111111111111111111111
|
||||
TLS 1.3 srv parse key_shares: trailing data after client_shares
|
||||
tls13_server_parse_key_shares_ext:"0024001d00201111111111111111111111111111111111111111111111111111111111111111ff":MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR
|
||||
|
||||
TLS 1.3 srv parse supported_groups: valid
|
||||
tls13_server_parse_supported_groups_ext:"0004001d0017":0:-1
|
||||
|
||||
TLS 1.3 srv parse supported_groups: empty list
|
||||
tls13_server_parse_supported_groups_ext:"0000":MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR
|
||||
|
||||
TLS 1.3 srv parse supported_groups: trailing data
|
||||
tls13_server_parse_supported_groups_ext:"0004001d0017ff":MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR
|
||||
|
||||
TLS 1.3 srv parse supported_groups: odd length
|
||||
tls13_server_parse_supported_groups_ext:"0003001d00":MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR
|
||||
|
||||
@ -4608,6 +4608,42 @@ exit:
|
||||
}
|
||||
/* END_CASE */
|
||||
|
||||
/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_PROTO_TLS1_3 */
|
||||
void tls13_server_parse_supported_groups_ext(data_t *ext, int expected_result,
|
||||
int expected_alert)
|
||||
{
|
||||
mbedtls_ssl_context ssl;
|
||||
mbedtls_ssl_config conf;
|
||||
int ret;
|
||||
|
||||
mbedtls_ssl_init(&ssl);
|
||||
mbedtls_ssl_config_init(&conf);
|
||||
MD_OR_USE_PSA_INIT();
|
||||
|
||||
TEST_EQUAL(mbedtls_ssl_config_defaults(&conf, MBEDTLS_SSL_IS_SERVER,
|
||||
MBEDTLS_SSL_TRANSPORT_STREAM,
|
||||
MBEDTLS_SSL_PRESET_DEFAULT), 0);
|
||||
TEST_EQUAL(mbedtls_ssl_setup(&ssl, &conf), 0);
|
||||
|
||||
ret = mbedtls_ssl_tls13_parse_supported_groups_ext(&ssl, ext->x,
|
||||
ext->x + ext->len);
|
||||
TEST_EQUAL(ret, expected_result);
|
||||
|
||||
if (expected_alert >= 0) {
|
||||
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(send_alert), 1);
|
||||
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(alert_type),
|
||||
(unsigned char) expected_alert);
|
||||
} else {
|
||||
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(send_alert), 0);
|
||||
}
|
||||
|
||||
exit:
|
||||
mbedtls_ssl_free(&ssl);
|
||||
mbedtls_ssl_config_free(&conf);
|
||||
MD_OR_USE_PSA_DONE();
|
||||
}
|
||||
/* END_CASE */
|
||||
|
||||
/* BEGIN_CASE depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_TEST_AT_LEAST_ONE_TLS1_3_CIPHERSUITE:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_ECDH:PSA_WANT_ALG_ECDSA_ANY:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384 */
|
||||
void reject_hrr_selecting_unoffered_group(void)
|
||||
{
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user