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-908: Server-side function_clause in TLS 1.3 handshake when client advertises unsupported signature_algs #4316

Closed
OTP-Maintainer opened this issue Apr 9, 2019 · 4 comments
Labels
bug Issue is reported as a bug priority:medium team:PS Assigned to OTP team PS
Milestone

Comments

@OTP-Maintainer
Copy link

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


While testing RabbitMQ with Erlang/OTP 22 built from the Git {{master}} branch, an SSL client (OpenSSL's {{s_client}}) is rejected by the Erlang SSL server. The error returned by {{ssl:handshake/1}} is:

{code:erlang}
{error,
 {tls_alert,
  {handshake_failure,
   "received SERVER ALERT: Fatal - Handshake Failure - malformed_handshake_data"}}}
{code}

The same scenario with Erlang/OTP 21 works just fine.

The difference is that the TLS 1.3 code path is taken because the client advertises TLS 1.3 as the highest/first version it supports, and only Erlang/OTP 22 supports this new version.

With Erlang/OTP 22, the code crashes here:
{code:erlang}
%% In tls_handshake_1_3.erl

do_check_cert_sign_algo(SignAlgo, SignHash, [Scheme|T]) ->
    %% HERE:
    %% The call below throws a `function_clause` exception because `Scheme`
    %% is set to `unassigned`.
    {Hash, Sign, _Curve} = ssl_cipher:scheme_to_components(Scheme),
    case compare_sign_algos(SignAlgo, SignHash, Sign, Hash) of
        true ->
            ok;
        _Else ->
            do_check_cert_sign_algo(SignAlgo, SignHash, T)
    end.
{code}

If I understand correctly, {{unassigned}} is the fallback value when a signature scheme is unsupported in {{ssl_cipher:signature_scheme/1}}. However, {{ssl_cipher:scheme_to_components/1}} doesn't expect to receive that atom.

The same crash doesn't happen in the TLS 1.2 code path because the {{unassigned}} values are filtered before {{ssl_cipher:scheme_to_components/1}} is called, by {{ssl_handshake:available_signature_algs/4}} function.

I attached the patch I used locally to fix the problem, but I don't know if this is the best approach.

I also attached the escript and the self-signed certificates I used to start a dummy SSL server. The {{s_client}} command line used is available in a comment at the top of the escript.
@OTP-Maintainer
Copy link
Author

dumbbell said:

By the way, here are the values of {{ClientSignAlgs}} and {{ClientSignAlgsCert}} as passed to {{tls_handshake_1_3:check_cert_sign_algo/4}}:

{code:erlang}
ClientSignAlgs=[ecdsa_secp256r1_sha256,ecdsa_secp384r1_sha384,
                ecdsa_secp521r1_sha512,ed25519,ed448,rsa_pss_pss_sha256,
                rsa_pss_pss_sha384,rsa_pss_pss_sha512,rsa_pss_rsae_sha256,
                rsa_pss_rsae_sha384,rsa_pss_rsae_sha512,rsa_pkcs1_sha256,
                rsa_pkcs1_sha384,rsa_pkcs1_sha512,unassigned,ecdsa_sha1,
                unassigned,rsa_pkcs1_sha1,unassigned,unassigned,unassigned,
                unassigned,unassigned]

ClientSignAlgsCert=undefined
{code}

@OTP-Maintainer
Copy link
Author

peterdmv said:

I have uploaded a patch that should solve this issue with two other problems that I've found while checking the code. Can you please try it if it works? Thanks!

@OTP-Maintainer
Copy link
Author

dumbbell said:

I tested the patch, it works as expected now. Thank you!

@OTP-Maintainer
Copy link
Author

peterdmv said:

Fixed in OTP-22.

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

No branches or pull requests

1 participant