Code Review Request: TLS 1.3 Implementation

Xuelei Fan xuelei.fan at oracle.com
Tue Jun 12 23:40:33 UTC 2018


On 6/12/2018 3:29 PM, Bradford Wetmore wrote:
> 
> 
> On 6/11/2018 6:36 PM, Xuelei Fan wrote:
>>>>>> SSLServerSocketImpl.java
>>>>>> ------------------------
>>>>>> 160:  You should allow multiple changes between server to client, 
>>>>>> and switch enabled protocols/ciphersuites accordingly.
>>>>>>
>>>> Yes, multiple changes are allowed.
>>>
>>> I didn't see a change here.  If you go from server (default) to 
>>> client, and then back again, shouldn't you reset to the original 
>>> server values? And set sslConfig.useClientMode()
>>>
>>> e.g.
>>>      sslServerSocket.setUseClientMode(true);
>>>      sslServerSocket.setUseClientMode(false);
>>>
>>> The current code won't change back to server.
>>>
>> I may miss something.  I think, it is able to change back.
>>
>> sslServerSocket.setUseClientMode(true) ->
>>    if it is server mode, set to use client mode; and if using default 
>> protocols, set to use client default protocols; if using default 
>> cipher suites, set to use client default cipher suites.  sslConfig is 
>> set to use client mode.
>>
>> sslServerSocket.setUseClientMode(false);
>>    if it is client mode, set to use server mode; and id using default 
>> protocols, set to use server default protocols; if using default 
>> cipher suites, set to use server default cipher suites.  sslConfig is 
>> set to use server mode.
> 
> You're right, I misread the code.  Thanks for bearing with me.
> 
> Can we move this sslConfig code (157-167) to a method in 
> SSLConfiguration instead?  The logic here in SSLServerSocketImpl uses 
> sslContext.* 8 times (e.g. 
> sslContext.getDefaultProtocolVersions(!useClientMode)).  This code is 
> very implementation-dependent on SSLConfiguration's internals, and thus 
> a good encapsulation candidate.
> 
Good point!

I will do it later.  The enhancement is tracked with:
     https://bugs.openjdk.java.net/browse/JDK-8204636

Thanks,
Xuelei



More information about the security-dev mailing list