CSR Review: 8208641: SSLSocket should throw an exception when configuring DTLS

[Xuelei Fan]

The "Default" algorithm defined in the SunJSSE provider is for TLS 
protocols.

Xuelei

On 8/8/2018 1:28 PM, Sean Mullan wrote:
> On 8/8/18 1:58 PM, Anthony Scarpino wrote:
>> I don't see where your example of getDefault() is any different than 
>> the current code path.  A user has to define a SSLContext when using 
>> getDefault() which as far as I can tell goes through the code path of 
>> SSLContext.get[Server]SocketFactory()
> 
> SSL{Server}SocketFactory.getDefault() says: "Otherwise, this method 
> returns SSLContext.getDefault().get{Server}SocketFactory(). If that call 
> fails, an inoperative factory is returned."
> 
> So it doesn't look like we have to do anything special for this case, 
> but as part of this fix, we should check/test that an inoperative 
> factory is returned if a DTLS context is the default.
> 
> --Sean
> 
>>
>> Tony
>>
>> On 08/08/2018 08:52 AM, Chris Hegarty wrote:
>>> +1 to everything Sean said, and just to add ...
>>>
>>> This change will prevent an SSLContext from giving out socket
>>> factories when it has been configured with DTLS. What about
>>> SSL[Server]SocketFactory::getDefault when the default SSL
>>> context is DTLS? I don’t see that getDefault can throw an UOE,
>>> should it?    Or should / could this be resolved at the socket
>>> factory level, when trying to create new sockets rather than at
>>> the factory retrieval time?
>>>
>>> -Chris.
>>>
>>>> On 8 Aug 2018, at 15:46, Sean Mullan <sean.mullan at oracle.com> wrote:
>>>>
>>>> On 8/7/18 8:05 PM, Xuelei Fan wrote:
>>>>> Hi Tony,
>>>>> The Specification section looks more like the implementation 
>>>>> details. We may change the implementation details again in the 
>>>>> future.  It may be more suitable to move it to the Solution 
>>>>> section, or just remove it.
>>>>
>>>> I agree, I would omit the diffs and put N/A for the Specification 
>>>> section.
>>>>
>>>>> In the Specification section, I may just say something like, "No 
>>>>> APIs changes.  The SunJSSE provider is updated to throw 
>>>>> UnsupportedOperationException if 
>>>>> SSLContext.SSLServerSocketFactory() or 
>>>>> SSLContext.SSLSocketFactory() get called for DTLS algorithms 
>>>>> SSLContext".
>>>>
>>>> I think the CSR should also say that the SunJSSE implementation of 
>>>> the engineGetSocketFactory and engineGetServerSocketFactory methods 
>>>> of SSLContextSpi have been changed to throw 
>>>> UnsupportedOperationException. That is the specific API behavior 
>>>> change.
>>>>
>>>> A few other comments on the CSR:
>>>>
>>>> "SSLContext.getSSLSocketFactory() and 
>>>> SSLContext.getSSLServerSocketFactory()"
>>>>
>>>> Typo, there is no "SSL" in the method names.
>>>>
>>>> The Compatibility Risk field has several typos ("there" -> "their", 
>>>> "how -> now", ...) and is a bit hard to understand. Wasn't TLS being 
>>>> used before instead of DTLS in certain scenarios? Because the API 
>>>> silently passed in certain cases, and now we will be throwing an 
>>>> Exception, maybe it might be better to say the risk is low.
>>>>
>>>> In the Summary section, it says "..., but it is not documented." I 
>>>> suggest opening a docs bug to improve the DTLS documentation so that 
>>>> it is more clear this scenario is not supported.
>>>>
>>>> I think the Interface Kind should be Java API since we are changing 
>>>> the behavior of an implementation of a standard API. I asked Joe 
>>>> Darcy this question yesterday, and he agreed.
>>>>
>>>> --Sean
>>>>
>>>>> Thanks,
>>>>> Xuelei
>>>>> On 8/7/2018 4:14 PM, Anthony Scarpino wrote:
>>>>>> Hi Xuelei,
>>>>>>
>>>>>> I have updated the csr and I believe I have addressed your comments.
>>>>>>
>>>>>> thanks
>>>>>>
>>>>>> Tony
>>>>>>
>>>>>> On 08/07/2018 01:43 PM, Xuelei Fan wrote:
>>>>>>> Hi Tony,
>>>>>>>
>>>>>>> Would you mind make it clear that this impact the JDK JSSE 
>>>>>>> provider only?  Third party's provider may be able to support 
>>>>>>> DTLS with SSLSocket.
>>>>>>>
>>>>>>> I think there may be no specification change.  The 
>>>>>>> SSLContext.getServerSocketFactory() and 
>>>>>>> SSLContext.getSocketFactory() defines the spec if the algorithm 
>>>>>>> is not supported by the underlying provider, 
>>>>>>> "UnsupportedOperationException - if the underlying provider does 
>>>>>>> not implement the operation.".  I may prefer to make it clear 
>>>>>>> that this is just a behavior change of the JDK JSSE provider 
>>>>>>> (SunJSSE).  The SunJSSE provider now throws 
>>>>>>> UnsupportedOperationException for creating 
>>>>>>> SSL(Server)SocketFactory with DTLS SSLContext, because it does 
>>>>>>> not actually support DTLS SSLSocket.
>>>>>>>
>>>>>>> In Solution section, "Throwing a UnsupportedOperationException 
>>>>>>> when getting a socket from the SSLServerSocketFactory or 
>>>>>>> SSLSocketFactory for DTLS."   I guess you meant, throwing a UOE 
>>>>>>> when calling SSLContext.getServerSocketFactory() and 
>>>>>>> SSLContext.getSocketFactory()?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Xuelei
>>>>>>>
>>>>>>> On 8/7/2018 12:17 PM, Anthony Scarpino wrote:
>>>>>>>> I need a review of a CSR for SSLSocket should throw an exception 
>>>>>>>> when configuring DTLS.  We are targeting this for 12 right now.
>>>>>>>>
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8209031
>>>>>>>>
>>>>>>>> thanks
>>>>>>>>
>>>>>>>> Tony
>>>>>>>>
>>>>>>
>>>
>>