Skip to content

Commit 39c201c

Browse files
authored
Merge pull request godotengine#103247 from Faless/mbedtls/backport_defragment_tls_handshake
[mbedTLS] Integrate TLS handshake defragmentation PR
2 parents f931a65 + fe84b84 commit 39c201c

File tree

12 files changed

+542
-56
lines changed

12 files changed

+542
-56
lines changed

doc/classes/EditorSettings.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,10 @@
11171117
<member name="network/tls/editor_tls_certificates" type="String" setter="" getter="">
11181118
The TLS certificate bundle to use for HTTP requests made within the editor (e.g. from the AssetLib tab). If left empty, the [url=https://github.com/godotengine/godot/blob/master/thirdparty/certs/ca-certificates.crt]included Mozilla certificate bundle[/url] will be used.
11191119
</member>
1120+
<member name="network/tls/enable_tls_v1.3" type="bool" setter="" getter="">
1121+
If [code]true[/code], enable TLSv1.3 negotiation.
1122+
[b]Note:[/b] Only supported when using Mbed TLS 3.0 or later (Linux distribution packages may be compiled against older system Mbed TLS packages), otherwise the maximum supported TLS version is always TLSv1.2.
1123+
</member>
11201124
<member name="project_manager/default_renderer" type="String" setter="" getter="">
11211125
The renderer type that will be checked off by default when creating a new project. Accepted strings are "forward_plus", "mobile" or "gl_compatibility".
11221126
</member>

doc/classes/ProjectSettings.xml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,9 +2204,8 @@
22042204
The CA certificates bundle to use for TLS connections. If this is set to a non-empty value, this will [i]override[/i] Godot's default [url=https://github.com/godotengine/godot/blob/master/thirdparty/certs/ca-certificates.crt]Mozilla certificate bundle[/url]. If left empty, the default certificate bundle will be used.
22052205
If in doubt, leave this setting empty.
22062206
</member>
2207-
<member name="network/tls/enable_tls_v1.3" type="bool" setter="" getter="" default="false">
2207+
<member name="network/tls/enable_tls_v1.3" type="bool" setter="" getter="" default="true">
22082208
If [code]true[/code], enable TLSv1.3 negotiation.
2209-
[b]Note:[/b] This is experimental, and may cause connections to fail in some cases (notably, if the remote server uses TLS handshake fragmentation).
22102209
[b]Note:[/b] Only supported when using Mbed TLS 3.0 or later (Linux distribution packages may be compiled against older system Mbed TLS packages), otherwise the maximum supported TLS version is always TLSv1.2.
22112210
</member>
22122211
<member name="physics/2d/default_angular_damp" type="float" setter="" getter="" default="1.0">

editor/editor_settings.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,7 @@ void EditorSettings::_load_defaults(Ref<ConfigFile> p_extra_config) {
974974

975975
// SSL
976976
EDITOR_SETTING_USAGE(Variant::STRING, PROPERTY_HINT_GLOBAL_FILE, "network/tls/editor_tls_certificates", _SYSTEM_CERTS_PATH, "*.crt,*.pem", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED);
977+
EDITOR_SETTING_BASIC(Variant::BOOL, PROPERTY_HINT_NONE, "network/tls/enable_tls_v1.3", true, "")
977978

978979
// Debug
979980
_initial_set("network/debug/remote_host", "127.0.0.1"); // Hints provided in setup_network

modules/mbedtls/register_types.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ void initialize_mbedtls_module(ModuleInitializationLevel p_level) {
5252
return;
5353
}
5454

55-
GLOBAL_DEF("network/tls/enable_tls_v1.3", false);
55+
GLOBAL_DEF("network/tls/enable_tls_v1.3", true);
5656

5757
#if MBEDTLS_VERSION_MAJOR >= 3
5858
int status = psa_crypto_init();

modules/mbedtls/tls_context_mbedtls.cpp

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232

3333
#include "core/config/project_settings.h"
3434

35+
#ifdef TOOLS_ENABLED
36+
#include "editor/editor_settings.h"
37+
#endif // TOOLS_ENABLED
38+
3539
static void my_debug(void *ctx, int level,
3640
const char *file, int line,
3741
const char *str) {
@@ -148,8 +152,17 @@ Error TLSContextMbedTLS::init_server(int p_transport, Ref<TLSOptions> p_options,
148152
}
149153

150154
#if MBEDTLS_VERSION_MAJOR >= 3
151-
if (Engine::get_singleton()->is_editor_hint() || !(bool)GLOBAL_GET("network/tls/enable_tls_v1.3")) {
152-
mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2);
155+
#ifdef TOOLS_ENABLED
156+
if (Engine::get_singleton()->is_editor_hint()) {
157+
if (!EditorSettings::get_singleton()->get_setting("network/tls/enable_tls_v1.3").operator bool()) {
158+
mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2);
159+
}
160+
} else
161+
#endif
162+
{
163+
if (!GLOBAL_GET("network/tls/enable_tls_v1.3").operator bool()) {
164+
mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2);
165+
}
153166
}
154167
#endif
155168

@@ -197,8 +210,17 @@ Error TLSContextMbedTLS::init_client(int p_transport, const String &p_hostname,
197210
}
198211

199212
#if MBEDTLS_VERSION_MAJOR >= 3
200-
if (Engine::get_singleton()->is_editor_hint() || !(bool)GLOBAL_GET("network/tls/enable_tls_v1.3")) {
201-
mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2);
213+
#ifdef TOOLS_ENABLED
214+
if (Engine::get_singleton()->is_editor_hint()) {
215+
if (!EditorSettings::get_singleton()->get_setting("network/tls/enable_tls_v1.3").operator bool()) {
216+
mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2);
217+
}
218+
} else
219+
#endif
220+
{
221+
if (!GLOBAL_GET("network/tls/enable_tls_v1.3").operator bool()) {
222+
mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2);
223+
}
202224
}
203225
#endif
204226

