diff --git a/src/internal.c b/src/internal.c index c4fcac5742..00335f4fd9 100644 --- a/src/internal.c +++ b/src/internal.c @@ -32343,13 +32343,17 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) else { if (DSH_CheckSessionId(ssl)) { /* RFC 7627 5.3: resumed session EMS state must match the - * ServerHello; abort on mismatch. Stateless (session-ticket) - * resumption - e.g. EAP-FAST, whose PAC is a TLS ticket - binds - * the EMS state in the ticket and need not re-advertise the - * extension, so this applies only to session-ID resumption. */ + * ServerHello; abort on mismatch. Covers ticket resumption + * too. Skip only EAP-FAST (sessionSecretCb supplied the PAC + * ticket's secret in DoServerHello), whose synthetic session + * has no negotiated EMS state to compare. */ if ( + #ifdef HAVE_SECRET_CALLBACK + !(ssl->sessionSecretCb != NULL #ifdef HAVE_SESSION_TICKET - ssl->session->ticketLen == 0 && + && ssl->session->ticketLen > 0 + #endif + ) && #endif ssl->session->haveEMS != ssl->options.haveEMS) { WOLFSSL_MSG("Resumed session EMS state does not match " diff --git a/tests/api.c b/tests/api.c index 96718dc334..8cf5d32fb0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35072,6 +35072,7 @@ TEST_CASE testCases[] = { #endif TEST_DECL(test_tls_ems_downgrade), TEST_DECL(test_tls_ems_resumption_downgrade), + TEST_DECL(test_tls_ems_resumption_server_downgrade), TEST_DECL(test_tls12_chacha20_poly1305_bad_tag), TEST_DECL(test_tls13_null_cipher_bad_hmac), TEST_DECL(test_scr_verify_data_mismatch), diff --git a/tests/api/test_tls_ext.c b/tests/api/test_tls_ext.c index 7da0976eb4..a1ac92656e 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -153,6 +153,166 @@ int test_tls_ems_resumption_downgrade(void) } +#if !defined(WOLFSSL_NO_TLS12) && defined(HAVE_EXTENDED_MASTER) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_SESSION_CACHE) +/* Remove the extended_master_secret extension from the ServerHello record at + * the head of the server-to-client memio buffer, patching up the record, + * handshake and extension-block lengths so the message still parses. + * Returns 0 on success. */ +static int StripEmsFromServerHello(struct test_memio_ctx* test_ctx) +{ + byte* buf = test_ctx->c_buff; + int len = test_ctx->c_len; + int recLen; + int hsLen; + int extsLenIdx; + int extsLen; + int idx; + int extsEnd; + + /* Record header: type(1) version(2) length(2) */ + if (len < 5 || buf[0] != handshake) + return -1; + recLen = (buf[3] << 8) | buf[4]; + if (5 + recLen > len) + return -1; + /* Handshake header: type(1) length(3) */ + if (recLen < HANDSHAKE_HEADER_SZ || buf[5] != server_hello) + return -1; + hsLen = (buf[6] << 16) | (buf[7] << 8) | buf[8]; + /* Skip version(2), random(32) to the session ID length, then skip the + * session ID, cipher suite(2) and compression(1) to the extensions + * length. */ + extsLenIdx = 5 + HANDSHAKE_HEADER_SZ + OPAQUE16_LEN + RAN_LEN + + OPAQUE8_LEN + buf[5 + HANDSHAKE_HEADER_SZ + OPAQUE16_LEN + + RAN_LEN] + + OPAQUE16_LEN + OPAQUE8_LEN; + if (extsLenIdx + OPAQUE16_LEN > 5 + recLen) + return -1; + extsLen = (buf[extsLenIdx] << 8) | buf[extsLenIdx + 1]; + idx = extsLenIdx + OPAQUE16_LEN; + extsEnd = idx + extsLen; + if (extsEnd > 5 + recLen) + return -1; + while (idx + 4 <= extsEnd) { + int extType = (buf[idx] << 8) | buf[idx + 1]; + int extLen = (buf[idx + 2] << 8) | buf[idx + 3]; + int rmLen = 4 + extLen; + + if (idx + rmLen > extsEnd) + return -1; + if (extType == HELLO_EXT_EXTMS) { + XMEMMOVE(buf + idx, buf + idx + rmLen, + (size_t)(len - idx - rmLen)); + recLen -= rmLen; + hsLen -= rmLen; + extsLen -= rmLen; + buf[3] = (byte)(recLen >> 8); + buf[4] = (byte)recLen; + buf[6] = (byte)(hsLen >> 16); + buf[7] = (byte)(hsLen >> 8); + buf[8] = (byte)hsLen; + buf[extsLenIdx] = (byte)(extsLen >> 8); + buf[extsLenIdx + 1] = (byte)extsLen; + test_ctx->c_len -= rmLen; + /* The ServerHello record sits wholly inside the first buffered + * message. */ + test_ctx->c_msg_sizes[0] -= rmLen; + return 0; + } + idx += rmLen; + } + return -1; +} + +/* Full handshake with EMS, then resume and strip the EMS extension from the + * ServerHello in transit. The client must catch the downgrade and abort + * (RFC 7627 Section 5.3). useTicket selects session-ticket resumption + * instead of session-ID resumption. */ +static int test_tls_ems_resumption_server_downgrade_ex(int useTicket) +{ + EXPECT_DECLS; + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + WOLFSSL_SESSION *session = NULL; + +#ifndef HAVE_SESSION_TICKET + (void)useTicket; +#endif + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); +#ifdef HAVE_SESSION_TICKET + if (useTicket) + ExpectIntEQ(wolfSSL_UseSessionTicket(ssl_c), WOLFSSL_SUCCESS); +#endif + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + ExpectNotNull(session = wolfSSL_get1_session(ssl_c)); + ExpectTrue(session->haveEMS); +#ifdef HAVE_SESSION_TICKET + if (useTicket) + ExpectIntGT(session->ticketLen, 0); +#endif + + wolfSSL_free(ssl_c); + ssl_c = NULL; + wolfSSL_free(ssl_s); + ssl_s = NULL; + test_memio_clear_buffer(&test_ctx, 0); + test_memio_clear_buffer(&test_ctx, 1); + + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_session(ssl_c, session), WOLFSSL_SUCCESS); + + /* ClientHello */ + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + /* Server flight accepting the resumption */ + ExpectIntEQ(wolfSSL_accept(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + /* Drop EMS from the ServerHello to simulate a downgrading server. */ + ExpectIntEQ(StripEmsFromServerHello(&test_ctx), 0); + /* The client must refuse to resume without EMS. */ + ExpectIntEQ(wolfSSL_connect(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), + WC_NO_ERR_TRACE(EXT_MASTER_SECRET_NEEDED_E)); + + wolfSSL_SESSION_free(session); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + return EXPECT_RESULT(); +} +#endif + +/* F-5807: a server that resumes an EMS session but omits the + * extended_master_secret extension from its ServerHello must be rejected by + * the client with EXT_MASTER_SECRET_NEEDED_E (RFC 7627 Section 5.3), on both + * session-ID and session-ticket resumption. */ +int test_tls_ems_resumption_server_downgrade(void) +{ + EXPECT_DECLS; +#if !defined(WOLFSSL_NO_TLS12) && defined(HAVE_EXTENDED_MASTER) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_SESSION_CACHE) + ExpectIntEQ(test_tls_ems_resumption_server_downgrade_ex(0), TEST_SUCCESS); +#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) + ExpectIntEQ(test_tls_ems_resumption_server_downgrade_ex(1), TEST_SUCCESS); +#endif +#endif + return EXPECT_RESULT(); +} + + #if !defined(WOLFSSL_NO_TLS12) && \ defined(BUILD_TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256) && \ defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) diff --git a/tests/api/test_tls_ext.h b/tests/api/test_tls_ext.h index eda61aeb14..9a0d838606 100644 --- a/tests/api/test_tls_ext.h +++ b/tests/api/test_tls_ext.h @@ -24,6 +24,7 @@ int test_tls_ems_downgrade(void); int test_tls_ems_resumption_downgrade(void); +int test_tls_ems_resumption_server_downgrade(void); int test_tls12_chacha20_poly1305_bad_tag(void); int test_tls13_null_cipher_bad_hmac(void); int test_scr_verify_data_mismatch(void);