From a29db7da2ee19ba2b8d43c114fdb69af3780d730 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Thu, 30 Nov 2023 14:06:14 +0800 Subject: [PATCH 1/7] tls13: early_data: cli: assign ciphersuite properly When early_data extension is enabled and sent in ClientHello, the client does not know if the server will accept early data and select the first proposed pre-shared key with a ciphersuite that is different from the ciphersuite associated to the selected pre-shared key. To address aforementioned case, we do associated verification when parsing early_data ext in EncryptedExtensions. Therefore we have to assign the ciphersuite in current handshake to session_negotiate later than the associated verification. This won't impact decryption of EncryptedExtensions since we compute handshake keys by the ciphersuite in handshake not via the one in session_negotiate. Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 44814b99f..d9a4b3e09 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1925,7 +1925,6 @@ static int ssl_tls13_postprocess_server_hello(mbedtls_ssl_context *ssl) mbedtls_ssl_set_inbound_transform(ssl, handshake->transform_handshake); MBEDTLS_SSL_DEBUG_MSG(1, ("Switch to handshake keys for inbound traffic")); - ssl->session_negotiate->ciphersuite = handshake->ciphersuite_info->id; ssl->session_in = ssl->session_negotiate; cleanup: @@ -2203,6 +2202,20 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) } #endif + /* + * When early_data extension is enabled and sent in ClientHello, the client + * does not know if the server will accept early data and select the first + * proposed pre-shared key with a ciphersuite that is different from the + * ciphersuite associated to the selected pre-shared key. To address + * aforementioned case, we do associated verification when parsing + * early_data ext in EncryptedExtensions. Therefore we have to assign + * the ciphersuite in current handshake to session_negotiate later than + * the associated verification. This won't impact decryption of + * EncryptedExtensions since we compute handshake keys by the ciphersuite + * in handshake not via the one in session_negotiate. + */ + ssl->session_negotiate->ciphersuite = handshake->ciphersuite_info->id; + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, buf_len)); From f4bad42670c60dbbd7c16ce16e91c0e377536941 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Thu, 30 Nov 2023 15:36:43 +0800 Subject: [PATCH 2/7] itls13: early_data: cli: improve comment Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index d9a4b3e09..bdb34247d 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2184,9 +2184,14 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) * - The selected cipher suite * - The selected ALPN [RFC7301] protocol, if any * - * We check here that when early data is involved the server - * selected the cipher suite associated to the pre-shared key - * as it must have. + * When parsing EncryptedExtensions, the client does not know if + * the server will accept early data and select the first proposed + * pre-shared key with a cipher suite that is different from the + * cipher suite associated to the selected pre-shared key. To address + * aforementioned case, when early data is involved, we check: + * - the selected pre-shared key is the first proposed one + * - the selected cipher suite same as the one associated with the + * pre-shared key. */ if (handshake->selected_identity != 0 || handshake->ciphersuite_info->id != @@ -2203,16 +2208,14 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) #endif /* - * When early_data extension is enabled and sent in ClientHello, the client - * does not know if the server will accept early data and select the first - * proposed pre-shared key with a ciphersuite that is different from the - * ciphersuite associated to the selected pre-shared key. To address - * aforementioned case, we do associated verification when parsing - * early_data ext in EncryptedExtensions. Therefore we have to assign - * the ciphersuite in current handshake to session_negotiate later than - * the associated verification. This won't impact decryption of - * EncryptedExtensions since we compute handshake keys by the ciphersuite - * in handshake not via the one in session_negotiate. + * Move `session_negotiate->ciphersuite` assignment here which after + * early data cipher suite check when receiving "early_data" extension + * in EncryptedExtensions. + * + * We compute transform_handshake by the cipher suite chosen from + * the server in `handshake`. `session_negotiate->ciphersuite` is the + * cipher suite negotiated in previous connection and it is not used for + * computing transform_handshake. */ ssl->session_negotiate->ciphersuite = handshake->ciphersuite_info->id; From 2bef7fbc8dd6e84b5e980747a4baed9552a42bf9 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 1 Dec 2023 12:02:54 +0800 Subject: [PATCH 3/7] tls13: early_data: cli: remove guard to fix failure Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index bdb34247d..2c76ad1c4 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2152,9 +2152,7 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) int ret; unsigned char *buf; size_t buf_len; -#if defined(MBEDTLS_SSL_EARLY_DATA) mbedtls_ssl_handshake_params *handshake = ssl->handshake; -#endif MBEDTLS_SSL_DEBUG_MSG(2, ("=> parse encrypted extensions")); From e72dfff1d68378da70757eeedb47e427c5f9186f Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 1 Dec 2023 12:05:12 +0800 Subject: [PATCH 4/7] tls13: early_data: cli: improve comment Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 2c76ad1c4..62e99cfec 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2207,8 +2207,7 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) /* * Move `session_negotiate->ciphersuite` assignment here which after - * early data cipher suite check when receiving "early_data" extension - * in EncryptedExtensions. + * early data cipher suite check. * * We compute transform_handshake by the cipher suite chosen from * the server in `handshake`. `session_negotiate->ciphersuite` is the From 03a00768c0dea338801e7966b263f5940151146f Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 1 Dec 2023 17:40:19 +0800 Subject: [PATCH 5/7] tls13: early_data: cli: improve comment This commit improves comment of the check for handshake parameters in Encrypted Extension. Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 62e99cfec..0cdb02b6c 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2182,14 +2182,15 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) * - The selected cipher suite * - The selected ALPN [RFC7301] protocol, if any * - * When parsing EncryptedExtensions, the client does not know if - * the server will accept early data and select the first proposed - * pre-shared key with a cipher suite that is different from the - * cipher suite associated to the selected pre-shared key. To address - * aforementioned case, when early data is involved, we check: - * - the selected pre-shared key is the first proposed one - * - the selected cipher suite same as the one associated with the - * pre-shared key. + * The server has sent an early data extension in its Encrypted + * Extension message thus accepted to receive early data. We + * check here that the additional constraints on the handshake + * parameters, when early data are exchanged, are met, + * namely: + * - the selected PSK for the handshake was the first one proposed + * by the client. + * - the selected ciphersuite for the handshake is the ciphersuite + * associated with the selected PSK. */ if (handshake->selected_identity != 0 || handshake->ciphersuite_info->id != From 9ae6534c201473aafb511fa8ec2b29817a88d00f Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 1 Dec 2023 17:46:06 +0800 Subject: [PATCH 6/7] tls13: early_data: cli: improve comment This commit improves comment of why we assign the identifier of the ciphersuite in handshake to `ssl->session_negotiate`. Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 0cdb02b6c..4273f38c0 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2207,13 +2207,14 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) #endif /* - * Move `session_negotiate->ciphersuite` assignment here which after - * early data cipher suite check. - * - * We compute transform_handshake by the cipher suite chosen from - * the server in `handshake`. `session_negotiate->ciphersuite` is the - * cipher suite negotiated in previous connection and it is not used for - * computing transform_handshake. + * In case the client has proposed a PSK associated with a ticket, + * `ssl->session_negotiate->ciphersuite` still contains at this point the + * identifier of the ciphersuite associated with the ticket. This is that + * way because, if an exchange of early data is agreed upon, we need + * it to check that the ciphersuite selected for the handshake is the + * ticket ciphersuite (see above). This information is not needed + * anymore thus we can now set it to the identifier of the ciphersuite + * used in this session under negotiation. */ ssl->session_negotiate->ciphersuite = handshake->ciphersuite_info->id; From 744577a429d3815169a590f5ca7d7b0118c99e9f Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 1 Dec 2023 22:33:59 +0800 Subject: [PATCH 7/7] tls13: early_data: cli: check a PSK has been selected in EE Signed-off-by: Yanray Wang --- library/ssl_tls13_client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 4273f38c0..1e1223e7e 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2187,12 +2187,14 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) * check here that the additional constraints on the handshake * parameters, when early data are exchanged, are met, * namely: + * - a PSK has been selected for the handshake * - the selected PSK for the handshake was the first one proposed * by the client. * - the selected ciphersuite for the handshake is the ciphersuite * associated with the selected PSK. */ - if (handshake->selected_identity != 0 || + if ((!mbedtls_ssl_tls13_key_exchange_mode_with_psk(ssl)) || + handshake->selected_identity != 0 || handshake->ciphersuite_info->id != ssl->session_negotiate->ciphersuite) {