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

[Artur Barashev]

On Sat, 21 Sep 2024 06:12:52 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

> I don't think we should worry too much about the server->client direction. If the client cancels the handshake, I don't expect it to handle the delayed DHE in ServerHello, which would be necessary to process the remainder of the server flight.

Actually when the client receives the delayed messages from the server it switches to encrypted mode just fine.

> src/java.base/share/classes/sun/security/ssl/SSLTransport.java line 151:
> 
>> 149: 
>> 150:                     // Switch server to use plaintext.
>> 151:                     context.handshakeContext.conContext.outputRecord.changeWriteCiphers(
> 
> now this is plain wrong. Please revert. It won't help once you fix the test case, see my comments there.

Done

> test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 166:
> 
>> 164:         logEngineStatus(serverEngine, serverResult);
>> 165:         runDelegatedTasks(serverEngine);
>> 166:         sTOc.clear();  // SH packet went missing.  Timeout on Client.
> 
> As I mentioned, TLS runs on top of TCP. Data might be truncated, but may never be received out of order To simulate this, you should never use `clear`, you can only use an alternating sequence of `flip` and `compact`.
> Please remove this line.

Yes, you are right, all done.

> test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 174:
> 
>> 172:         logEngineStatus(serverEngine, serverResult);
>> 173:         runDelegatedTasks(serverEngine);
>> 174:         sTOc.clear();  // CCS packet went missing.  Timeout on Client.
> 
> remove this line too

Done

> test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 182:
> 
>> 180:         logEngineStatus(serverEngine, serverResult);
>> 181:         runDelegatedTasks(serverEngine);
>> 182:         sTOc.clear();  // EE/etc. packet went missing.  Timeout on Client.
> 
> remove this line too

Done

> test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 202:
> 
>> 200:         runDelegatedTasks(serverEngine);
>> 201: 
>> 202:         cTOs.clear();
> 
> replace with cTOs.compact()

Done

> test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 235:
> 
>> 233:         runDelegatedTasks(clientEngine);
>> 234: 
>> 235:         sTOc.clear();
> 
> replace with sTOc.compact()

Done

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

PR Comment: https://git.openjdk.org/jdk/pull/21043#issuecomment-2369026325
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1771888323
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1771888185
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1771887783
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1771887716
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1771887620
PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1771887502