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

Merge 8134d60565d244d019d77c1324e3dc0ac76e1d80 into 03e1438089654fb884ac48e0f3cd0049cfe2c102

Browse Source
This commit is contained in:
Joachim Bauernberger 2026-10-27 10:46:10 -04:00 committed by GitHub
commit b216cf425c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 208 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ChangeLog.d/sni-parse-strict.txt Normal file
View File

@ -0,0 +1,9 @@
Bugfix
* The TLS server now rejects a ClientHello whose server_name extension
is malformed: an empty server_name_list, a list that does not fill the
extension, an empty host_name, more than one host_name entry (forbidden
by RFC 6066), or an entry after the host_name that overruns the list.
Previously the first host_name was used and the rest of the extension
was not checked. These are rejected with a decode_error alert. A
host_name containing a null byte is rejected with an unrecognized_name
alert instead of being passed to the ServerName callback.

5
include/mbedtls/ssl.h
View File

@ -4083,6 +4083,11 @@ void mbedtls_ssl_set_hs_authmode(mbedtls_ssl_context *ssl,
* then must return 0. If no matching name is found, the
* callback may return non-zero to abort the handshake.
*
* \note The callback is only called once the whole ServerName
* extension has been validated. \c hostname is not
* null-terminated; it is at least one byte long and does
* not contain any null byte.
*
* \param conf SSL configuration
* \param f_sni verification function
* \param p_sni verification parameter

82
library/ssl_tls.c
View File

@ -8372,6 +8372,13 @@ int mbedtls_ssl_write_sig_alg_ext(mbedtls_ssl_context *ssl, unsigned char *buf,
* struct {
* ServerName server_name_list<1..2^16-1>
* } ServerNameList;
*
* RFC 6066, section 3: "The ServerNameList MUST NOT contain more than one
* name of the same name_type." Entries of unknown name types begin with a
* 16-bit length field and are skipped.
*
* The whole list is validated before the ServerName callback is called, so
* that the callback never acts on a malformed extension.
*/
MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_parse_server_name_ext(mbedtls_ssl_context *ssl,
@ -8380,8 +8387,9 @@ int mbedtls_ssl_parse_server_name_ext(mbedtls_ssl_context *ssl,
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
const unsigned char *p = buf;
size_t server_name_list_len, hostname_len;
const unsigned char *server_name_list_end;
size_t server_name_list_len, name_len;
const unsigned char *hostname = NULL;
size_t hostname_len = 0;
MBEDTLS_SSL_DEBUG_MSG(3, ("parse ServerName extension"));
@ -8389,37 +8397,63 @@ int mbedtls_ssl_parse_server_name_ext(mbedtls_ssl_context *ssl,
server_name_list_len = MBEDTLS_GET_UINT16_BE(p, 0);
p += 2;
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, server_name_list_len);
server_name_list_end = p + server_name_list_len;
while (p < server_name_list_end) {
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, server_name_list_end, 3);
hostname_len = MBEDTLS_GET_UINT16_BE(p, 1);
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, server_name_list_end,
hostname_len + 3);
/* The list must not be empty and must fill the extension exactly. */
if (server_name_list_len == 0 ||
server_name_list_len != (size_t) (end - p)) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad server_name list length"));
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR,
MBEDTLS_ERR_SSL_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
while (p < end) {
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, 3);
name_len = MBEDTLS_GET_UINT16_BE(p, 1);
MBEDTLS_SSL_CHK_BUF_READ_PTR(p, end, name_len + 3);
if (p[0] == MBEDTLS_TLS_EXT_SERVERNAME_HOSTNAME) {
/* sni_name is intended to be used only during the parsing of the
* ClientHello message (it is reset to NULL before the end of
* the message parsing). Thus it is ok to just point to the
* reception buffer and not make a copy of it.
*/
ssl->handshake->sni_name = p + 3;
ssl->handshake->sni_name_len = hostname_len;
if (ssl->conf->f_sni == NULL) {
return 0;
if (hostname != NULL || name_len == 0) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad server_name host_name entry"));
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR,
MBEDTLS_ERR_SSL_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
ret = ssl->conf->f_sni(ssl->conf->p_sni,
ssl, p + 3, hostname_len);
if (ret != 0) {
MBEDTLS_SSL_DEBUG_RET(1, "ssl_sni_wrapper", ret);
/* A host name never contains a null byte. Reject it rather than
* pass a name to the callback that a C string comparison would
* truncate. */
if (memchr(p + 3, '\0', name_len) != NULL) {
MBEDTLS_SSL_DEBUG_MSG(1, ("null byte in server_name host_name"));
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME,
MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME);
return MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME;
}
return 0;
hostname = p + 3;
hostname_len = name_len;
}
p += hostname_len + 3;
p += name_len + 3;
}
if (hostname == NULL) {
return 0;
}
/* sni_name is intended to be used only during the parsing of the
* ClientHello message (it is reset to NULL before the end of
* the message parsing). Thus it is ok to just point to the
* reception buffer and not make a copy of it.
*/
ssl->handshake->sni_name = hostname;
ssl->handshake->sni_name_len = hostname_len;
if (ssl->conf->f_sni == NULL) {
return 0;
}
ret = ssl->conf->f_sni(ssl->conf->p_sni, ssl, hostname, hostname_len);
if (ret != 0) {
MBEDTLS_SSL_DEBUG_RET(1, "ssl_sni_wrapper", ret);
MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME,
MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME);
return MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME;
}
return 0;

45
tests/suites/test_suite_ssl.data
View File

@ -3653,3 +3653,48 @@ 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
Parse server_name: single host_name
ssl_parse_server_name_ext:"000e00000b6578616d706c652e636f6d":TEST_SNI_CB_NONE:0:0:"6578616d706c652e636f6d"
Parse server_name: single host_name, callback accepts
ssl_parse_server_name_ext:"000e00000b6578616d706c652e636f6d":TEST_SNI_CB_ACCEPT:0:0:"6578616d706c652e636f6d"
Parse server_name: single host_name, callback rejects
ssl_parse_server_name_ext:"000e00000b6578616d706c652e636f6d":TEST_SNI_CB_REJECT:MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME:MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME:"6578616d706c652e636f6d"
Parse server_name: unknown name type skipped before host_name
ssl_parse_server_name_ext:"001401000361626300000b6578616d706c652e636f6d":TEST_SNI_CB_ACCEPT:0:0:"6578616d706c652e636f6d"
Parse server_name: unknown name type only
ssl_parse_server_name_ext:"0006010003616263":TEST_SNI_CB_ACCEPT:0:0:""
Parse server_name: truncated list length
ssl_parse_server_name_ext:"00":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: empty server_name_list
ssl_parse_server_name_ext:"0000":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: trailing data after server_name_list
ssl_parse_server_name_ext:"000e00000b6578616d706c652e636f6dff":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: server_name_list longer than extension
ssl_parse_server_name_ext:"000f00000b6578616d706c652e636f6d":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: truncated entry header
ssl_parse_server_name_ext:"00020000":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: host_name longer than server_name_list
ssl_parse_server_name_ext:"0003000005":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: empty host_name
ssl_parse_server_name_ext:"0003000000":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: two host_name entries
ssl_parse_server_name_ext:"001c00000b6578616d706c652e636f6d00000b6578616d706c652e6e6574":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: malformed entry after host_name
ssl_parse_server_name_ext:"001300000b6578616d706c652e636f6d0100056162":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_DECODE_ERROR:MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR:""
Parse server_name: null byte in host_name
ssl_parse_server_name_ext:"000e00000b6578616d706c6500636f6d":TEST_SNI_CB_ACCEPT:MBEDTLS_ERR_SSL_UNRECOGNIZED_NAME:MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME:""

91
tests/suites/test_suite_ssl.function
View File

@ -637,6 +637,36 @@ static void tls_tweak_in_msglen(void *ctx, int level,
}
#endif
/* Modes of the ServerName callback in ssl_parse_server_name_ext. These are
* used as expression parameters in test_suite_ssl.data, which the test
* framework's get_expression() evaluates regardless of a test case's
* dependencies, so they must be defined whenever the suite is built, not
* only under SNI && SRV_C. */
#define TEST_SNI_CB_NONE 0
#define TEST_SNI_CB_ACCEPT 1
#define TEST_SNI_CB_REJECT 2
#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && defined(MBEDTLS_SSL_SRV_C)
typedef struct {
int ret;
int calls;
const unsigned char *name;
size_t name_len;
} test_sni_cb_ctx_t;
static int test_sni_cb(void *p_ctx, mbedtls_ssl_context *ssl,
const unsigned char *name, size_t name_len)
{
test_sni_cb_ctx_t *ctx = (test_sni_cb_ctx_t *) p_ctx;
(void) ssl;
ctx->calls++;
ctx->name = name;
ctx->name_len = name_len;
return ctx->ret;
}
#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_SRV_C */
/* END_HEADER */
@ -7597,3 +7627,64 @@ exit:
PSA_DONE();
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_SSL_SERVER_NAME_INDICATION:MBEDTLS_SSL_SRV_C */
void ssl_parse_server_name_ext(data_t *ext_data, int sni_cb_mode,
int expected_ret, int expected_alert,
data_t *expected_hostname)
{
mbedtls_ssl_context ssl;
mbedtls_ssl_config conf;
test_sni_cb_ctx_t sni_ctx = { 0, 0, NULL, 0 };
const unsigned char *sni_name;
size_t sni_name_len;
mbedtls_ssl_init(&ssl);
mbedtls_ssl_config_init(&conf);
USE_PSA_INIT();
TEST_EQUAL(mbedtls_ssl_config_defaults(&conf, MBEDTLS_SSL_IS_SERVER,
MBEDTLS_SSL_TRANSPORT_STREAM,
MBEDTLS_SSL_PRESET_DEFAULT),
0);
if (sni_cb_mode != TEST_SNI_CB_NONE) {
sni_ctx.ret = (sni_cb_mode == TEST_SNI_CB_REJECT) ? -1 : 0;
mbedtls_ssl_conf_sni(&conf, test_sni_cb, &sni_ctx);
}
TEST_EQUAL(mbedtls_ssl_setup(&ssl, &conf), 0);
TEST_EQUAL(mbedtls_ssl_parse_server_name_ext(&ssl, ext_data->x,
ext_data->x + ext_data->len),
expected_ret);
if (expected_alert != 0) {
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(send_alert), 1);
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(alert_type), expected_alert);
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(alert_reason), expected_ret);
} else {
TEST_EQUAL(ssl.MBEDTLS_PRIVATE(send_alert), 0);
}
/* The host name is recorded, and the callback called with it, only
* once the whole extension has been validated. */
sni_name = ssl.MBEDTLS_PRIVATE(handshake)->sni_name;
sni_name_len = ssl.MBEDTLS_PRIVATE(handshake)->sni_name_len;
if (expected_hostname->len == 0) {
TEST_ASSERT(sni_name == NULL);
TEST_EQUAL(sni_ctx.calls, 0);
} else {
TEST_MEMORY_COMPARE(sni_name, sni_name_len,
expected_hostname->x, expected_hostname->len);
if (sni_cb_mode != TEST_SNI_CB_NONE) {
TEST_EQUAL(sni_ctx.calls, 1);
TEST_MEMORY_COMPARE(sni_ctx.name, sni_ctx.name_len,
expected_hostname->x, expected_hostname->len);
}
}
exit:
mbedtls_ssl_free(&ssl);
mbedtls_ssl_config_free(&conf);
USE_PSA_DONE();
}
/* END_CASE */