Hi Xuelei,
Please check the update:
http://cr.openjdk.java.net/~rhalade/8049432/webrev.02/
Please help to push it if OK.
Full comments:
8049432: Need new tests for testing TLS property
jdk.tls.client.protocols with various protocols and values
Reviewed-by: xuelei
Contributed-by: Zaiyao Liu <zaiyao....@oracle.com>
Thanks and Regards.
Kevin
于 2014/12/4 19:12, Xuelei Fan 写道:
On 12/4/2014 4:39 PM, zaiyao liu wrote:
Hi Xuelei,
From the document:
http://docs.oracle.com/javase/8/docs/technotes/guides/security/SunProviders.html#SunJSSEProvider
Please refer to:
http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#SSLContext
The |SunJSSE| provider supports the following |protocol| parameters:
Protocol Enabled by Default for Client Enabled by Default for Server
SSLv3 Yes Yes
TLSv1 Yes Yes
TLSv1.1 Yes Yes
TLSv1.2 Yes Yes
SSLv2Hello Footnote 1
<http://docs.oracle.com/javase/8/docs/technotes/guides/security/SunProviders.html#sslv2protonote>
No Yes
Also I got following error when set contextProtocol as SSL and TLS:
Caused by: java.lang.IllegalArgumentException: jdk.tls.client.protocols:
TLS is not a standard SSL protocol name
at
sun.security.ssl.SSLContextImpl$CustomizedSSLContext.<clinit>(SSLContextImpl.java:615)
What's your suggestion for test the "SSL" and "TLS"?
SSLContext protocol name is not SSL/TLS protocol name. Maybe confusing,
but they are two concepts. "SSL" and "TLS" are not SSL/TLS protocols,
they are default SSLContext algorithms. One cannot specify SSLContext
protocol to SSL/TLS protocol. SSLContext.getInstance("SSL") should
work. But "jdk.tls.client.protocols=SSL" will not work.
Xuelei
Thanks
Kevin
于 2014/12/4 15:16, Xuelei Fan 写道:
On 12/4/2014 1:55 PM, zaiyao liu wrote:
Hi Xuelei,
Thanks for careful review. please check the update:
http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8049432/webrev/
Looks fine to me.
I am not very clear about "4. Do you want to test the "SSL" and "TLS"
protocol of SSLContext ()?", Can you explain it?
"SSL" and "TLS" are two protocol algorithm for SSLContext.
SSLContext.getInstance("SSL");
SSLContext.getInstance("TLS");
Xuelei
Thanks again.
Kevin
于 2014/12/3 16:50, Xuelei Fan 写道:
Looks fine to me. Only a few minor comments:
1.
201 String[] expecteSupported_protos = new String[] {
202 "SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1",
"TLSv1.2"
203 };
This variable can be define as a static class variable:
public class TLSClientPropertyTest {
private String[] expecteSupportedProtos = new String[] {
"SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"
};
}
Per Java coding conversions, better to use "theVariableName" style,
rather than link with "-" or "_" (expecteSupported_protos vs
expecteSupportedProtos).
2.
60 String protocol;
I think, in the test, this is used as SSLContext protocol. It would be
nice if the variable name is instinctive. I may suggest to use
"contextProtocol".
The same for the "expectedProto" parameter name.
3.
67 expectedDefaultProtos = new String[] {
68 "TLSv1.1", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"
69 };
Is it intend to have duplicated "TLSv1.1"? It's OK to me, but better to
add a comment about why duplicated protocols are listed here.
4.
Do you want to test the "SSL" and "TLS" protocol of SSLContext ()?
Thanks,
Xuelei
On 12/3/2014 2:15 PM, zaiyao liu wrote:
Hi Xuelei,
Can you help to review this code change?
Thanks and Regards.
Kevin
于 2014/9/10 10:29, zaiyao liu 写道:
Hi All,
Please review the code change,the purpose of this fix is to address
following:
-Sets the property jdk.tls.client.protocols to one of this
protocols:SSLv3,TLSv1,TLSv1,TLSv1.1,TLSv1.2 and TLSV(invalid) or
removes this
property (if any),then validates the default, supported and current
protocols in the SSLContext.
Webrev: http://cr.openjdk.java.net/~tyan/kevin/JDK-8049432/webrev01/
<http://cr.openjdk.java.net/%7Etyan/kevin/JDK-8049432/webrev01/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8049432
Thanks
Kevin
