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

Resolve some TLSv1.3 alert issues #6887

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@mattcaswell
Copy link
Member

mattcaswell commented Aug 7, 2018

Ensure that we write out alerts correctly after early_data. If we sent early_data and then received back an HRR, the enc_write_ctx was stale resulting in errors if an alert needed to be sent.

Thanks to Quarkslab for reporting this.

In any case it makes little sense to encrypt alerts using the client_early_traffic_secret, so we add special handling for alerts sent after early_data. All such alerts are sent in plaintext (until we're using the handshake traffic secret).

Additionally, on the server side at certain points in the handshake we could receive either a plaintext or
an encrypted alert from the client. We should tolerate both where appropriate.

Testing this is quite challenging. I've not been able to come up with tests for some of these corner cases, but I have tested the straight forward case of an unencrypted alert being sent where the server might normally expect an encrypted handshake message.

mattcaswell added some commits Aug 7, 2018

Ensure that we write out alerts correctly after early_data
If we sent early_data and then received back an HRR, the enc_write_ctx
was stale resulting in errors if an alert needed to be sent.

Thanks to Quarkslab for reporting this.

In any case it makes little sense to encrypt alerts using the
client_early_traffic_secret, so we add special handling for alerts sent
after early_data. All such alerts are sent in plaintext.
Tolerate encrypted or plaintext alerts
At certain points in the handshake we could receive either a plaintext or
an encrypted alert from the client. We should tolerate both where
appropriate.
Add a test for unencrypted alert
Test that a server can handle an unecrypted alert when normally the next
message is encrypted.

@richsalz richsalz added the ready label Aug 7, 2018

@mattcaswell mattcaswell added this to the 1.1.1 milestone Aug 8, 2018

@mattcaswell

This comment has been minimized.

Copy link
Member Author

mattcaswell commented Aug 8, 2018

Pushed. Thanks.

@mattcaswell mattcaswell closed this Aug 8, 2018

levitte pushed a commit that referenced this pull request Aug 8, 2018

Ensure that we write out alerts correctly after early_data
If we sent early_data and then received back an HRR, the enc_write_ctx
was stale resulting in errors if an alert needed to be sent.

Thanks to Quarkslab for reporting this.

In any case it makes little sense to encrypt alerts using the
client_early_traffic_secret, so we add special handling for alerts sent
after early_data. All such alerts are sent in plaintext.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #6887)

levitte pushed a commit that referenced this pull request Aug 8, 2018

Tolerate encrypted or plaintext alerts
At certain points in the handshake we could receive either a plaintext or
an encrypted alert from the client. We should tolerate both where
appropriate.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #6887)

levitte pushed a commit that referenced this pull request Aug 8, 2018

Add a test for unencrypted alert
Test that a server can handle an unecrypted alert when normally the next
message is encrypted.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #6887)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment