Skip to content

Fail to complete TLS handshake to httpstat.us by TLS1.3 #8669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BensonLiou opened this issue Jan 4, 2024 · 3 comments · Fixed by #8697
Closed

Fail to complete TLS handshake to httpstat.us by TLS1.3 #8669

BensonLiou opened this issue Jan 4, 2024 · 3 comments · Fixed by #8697
Labels
bug component-tls13 size-s Estimated task size: small (~2d)

Comments

@BensonLiou
Copy link
Contributor

Summary

Received "protocol version (70)" error from server while connecting by ssl_client2

System information

Mbed TLS version (number or commit id):
593e9cb
Operating system and version:
WLS2 ubuntu (5.15.90.1-microsoft-standard-WSL2)
Configuration (if not default, please attach mbedtls_config.h):

diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h
index a3e3f8347..e19030589 100644
--- a/include/mbedtls/mbedtls_config.h
+++ b/include/mbedtls/mbedtls_config.h
@@ -1774,7 +1774,7 @@
  *
  * Uncomment this macro to enable the support for TLS 1.3.
  */
-//#define MBEDTLS_SSL_PROTO_TLS1_3
+#define MBEDTLS_SSL_PROTO_TLS1_3

 /**
  * \def MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE
@@ -1796,7 +1796,7 @@
  * effect on the build.
  *
  */
-//#define MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE
+#define MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE

Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)
Additional environment information:
NA

Expected behavior

Complete the SSL handshake

Actual behavior

Received "protocol version (70)" error from server

Steps to reproduce

execute
programs/ssl/ssl_client2 server_name=httpstat.us server_port=443 auth_mode=none force_version=tls13

Additional information

There is no issue on openssl. I could connect to same server by
openssl s_client -connect httpstat.us:443 -tls1_3

I found the random number is different in two ClientHello. According to RFC8446, two ClientHello have to be same.
I could fix this issue with following patch

diff --git a/library/ssl_client.c b/library/ssl_client.c
index 7a7840662..d3303057a 100644
--- a/library/ssl_client.c
+++ b/library/ssl_client.c
@@ -802,10 +802,13 @@ static int ssl_prepare_client_hello(mbedtls_ssl_context *ssl)
         (ssl->handshake->cookie == NULL))
 #endif
     {
-        ret = ssl_generate_random(ssl);
-        if (ret != 0) {
-            MBEDTLS_SSL_DEBUG_RET(1, "Random bytes generation failed", ret);
-            return ret;
+        if (ssl->handshake->hello_retry_request_count == 0)
+        {
+            ret = ssl_generate_random(ssl);
+            if (ret != 0) {
+                MBEDTLS_SSL_DEBUG_RET(1, "Random bytes generation failed", ret);
+                return ret;
+            }
         }
     }

Please check if this patch is good enough

@BensonLiou BensonLiou changed the title Fail to connect to httpstat.us by TLS1.3 Fail to complete TLS handshake to httpstat.us by TLS1.3 Jan 5, 2024
@ronald-cron-arm
Copy link
Contributor

Thanks for the report. The random bytes in the second ClientHello should indeed be the same as the ones in the first ClientHello. We have to fix that. The proposed patch seems to go on the right direction but probably misses some "#if defined(MBEDTLS_SSL_PROTO_TLS1_3)" guard as the hello_retry_request_count field does not exist if TLS 1.3 is not enabled in the build.

@ronald-cron-arm ronald-cron-arm added bug component-tls13 size-s Estimated task size: small (~2d) labels Jan 9, 2024
@BensonLiou
Copy link
Contributor Author

Thanks for the suggestion. Will you create a PR or I should do it?

@tom-cosgrove-arm
Copy link
Contributor

If you would like to create a PR it would be welcome

BensonLiou added a commit to BensonLiou/mbedtls that referenced this issue Mar 14, 2024
   * In TLS 1.3 clients, fix an interoperability problem due to the client
     generating a new random after a HelloRetryRequest. Fixes Mbed-TLS#8669.

Signed-off-by: BensonLiou <[email protected]>
@github-project-automation github-project-automation bot moved this to [3.6] TLS 1.3 misc for LTS in Backlog for Mbed TLS Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-tls13 size-s Estimated task size: small (~2d)
Projects
Status: [3.6] TLS 1.3 misc for LTS
Development

Successfully merging a pull request may close this issue.

3 participants