Ok, thanks. Looks like the “Default” case is not-an-issue. -Chris.
> On 8 Aug 2018, at 22:29, Xuelei Fan <xuelei....@oracle.com> wrote: > > 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.mul...@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 >>>>>>>>> >>>>>>> >>>> >>>