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

[Bradford Wetmore]

On Fri, 25 Oct 2024 13:30:43 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

>> Check for unexpected plaintext alert message during TLSv1.3 handshake. This can happen if client doesn't receive ServerHello due to network timeout and tries to close the connection by sending an alert message.
>
> Artur Barashev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 28 additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8331682
>  - Use more appropriate exception with the alert description
>  - Update Copyright
>  - Update @library directive
>  - Merge branch 'master' into JDK-8331682
>  - Produce appropriate exception message. Update tests.
>  - Adjust line length
>  - Additional error checking
>  - Write and read to/from server in a single pass. Use SocketChannel.
>  - Return null if there is no record we attempted to decode
>  - ... and 18 more: https://git.openjdk.org/jdk/compare/22220c1b...aef08dd0

src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 1862:

> 1860: 
> 1861:                 if (bb.remaining() <= tagSize) {
> 1862:                     // Check for unexpected plaintext alert.

We generally try to keep the security/JSSE code to <=80 chars.  It makes it **MUCH** easier to read in side-by-side comparisons on github or IJ, so that you don't have to vertical-scroll.

src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 1872:

> 1870:                         }
> 1871: 
> 1872:                         throw new GeneralSecurityException(msg);

Why a `GeneralSecurityException` instead of `SSLHandshakeException`?

test/lib/jdk/test/lib/security/SecurityUtils.java line 130:

> 128:     }
> 129: 
> 130:     public static void inspectTlsBuffer(ByteBuffer buffer) throws IOException {

I'm not sure how useful the information provided by the call this really is, and whether it's worth introducing in a separate library.  Was the output really useful in your debugging?

test/lib/jdk/test/lib/security/SecurityUtils.java line 144:

> 142:             int contentLen = getInt16(packet);                 // pos: 3, 4
> 143: 
> 144:             System.err.printf(

If you keep, <= 80 chars

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1817229877
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1817253320
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1817340758
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1817259692