Looks fine to me.


On 9/22/2016 7:40 AM, Valerie Peng wrote:
Alright, I included the hex value of the version to the exception message.
In addition, one of the regression test was using 0x400 as the version
value and that has to be removed now that the version check has been


On 9/21/2016 10:49 AM, Seán Coffey wrote:
Hey Valerie,

There are a few calls in this code where an exception is thrown if a
bad version is received. It's code that already existed, but would you
mind enhancing the exceptions to print the version while editing the
code there ?
e.g. P11TlsKeyMaterialGenerator.java
+             throw new InvalidAlgorithmParameterException
+                    ("Only" + (supportSSLv3? " SSL 3.0,": "") +
+                     " TLS 1.0, and TLS 1.1 are supported");
On 21/09/16 18:28, Valerie Peng wrote:
Good catch, I have fixed all three and updated the webrev:

Thanks for the prompt review~

On 9/20/2016 8:11 PM, Xuelei Fan wrote:
There is a bug in the previous code. "&&" should be replaced with "||".
-   (version < 0x0300) && (version > 0x0302)
+   (version < 0x0300) || (version > 0x0302)

The other two have the same issues.  Otherwise, looks fine to me.

BTW, if client request to negotiate SSLv3, the server may not be
able to select other crypto provider that supports SSLv3 at
present.  We may want a further enhancement later.  As SSLv3 is
fading out, this enhancement may be not our priority.  I filed a P3
RFE (JDK-8166425) for the tracking.


On 9/20/2016 8:31 AM, Valerie Peng wrote:

Could you please help reviewing this change?

There are quite a few test failures on Solaris 12 due to the
removal of
Solaris PKCS11 SSL3 mechanisms which SunPKCS11 provider assume to be
always present. I updated relevant classes as well as regression tests
to skip SSL3 testing when the support isn't there.

Bug: https://bugs.openjdk.java.net/browse/JDK-8136355
Webrev: http://cr.openjdk.java.net/~valeriep/8136355/webrev.00/


Reply via email to