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

[Daniel Jeliński]

On Tue, 8 Oct 2024 14:59:23 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 incrementally with one additional commit since the last revision:
> 
>   Return null if there is no record we attempted to decode

test/jdk/javax/net/ssl/TLSv13/SSLSocketNoServerHelloClientShutdown.java line 122:

> 120:                     SSLEngineResult clientResult;
> 121:                     // Client-side plain TCP socket.
> 122:                     clientSocket = new Socket("localhost", port);

if you use a SocketChannel instead, you'll be able to deal with ByteBuffers directly instead of going through byte arrays.

test/jdk/javax/net/ssl/TLSv13/SSLSocketNoServerHelloClientShutdown.java line 134:

> 132: 
> 133:                     // Send client_hello, read and store all available messages from the server.
> 134:                     while (delayed.size() < 6) {

Could you drop the delayed queue, and instead send user_canceled/close_notify before you start reading from the socket? It would simplify the code and improve the coverage.

test/jdk/javax/net/ssl/TLSv13/SSLSocketNoServerHelloClientShutdown.java line 162:

> 160:                     // Send user_canceled and close_notify alerts to server. Server should process
> 161:                     // 2 unencrypted alerts and send its own close_notify alert back to the client.
> 162:                     ByteBuffer serverCloseNotify = clientWriteRead();

Make sure to read until EOF

test/jdk/javax/net/ssl/TLSv13/SSLSocketNoServerHelloClientShutdown.java line 196:

> 194:     }
> 195: 
> 196:     protected ByteBuffer clientWriteRead() throws IOException {

could you split the write and read methods?

test/jdk/javax/net/ssl/TLSv13/SSLSocketNoServerHelloClientShutdown.java line 204:

> 202:         inspectTlsBuffer(cTOs);
> 203: 
> 204:         byte[] outbound = new byte[cTOs.limit() - cTOs.position()];

limit() - position() is equivalent to remaining()

test/jdk/javax/net/ssl/TLSv13/SSLSocketNoServerHelloClientShutdown.java line 215:

> 213: 
> 214:         try {
> 215:             len = is.read(inbound);

this method might return a partial TLS record or multiple records. SSLEngine expects to receive the entire record in one buffer.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1793038277
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1793030301
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1793049415
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1793039563
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1793040046
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1793045659