On Tue, 8 Oct 2024 14:59:23 GMT, Artur Barashev <abaras...@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
