Handle TLS 1.3 closure alerts consistently #9566
Conversation
CT Test Results 2 files 66 suites 48m 7s ⏱️ Results for commit fe9e907. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
| {#ssl_tls{type = ?ALERT, | ||
| version = ?TLS_1_3, %% Internally use real version | ||
| fragment = <<?FATAL,?USER_CANCELED>>}, ConnectionStates0}; | ||
| fragment = <<?WARNING,ClosureAlert>>}, ConnectionStates0}; |
There was a problem hiding this comment.
I do not agree with this change. This only happens when the handshake is canceled before the TLS-1.3 handshake encryption has come in to play, which means that this would require a server to wait for a close notify or handshake timeout, and that could be used to try to DoS-attack. I think we should only change that we should not care which level was sent as long as it is one of WARNING or FATAL (otherwise it is not a valid packet), and we should keep fatal here so that this is always ends up as immediate closure.
There was a problem hiding this comment.
Note that although TLS-1.3 does not care about the level on protocol level we can use this legacy field to signal immediate closure here instead inventing a new mechanism to deal with this corner case.
RFC 8446 section 6.2 states a pre-connection user_canceled alert should be followed by a close_notify alert. In these states, these alerts may be sent in the clear.
This PR fixes the handling of both alerts by:
This PR also lowers the logging level of the user_canceled alert since this is a perfectly valid client action which should not spam the server log.