Re: RFR JDK-8219989 : Retire the com.sun.net.ssl.internal.ssl.Provider name
Good to have a CSR. Please review:
CSR: https://bugs.openjdk.java.net/browse/JDK-8240974
Thanks,
Xuelei
On 3/12/2020 2:29 PM, Sean Mullan wrote:
The fix looks good, although I think you should also file a CSR since
this provider name was at one time documented and supported and you
mentioned that it was still supported in the previous CSR when the
com.sun.net.ssl package was removed [1].
Thanks,
Sean
[1] https://bugs.openjdk.java.net/browse/JDK-8218932
On 3/12/20 1:39 PM, Xuelei Fan wrote:
Hi,
Could I get the following update reviewed?
Bug#: https://bugs.openjdk.java.net/browse/JDK-8219989
Webrev: http://cr.openjdk.java.net/~xuelei/8219989/webrev.00/
Release note task: https://bugs.openjdk.java.net/browse/JDK-8240967
This is a request to remove the legacy SunJSSE provider name,
"com.sun.net.ssl.internal.ssl.Provider". The "SunJSSE" should be used
instead (for example, "SSLContext.getInstance("TLS", "SunJSSE")").
The use of the legacy provider name should be rare now. But please
let me know if you have concerns before March 19, 2019.
Thanks,
Xuelei
Re: RFR: 8238566: java.security.Provider$Service.supportsParameter() is racy
No problem, thanks for double checking. ;)
Valerie
On 3/12/2020 4:42 AM, Daniel Fuchs wrote:
Hi Valerie,
Please ignore my comment. Sorry for the noise.
I somehow clicked on the wrong webrev link :-(
best regards,
-- daniel
On 12/03/2020 11:35, Daniel Fuchs wrote:
Hi Valerie,
Given that hasKeyAttributes is already decelared as volatile,
may I suggest the following change that uses double locking?
It will avoid synchronizing in the happy case where `hasKeyAttributes`
has already been computed.
1924 private boolean hasKeyAttributes() {
1925 Boolean b = hasKeyAttributes;
if (b != null) return b;
synchronized (this) {
b = hasKeyAttributes;
1926 if (b != null) return b;
1927 String s;
1928 s = getAttribute("SupportedKeyFormats");
...
1948 hasKeyAttributes = b;
1949 }
1950 return b;
1951 }
best regards,
-- daniel
On 11/03/2020 20:31, Valerie Peng wrote:
Anyone can help reviewing this?
I addressed this by applying the double-checked-locking pattern for
lazy initialization of instance fields.
Existing code already have most of the code structured for the
pattern but misses the second check.
Bug: https://bugs.openjdk.java.net/browse/JDK-8238566
Webrev: http://cr.openjdk.java.net/~valeriep/8238566/webrev.00/
Thanks,
Valerie
Re: RFR JDK-8227024 : Remove the deprecated javax.security.cert APIs
Hi Xuelei, Changes look good. It may be worthwhile for the CSR to mention that there are existing methods which replace the to-be-removed ones so apps have a migration path. Thanks, Valerie On 3/12/2020 9:47 AM, Xuelei Fan wrote: Hi, Could I get the following update reviewed? CSR: https://bugs.openjdk.java.net/browse/JDK-8227395 Webrev: http://cr.openjdk.java.net/~xuelei/8227024/webrev.00/ The legacy javax.security.cert APIs and the dependent were initially deprecated in Java SE 9 and marked for removal in Java SE 13. Applications should use the java.security.cert APIs for now. This is a request to remove the deprecated javax.security.cert APIs. The use of the legacy APIs should be rare now. But please let me know if you have concerns before March 19, 2019. Thanks & Regards, Xuelei
Re: RFR JDK-8227024 : Remove the deprecated javax.security.cert APIs
Looks good to me. --Sean On 3/12/20 12:47 PM, Xuelei Fan wrote: Hi, Could I get the following update reviewed? CSR: https://bugs.openjdk.java.net/browse/JDK-8227395 Webrev: http://cr.openjdk.java.net/~xuelei/8227024/webrev.00/ The legacy javax.security.cert APIs and the dependent were initially deprecated in Java SE 9 and marked for removal in Java SE 13. Applications should use the java.security.cert APIs for now. This is a request to remove the deprecated javax.security.cert APIs. The use of the legacy APIs should be rare now. But please let me know if you have concerns before March 19, 2019. Thanks & Regards, Xuelei
Re: RFR JDK-8219989 : Retire the com.sun.net.ssl.internal.ssl.Provider name
The fix looks good, although I think you should also file a CSR since
this provider name was at one time documented and supported and you
mentioned that it was still supported in the previous CSR when the
com.sun.net.ssl package was removed [1].
Thanks,
Sean
[1] https://bugs.openjdk.java.net/browse/JDK-8218932
On 3/12/20 1:39 PM, Xuelei Fan wrote:
Hi,
Could I get the following update reviewed?
Bug#: https://bugs.openjdk.java.net/browse/JDK-8219989
Webrev: http://cr.openjdk.java.net/~xuelei/8219989/webrev.00/
Release note task: https://bugs.openjdk.java.net/browse/JDK-8240967
This is a request to remove the legacy SunJSSE provider name,
"com.sun.net.ssl.internal.ssl.Provider". The "SunJSSE" should be used
instead (for example, "SSLContext.getInstance("TLS", "SunJSSE")").
The use of the legacy provider name should be rare now. But please let
me know if you have concerns before March 19, 2019.
Thanks,
Xuelei
Re: RFR JDK-8227024 : Remove the deprecated javax.security.cert APIs
Hi Xuelei, Looks good to me. Hai-May > On Mar 12, 2020, at 10:34 AM, Xuelei Fan wrote: > > And the release note task: > https://bugs.openjdk.java.net/browse/JDK-8240968 > > Xuelei > > On 3/12/2020 9:47 AM, Xuelei Fan wrote: >> Hi, >> Could I get the following update reviewed? >> CSR: https://bugs.openjdk.java.net/browse/JDK-8227395 >> Webrev: http://cr.openjdk.java.net/~xuelei/8227024/webrev.00/ >> The legacy javax.security.cert APIs and the dependent were initially >> deprecated in Java SE 9 and marked for removal in Java SE 13. Applications >> should use the java.security.cert APIs for now. This is a request to remove >> the deprecated javax.security.cert APIs. >> The use of the legacy APIs should be rare now. But please let me know if >> you have concerns before March 19, 2019. >> Thanks & Regards, >> Xuelei
Re: RFR JDK-8219989 : Retire the com.sun.net.ssl.internal.ssl.Provider name
Hi Xuelei,
Looks good to me.
Hai-May
> On Mar 12, 2020, at 10:39 AM, Xuelei Fan wrote:
>
> Hi,
>
> Could I get the following update reviewed?
>
> Bug#: https://bugs.openjdk.java.net/browse/JDK-8219989
> Webrev: http://cr.openjdk.java.net/~xuelei/8219989/webrev.00/
> Release note task: https://bugs.openjdk.java.net/browse/JDK-8240967
>
> This is a request to remove the legacy SunJSSE provider name,
> "com.sun.net.ssl.internal.ssl.Provider". The "SunJSSE" should be used
> instead (for example, "SSLContext.getInstance("TLS", "SunJSSE")").
>
> The use of the legacy provider name should be rare now. But please let me
> know if you have concerns before March 19, 2019.
>
> Thanks,
> Xuelei
RFR JDK-8219989 : Retire the com.sun.net.ssl.internal.ssl.Provider name
Hi,
Could I get the following update reviewed?
Bug#: https://bugs.openjdk.java.net/browse/JDK-8219989
Webrev: http://cr.openjdk.java.net/~xuelei/8219989/webrev.00/
Release note task: https://bugs.openjdk.java.net/browse/JDK-8240967
This is a request to remove the legacy SunJSSE provider name,
"com.sun.net.ssl.internal.ssl.Provider". The "SunJSSE" should be used
instead (for example, "SSLContext.getInstance("TLS", "SunJSSE")").
The use of the legacy provider name should be rare now. But please let
me know if you have concerns before March 19, 2019.
Thanks,
Xuelei
Re: RFR JDK-8227024 : Remove the deprecated javax.security.cert APIs
And the release note task: https://bugs.openjdk.java.net/browse/JDK-8240968 Xuelei On 3/12/2020 9:47 AM, Xuelei Fan wrote: Hi, Could I get the following update reviewed? CSR: https://bugs.openjdk.java.net/browse/JDK-8227395 Webrev: http://cr.openjdk.java.net/~xuelei/8227024/webrev.00/ The legacy javax.security.cert APIs and the dependent were initially deprecated in Java SE 9 and marked for removal in Java SE 13. Applications should use the java.security.cert APIs for now. This is a request to remove the deprecated javax.security.cert APIs. The use of the legacy APIs should be rare now. But please let me know if you have concerns before March 19, 2019. Thanks & Regards, Xuelei
RFR JDK-8227024 : Remove the deprecated javax.security.cert APIs
Hi, Could I get the following update reviewed? CSR: https://bugs.openjdk.java.net/browse/JDK-8227395 Webrev: http://cr.openjdk.java.net/~xuelei/8227024/webrev.00/ The legacy javax.security.cert APIs and the dependent were initially deprecated in Java SE 9 and marked for removal in Java SE 13. Applications should use the java.security.cert APIs for now. This is a request to remove the deprecated javax.security.cert APIs. The use of the legacy APIs should be rare now. But please let me know if you have concerns before March 19, 2019. Thanks & Regards, Xuelei
Re: [RFR] 8166597: Crypto support for the EdDSA Signature Algorithm (JEP 339)
Here are some more comments, all minor. I will probably need one more round of review. - src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java 362 /* XDH does not require native implementation */ Looks like a cut-paste error. I think you can remove this line. - src/java.base/share/classes/java/security/interfaces/EdECPrivateKey.java - src/java.base/share/classes/java/security/interfaces/EdECPublicKey.java - src/java.base/share/classes/java/security/spec/EdECPoint.java In the class description of these 3 classes, you are missing a at the start of the 2nd paragraph. Otherwise when you generate javadoc, it won't create a new paragraph. - src/java.base/share/classes/java/security/spec/EdDSAParameterSpec.java 71 * @param context the context that is bound to the signature This should probably say that the parameter is copied. Also getContext() should say that it returns a copy (if not null). 92 * Get the context that should be used, if any Missing period at end of sentence. - src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdDSAPublicKeyImpl.java 58 // set the high-order bit of the .. of the ? 73 // construct the EdECPoint representation 74 This comment applies to code after line 74, so I think it would make more sense if you removed line 74 and added a blank line before 73. - src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdECOperations.java It would be useful to have some additional comments in this code. --Sean On 2/25/20 3:49 PM, Anthony Scarpino wrote: Hi I need a code review for the EdDSA support in JEP 339. The code builds on the existing java implemented constant time classes used for XDH and the NIST curves. The change also adds classes to the public API to support EdDSA operations. All information about the JEP is located at: JEP 339: https://bugs.openjdk.java.net/browse/JDK-8199231 CSR: https://bugs.openjdk.java.net/browse/JDK-8190219 webrev: https://cr.openjdk.java.net/~ascarpino/8166597/webrev/ thanks Tony
RFR 8240848: ArrayIndexOutOfBoundsException buf for TextCallbackHandler
Please take a review at http://cr.openjdk.java.net/~weijun/8240848/webrev.00/ The problem is that selection has a different meaning for a specified optionType (the option value) and an unspecified one (the option index). I also take this chance to make ConfirmationCallback more robust. Thanks, Max
Re: RFR: 8238566: java.security.Provider$Service.supportsParameter() is racy
Hi Valerie,
Please ignore my comment. Sorry for the noise.
I somehow clicked on the wrong webrev link :-(
best regards,
-- daniel
On 12/03/2020 11:35, Daniel Fuchs wrote:
Hi Valerie,
Given that hasKeyAttributes is already decelared as volatile,
may I suggest the following change that uses double locking?
It will avoid synchronizing in the happy case where `hasKeyAttributes`
has already been computed.
1924 private boolean hasKeyAttributes() {
1925 Boolean b = hasKeyAttributes;
if (b != null) return b;
synchronized (this) {
b = hasKeyAttributes;
1926 if (b != null) return b;
1927 String s;
1928 s = getAttribute("SupportedKeyFormats");
...
1948 hasKeyAttributes = b;
1949 }
1950 return b;
1951 }
best regards,
-- daniel
On 11/03/2020 20:31, Valerie Peng wrote:
Anyone can help reviewing this?
I addressed this by applying the double-checked-locking pattern for
lazy initialization of instance fields.
Existing code already have most of the code structured for the pattern
but misses the second check.
Bug: https://bugs.openjdk.java.net/browse/JDK-8238566
Webrev: http://cr.openjdk.java.net/~valeriep/8238566/webrev.00/
Thanks,
Valerie
Re: RFR: 8238566: java.security.Provider$Service.supportsParameter() is racy
Hi Valerie,
Given that hasKeyAttributes is already decelared as volatile,
may I suggest the following change that uses double locking?
It will avoid synchronizing in the happy case where `hasKeyAttributes`
has already been computed.
1924 private boolean hasKeyAttributes() {
1925 Boolean b = hasKeyAttributes;
if (b != null) return b;
synchronized (this) {
b = hasKeyAttributes;
1926 if (b != null) return b;
1927 String s;
1928 s = getAttribute("SupportedKeyFormats");
...
1948 hasKeyAttributes = b;
1949 }
1950 return b;
1951 }
best regards,
-- daniel
On 11/03/2020 20:31, Valerie Peng wrote:
Anyone can help reviewing this?
I addressed this by applying the double-checked-locking pattern for lazy
initialization of instance fields.
Existing code already have most of the code structured for the pattern
but misses the second check.
Bug: https://bugs.openjdk.java.net/browse/JDK-8238566
Webrev: http://cr.openjdk.java.net/~valeriep/8238566/webrev.00/
Thanks,
Valerie
Re: 8237219: Disabling the native SunEC implementation
Another webrev update with Max's recent comments. https://cr.openjdk.java.net/~ascarpino/8237219/webrev.03 Also I still need a reviewer for the CSR. thanks Tony On 3/2/20 4:40 PM, Anthony Scarpino wrote: Hi I need a review of the CSR and webrev for disabling by default the native SunEC curves from the API. With the recent verification changes in JDK-8237218, SunJCE is long dependent on the native code for verifying the constant-time curves. This disabling can be undone with setting a system property, jdk.sunec.disableNative. I'm doing a simultaneous review as changes for one will likely affect the other. CSR: https://bugs.openjdk.java.net/browse/JDK-8238911 webrev: https://cr.openjdk.java.net/~ascarpino/8237219/ The curves affected are: secp112r1, secp112r2, secp128r1, secp128r2, secp160k1, secp160r1, secp160r2, secp192k1, secp192r1, secp224k1, secp224r1, secp256k1, sect113r1, sect113r2, sect131r1, sect131r2, sect163k1, sect163r1, sect163r2, sect193r1, sect193r2, sect233k1, sect233r1, sect239k1, sect283k1, sect283r1, sect409k1, sect409r1, sect571k1, sect571r1, X9.62 c2tnb191v1, X9.62 c2tnb191v2, X9.62 c2tnb191v3, X9.62 c2tnb239v1, X9.62 c2tnb239v2, X9.62 c2tnb239v3, X9.62 c2tnb359v1, X9.62 c2tnb431r1, X9.62 prime192v2, X9.62 prime192v3, X9.62 prime239v1, X9.62 prime239v2, X9.62 prime239v3, brainpoolP256r1 brainpoolP320r1, brainpoolP384r1, brainpoolP512r1 Tony