thirdparty/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,7 @@ File extracted from upstream release tarball:
627627
Patches:
628628

629629
- `0001-msvc-2019-psa-redeclaration.patch` (GH-90535)
630+
- `0002-pr-9981-defragment-incoming-tls-handshake-messages.patch` (GH-103247)
630631

631632

632633
## meshoptimizer

thirdparty/mbedtls/include/mbedtls/ssl.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1724,7 +1724,16 @@ struct mbedtls_ssl_context {
17241724
int MBEDTLS_PRIVATE(early_data_state);
17251725
#endif
17261726

1727-
unsigned MBEDTLS_PRIVATE(badmac_seen); /*!< records with a bad MAC received */
1727+
/** Multipurpose field.
1728+
*
1729+
* - DTLS: records with a bad MAC received.
1730+
* - TLS: accumulated length of handshake fragments (up to \c in_hslen).
1731+
*
1732+
* This field is multipurpose in order to preserve the ABI in the
1733+
* Mbed TLS 3.6 LTS branch. Until 3.6.2, it was only used in DTLS
1734+
* and called `badmac_seen`.
1735+
*/
1736+
unsigned MBEDTLS_PRIVATE(badmac_seen_or_in_hsfraglen);
17281737

17291738
#if defined(MBEDTLS_X509_CRT_PARSE_C)
17301739
/** Callback to customize X.509 certificate chain verification */

thirdparty/mbedtls/library/ssl_misc.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1829,10 +1829,11 @@ void mbedtls_ssl_set_timer(mbedtls_ssl_context *ssl, uint32_t millisecs);
18291829
MBEDTLS_CHECK_RETURN_CRITICAL
18301830
int mbedtls_ssl_check_timer(mbedtls_ssl_context *ssl);
18311831

1832-
void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl);
1832+
void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl);
1833+
void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl);
1834+
void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl);
18331835
void mbedtls_ssl_update_out_pointers(mbedtls_ssl_context *ssl,
18341836
mbedtls_ssl_transform *transform);
1835-
void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl);
18361837

18371838
MBEDTLS_CHECK_RETURN_CRITICAL
18381839
int mbedtls_ssl_session_reset_int(mbedtls_ssl_context *ssl, int partial);

thirdparty/mbedtls/library/ssl_msg.c

Lines changed: 103 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "constant_time_internal.h"
2626
#include "mbedtls/constant_time.h"
2727

28+
#include <limits.h>
2829
#include <string.h>
2930

3031
#if defined(MBEDTLS_USE_PSA_CRYPTO)
@@ -3220,13 +3221,17 @@ static uint32_t ssl_get_hs_total_len(mbedtls_ssl_context const *ssl)
32203221

