On Wed, 9 Apr 2025 03:28:40 GMT, Nibedita Jena <d...@openjdk.org> wrote:
> Session resumption without server side state was added under > [JDK-8211018](https://bugs.openjdk.org/browse/JDK-8211018). > While it is TLSv1.2 session resumption, the client hello message is being > parsed in SSLSessionImpl for each extensions. > > Customer has reported handshake failure and is reproducible locally with > exception NegativeArraySizeExceptions when there is ServerNameIndication with > size > 127. > According to RFC 3546, the host_name limit allowed is 255. > With a sample testcase when the host_name length is > 127, exception is > thrown: > javax.net.ssl|DEBUG|71|Thread-1|2025-04-06 17:13:07.278 > UTC|ClientHello.java:825|Negotiated protocol version: TLSv1.2 > javax.net.ssl|WARNING|71|Thread-1|2025-04-06 17:13:07.281 > UTC|SSLSocketImpl.java:1672|handling exception ( > "throwable" : { > java.lang.NegativeArraySizeException: -1 > at > java.base/sun.security.ssl.SSLSessionImpl.<init>(SSLSessionImpl.java:399) > at > java.base/sun.security.ssl.SessionTicketExtension$T12CHSessionTicketConsumer.consume(SessionTicketExtension.java:468) > > e.g. > int l = buf.get(); > b = new byte[l]; <-------------------- NegativeArraySizeException thrown > here when > 127 > > For TLSv1.3, its not an issue until length > 255. > > According to RFC 5077, PSK identity length allowed is <0..2^16-1> and so its > value conversion being taken care of under this change. > Master secret is allowed for 48 bytes - master_secret[48], shouldnt be an > issue. Changes requested by djelinski (Reviewer). src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java line 359: > 357: > 358: // PSK identity > 359: i = Byte.toUnsignedInt(buf.get()); Could you rewrite this constructor to use the helper methods from `sun.security.ssl.Record` instead of reading data from the buffer directly? test/jdk/sun/security/ssl/SSLSessionImpl/ResumeClientTLS12withSNI.java line 115: > 113: passphrase); > 114: > 115: SSLContext sslCtx = SSLContext.getInstance(sslProtocol); Use SSLContextTemplate methods to generate the context, if possible test/jdk/sun/security/ssl/SSLSessionImpl/ResumeClientTLS12withSNI.java line 268: > 266: clientEngine.setUseClientMode(true); > 267: SSLParameters cliSSLParams = clientEngine.getSSLParameters(); > 268: cliSSLParams.setServerNames(List.of(SNI_NAME)); you could also use `SNI_NAME` as the host parameter of the `createSSLEngine` method above; this way you wouldn't need to set it here. test/jdk/sun/security/ssl/SSLSessionImpl/ResumeClientTLS12withSNI.java line 392: > 390: System.out.println(this.sslc.getProvider().getName() + " " + > this.sslc.getProtocol() + " - Session resumption SUCCEEDED"); > 391: } else { > 392: System.out.println(this.sslc.getProvider().getName() + " " + > this.sslc.getProtocol() + " - Session resumption FAILED"); should the test fail if this happens? Either change this to throw an exception, or remove this method. test/jdk/sun/security/ssl/SSLSessionImpl/keystore_san.p12 line 1: > 1: 0��0�� *�H�� Can you use the SSLContextTemplate class instead? If not, please add a readme describing how this file can be regenerated. ------------- PR Review: https://git.openjdk.org/jdk/pull/24535#pullrequestreview-2752317840 PR Review Comment: https://git.openjdk.org/jdk/pull/24535#discussion_r2034632391 PR Review Comment: https://git.openjdk.org/jdk/pull/24535#discussion_r2034639617 PR Review Comment: https://git.openjdk.org/jdk/pull/24535#discussion_r2034645788 PR Review Comment: https://git.openjdk.org/jdk/pull/24535#discussion_r2034654626 PR Review Comment: https://git.openjdk.org/jdk/pull/24535#discussion_r2034624226
