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

OpenSSL 1.0.2* doesn’t preserve digests for SNI #4554

Closed
masaori335 opened this issue Oct 19, 2017 · 5 comments
Closed

OpenSSL 1.0.2* doesn’t preserve digests for SNI #4554

masaori335 opened this issue Oct 19, 2017 · 5 comments

Comments

@masaori335
Copy link

SHA1 is always used as digest of signature algorithm, when SSL_set_SSL_CTX() is called for SNI.
We only see this issue with OpenSSL 1.0.2*. We don’t see this issue with 1.0.1* or 1.1.0*.

This was originally fixed by 4e05aed for 1.0.1* and 14e14bf for master. The change for master was cherry-picked to 1.0.2* (1ce95f1) but it looks not enough.
IMO, it should copy CERT_PKEYs like the fix for 1.0.1* or negotiate signature algorithm from copied peer_sigalgs again.

@davidben
Copy link
Contributor

davidben commented Oct 19, 2017

Do you all call SSL_set_SSL_CTX in SSL_CTX_set_cert_cb's callback by any chance? I haven't tested it but, from source inspection, it looks like the issue might be that 1.0.2 calls tls1_set_server_sigalgs before the certificate decisions are actually finalized by cert_cb. Either the digest decisions should be copied over as you suggest, or those should happen in the other order.

Edit: Changing their order will probably upset SSL_check_chain. digest decisions should be copied over.

@masaori335
Copy link
Author

masaori335 commented Oct 19, 2017

Do you all call SSL_set_SSL_CTX in SSL_CTX_set_cert_cb's callback by any chance?

Yes, we are. This is originally reported to Apache Traffic Server. ( apache/trafficserver#2482 ) If you need details of how we call, please refer below.

@davidben
Copy link
Contributor

Ah okay. If my theory is right, I believe you'll find the OpenSSL bug no longer reproduces if you turn TS_USE_CERT_CB off and use the other callback. (But then you can't load the certificate asynchronously which seems to be why you all are using cert_cb.) That might explain why folks didn't notice the 1.0.2 backport was incomplete.

@masaori335
Copy link
Author

Right. I don't see this issue after I turned off TS_USE_CERT_CB.

davidben added a commit to davidben/openssl that referenced this issue Oct 23, 2017
1ce95f1 was incomplete and did not
handle the case when SSL_set_SSL_CTX was called from the cert_cb
callback rather than the SNI callback. The consequence is any server
using OpenSSL 1.0.2 and the cert_cb callback for SNI only ever signs a
weak digest, SHA-1, even when connecting to clients which use secure
ones.

Fix this and add regression tests for both this and the original issue.

Fixes openssl#4554.
levitte pushed a commit that referenced this issue Nov 1, 2017
1ce95f1 was incomplete and did not
handle the case when SSL_set_SSL_CTX was called from the cert_cb
callback rather than the SNI callback. The consequence is any server
using OpenSSL 1.0.2 and the cert_cb callback for SNI only ever signs a
weak digest, SHA-1, even when connecting to clients which use secure
ones.

Fix this and add regression tests for both this and the original issue.

Fixes #4554.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #4577)
@masaori335
Copy link
Author

This is fixed in 1.0.2m. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants