TLS handshake failure when ciphers and versions are returned from sni_fun.

[eseres]

Describe the bug

Specifying the ciphers and versions SSL options for ssl:handshake/2 succeeds. Returning the same exact options from the sni_fun, fails.

To Reproduce

Build and run the following code and observe the output. With the first ssl:handshake(...) line included and the second one commented out, the openssl command will succeed.

% git clone https://github.com/michaelklishin/tls-gen.git
% make -C tls-gen/basic
% cp tls-gen/basic/result/server_$(hostname)_certificate.pem server_certificate.pem
% cp tls-gen/basic/result/server_$(hostname)_key.pem server_key.pem
% cp tls-gen/basic/result/ca_certificate.pem .
% erlc signature_issue.erl && erl -noshell -s signature_issue start -s init stop
% openssl version
% OpenSSL 3.0.3 3 May 2022 (Library: OpenSSL 3.0.3 3 May 2022)
% openssl s_client -connect localhost:9999 -CAfile ca_certificate.pem -servername $(hostname) -cipher DEFAULT@SECLEVEL=2

-module(signature_issue).
-export([start/0]).

start() ->
    ssl:start(),
    Versions = ['tlsv1.2' ,'tlsv1.1' ,'tlsv1'],
    Ciphers = lists:usort(ssl:filter_cipher_suites(lists:flatten([ssl:cipher_suites(all ,V) || V <- Versions]), [])),
    CiphersAndVersions = [{versions, Versions}, {ciphers, Ciphers}],
    SslOpts = [
      {active, true},
      {verify, verify_peer},
      {cacertfile, "./ca_certificate.pem"},
      {certfile,   "./server_certificate.pem"},
      {keyfile,    "./server_key.pem"},
      {log_level, error}
    ],
    {ok, ListenSocket} = gen_tcp:listen(9999, [{active, false}, binary]),
    {ok, AcceptSocket} = gen_tcp:accept(ListenSocket),
    case ssl:handshake(AcceptSocket, [{sni_fun, fun (_SNI) -> SslOpts end}] ++ CiphersAndVersions) of % works
  % case ssl:handshake(AcceptSocket, [{sni_fun, fun (_SNI) -> SslOpts ++ CiphersAndVersions end}]) of % fails
      {ok ,Socket} -> 
        {ok, [Protocol]} = ssl:connection_information(Socket, [protocol]),
        io:format("Success: ~p~n", [Protocol]),
        ssl:close(Socket);
      {error ,Error} -> io:format("Error: ~p~n", [Error])
    end,
    ok.

Now comment out the first ssl:handshake(...) line and include the second one instead. Observe in the openssl command output how the handshake now fails and no cipher will be indicated as selected for the SSL-Session:

SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : 0000
    Session-ID: 16EFE7A8788E7DB12AACB7DDD026044F0339E3D262F74376854FA29337C06371
    Session-ID-ctx: 
    Master-Key: 

The error message from openssl is:

0016210901000000:error:0A000172:SSL routines:tls12_check_peer_sigalg:wrong signature type:ssl/t1_lib.c:1567:

Expected behavior

It should not make a difference whether the ciphers and versions are specifying directly to ssl:handshake/2 or returned from the sni_fun option. The SSL handshake should succeed in both cases.

Affected versions

Erlang/OTP 24 [erts-12.3.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit] [dtrace]

Additional context

It seems that ciphers option can be returned from the sni_fun as long as the versions options is given at the top level directly to ssl:handshake/2. That configuration works.

Also, specifying {versions, ['tlsv1.2']} at the top level and returning {versions, ['tlsv1.2', 'tlsv1.1', tlsv1]} from the sni_fun and enforcing openssl to use 'tlsv1.1', will also fail - meaning that the versions returned from sni_fun will not override the options given at the top level. See corresponding ssl:handshake(...) line and openssl command below:

  case ssl:handshake(AcceptSocket, [{sni_fun, fun (_SNI) -> SslOpts ++ CiphersAndVersions end}] ++ [{versions, ['tlsv1.2']}]) of % fails

$ tls_signature_issue % openssl s_client -connect localhost:9999 -CAfile ca_certificate.pem -servername $(hostname) -no_tls1_2

Going in the opposite direction and providing all supported TLS versions at the top level and reducing them in the sni_fun appears to work.

[IngelaAndin]

I have reproduced the reported behaviour and will look into it next week

[IngelaAndin]

Please try my PR.

[IngelaAndin]

By the way, you would get the same set of ciphers as in your example doing only

Ciphers = ssl:filter_cipher_suites(ssl:cipher_suites(all ,HighestVersion), []).

All is an inclusive option (there is also the possibility to do an exclusive listing).
And the call to lists:usort will probably change the order in a way that you would not want if you for instance used it in the client or with the option of honoring the server order.

[csrl]

Thank you @IngelaAndin . Can this be included in a 24.x release as well?

[IngelaAndin]

I already prepared it to be piggybacked on the next OTP-24 patch, that is it will be included the next time an OTP-24 patch is made but it will not be the reason for making the patch.