On Fri, 20 Sep 2024 21:35:53 GMT, Artur Barashev <d...@openjdk.org> wrote:
>> https://bugs.openjdk.org/browse/JDK-8331682 > > Artur Barashev has updated the pull request incrementally with one additional > commit since the last revision: > > - Switch server to use plaintext after getting the unexpected plaintext > alert message during TLSv1.3 handshake > - Always send user_cancelled alert before close_notify alert during > handshake. This is actually a different issue which was discovered during > this fix. > - Update tests accordingly 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. 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. 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. 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 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 test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 202: > 200: runDelegatedTasks(serverEngine); > 201: > 202: cTOs.clear(); replace with cTOs.compact() test/jdk/javax/net/ssl/TLSv13/SSLEngineNoServerHelloClientShutdown.java line 235: > 233: runDelegatedTasks(clientEngine); > 234: > 235: sTOc.clear(); replace with sTOc.compact() ------------- PR Review: https://git.openjdk.org/jdk/pull/21043#pullrequestreview-2319660975 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1769483330 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1769483458 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1769483511 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1769483537 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1769483598 PR Review Comment: https://git.openjdk.org/jdk/pull/21043#discussion_r1769483652
