coffee/mbedtls
1
0
Fork 0
mirror of https://github.com/Mbed-TLS/mbedtls.git synced 2026-10-28 18:07:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

Validate the TLS 1.3 server key_share extension

Browse Source 
The TLS 1.3 server accepted a malformed key_share extension in the
ClientHello and then closed the connection without sending an alert:

- a zero-length key_exchange was accepted;
- a key_exchange whose length did not match its group was only caught
  later, by the key agreement, which returned without pending an alert;
- data trailing the client_shares list was ignored.

Reject these in ssl_tls13_parse_key_shares_ext() and in the shared
key-exchange reader mbedtls_ssl_tls13_read_public_xxdhe_share():

- a zero-length key_exchange and trailing data after client_shares are
  rejected with a decode_error alert;
- a key_exchange whose length does not match the group is rejected with
  an illegal_parameter alert, which is what OpenSSL does.

The shared reader is also used by the client, so it now checks that a
ServerHello key_share is consumed exactly.

Expose ssl_tls13_parse_key_shares_ext() with MBEDTLS_STATIC_TESTABLE and
add unit tests that feed crafted key_share extensions to the server
parser.

Fixes #10887.
Fixes #10737.

Signed-off-by: Joachim Bauernberger <joachim.bauernberger@protonmail.com>
This commit is contained in:
Joachim Bauernberger 2026-10-25 18:55:15 +01:00
parent cac9589a97
commit e462a2ea4e
8 changed files with 210 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ChangeLog.d/tls13-key-share-validation.txt Normal file
View File

@ -0,0 +1,8 @@
Bugfix
* On the TLS 1.3 server, reject a malformed key_share extension in the
ClientHello instead of closing the connection without an alert. A
zero-length key_exchange and trailing data after the client_shares list
are now rejected with a decode_error alert, and a key_exchange whose
length does not match the group with an illegal_parameter alert. The
shared key-exchange reader now also checks that a ServerHello key_share
is consumed exactly on the client side. Fixes #10887. Fixes #10737.

1
library/ssl_misc.h
View File