32213222
int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
32223223
{
3223-
if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl)) {
3224+
/* First handshake fragment must at least include the header. */
3225+
if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl) && ssl->in_hslen == 0) {
32243226
MBEDTLS_SSL_DEBUG_MSG(1, ("handshake message too short: %" MBEDTLS_PRINTF_SIZET,
32253227
ssl->in_msglen));
32263228
return MBEDTLS_ERR_SSL_INVALID_RECORD;
32273229
}
32283230

3229-
ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl);
3231+
if (ssl->in_hslen == 0) {
3232+
ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl);
3233+
ssl->badmac_seen_or_in_hsfraglen = 0;
3234+
}
32303235

32313236
MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen ="
32323237
" %" MBEDTLS_PRINTF_SIZET ", type = %u, hslen = %"
@@ -3292,10 +3297,67 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
32923297
}
32933298
} else
32943299
#endif /* MBEDTLS_SSL_PROTO_DTLS */
3295-
/* With TLS we don't handle fragmentation (for now) */
3296-
if (ssl->in_msglen < ssl->in_hslen) {
3297-
MBEDTLS_SSL_DEBUG_MSG(1, ("TLS handshake fragmentation not supported"));
3298-
return MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE;
3300+
if (ssl->badmac_seen_or_in_hsfraglen <= ssl->in_hslen) {
3301+
int ret;
3302+
const size_t hs_remain = ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen;
3303+
MBEDTLS_SSL_DEBUG_MSG(3,
3304+
("handshake fragment: %u .. %"
3305+
MBEDTLS_PRINTF_SIZET " of %"
3306+
MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET,
3307+
ssl->badmac_seen_or_in_hsfraglen,
3308+
(size_t) ssl->badmac_seen_or_in_hsfraglen +
3309+
(hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen),
3310+
ssl->in_hslen, ssl->in_msglen));
3311+
if (ssl->in_msglen < hs_remain) {
3312+
/* ssl->in_msglen is a 25-bit value since it is the sum of the
3313+
* header length plus the payload length, the header length is 4
3314+
* and the payload length was received on the wire encoded as
3315+
* 3 octets. We don't support 16-bit platforms; more specifically,
3316+
* we assume that both unsigned and size_t are at least 32 bits.
3317+
* Therefore there is no possible integer overflow here.
3318+
*/
3319+
ssl->badmac_seen_or_in_hsfraglen += (unsigned) ssl->in_msglen;
3320+
ssl->in_hdr = ssl->in_msg + ssl->in_msglen;
3321+
ssl->in_msglen = 0;
3322+
mbedtls_ssl_update_in_pointers(ssl);
3323+
return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING;
3324+
}
3325+
if (ssl->badmac_seen_or_in_hsfraglen > 0) {
3326+
/*
3327+
* At in_first_hdr we have a sequence of records that cover the next handshake
3328+
* record, each with its own record header that we need to remove.
3329+
* Note that the reassembled record size may not equal the size of the message,
3330+
* there may be more messages after it, complete or partial.
3331+
*/
3332+
unsigned char *in_first_hdr = ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
3333+
unsigned char *p = in_first_hdr, *q = NULL;
3334+
size_t merged_rec_len = 0;
3335+
do {
3336+
mbedtls_record rec;
3337+
ret = ssl_parse_record_header(ssl, p, mbedtls_ssl_in_hdr_len(ssl), &rec);
3338+
if (ret != 0) {
3339+
return ret;
3340+
}
3341+
merged_rec_len += rec.data_len;
3342+
p = rec.buf + rec.buf_len;
3343+
if (q != NULL) {
3344+
memmove(q, rec.buf + rec.data_offset, rec.data_len);
3345+
q += rec.data_len;
3346+
} else {
3347+
q = p;
3348+
}
3349+
} while (merged_rec_len < ssl->in_hslen);
3350+
ssl->in_hdr = in_first_hdr;
3351+
mbedtls_ssl_update_in_pointers(ssl);
3352+
ssl->in_msglen = merged_rec_len;
3353+
/* Adjust message length. */
3354+
MBEDTLS_PUT_UINT16_BE(merged_rec_len, ssl->in_len, 0);
3355+
ssl->badmac_seen_or_in_hsfraglen = 0;
3356+
MBEDTLS_SSL_DEBUG_BUF(4, "reassembled record",
3357+
ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + merged_rec_len);
3358+
}
3359+
} else {
3360+
return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
32993361
}
33003362

33013363
return 0;
@@ -4640,6 +4702,16 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl)
46404702
return MBEDTLS_ERR_SSL_INTERNAL_ERROR;
46414703
}
46424704

