Skip to content
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

ERL-910: TLS 1.3: Erlang client hangs during handshake with Erlang server #3704

Closed
OTP-Maintainer opened this issue Apr 11, 2019 · 3 comments
Labels
not a bug Issue is determined as not a bug by OTP priority:medium team:PS Assigned to OTP team PS

Comments

@OTP-Maintainer
Copy link

Original reporter: dumbbell
Affected version: OTP-22.0
Component: ssl
Migrated from: https://bugs.erlang.org/browse/ERL-910


With ERL-908 fixed, we still hit issues when testing RabbitMQ with Erlang 22 built from the Git {{master}} branch.

An Erlang client hangs when trying to connect to an Erlang server using TLS 1.3. I'm going to attach:
* the test script I used to start the server and the client; it configures dbg to track calls into the SSL application
* the self-signed TLS certificates/keys
* the output of dbg
* a network capture

The client hangs in {{ssl:connect()}} or {{ssl:handshake()}} (when usign {{gen_tcp}} to open the underlying TCP connection) because the state machine doesn't complete the handshake and never replies to the caller.

After spending quite some time reading the code, I see two problems:

The first one is that after handling the Server Hello record from the server, the client considers that the negotiated version is TLS 1.2: in {{tls_handshake:hello()}}, it only considers the {{Version}} field of the Server Hello ({{#server_hello.server_version}}), which is always TLS 1.2 starting from TLS 1.3, instead of the effective version stored in {{#server_hello_selected_version}}.

I tried the following patch to fix this:
{code:diff}
+++ b/lib/ssl/src/tls_handshake.erl
@@ -173,9 +173,17 @@ hello(#server_hello{server_version = Version, random = Random,
                    session_id = SessionId, extensions = HelloExt},
       #ssl_options{versions = SupportedVersions} = SslOpt,
       ConnectionStates0, Renegotiation) ->
-    case tls_record:is_acceptable_version(Version, SupportedVersions) of
+    EffectiveVersion = case HelloExt of
+                           #{server_hello_selected_version :=
+                             #server_hello_selected_version{
+                                selected_version = V}} ->
+                               V;
+                           _ ->
+                               Version
+                       end,
+    case tls_record:is_acceptable_version(EffectiveVersion, SupportedVersions) of
        true ->
-           handle_server_hello_extensions(Version, SessionId, Random, CipherSuite,
+           handle_server_hello_extensions(EffectiveVersion, SessionId, Random, CipherSuite,
                                           Compression, HelloExt, SslOpt,
                                            ConnectionStates0, Renegotiation);
        false ->
{code}

The second issue is that after Server Hello, the server sends the remaining handshake records wrapped inside Application Data records and the state machine doesn't seem to handle that: it calls {{ssl_connection:read_application_data()}} which, if I understand correctly, sends the data to the process owning the socket. Therefore those records are never handled by the state machine. This includes the Server Hello Done record, which explains why the state machine never replies to the initial caller.

I tried to prepare a patch but couldn't, my knowledge of the SSL application code is way too limited. In particular, I don't know how to decipher the fragment in the Application Data record to pass it to {{tls_handshake:get_tls_handshake()}}.

To start the test script for the server side:
{code}
escript ./ssl.escript server 2>&1 | tee server.txt
{code}

To start the test script for the client side:
{code}
escript ./ssl.escript client2 2>&1 | tee client.txt
{code}
@OTP-Maintainer
Copy link
Author

peterdmv said:

The client side for TLS 1.3 is not yet implemented. Please check the [Standards Compliance| http://blog.erlang.org/cd/docs/master/lib/ssl-9.2.1/doc/html/standards_compliance.html#tls-1.3 ] for more information about the current capabilites.

@OTP-Maintainer
Copy link
Author

dumbbell said:

Ok, thank you for the pointer and sorry for the noise!

I will take a deeper look on the Ranch/RabbitMQ side to see why it tries to negotiate TLS 1.3...

@OTP-Maintainer
Copy link
Author

ingela said:

Will be implemented during OTP-22 

@OTP-Maintainer OTP-Maintainer added not a bug Issue is determined as not a bug by OTP team:PS Assigned to OTP team PS priority:medium labels Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not a bug Issue is determined as not a bug by OTP priority:medium team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

1 participant