@ -2613,6 +2613,7 @@ mbedtls_ssl_mode_t mbedtls_ssl_get_mode_from_ciphersuite(
MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_tls13_read_public_xxdhe_share(mbedtls_ssl_context *ssl,
uint16_t named_group,
const unsigned char *buf,
size_t buf_len);

2
library/ssl_tls13_client.c
View File

@ -498,7 +498,7 @@ static int ssl_tls13_parse_key_share_ext(mbedtls_ssl_context *ssl,
mbedtls_ssl_tls13_named_group_is_ffdh(group)) {
MBEDTLS_SSL_DEBUG_MSG(2,
("DHE group name: %s", mbedtls_ssl_named_group_to_str(group)));
ret = mbedtls_ssl_tls13_read_public_xxdhe_share(ssl, p, end - p);
ret = mbedtls_ssl_tls13_read_public_xxdhe_share(ssl, group, p, end - p);
if (ret != 0) {
return ret;
}

131
library/ssl_tls13_generic.c
View File

@ -1482,35 +1482,6 @@ int mbedtls_ssl_reset_transcript_for_hrr(mbedtls_ssl_context *ssl)
#if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_EPHEMERAL_ENABLED)
int mbedtls_ssl_tls13_read_public_xxdhe_share(mbedtls_ssl_context *ssl,
const unsigned char *buf,
size_t buf_len)
{
uint8_t *p = (uint8_t *) buf;
const uint8_t *end = buf + buf_len;
mbedtls_ssl_handshake_params *handshake = ssl->handshake;
/* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, 2);
uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE(p, 0);
p += 2;
/* Check if key size is consistent with given buffer length. */
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, peerkey_len);
/* Store peer's ECDH/FFDH public key. */
if (peerkey_len > sizeof(handshake->xxdh_psa_peerkey)) {
MBEDTLS_SSL_DEBUG_MSG(1, ("Invalid public key length: %u > %" MBEDTLS_PRINTF_SIZET,
(unsigned) peerkey_len,
sizeof(handshake->xxdh_psa_peerkey)));
return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE;
}
memcpy(handshake->xxdh_psa_peerkey, p, peerkey_len);
handshake->xxdh_psa_peerkey_len = peerkey_len;
return 0;
}
#if defined(PSA_WANT_ALG_FFDH)
static psa_status_t mbedtls_ssl_get_psa_ffdh_info_from_tls_id(
uint16_t tls_id, size_t *bits, psa_key_type_t *key_type)
@ -1552,6 +1523,108 @@ static psa_status_t mbedtls_ssl_get_psa_ffdh_info_from_tls_id(
}
#endif /* PSA_WANT_ALG_FFDH */
/* Length of the key_exchange field of a KeyShareEntry for the given named
* group, i.e. the exact size of a well-formed ECDHE/FFDHE public key. */
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_tls13_named_group_share_len(uint16_t named_group,
size_t *share_len)
{
#if defined(PSA_WANT_ALG_ECDH)
if (mbedtls_ssl_tls13_named_group_is_ecdhe(named_group)) {
psa_key_type_t key_type;
size_t bits;
if (mbedtls_ssl_get_psa_curve_info_from_tls_id(
named_group, &key_type, &bits) != PSA_SUCCESS) {
return MBEDTLS_ERR_SSL_INTERNAL_ERROR;
}
if (PSA_KEY_TYPE_ECC_GET_FAMILY(key_type) == PSA_ECC_FAMILY_MONTGOMERY) {
/* Montgomery curves use the raw u-coordinate: X25519 32, X448 56. */
*share_len = PSA_BITS_TO_BYTES(bits);
} else {
/* Weierstrass curves use the uncompressed point 0x04 || X || Y:
* secp256r1 65, secp384r1 97, secp521r1 133. */
*share_len = 2 * PSA_BITS_TO_BYTES(bits) + 1;
}
return 0;
}
#endif /* PSA_WANT_ALG_ECDH */
#if defined(PSA_WANT_ALG_FFDH)
if (mbedtls_ssl_tls13_named_group_is_ffdh(named_group)) {
psa_key_type_t key_type;
size_t bits;
if (mbedtls_ssl_get_psa_ffdh_info_from_tls_id(
named_group, &bits, &key_type) != PSA_SUCCESS) {
return MBEDTLS_ERR_SSL_INTERNAL_ERROR;
}
/* FFDHE public keys are the size of the group prime. */
*share_len = PSA_BITS_TO_BYTES(bits);
return 0;
}
#endif /* PSA_WANT_ALG_FFDH */
(void) named_group;
(void) share_len;
return MBEDTLS_ERR_SSL_INTERNAL_ERROR;
}
int mbedtls_ssl_tls13_read_public_xxdhe_share(mbedtls_ssl_context *ssl,
uint16_t named_group,
const unsigned char *buf,
size_t buf_len)
{
uint8_t *p = (uint8_t *) buf;
const uint8_t *end = buf + buf_len;
mbedtls_ssl_handshake_params *handshake = ssl->handshake;
size_t expected_len;
int ret;
/* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, 2);
uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE(p, 0);
p += 2;
/* Check if key size is consistent with given buffer length. */
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, peerkey_len);
/* key_exchange is the whole KeyShareEntry; no trailing data (RFC 8446 4.2.8). */
if ((size_t) (end - p) != peerkey_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("key_share entry 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;
}
/* A share whose length does not match the group is illegal_parameter. */
ret = ssl_tls13_named_group_share_len(named_group, &expected_len);
if (ret != 0) {
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR,
MBEDTLS_ERR_SSL_INTERNAL_ERROR);
return ret;
}
if (peerkey_len != expected_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("Invalid public key length: %u, expected %"
MBEDTLS_PRINTF_SIZET " for group %04x",
(unsigned) peerkey_len, expected_len,
(unsigned) named_group));
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER,
MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER);
return MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER;
}
/* Store peer's ECDH/FFDH public key. */
if (peerkey_len > sizeof(handshake->xxdh_psa_peerkey)) {
MBEDTLS_SSL_DEBUG_MSG(1, ("Invalid public key length: %u > %" MBEDTLS_PRINTF_SIZET,
(unsigned) peerkey_len,
sizeof(handshake->xxdh_psa_peerkey)));
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR,
MBEDTLS_ERR_SSL_INTERNAL_ERROR);
return MBEDTLS_ERR_SSL_INTERNAL_ERROR;
}
memcpy(handshake->xxdh_psa_peerkey, p, peerkey_len);
handshake->xxdh_psa_peerkey_len = peerkey_len;
return 0;
}
int mbedtls_ssl_tls13_generate_and_write_xxdh_key_exchange(
mbedtls_ssl_context *ssl,
uint16_t named_group,

7
library/ssl_tls13_invasive.h
View File

@ -16,6 +16,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(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_EPHEMERAL_ENABLED)
int mbedtls_ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end);
#endif
#endif /* MBEDTLS_TEST_HOOKS */
#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */

28
library/ssl_tls13_server.c
View File

