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

rework handshake length checks per review

Browse Source 
ClientHello: narrow the session-id check to the session id itself, check
the cookie length byte before reading it, and let the ciphersuite-list
length check cover both transports.

CertificateRequest: make the signature-algorithms check also reserve the
two certificate-authorities length bytes instead of adding a separate
check, use the available < needed pattern throughout, and reject an
odd-length signature-algorithms list.

Add injected truncated-ClientHello test cases (TLS 1.2-only builds).

Signed-off-by: Naveed <naveed@bugqore.com>
This commit is contained in:
Naveed 2026-08-07 15:48:03 +05:30 committed by “rootvector2”
parent 46771343fc
commit 8a69d73b79
4 changed files with 51 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
ChangeLog.d/tls12-handshake-length-read-bounds.txt
View File

@ -1,6 +1,10 @@
Bugfix
* Fix a one-byte buffer over-read when parsing a TLS 1.2 ClientHello or
CertificateRequest. The length checks preceding the ciphersuite-list
length (server side) and the certificate-authorities length (client
side) reserved one byte too few, so a truncated message could cause the
two-byte length field to be read one byte past the end of the message.
* Fix a buffer over-read when parsing a TLS 1.2 ClientHello or
CertificateRequest that is truncated inside the two-byte length field
of the ciphersuite list (server side) or of the certificate
authorities list (client side): the length checks preceding those
fields reserved too few bytes, so the length field could be read past
the end of the message. Such messages are now rejected before the
read.
* The TLS 1.2 client now rejects a CertificateRequest message whose
supported_signature_algorithms list has an odd length.

41
library/ssl_tls12_client.c
View File

@ -2217,7 +2217,7 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
buf = ssl->in_msg;
/* certificate_types */
if (ssl->in_hslen <= mbedtls_ssl_hs_hdr_len(ssl)) {
if (ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 1) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -2227,16 +2227,10 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
n = cert_type_len;
/*
* In the subsequent code there are two paths that read from buf:
* * the length of the signature algorithms field (if minor version of
* SSL is 3),
* * distinguished name length otherwise.
* Both reach at most the index:
* ...hdr_len + 2 + n,
* therefore the buffer length at this point must be greater than that
* regardless of the actual code path.
* The two-byte supported_signature_algorithms length is read next,
* at offset ...hdr_len + 1 + n.
*/
if (ssl->in_hslen <= mbedtls_ssl_hs_hdr_len(ssl) + 2 + n) {
if (ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 1 + n + 2) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -2247,18 +2241,13 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
sig_alg_len = MBEDTLS_GET_UINT16_BE(buf, mbedtls_ssl_hs_hdr_len(ssl) + 1 + n);
/*
* The furthest access in buf is in the loop few lines below:
* sig_alg[i + 1],
* where:
* sig_alg = buf + ...hdr_len + 3 + n,
* max(i) = sig_alg_len - 1.
* Therefore the furthest access is:
* buf[...hdr_len + 3 + n + sig_alg_len - 1 + 1],
* which reduces to:
* buf[...hdr_len + 3 + n + sig_alg_len],
* which is one less than we need the buf to be.
* The list is a sequence of two-byte values, so its length must be
* even. The message must have room for the list itself, plus the
* two-byte certificate_authorities length that follows it and is
* read below.
*/
if (ssl->in_hslen <= mbedtls_ssl_hs_hdr_len(ssl) + 3 + n + sig_alg_len) {
if (sig_alg_len % 2 != 0 ||
ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 1 + n + 2 + sig_alg_len + 2) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(
ssl,
@ -2278,16 +2267,6 @@ static int ssl_parse_certificate_request(mbedtls_ssl_context *ssl)
n += 2 + sig_alg_len;
/* The signature-algorithms check above leaves room for only one of the two
* distinguished-name length bytes, so make sure both are in the message
* before reading them. */
if (ssl->in_hslen < mbedtls_ssl_hs_hdr_len(ssl) + 3 + n) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate request message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
/* certificate_authorities */
dn_len = MBEDTLS_GET_UINT16_BE(buf, mbedtls_ssl_hs_hdr_len(ssl) + 1 + n);

17
library/ssl_tls12_server.c
View File

@ -1016,7 +1016,7 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
sess_len = buf[34];
if (sess_len > sizeof(ssl->session_negotiate->id) ||
sess_len + 34 + 2 > msg_len) { /* 2 for cipherlist length field */
sess_len + 35 > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -1037,9 +1037,16 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
cookie_offset = 35 + sess_len;
if (cookie_offset + 1 > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
return MBEDTLS_ERR_SSL_DECODE_ERROR;
}
cookie_len = buf[cookie_offset];
if (cookie_offset + 1 + cookie_len + 2 > msg_len) {
if (cookie_offset + 1 + cookie_len > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR);
@ -1085,9 +1092,9 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl)
#endif /* MBEDTLS_SSL_PROTO_DTLS */
ciph_offset = 35 + sess_len;
/* The two-byte ciphersuite list length field must be within the message.
* The DTLS branch above reserves it via the cookie length check; the
* session id length check does not, so guard the read here. */
/* The two-byte ciphersuite list length field must be within the
* message: the checks above only guarantee that the fields before it
* (session id, and for DTLS the cookie) are. */
if (ciph_offset + 2 > msg_len) {
MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message"));
mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,

20
tests/suites/test_suite_ssl.tls-defrag.data
View File

@ -63,6 +63,26 @@ Inject ClientHello - TLS 1.2 unknown ciphersuite (for reference)
depends_on:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_GCM:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_1
inject_client_content_on_the_wire:MBEDTLS_PK_ECDSA:MBEDTLS_SSL_CLIENT_HELLO:"160303002f0100002b03030123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef000002cccc01000000":"got no ciphersuites in common":MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE
# See "ClientHello breakdown" above
# ClientHello body ending right after the session id: version (2) +
# random (32) + session id length (1) + session id (3) = 38 bytes, the
# minimum accepted, with the ciphersuite list length field missing
# entirely. Only run in TLS 1.2-only builds: when TLS 1.3 is enabled,
# the TLS 1.3 code parses the ClientHello first and rejects it itself.
Inject ClientHello - TLS 1.2 truncated after session id
depends_on:MBEDTLS_SSL_PROTO_TLS1_2:!MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_GCM:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_1
inject_client_content_on_the_wire:MBEDTLS_PK_ECDSA:MBEDTLS_SSL_CLIENT_HELLO:"160303002a0100002603030123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef035f5f5f":"bad client hello message":MBEDTLS_ERR_SSL_DECODE_ERROR
# See "ClientHello breakdown" above
# ClientHello body ending inside the ciphersuite list length: version (2) +
# random (32) + session id length (1) + session id (2) + first length
# byte (1) = 38 bytes. Reading the full two-byte length field used to
# over-read one byte past the end of the message.
# Only run in TLS 1.2-only builds, see above.
Inject ClientHello - TLS 1.2 truncated inside ciphersuite list length
depends_on:MBEDTLS_SSL_PROTO_TLS1_2:!MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_GCM:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_1
inject_client_content_on_the_wire:MBEDTLS_PK_ECDSA:MBEDTLS_SSL_CLIENT_HELLO:"160303002a0100002603030123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef025f5f00":"bad client hello message":MBEDTLS_ERR_SSL_DECODE_ERROR
# See "ClientHello breakdown" above
# ephemeral with secp256r1 + MBEDTLS_TLS1_3_AES_128_GCM_SHA256
Inject ClientHello - TLS 1.3 good (for reference)