RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

[Rahul]

Updated patch after considering the impact of returning default parameters on the http client.
TLS versions earlier limited to 1.2 and above by client, now will support all versions(wrt the scenarios for this bug).
    
    Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
    Issue: https://bugs.openjdk.java.net/browse/JDK-8239594
    
    Webrev: http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.01/
    
    -- Rahul

On 27/03/2020, 16:23, "net-dev on behalf of Xuelei Fan" <net-dev-bounces at openjdk.java.net on behalf of xuelei.fan at oracle.com> wrote:

    On 3/27/2020 5:52 AM, Chris Hegarty wrote:
    > Xuelei,
    > 
    > Before commenting further on the interaction of the HTTP Client with various contorted configurations, I would like to get a better understanding of the `jdk.tls.client.protocols` property.
    > 
    > Is there a specification or other documentation describing `jdk.tls.client.protocols` ?
    > 
    See the jdk.tls.client.protocols line in table 'Table 8-3 System 
    Properties and Customized Items" in JSSE Reference Guides:
    
    "https://docs.oracle.com/en/java/javase/14/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-A41282C3-19A3-400A-A40F-86F4DA22ABA9
    
    For your quick reference, I copied the note here:
    
    ---------------
    Customized Item:
    Default handshaking protocols for TLS/DTLS clients.
    
    Notes:
    To enable specific SunJSSE protocols on the client, specify them in a 
    comma-separated list within quotation marks; all other supported 
    protocols are not enabled on the client
    For example,
    
         If jdk.tls.client.protocols="TLSv1,TLSv1.1", then the default 
    protocol settings on the client for TLSv1 and TLSv1.1 are enabled, while 
    SSLv3, TLSv1.2, TLSv1.3, and SSLv2Hello are not enabled
    
         If jdk.tls.client.protocols="DTLSv1.2" , then the protocol setting 
    on the client for DTLS1.2 is enabled, while DTLS1.0 is not enabled
    ---------------
    
    
    > It is my understanding that the property only affects the *default* protocol’s ( not the supported protocols ) of the *default* context. That is, the context returned by `SSLContext.getInstance("Default”)`,
    It is correct that the property impact the default SSLContext only.  The 
    default SSLContext instance could get from:
         SSLContext.getInstance("Default");
         SSLContext.getInstance("TLS");
         SSLContext.getInstance("DTLS");
    
    
    > and the protocol values returned by the following invocation on that context `getDefaultSSLParameters().getProtocols()`. Is this correct? If not, what does it do?
    Yes.
    
    Xuelei
    
    > -Chris.
    > 
    >> On 26 Mar 2020, at 16:58, Xuelei Fan <xuelei.fan at oracle.com> wrote:
    >>
    >> With this update, the logic looks like: if TLSv1.3 is not enabled in the SSLContext, use TLSv1.2 instead;  Otherwise, use TLSv1.3 and TLSv1.2.
    >>
    >> There may be a couple of issues:
    >> 1. TLSv1.2 may be not enabled, although TLSv1.3 is enabled.
    >> For example:
    >>    System.setProperty("jdk.tls.client.protocols", "TLSv1.3")
    >>    System.setProperty("jdk.tls.client.protocols", "TLSv1.1, TLSv1.0")
    >>
    >> 2. TLSv1.2 may be not supported in the SSLContext.
    >> For example:
    >>    SSLContext context = SSLContext.getInstance("DTLS");
    >>    HttpClient.newBuilder().sslContext(context)...
    >>
    >> 3. The application may not want to use TLS 1.2.
    >> For example:
    >>    System.setProperty("jdk.tls.client.protocols", "TLSv1.1, TLSv1.0")
    >>
    >> The System property may be shared by code other than httpclient.  So the setting may not consider the impact on httpclient.
    >>
    >> I may use enabled protocols only. If no TLSv1.2/TLSv1.3, I may use an empty protocol array, and test to see what happens in the httpclient implementation stack.
    >>
    >> Xuelei
    >>
    >> On 3/26/2020 9:28 AM, Sean Mullan wrote:
    >>> Cross-posting to security-dev as this involves TLS/SSL configuration.
    >>> --Sean
    >>> On 3/26/20 10:02 AM, rahul.r.yadav at oracle.com wrote:
    >>>> Hello,
    >>>>
    >>>> Request to have my fix reviewed for issues:
    >>>>
    >>>>       JDK-8239595 : ssl context version is not respected
    >>>>       JDK-8239594 : jdk.tls.client.protocols is not respected
    >>>>
    >>>> The fix updates jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
    >>>> to use ctx.getDefaultSSLParameters()instead of ctx.getSupportedSSLParameters(),
    >>>> as the latter does not respect the context parameters set by the user.
    >>>>
    >>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
    >>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8239594
    >>>>
    >>>> Webrev: http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/
    >>>>
    >>>> -- Rahul
    >