4705+
if (ssl->badmac_seen_or_in_hsfraglen != 0) {
4706+
/* Not all handshake fragments have arrived, do not consume. */
4707+
MBEDTLS_SSL_DEBUG_MSG(3,
4708+
("waiting for more fragments (%u of %"
4709+
MBEDTLS_PRINTF_SIZET ", %" MBEDTLS_PRINTF_SIZET " left)",
4710+
ssl->badmac_seen_or_in_hsfraglen, ssl->in_hslen,
4711+
ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen));
4712+
return 0;
4713+
}
4714+
46434715
/*
46444716
* Get next Handshake message in the current record
46454717
*/
@@ -4665,6 +4737,7 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl)
46654737
ssl->in_msglen -= ssl->in_hslen;
46664738
memmove(ssl->in_msg, ssl->in_msg + ssl->in_hslen,
46674739
ssl->in_msglen);
4740+
MBEDTLS_PUT_UINT16_BE(ssl->in_msglen, ssl->in_len, 0);
46684741

46694742
MBEDTLS_SSL_DEBUG_BUF(4, "remaining content in record",
46704743
ssl->in_msg, ssl->in_msglen);
@@ -4967,10 +5040,12 @@ static int ssl_get_next_record(mbedtls_ssl_context *ssl)
49675040
return ret;
49685041
}
49695042

4970-
if (ssl->conf->badmac_limit != 0 &&
4971-
++ssl->badmac_seen >= ssl->conf->badmac_limit) {
4972-
MBEDTLS_SSL_DEBUG_MSG(1, ("too many records with bad MAC"));
4973-
return MBEDTLS_ERR_SSL_INVALID_MAC;
5043+
if (ssl->conf->badmac_limit != 0) {
5044+
++ssl->badmac_seen_or_in_hsfraglen;
5045+
if (ssl->badmac_seen_or_in_hsfraglen >= ssl->conf->badmac_limit) {
5046+
MBEDTLS_SSL_DEBUG_MSG(1, ("too many records with bad MAC"));
5047+
return MBEDTLS_ERR_SSL_INVALID_MAC;
5048+
}
49745049
}
49755050

49765051
/* As above, invalid records cause
@@ -5345,7 +5420,7 @@ void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl)
53455420
} else
53465421
#endif
53475422
{
5348-
ssl->in_ctr = ssl->in_hdr - MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
5423+
ssl->in_ctr = ssl->in_buf;
53495424
ssl->in_len = ssl->in_hdr + 3;
53505425
#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
53515426
ssl->in_cid = ssl->in_len;
@@ -5361,24 +5436,35 @@ void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl)
53615436
* Setup an SSL context
53625437
*/
53635438

5364-
void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl)
5439+
void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl)
5440+
{
5441+
#if defined(MBEDTLS_SSL_PROTO_DTLS)
5442+
if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
5443+
ssl->in_hdr = ssl->in_buf;
5444+
} else
5445+
#endif /* MBEDTLS_SSL_PROTO_DTLS */
5446+
{
5447+
ssl->in_hdr = ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
5448+
}
5449+
5450+
/* Derive other internal pointers. */
5451+
mbedtls_ssl_update_in_pointers(ssl);
5452+
}
5453+
5454+
void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl)
53655455
{
53665456
/* Set the incoming and outgoing record pointers. */
53675457
#if defined(MBEDTLS_SSL_PROTO_DTLS)
53685458
if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
53695459
ssl->out_hdr = ssl->out_buf;
5370-
ssl->in_hdr = ssl->in_buf;
53715460
} else
53725461
#endif /* MBEDTLS_SSL_PROTO_DTLS */
53735462
{
53745463
ssl->out_ctr = ssl->out_buf;
5375-
ssl->out_hdr = ssl->out_buf + 8;
5376-
ssl->in_hdr = ssl->in_buf + 8;
5464+
ssl->out_hdr = ssl->out_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
53775465
}
5378-
53795466
/* Derive other internal pointers. */
53805467
mbedtls_ssl_update_out_pointers(ssl, NULL /* no transform enabled */);
5381-
mbedtls_ssl_update_in_pointers(ssl);
53825468
}
53835469

53845470
/*

0 commit comments

Comments
 (0)