Skip to content

Commit f6ef146

Browse files
committed
EarlySanityCheckMsgReceived: version_negotiated should always be checked
Multiple handshake messages in one record will fail the MsgCheckBoundary() check on the client side when the client is set to TLS 1.3 but allows downgrading. --> ClientHello <-- ServerHello + rest of TLS 1.2 flight Client returns OUT_OF_ORDER_E because in TLS 1.3 the ServerHello has to be the last message in a record. In TLS 1.2 the ServerHello can be in the same record as the rest of the server's first flight.
1 parent 8a45f43 commit f6ef146

2 files changed

Lines changed: 52 additions & 10 deletions

File tree

src/internal.c

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10971,18 +10971,11 @@ int EarlySanityCheckMsgReceived(WOLFSSL* ssl, byte type, word32 msgSz)
1097110971
{
1097210972
int ret = 0;
1097310973
#ifndef WOLFSSL_DISABLE_EARLY_SANITY_CHECKS
10974-
byte version_negotiated = 0;
10975-
10976-
WOLFSSL_ENTER("EarlySanityCheckMsgReceived");
10977-
10978-
#ifdef WOLFSSL_DTLS
1097910974
/* Version has only been negotiated after we either send or process a
1098010975
* ServerHello message */
10981-
if (ssl->options.dtls)
10982-
version_negotiated = ssl->options.serverState >= SERVER_HELLO_COMPLETE;
10983-
else
10984-
#endif
10985-
version_negotiated = 1;
10976+
byte version_negotiated = ssl->options.serverState >= SERVER_HELLO_COMPLETE;
10977+
10978+
WOLFSSL_ENTER("EarlySanityCheckMsgReceived");
1098610979

1098710980
if (version_negotiated)
1098810981
ret = MsgCheckEncryption(ssl, type, ssl->keys.decryptedCur == 1);

tests/api.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68852,6 +68852,54 @@ static int test_self_signed_stapling(void)
6885268852
return EXPECT_RESULT();
6885368853
}
6885468854

68855+
static int test_tls_multi_handshakes_one_record(void)
68856+
{
68857+
EXPECT_DECLS;
68858+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_NO_TLS12)
68859+
struct test_memio_ctx test_ctx;
68860+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
68861+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
68862+
int newRecIdx = RECORD_HEADER_SZ;
68863+
int idx = 0;
68864+
68865+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
68866+
68867+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
68868+
wolfTLS_client_method, wolfTLSv1_2_server_method), 0);
68869+
68870+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
68871+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
68872+
ExpectIntEQ(wolfSSL_accept(ssl_s), -1);
68873+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
68874+
68875+
/* Combine server handshake msgs into one record */
68876+
while (idx < test_ctx.c_len) {
68877+
word16 recLen;
68878+
68879+
ato16(((RecordLayerHeader*)(test_ctx.c_buff + idx))->length, &recLen);
68880+
idx += RECORD_HEADER_SZ;
68881+
68882+
XMEMMOVE(test_ctx.c_buff + newRecIdx, test_ctx.c_buff + idx,
68883+
(size_t)recLen);
68884+
68885+
newRecIdx += recLen;
68886+
idx += recLen;
68887+
}
68888+
c16toa(newRecIdx - RECORD_HEADER_SZ,
68889+
((RecordLayerHeader*)test_ctx.c_buff)->length);
68890+
test_ctx.c_len = newRecIdx;
68891+
68892+
ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
68893+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
68894+
68895+
wolfSSL_free(ssl_c);
68896+
wolfSSL_free(ssl_s);
68897+
wolfSSL_CTX_free(ctx_c);
68898+
wolfSSL_CTX_free(ctx_s);
68899+
#endif
68900+
return EXPECT_RESULT();
68901+
}
68902+
6885568903
/*----------------------------------------------------------------------------*
6885668904
| Main
6885768905
*----------------------------------------------------------------------------*/
@@ -70152,6 +70200,7 @@ TEST_CASE testCases[] = {
7015270200
TEST_DECL(test_dtls_empty_keyshare_with_cookie),
7015370201
TEST_DECL(test_tls13_pq_groups),
7015470202
TEST_DECL(test_tls13_early_data),
70203+
TEST_DECL(test_tls_multi_handshakes_one_record),
7015570204
/* This test needs to stay at the end to clean up any caches allocated. */
7015670205
TEST_DECL(test_wolfSSL_Cleanup)
7015770206
};

0 commit comments

Comments
 (0)