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-1206: ssl sni_hosts malformed_handshake_data #4260

Closed
OTP-Maintainer opened this issue Mar 26, 2020 · 13 comments
Closed

ERL-1206: ssl sni_hosts malformed_handshake_data #4260

OTP-Maintainer opened this issue Mar 26, 2020 · 13 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: essen
Affected version: OTP-23.0-rc2
Fixed in version: OTP-23.0
Component: ssl
Migrated from: https://bugs.erlang.org/browse/ERL-1206


Under a specific configuration of ssl we are getting the following system reports:

{code}
*** System report during acceptor_SUITE:ssl_sni_echo/1 in ssl 2020-03-25 18:27:00.926 ***
=NOTICE REPORT==== 25-Mar-2020::18:27:00.926666 ===
TLS server: In state hello at tls_handshake.erl:231 generated SERVER ALERT: Fatal - Handshake Failure
 - malformed_handshake_data

*** System report during acceptor_SUITE:ssl_sni_echo/1 in ssl 2020-03-25 18:27:00.935 ***
=NOTICE REPORT==== 25-Mar-2020::18:27:00.935747 ===
TLS client: In state hello received SERVER ALERT: Fatal - Handshake Failure
{code}

The server configuration is 
{noformat}
[{sni_hosts, [{"localhost", Opts}]}]
{noformat}
 where Opts has cert/key self-generated (using the old erl_make_certs) and also contains 
{noformat}
{versions, ['tlsv1.2']}
{noformat}
.

The client has no particular configuration.

Forcing the client to use TLS 1.2 "fixes" the problem. Tests that do not use sni_hosts but are otherwise configured the same do not have this issue.

This is the relevant test triggering this issue: https://github.com/ninenines/ranch/blob/master/test/acceptor_SUITE.erl#L596

If this is an actual bug and not my misunderstanding I can open a ticket.

Note that we've restricted the server to TLS 1.2 to fix other issues that I do not believe to be bugs in ssl. I haven't investigated it but since it gets us insufficient security errors and that the self-generated certificates use insecure algorithms I'm guessing it's probably the issue. We will switch from erl_make_certs to the more modern approach of generating certificates for tests in a future release.
@OTP-Maintainer
Copy link
Author

ingela said:

Is this still failing with master of today?

@OTP-Maintainer
Copy link
Author

essen said:

I'll try this afternoon.

@OTP-Maintainer
Copy link
Author

essen said:

Yes this is still failing.

@OTP-Maintainer
Copy link
Author

ingela said:

Ok, thanks for trying.  Is your test using RSA certs?  And would using ECDSA certs make the problem go away? If you need to generate ECDSA test certs you can use:

 
{code:java}
public_key:pkix_test_data(#{server_chain => #{root => [], intermediates => [[]], peer => []},client_chain => #{root => [], intermediates => [[]], peer => []}}). 
{code}
 

@OTP-Maintainer
Copy link
Author

essen said:

Yes it's using RSA certs. It's using the old erl_make_certs and we'll of course need to replace that very soon so thanks for the snippet.

Note that this particular issue only happens when sni_hosts is used, the same test with the certs given directly works fine. So even if the certs are a bit outdated I would not expect an error when using sni_hosts.

@OTP-Maintainer
Copy link
Author

ingela said:

Yes of course not. I really can not understand what this has to do with SNI. So I am curious if SNI with ECDSA certs and not enforcing TLS-1.2 will work. We have added RSASSA-PSS algorithms that  also have bearing on the TLS-1.2 protocol. We already fixed two interop issues related to this. Maybe there are more... however I think that then the  certs ought to cause a problem even without SNI.

@OTP-Maintainer
Copy link
Author

essen said:

I don't know what's really happening, however the tests with/without SNI can be seen here:

* https://github.com/ninenines/ranch/blob/master/test/acceptor_SUITE.erl#L546 without
* https://github.com/ninenines/ranch/blob/master/test/acceptor_SUITE.erl#L624 with

As you can see the only real difference is the use of sni_hosts, and of course the restriction to TLS 1.2 that i mentioned before as a workaround. The test can probably be extracted to run in the shell but I will be busy until Thursday.

@OTP-Maintainer
Copy link
Author

peterdmv said:

I think we have found the culprit, it is the 1024 bit RSA key in your certificate. We are still investigating if it is the length or the format of the key that causes this trouble. Possible workaround right now is to not allow the sha512 hash as signature algorithm by adding this option to ssl: 
 {signature_algs, [ecdsa_secp521r1_sha512,ecdsa_secp384r1_sha384,ecdsa_secp256r1_sha256, rsa_pss_pss_sha384, rsa_pss_pss_sha256, rsa_pss_rsae_sha384, rsa_pss_rsae_sha256, rsa_pkcs1_sha512, rsa_pkcs1_sha384, rsa_pkcs1_sha256, ecdsa_sha1, rsa_pkcs1_sha1

{sha512,ecdsa}, \{sha512,rsa}, \{sha384,ecdsa}, \{sha384,rsa}, \{sha256,ecdsa}, \{sha256,rsa}, \{sha224,ecdsa}, \{sha224,rsa}, \{sha,ecdsa}, \{sha,rsa},\{sha,dsa}]},
 (to reproduce this list: tls_v1:default_signature_algs(\{3,4}) – [rsa_pss_rsae_sha512, rsa_pss_pss_sha512])

@OTP-Maintainer
Copy link
Author

peterdmv said:

It looks like the openssl crypto lib rejects to generate an RSA-PSS signature with a salt length greater than 62 with this specific RSA key but the TLS 1.3 protocol requires it to have the same salt length as the hash algorithm that is 64 in case of sha512.

@OTP-Maintainer
Copy link
Author

essen said:

Can this error be detected and produce a more readable error? That would be an appropriate resolution of this ticket I think.

@OTP-Maintainer
Copy link
Author

peterdmv said:

https://github.com/openssl/openssl/blob/64e54bf5c6657bf423d3ba463f31095d598d94e7/crypto/rsa/rsa_pss.c#L89
if (sLen == RSA_PSS_SALTLEN_MAX) {
        sLen = emLen - hLen - 2;
 } else if (sLen > emLen - hLen - 2) { /* sLen can be small negative */
        RSAerr(RSA_F_RSA_VERIFY_PKCS1_PSS_MGF1, RSA_R_DATA_TOO_LARGE);
        goto err;
 }

According to openssl the salt is too large for this key.

@OTP-Maintainer
Copy link
Author

peterdmv said:

I think we can improve the signature algorithm selection in TLS 1.3 to handle this special case. We will check the size of the RSA key and filter those signature algorithms that are not compatible due to the size contraints in openssl crypto.

@OTP-Maintainer
Copy link
Author

peterdmv said:

A fix has been pushed to master. It will be released in OTP 23.
[https://github.com/erlang/otp/commit/0a6cf7f2605db284939232cc6902036aaf772c9a]

@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-23.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