The updated webrev: http://cr.openjdk.java.net/~xuelei/7093640/webrev.01/
Xuelei On 12/18/2013 2:11 PM, Xuelei Fan wrote: > 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 >>> >
