Skip to content

Commit

Permalink
Merge pull request #7626 from lealem47/parseServerHello
Browse files Browse the repository at this point in the history
Improved fix for TLS1.3 to TLS1.2 client downgrade
  • Loading branch information
JacobBarthelmeh authored Jun 6, 2024
2 parents c822303 + 5a1ac27 commit d09f955
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 11 deletions.
20 changes: 9 additions & 11 deletions src/tls13.c
Original file line number Diff line number Diff line change
Expand Up @@ -5298,13 +5298,6 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
return ret;
}
#endif /* WOLFSSL_DTLS13 */

#ifndef WOLFSSL_NO_TLS12
return DoServerHello(ssl, input, inOutIdx, helloSz);
#else
SendAlert(ssl, alert_fatal, wolfssl_alert_protocol_version);
return VERSION_ERROR;
#endif
}
}

Expand All @@ -5330,8 +5323,9 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
/* restore message type */
*extMsgType = args->extMsgType;

if (args->totalExtSz > 0) {
/* Parse and handle extensions. */
/* Parse and handle extensions, unless lower than TLS1.3. In that case,
* extensions will be parsed in DoServerHello. */
if (args->totalExtSz > 0 && IsAtLeastTLSv1_3(ssl->version)) {
ret = TLSX_Parse(ssl, input + args->idx, args->totalExtSz,
*extMsgType, NULL);
if (ret != 0) {
Expand All @@ -5350,7 +5344,9 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
ssl->msgsReceived.got_hello_retry_request = 1;
ssl->msgsReceived.got_server_hello = 0;
}
}

if (args->totalExtSz > 0) {
args->idx += args->totalExtSz;
}

Expand All @@ -5359,7 +5355,9 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
DtlsCIDOnExtensionsParsed(ssl);
#endif /* WOLFSSL_DTLS_CID */

*inOutIdx = args->idx;
if (IsAtLeastTLSv1_3(ssl->version)) {
*inOutIdx = args->idx;
}

ssl->options.serverState = SERVER_HELLO_COMPLETE;

Expand Down Expand Up @@ -5403,7 +5401,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
else
ssl->chVersion.minor = TLSv1_2_MINOR;
/* Complete TLS v1.2 processing of ServerHello. */
ret = CompleteServerHello(ssl);
ret = DoServerHello(ssl, input, inOutIdx, helloSz);
#else
WOLFSSL_MSG("Client using higher version, fatal error");
WOLFSSL_ERROR_VERBOSE(VERSION_ERROR);
Expand Down
62 changes: 62 additions & 0 deletions tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -68416,6 +68416,67 @@ static int test_extra_alerts_wrong_cs(void)
}
#endif

#if defined(WOLFSSL_TLS13) && !defined(WOLFSSL_NO_TLS12) && \
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES)

#define TEST_CS_DOWNGRADE_CLIENT "ECDHE-RSA-AES256-GCM-SHA384"

byte test_wrong_cs_downgrade_sh[] = {
0x16, 0x03, 0x03, 0x00, 0x56, 0x02, 0x00, 0x00, 0x52, 0x03, 0x03, 0x10,
0x2c, 0x88, 0xd9, 0x7a, 0x23, 0xc9, 0xbd, 0x11, 0x3b, 0x64, 0x24, 0xab,
0x5b, 0x45, 0x33, 0xf6, 0x2c, 0x34, 0xe4, 0xcf, 0xf4, 0x78, 0xc8, 0x62,
0x06, 0xc7, 0xe5, 0x30, 0x39, 0xbf, 0xa1, 0x20, 0xa3, 0x06, 0x74, 0xc3,
0xa9, 0x74, 0x52, 0x8a, 0xfb, 0xae, 0xf0, 0xd8, 0x6f, 0xb2, 0x9d, 0xfe,
0x78, 0xf0, 0x3f, 0x51, 0x8f, 0x9c, 0xcf, 0xbe, 0x61, 0x43, 0x9d, 0xf8,
0x85, 0xe5, 0x2f, 0x54,
0xc0, 0x2f, /* ECDHE-RSA-AES128-GCM-SHA256 */
0x00, 0x00, 0x0a, 0x00, 0x0b, 0x00,
0x02, 0x01, 0x00, 0x00, 0x17, 0x00, 0x00
};

static int test_wrong_cs_downgrade(void)
{
EXPECT_DECLS;
#ifdef BUILD_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL;
WOLFSSL *ssl_c = NULL;

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL,
wolfSSLv23_client_method, NULL), 0);

ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, TEST_CS_DOWNGRADE_CLIENT),
WOLFSSL_SUCCESS);

/* CH */
ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS);
ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
WOLFSSL_ERROR_WANT_READ);

/* consume CH */
test_ctx.s_len = 0;
/* inject SH */
XMEMCPY(test_ctx.c_buff, test_wrong_cs_downgrade_sh,
sizeof(test_wrong_cs_downgrade_sh));
test_ctx.c_len = sizeof(test_wrong_cs_downgrade_sh);

ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS);
ExpectIntNE(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR),
WOLFSSL_ERROR_WANT_READ);

wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
#endif
return EXPECT_RESULT();
}
#else
static int test_wrong_cs_downgrade(void)
{
return TEST_SKIPPED;
}
#endif

#if !defined(WOLFSSL_NO_TLS12) && defined(WOLFSSL_EXTRA_ALERTS) && \
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_SP_MATH)

Expand Down Expand Up @@ -74020,6 +74081,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_ticket_nonce_malloc),
#endif
TEST_DECL(test_ticket_ret_create),
TEST_DECL(test_wrong_cs_downgrade),
TEST_DECL(test_extra_alerts_wrong_cs),
TEST_DECL(test_extra_alerts_skip_hs),
TEST_DECL(test_extra_alerts_bad_psk),
Expand Down

0 comments on commit d09f955

Please sign in to comment.