@ -17,6 +17,7 @@
#include "mbedtls/psa_util.h"
#include "ssl_tls13_keys.h"
#include "ssl_tls13_invasive.h"
#include "ssl_debug_helpers.h"
@ -891,9 +892,10 @@ static int ssl_tls13_parse_supported_groups_ext(mbedtls_ssl_context *ssl,
* - A negative value for fatal errors.
*/
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end)
MBEDTLS_STATIC_TESTABLE
int mbedtls_ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end)
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
unsigned char const *p = buf;
@ -916,6 +918,14 @@ static int ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
ssl->handshake->offered_group_id = 0;
client_shares_end = p + client_shares_len;
/* client_shares is the whole extension; no trailing data (RFC 8446 4.2.8). */
if (client_shares_end != end) {
MBEDTLS_SSL_DEBUG_MSG(1, ("key_share 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;
}
/* We try to find a suitable key share entry and copy it to the
* handshake context. Later, we have to find out whether we can do
* something with the provided key share or whether we have to
@ -937,6 +947,14 @@ static int ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
group = MBEDTLS_GET_UINT16_BE(p, 0);
key_exchange_len = MBEDTLS_GET_UINT16_BE(p, 2);
p += 4;
/* key_exchange is opaque<1..2^16-1>: a zero-length share is malformed. */
if (key_exchange_len == 0) {
MBEDTLS_SSL_DEBUG_MSG(1, ("key_share entry with empty key_exchange"));
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR,
MBEDTLS_ERR_SSL_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
key_exchange = p;
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, client_shares_end, key_exchange_len);
p += key_exchange_len;
@ -959,7 +977,7 @@ static int ssl_tls13_parse_key_shares_ext(mbedtls_ssl_context *ssl,
mbedtls_ssl_named_group_to_str(group),
group));
ret = mbedtls_ssl_tls13_read_public_xxdhe_share(
ssl, key_exchange - 2, key_exchange_len + 2);
ssl, group, key_exchange - 2, key_exchange_len + 2);
if (ret != 0) {
return ret;
}
@ -1555,7 +1573,7 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl,
* contains the endpoint's cryptographic parameters for
* ECDHE/DHE key establishment methods.
*/
ret = ssl_tls13_parse_key_shares_ext(
ret = mbedtls_ssl_tls13_parse_key_shares_ext(
ssl, p, extension_data_end);
if (ret == SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH) {
MBEDTLS_SSL_DEBUG_MSG(2, ("No usable share for key agreement."));

23
tests/suites/test_suite_ssl.data
View File

@ -3650,3 +3650,26 @@ tls13_record_boundary_alignment:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_HS_FINISHED
TLS 1.3 EndOfEarlyData record boundary alignment enforcement
depends_on:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_SESSION_TICKETS
tls13_record_boundary_alignment:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_HS_END_OF_EARLY_DATA
TLS 1.3 srv parse key_shares: valid x25519
depends_on:PSA_WANT_ALG_ECDH:PSA_WANT_ECC_MONTGOMERY_255
tls13_server_parse_key_shares_ext:"0024001d00201111111111111111111111111111111111111111111111111111111111111111":0:-1
TLS 1.3 srv parse key_shares: zero-length share
tls13_server_parse_key_shares_ext:"0004001d0000":MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR
TLS 1.3 srv parse key_shares: x25519 share too short (31)
depends_on:PSA_WANT_ALG_ECDH:PSA_WANT_ECC_MONTGOMERY_255
tls13_server_parse_key_shares_ext:"0023001d001f11111111111111111111111111111111111111111111111111111111111111":MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER:MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER
TLS 1.3 srv parse key_shares: x25519 share too long (33)
depends_on:PSA_WANT_ALG_ECDH:PSA_WANT_ECC_MONTGOMERY_255
tls13_server_parse_key_shares_ext:"0025001d0021111111111111111111111111111111111111111111111111111111111111111111":MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER:MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER
TLS 1.3 srv parse key_shares: ffdhe2048 share wrong length (255)
depends_on:PSA_WANT_ALG_FFDH:PSA_WANT_DH_RFC7919_2048
tls13_server_parse_key_shares_ext:"0103010000ff111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111":MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER:MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER
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

45
tests/suites/test_suite_ssl.function
View File

@ -4563,6 +4563,51 @@ exit:
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_EPHEMERAL_ENABLED */
void tls13_server_parse_key_shares_ext(data_t *ext, int expected_result,
int expected_alert)
{
mbedtls_ssl_context ssl;
mbedtls_ssl_config conf;
int ret;
/* Offer both an ECDHE and an FFDHE group so a well-formed share for either
* reaches the key-exchange reader, whatever the build's default groups. */
static const uint16_t groups[] = {
MBEDTLS_SSL_IANA_TLS_GROUP_X25519,
MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE2048,
MBEDTLS_SSL_IANA_TLS_GROUP_NONE
};
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);
mbedtls_ssl_conf_groups(&conf, groups);
TEST_EQUAL(mbedtls_ssl_setup(&ssl, &conf), 0);
ret = mbedtls_ssl_tls13_parse_key_shares_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)
{