On Fri, 25 Oct 2024 22:00:05 GMT, Bradford Wetmore <wetm...@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
