Thank you for the code review. On 12/18/2013 12:27 PM, Bradford Wetmore wrote: > Hi Xuelei, > > I looked as several of the new tests but not all, and looked at the > existing tests plus new code. > > SSLContextImpl.java > =================== > > 227: As I mentioned in Instant Message tonight, I'm not sure this code > is needed. I think these values are being set as part of the > super.engineGetDefaultSSLParameters, so unless your change somehow > tweaks them, this can probably go. Thanks for checking this out. > The code is not needed. Removed.
> 631: Minor nit, you could tighten up the exception message a little: > > PROPERTY_NAME + ": " + protocols[i] + > " is not a standard TLS protocol name"; > OK. > IllegalProtocolProperty.java and others > ============================ > Minor nit, you won't be able to run this from the command line in case > someone wants to do so, I generally do a System.getProperty() on the > first line instead of a @run option. > It is easier to update the parameter, without need to looking the code. Let's use this style for now. Thanks, Xuelei > Brad > > > > On 12/17/2013 2:08 AM, Xuelei Fan wrote: >> Hi, >> >> This is a request to enabled TLS 1.2 for client-side default contexts. >> Please review this update. >> >> webrev: http://cr.openjdk.java.net/~xuelei/7093640/webrev.00/ >> >> We are still concern about the version intolerance issue with some older >> SSL/TLS server implementation. As a workaround, a new system property, >> "jdk.tls.client.protocols", is defined to configure the protocols in >> default contexts. >> >> By default, TLS 1.1 and TLS 1.2 (plus other supported and safe >> protocols) are enabled unless the system property is explicit configured >> and does not contain "TLSv1.1" or "TLSv1.2". >> >> The property string is a list of comma separated standard SSL protocol >> names. The syntax of the property string can be described as this Java >> BNF-style: >> ClientProtocols: >> ('"' SSLProtocolNames '"') | SSLProtocolNames >> SSLProtocolNames: >> SSLProtocolName { , SSLProtocolName } >> SSLProtocolName: >> (see below) >> >> The "SSLProtocolName" is the standard SSL protocol name as described in >> the "Java Cryptography Architecture Standard Algorithm Name >> Documentation". If the property value does not comply to the above >> syntax, or the specified value of SSLProtocolName is not a supported SSL >> protocol name, the instantiation of the SSLContext provider service (via >> SSLContext.getInstance() methods) may generate a >> java.security.NoSuchAlgorithmException. Please note that the protocol >> name is case-sensitive. >> >> If the system property is not set or is empty, the default enabled >> protocol setting in both client and server looks like: >> >> Protocol Enabled Enabled >> for Client for Server >> -------- ---------- ---------- >> SSLv3 Yes Yes >> TLSv1 Yes Yes >> TLSv1.1 Yes Yes >> TLSv1.2 Yes Yes >> SSLv2Hello No Yes >> >> >> If the system property is set to "TLSv1,TLSv1.1", the default enabled >> protocol setting in both client and server looks like: >> >> Protocol Enabled Enabled >> for Client for Server >> -------- ---------- ---------- >> SSLv3 No Yes >> TLSv1 Yes Yes >> TLSv1.1 Yes Yes >> TLSv1.2 No Yes >> SSLv2Hello No Yes >> >> This update does not impact the API specification of JSSE, JSSE server >> side and third party's provider. >> >> Thanks, >> Xuelei >>
