Re: RFR: 8270344: Session resumption errors [v4]
On Thu, 19 Aug 2021 19:51:36 GMT, Xue-Lei Andrew Fan wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> maxProtocolVersion refactoring
>
> test/jdk/sun/security/ssl/SSLSessionImpl/InvalidateSession.java line 60:
>
>> 58: System.setProperty("javax.net.ssl.keyStorePassword", passwd);
>> 59: System.setProperty("javax.net.ssl.trustStore", trustFilename);
>> 60: System.setProperty("javax.net.ssl.trustStorePassword", passwd);
>
> It is not recommended to use the binary key store files for JSSE test cases.
> Please refer to test/jdk/javax/net/ssl/templates/SSLContextTemplate.java for
> a replacement.
Good suggestion - done.
> test/jdk/sun/security/ssl/SSLSessionImpl/InvalidateSession.java line 173:
>
>> 171: }
>> 172: }
>> 173: }
>
> Is a new line required in the end of file? I see red symbol in the review
> board, I think the symbol may be generated by the GitHub.
not sure it matters, but added a new line
-
PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v4]
On Thu, 19 Aug 2021 13:07:59 GMT, Sean Coffey wrote:
>> Corner case where a session resumption can fail if the TLS server changes
>> supported protocol versions in relation to a cached SSLSession. This is
>> primarily an issue where the legacy TLS version is used in place of the
>> newer "supported_versions" TLS extension.
>
> Sean Coffey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> maxProtocolVersion refactoring
Changes requested by xuelei (Reviewer).
src/java.base/share/classes/sun/security/ssl/ClientHello.java line 547:
> 545: // handshake output stream, so that the output
> records
> 546: // (at the record layer) have the correct version
> 547: chc.setVersion(sessionVersion);
The removing of the call to "setVersion()" has an impact, I think. I think the
declaration of this method could be removed in HandshakeContext class, and set
the HandshakeContext.conContext.protocolVersion to
HandshakeContext.maximumActiveProtocol in the HandshakeContext.initialize()
method.
test/jdk/sun/security/ssl/SSLSessionImpl/InvalidateSession.java line 60:
> 58: System.setProperty("javax.net.ssl.keyStorePassword", passwd);
> 59: System.setProperty("javax.net.ssl.trustStore", trustFilename);
> 60: System.setProperty("javax.net.ssl.trustStorePassword", passwd);
It is not recommended to use the binary key store files for JSSE test cases.
Please refer to test/jdk/javax/net/ssl/templates/SSLContextTemplate.java for a
replacement.
test/jdk/sun/security/ssl/SSLSessionImpl/InvalidateSession.java line 173:
> 171: }
> 172: }
> 173: }
Is a new line required in the end of file? I see red symbol in the review
board, I think the symbol may be generated by the GitHub.
-
PR: https://git.openjdk.java.net/jdk/pull/5110
Re: RFR: 8270344: Session resumption errors [v4]
> Corner case where a session resumption can fail if the TLS server changes > supported protocol versions in relation to a cached SSLSession. This is > primarily an issue where the legacy TLS version is used in place of the newer > "supported_versions" TLS extension. Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: maxProtocolVersion refactoring - Changes: - all: https://git.openjdk.java.net/jdk/pull/5110/files - new: https://git.openjdk.java.net/jdk/pull/5110/files/86ae055f..c12551ad Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5110&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5110&range=02-03 Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5110.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5110/head:pull/5110 PR: https://git.openjdk.java.net/jdk/pull/5110
