A new proposal to add methods to HttpsURLConnection to access SSLSession

[Sean Mullan]

On 11/5/18 1:52 PM, Xuelei Fan wrote:
> Hi Chris and Sean,
> 
> There are a few feedback for the CSR approval.  I updated to use 
> Optional<SSLSession> for the returned value.  For more details, please 
> refer to:
>    https://bugs.openjdk.java.net/browse/JDK-8213161
>    http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/

I didn't see a test for SecureCacheResponse - is it possible? You could 
also add a test case for IllegalStateExc.

lines 108-113 of HttpsSession.java should be indented 4 more spaces. 
Otherwise looks ok to me.

--Sean

> 
> 
> Please let me know if you are OK with this change.
> 
> Thanks,
> Xuelei
> 
> On 11/2/2018 11:42 AM, Xuelei Fan wrote:
>> Thanks for the review and suggestions, Chris and Sean.
>>
>> I just finalized the CSR.
>>
>> Thanks,
>> Xuelei
>>
>> On 11/2/2018 5:58 AM, Chris Hegarty wrote:
>>> Thanks for the updates Xuelei, some minor comments inline..
>>>
>>>> On 1 Nov 2018, at 23:42, Xuelei Fan <xuelei.fan at oracle.com 
>>>> <mailto:xuelei.fan at oracle.com>> wrote:
>>>>
>>>> On 11/1/2018 11:24 AM, Sean Mullan wrote:
>>>>> On 10/31/18 11:52 AM, Chris Hegarty wrote:
>>>>>> Xuelei,
>>>>>>
>>>>>> On 30/10/18 20:55, Xuelei Fan wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> For the current HttpsURLConnection, there is not much security 
>>>>>>> parameters exposed in the public APIs.  An application may need 
>>>>>>> richer information for the underlying TLS connections, for 
>>>>>>> example the negotiated TLS protocol version.
>>>>>>>
>>>>>>> Please let me know if you have concerns to add a new method 
>>>>>>> HttpsURLConnection.getSSLSession() and deprecate the duplicated 
>>>>>>> methods, by the end of Nov. 2, 2018.
>>>>>>>
>>>>>>> Here is the proposal:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8213161
>>>>> Are there any security issues associated with returning the 
>>>>> SSLSession, since it is mutable?
>>>> It should be fine.  The update APIs of the session (invalidating, 
>>>> bind values) does not impact the connection.
>>>
>>> Alternatively, as is done in the new HTTP Client, an immutable
>>> SSLSession instance can be returned.
>>>
>>>>> +     *           SHOULD override this method with appropriate 
>>>>> implementation.
>>>>> s/appropriate/an appropriate/
>>>>> I would probably not capitalize "SHOULD" and just say "should". 
>>>>> "SHOULD" is more common in RFCs. I don't see that much in javadocs.
>>>>> +     * @implNote The JDK Reference Implementation supports this 
>>>>> operation.
>>>>> +     *           As an application may have to use this operation 
>>>>> for more
>>>>> +     *           security parameters, it is recommended to support 
>>>>> this
>>>>> +     *           operation in all implementations.
>>>>> I think it should be obvious that the JDK implementation would 
>>>>> override this method so not sure that first sentence is necessary. 
>>>>> The other sentence seems like it could be combined with the 
>>>>> previous sentence, ex:
>>>>> "Subclasses should override this method with an appropriate 
>>>>> implementation since an application may need to access additional 
>>>>> parameters associated with the SSL session."
>>>> Updated accordingly, in the CSR and webrev:
>>>> https://bugs.openjdk.java.net/browse/JDK-8213161
>>>
>>> The CSR looks good. I made a few minor edits to the verbiage
>>> and added myself as reviewer.
>>>
>>> The title will need to be updated to reflect the addition of the
>>> new method in SecureCacheResponse. Maybe:
>>>
>>> "Add SSLSession accessors to HttpsURLConnection and
>>> SecureCacheResponse"
>>>
>>> -Chris.
>>>
>>>
>>>