RFR: 8331682: Slow networks/Impatient clients can potentially send unencrypted TLSv1.3 alerts that won't parse on the server [v24]

[Artur Barashev]

On Fri, 25 Oct 2024 22:00:05 GMT, Bradford Wetmore <wetmore at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 1866:
>> 
>>> 1864:                         // In TLSv1.3 alert level can be ignored, we only get the alert.
>>> 1865:                         final String msg = "Unexpected plaintext alert received: "
>>> 1866:                                 + Alert.nameOf(bb.get(bb.position() + 1));
>> 
>> . I would also check the bb size, in case of IndexOutOfBoundsException.
>> . I may also dump the alert level.
>> . It may be not a plaintext alert.  From the context, I don't think we are sure of it.
>> 
>> Considering above, I may just dump the hex bytes for the remaining bytes in bb.  For example:
>>    "Unexpected alert message received [hex bytes]"
>
> 1. If we got through the `bb.remaining() <= tagSize` arm, we're not going to be encrypted.  I think we are safe here.
> 2. I would dump the alert level (fatal/warning) + reason.
> 3. `bb` size definitely needs to be checked.  e.g. if `bb.remaining() == 0`, the `bb.position() + 1` will IOOBE.

- Alert level is ignored in TLSv1.3, but we can sure include it it as well
- Why are we not sure that alert is plaintext? If alert is not a plaintext then contentType will be 23 and we throw BadPaddingException as we should
- About the bb size: the upstream code (SSLEngineInputRecord/SSLSocketInputRecord) actually makes sure those bytes are present. The above `if (contentType == ContentType.CHANGE_CIPHER_SPEC.id)` code also doesn't include the bb size check.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1817452625