Re: RFR 8199198: Remove unused functions in jdk.crypto.mscapi native code

2018-03-07 Thread Vincent Ryan
Hello Max, Your change looks fine to me. Thanks. > On 7 Mar 2018, at 02:19, Weijun Wang wrote: > > Please take a review at > > http://cr.openjdk.java.net/~weijun/8199198/webrev.00 > > I just removed the 3 unused functions. They were there from the beginning > (2005) but had never existed i

Re: hello, im a new contributor

2017-09-01 Thread Vincent Ryan
9/jdk9/jdk/file/51f5d60713b5/src/java.base/share/classes/java/util/jar/Manifest.java#l157 > > <http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/51f5d60713b5/src/java.base/share/classes/java/util/jar/Manifest.java#l157> > So I'll try to remove it first including the

Re: hello, im a new contributor

2017-09-01 Thread Vincent Ryan
Hello Philipp, I’m happy to sponsor your fix for JDK 10. Have you followed these steps: http://openjdk.java.net/contribute/ <http://openjdk.java.net/contribute/> ? Thanks. > On 1 Sep 2017, at 08:58, Vincent Ryan wrote: > > Moved to security-dev > > >> On 1 Sep 2

Re: hello, im a new contributor

2017-09-01 Thread Vincent Ryan
Moved to security-dev > On 1 Sep 2017, at 08:28, Philipp Kunz wrote: > > Hello everyone > > I have been developing with Java for around 17 years now and when I > encountered some bug I decided to attempt to fix it: > https://bugs.openjdk.java.net/browse/JDK-6695402. This also looks like it m

Re: [10] RFR 8173181: Empty string alias in KeyStore throws StringIndexOutOfBoundsException for getEntry()

2017-08-24 Thread Vincent Ryan
> On 24 Aug 2017, at 15:50, Weijun Wang wrote: > > 1. Maybe just check length > 1? Yes. I’ll make the change. > > 2. The test's @summary seems unrelated. An empty alias is supported in JKS. > > --Max > >> On Aug 24, 2017, at 10:27 PM, Vincent Ryan

[10] RFR 8173181: Empty string alias in KeyStore throws StringIndexOutOfBoundsException for getEntry()

2017-08-24 Thread Vincent Ryan
Please review this fix to correct support for an empty string alias in PKCS12 keystores. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8173181 Webrev: http://cr.openjdk.java.net/~vinnie/8173181/webrev.00/

Re: RFR: JDK-8159544: Remove deprecated classes in com.sun.security.auth.**

2017-08-18 Thread Vincent Ryan
Your changes look fine to me. Thanks. > On 17 Aug 2017, at 20:08, Sean Mullan wrote: > > Please review this JDK 10 change to remove the deprecated classes in > com.sun.security.auth.** that have been previously marked with > forRemoval=true in JDK 9. > > webrev: http://cr.openjdk.java.net/~m

Re: RFR[10] 8185620: MSCAPI test leaves too many entries in keystore

2017-08-14 Thread Vincent Ryan
Your fix looks good to me. Thanks. > On 14 Aug 2017, at 03:44, sha.ji...@oracle.com wrote: > > Please review this fix. > Thanks! > > John Jiang > > > On 07/08/2017 13:03, sha.ji...@oracle.com wrote: >> Hi, >> Test sun/security/mscapi/SmallPrimeExponentP.java adds some entries into >> Windows-

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-12 Thread Vincent Ryan
ve. > > latest webrev: http://cr.openjdk.java.net/~apetcher/8182999/webrev.02/ > <http://cr.openjdk.java.net/~apetcher/8182999/webrev.02/> > On 7/12/2017 10:42 AM, Vincent Ryan wrote: >> Looks fine to me too. >> >> We should investigate how best to support simi

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-12 Thread Vincent Ryan
Looks fine to me too. We should investigate how best to support similar behaviour for the SunPKCS11 provider. To track this issue I’ve filed a related bug 8184290 : SunPKCS11 throws ProviderException for unsupported curves > On 10 Jul 2017, a

Re: [10] RFR 8183509: keytool should not allow multiple commands

2017-07-07 Thread Vincent Ryan
Looks fine to me. > On 7 Jul 2017, at 15:55, Weijun Wang wrote: > > Updated at > > http://cr.openjdk.java.net/~weijun/8183509/webrev.01/ > > Changes: > > http://cr.openjdk.java.net/~weijun/8183509/webrev.01/interdiff.patch.html > > Thanks > Max > >> On Jul 7, 2017, at 8:00 PM, Sean Mull

[9] RFR 8181978: Keystore probing mechanism fails for large PKCS12 keystores

2017-06-13 Thread Vincent Ryan
Martin has reported a serious regression involving PKCS12 keystores in JDK 9. It affects large PKCS12 keystores loaded using the new KeyStore.getInstance(File, xxx) method. The error is due to a typo in the masks used by the keystore type detection mechanism. Bug: https://bugs.openjdk.java.net/b

Re: RFR 8181841: A TSA server returns timestamp with precision higher than milliseconds

2017-06-12 Thread Vincent Ryan
This approach looks fine to me given the limitation on the precision of Date. Just one issue: why remove the upper bound at l.277 in DerInputBuffer.java Thanks. > On 12 Jun 2017, at 05:22, Weijun Wang wrote: > > Please review this fix at > > http://cr.openjdk.java.net/~weijun/8181841/webrev

Re: RFR 8172244: AIOOBE in KeyStore.getCertificateAlias on Windows

2017-05-23 Thread Vincent Ryan
Your fix looks fine to me. Thanks. > On 22 May 2017, at 20:57, Adam Petcher wrote: > > This is a bug fix related to keys without certificates in the Windows key > store. When a key doesn't have a certificate, the native code will set the > list of certificates to an empty list. Some of the Jav

Re: RFR 8176457: Add verbose option to java.security.debug

2017-05-02 Thread Vincent Ryan
Your change looks fine to me Tony. Thanks. > On 3 May 2017, at 01:13, Anthony Scarpino wrote: > > I need a review for adding the verbose option to debugging so that the output > is not overwhelmed with unnecessary info. > > http://cr.openjdk.java.net/~ascarpino/8176457/webrev/ > > Tony >

Re: [10] RFR: 8175029: StackOverflowError in X509CRL and X509Certificate.verify(PublicKey, Provider)

2017-03-31 Thread Vincent Ryan
This fix looks fine to me. Thanks. > On 31 Mar 2017, at 16:34, Sean Mullan wrote: > > New webrev uploaded to address your comments: > http://cr.openjdk.java.net/~mullan/webrevs/8175029/webrev.01/ > > --Sean > > On 3/31/17 9:11 AM, Sean Mullan wrote: >> On 3/31/17 9:08 AM, Weijun Wang wrote:

Re: RFR: JDK-8176503: Disable SHA-1 TLS Server Certificates

2017-03-13 Thread Vincent Ryan
That change looks fine to me. Thanks. > On 13 Mar 2017, at 14:57, Sean Mullan wrote: > > Please review this configuration change to disable SHA-1 TLS server > certificates by default in JDK 9. In order to be disabled, the certificates > must chain back to trusted root certificate in the cacer

Re: RFR 8175846: Provide javadoc descriptions for jdk.policytool and jdk.crypto.* modules

2017-03-07 Thread Vincent Ryan
Your comments look fine to me. > On 7 Mar 2017, at 14:44, Weijun Wang wrote: > > Ping again. > > On 03/01/2017 09:02 AM, Weijun Wang wrote: >> Please review the patch below. I don't know if all those jdk.crypto.* >> modules will be listed in the doc, but it's nice to include a comment. >> >>

Re: RFR: 8174837: Add "since=9" to deprecated ContentSigner and ContentSignerParameters classes

2017-02-13 Thread Vincent Ryan
OT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. > * > * This code is free software; you can redistribute it and/or modify it > @@ -36,7 +36,7 @@ > * @author Vincent Ryan > * @deprecated This class has been deprecated. > */ > -@Deprecated > +@Deprecated(since="9") > public interface ContentSignerParameters {

[9] 8173956: KeyStore regression due to default keystore being changed to PKCS12

2017-02-06 Thread Vincent Ryan
Please review this fix to correct support for mixed-case aliases in PKCS12 keystores. Thanks. Webrev: http://cr.openjdk.java.net/~vinnie/8173956/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8173956

Re: Feedback on SSLEngine.setHandshakeApplicationProtocolSelector()

2017-01-16 Thread Vincent Ryan
That’s good news. Thanks for validating the new mechanism in Jetty. > On 12 Jan 2017, at 19:04, Simone Bordet wrote: > > Hi, > > On Wed, Jan 11, 2017 at 5:57 PM, Simone Bordet > wrote: >> Hi, >> >> I just wanted to report that I have implemented the new mechanism >> provided by SSLEngine.se

Re: [9] RFR 8171443: (spec) An ALPN callback function may also ignore ALPN

2016-12-22 Thread Vincent Ryan
to throw an exception instead. I’d prefer to keep the current behaviour. Thanks. > On 22 Dec 2016, at 11:06, Simone Bordet wrote: > > Vincent, > > On Thu, Dec 22, 2016 at 11:38 AM, Vincent Ryan > wrote: >> Please review this spec change to allow an ALPN callback func

[9] RFR 8171443: (spec) An ALPN callback function may also ignore ALPN

2016-12-22 Thread Vincent Ryan
Please review this spec change to allow an ALPN callback function to also disable ALPN usage and return no ALPN extension value during a TLS handshake. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8171443 Webrev: http://cr.openjdk.java.net/~vinnie/8171443/webrev.01/

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-16 Thread Vincent Ryan
> 536: Minor nit: suggest renaming hasCallback to something a little more > descriptive. By the time you drop 400 lines, you may have forgotten the > variable meaning. hasAPCallback? > > 535: A comments describing your current logic would be nice. > > if (th

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-15 Thread Vincent Ryan
n scenarios looks like: > > In my previous response, I suggested a different approach where everything > ALPN is done together. I think it may be similar to your N1-4. > > I look forward to the ServerHandshaker change next week. > > Brad > > > On 12/9/2016 1:08

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-09 Thread Vincent Ryan
Thanks for your detailed review Brad. I’ve generated a new webrev: http://cr.openjdk.java.net/~vinnie/8170282/webrev.01/ > On 9 Dec 2016, at 01:34, Bradford Wetmore wrote: > > > Hi Vinnie, > > On 12/8/2016 2:18 PM, Vincent Ryan wrote: >> The Java Servlet Expect

Re: [9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-08 Thread Vincent Ryan
problem and the existing API is still available for the common case. > On 8 Dec 2016, at 23:13, David M. Lloyd wrote: > > On 12/08/2016 04:18 PM, Vincent Ryan wrote: >> The Java Servlet Expect Group reported that they have identified a specific >> HTTP2 server use-case tha

[9] RFR 8170282: Enable ALPN parameters to be supplied during the TLS handshake

2016-12-08 Thread Vincent Ryan
The Java Servlet Expect Group reported that they have identified a specific HTTP2 server use-case that cannot be easily addressed using the existing ALPN APIs. This changeset fixes that problem. It supports a new callback mechanism to allow TLS server applications to set an application protocol

Re: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Vincent Ryan
ated the copyright messages. Thanks. > New webrev: > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/ > > Best regards, > Goetz. > > (Am I correct that your openJdk name is Vinnie?) Yes. > >> -Original Message- >> From: Vincent Ryan [ma

Re: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Vincent Ryan
Hello Goetz, Please modify the bug summary to reference ECC too. Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source code headers will need to be updated/added. BTW p11_mutex.c is listed below but appears to be missing from the webrev. Thanks. > On 30 Nov 2016, at 13

Re: RFR 8170035: When determining the ciphersuite lists, there is no debug output for disabled suites

2016-11-22 Thread Vincent Ryan
Looks fine to me. Thanks. > On 22 Nov 2016, at 18:45, Jamil Nimeh wrote: > > Hello all, > > This is a short webrev that adds extra debug output that will show users > which ciphers are not in the enabled list due to being disabled by things > like jdk.tls.disabledAlgorithms, etc. > > Bug: h

Re: RFR: 8168861 AnchorCertificates uses hardcoded password for cacerts keystore

2016-11-11 Thread Vincent Ryan
Your fix looks good to me Tony. Thanks. > On 10 Nov 2016, at 21:55, Anthony Scarpino > wrote: > > Hi, > > I need a one line review to null out the password field in the keystore load > op.. > > http://cr.openjdk.java.net/~ascarpino/8168861/webrev/ > > thanks > > Tony

Re: Code Review Request, JDK-8166103 Allow certs with unknown critical extension in SunX509 validator

2016-11-11 Thread Vincent Ryan
Your changes look fine to me. Just a minor language correction: ‘to use’ -> ‘using’ (2 instances) > On 11 Nov 2016, at 05:11, Xuelei Fan wrote: > > Hi, > > Please review this bug fix: > > http://cr.openjdk.java.net/~xuelei/8166103/webrev.00/ > > The current validator implementations only al

Re: RFR : (XS) 8166432:Bad 8u112 merge of sun/security/tools/jarsigner/warnings/Test.java

2016-11-09 Thread Vincent Ryan
Looks fine to me. Thanks. > On 9 Nov 2016, at 11:26, Seán Coffey wrote: > > A bad test code merge occurred when 8u112 and CPU code was being merged a few > weeks ago. Some extra functionality added to a helper test was lost. This fix > restores it. 8u applicable only. > > diff --git a/test/s

Re: RFR: 8168851: Tighten permissions granted to the java.smartcardio module

2016-10-27 Thread Vincent Ryan
ule: > > https://bugs.openjdk.java.net/browse/JDK-8168851 > http://cr.openjdk.java.net/~mullan/webrevs/8168851/webrev.00/ > > --Sean > > On 10/27/2016 12:28 PM, Vincent Ryan wrote: >> Your fix looks fine to me Sean. >> >> >>> On 27 Oct 2016, at 17:09, Sean Mullan

Re: RFR: 8167512: Tighten permissions granted to the jdk.crypto.ucrypto module

2016-10-27 Thread Vincent Ryan
Your fix looks fine to me Sean. > On 27 Oct 2016, at 17:09, Sean Mullan wrote: > > Please review this change to tighten or remove unnecessary permissions > granted to the jdk.crypto.ucrypto module: > > https://bugs.openjdk.java.net/browse/JDK-8167512 > http://cr.openjdk.java.net/~mullan/webrev

Re: PING : Re: [jdk9] (S) RFR: 8165463: Native implementation of sunmscapi should use operator new (nothrow) for allocations

2016-10-21 Thread Vincent Ryan
Your fix looks good Ivan. Thanks. > On 21 Oct 2016, at 22:06, Ivan Gerasimov wrote: > > Hello! > > Could someone with the Reviewer's rights please help review and approve this > fix? > > The latest webrev can be found here: > http://cr.openjdk.java.net/~igerasim/8165463/01/webrev/ > > Thank

Re: RFR 8168374: TsacertOptionTest.java fails on all platforms

2016-10-19 Thread Vincent Ryan
Your fix looks fine to me. Thanks. > On 20 Oct 2016, at 05:21, Wang Weijun wrote: > > Please review this test change: > > diff --git a/test/sun/security/tools/jarsigner/TsacertOptionTest.java > b/test/sun/security/tools/jarsigner/TsacertOptionTest.java > --- a/test/sun/security/tools/jarsigner

Re: RFR: 8168078: Remove permission to read all system properties granted to the jdk.crypto.ec module

2016-10-17 Thread Vincent Ryan
Your fix looks fine to me Sean. Thanks. > On 17 Oct 2016, at 15:51, Sean Mullan wrote: > > Please review this fix to remove granting the unnecessary PropertyPermission > to the jdk.crypto.ec module: > > http://cr.openjdk.java.net/~mullan/webrevs/8168078/webrev.00/ > > The existing TestEC tes

[9] RFR 8167371: KeyStoreSpi.engineSetEntry should throw an Exception if password protection alg is specified

2016-10-13 Thread Vincent Ryan
Please review this fix to add a check to the default implementation of KeyStore setEntry and getEntry (in KeyStoreSpi). An exception is thrown if a password protection algorithm is specified. An existing test has been updated to validate the fix. Keystore implementations that support a user-supp

Re: Java 9 - Java Cryptography Extension (JCE) Unlimited Strength Jurisdiction Policy Files

2016-10-05 Thread Vincent Ryan
JDK-8061842 has added improvements in JDK 9 to simplify the management of crypto strength. See README.txt for details. Adding, 'Secu

Re: RFR[9] JDK-8077138: Some PKCS11 tests fail because NSS library is not initialized

2016-09-13 Thread Vincent Ryan
The code changes look fine (but your webrev shows zero lines changed throughout). > On 13 Sep 2016, at 10:43, John Jiang wrote: > > Hi, > Please review this patch for fixing JDK-8077138. > The solution is re-building NSS libraries with VS2013, and then the new NSS > DLLs can depend on msvcr12

Re: RFR: 8024714: In java.security file, ocsp.responderCertSubjectName should not contain quotes

2016-08-26 Thread Vincent Ryan
Looks fine to me. > On 26 Aug 2016, at 13:13, Sean Mullan wrote: > > https://bugs.openjdk.java.net/browse/JDK-8024714 > > A trivial fix to remove the quotes from the ocsp property name examples in > the java.security file as they should not be included and will cause a > parsing exception if

Re: [9] RFR 8164494: SunPKCS11-Solaris requires a non-empty PBE password

2016-08-20 Thread Vincent Ryan
> > --Max > > On 8/20/2016 5:18, Valerie Peng wrote: >> Looks fine to me. >> Thanks, >> Valerie >> >> On 8/19/2016 9:57 AM, Vincent Ryan wrote: >>> Please review this fix to PBE key derivation function which detects >>> when a non-emp

[9] RFR 8164494: SunPKCS11-Solaris requires a non-empty PBE password

2016-08-19 Thread Vincent Ryan
Please review this fix to PBE key derivation function which detects when a non-empty password is supplied to the SunPKCS11-Solaris JCE provider and fails over to the SunJCE provider instead. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8164494 Webrev: http://cr.openjdk.java.net/~vinnie/

[9] RFR 8164398: Add test sun/security/krb5/auto/EmptyPassword.java to ProblemList

2016-08-18 Thread Vincent Ryan
Please approve this change to add a failing test to jdk/test/ProblemList.txt so we can investigate further. Thanks. diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -289,6 +289,8 @@ sun/security/ssl/SSLSocketImpl/AsyncSSLSocketCl

Re: [9] RFR 8078661: [SunPKCS11] Fails to cast into RSAPrivateCrtKey after RSA KeyPair Generation

2016-08-17 Thread Vincent Ryan
Your fix looks good to me. Thanks. > On 17 Aug 2016, at 02:24, Valerie Peng wrote: > > > Anyone has time to review a straightforward fix? The current PKCS11 code > assume that if public exponent is available for RSA Private Key, then it's a > RSA CRT key. However, not all vendor implementatio

Re: PING - [jdk9] RFR: 8153438: Avoid repeated "Please insert a smart card" popup windows

2016-08-16 Thread Vincent Ryan
That fix looks fine. Is there any significant performance impact due to calling CryptAcquireCertificatePrivateKey twice? Thanks. > On 16 Aug 2016, at 13:56, Ivan Gerasimov wrote: > > A gentle reminder. > > Would you please help review at your convenience. > > With kind regards, > Ivan > > >

Re: [jdk9] RFR: 8163896: Finalizing one key of a KeyPair invalidates the other key

2016-08-15 Thread Vincent Ryan
Thanks Ivan. Your fix looks good. > On 13 Aug 2016, at 02:15, Ivan Gerasimov wrote: > > Hello! > > It was noticed, that SunMSCAPI implementation of a KeyPair has a problem: The > public and the private keys share the same native handles. > > As the result, when one of the keys is finalized,

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-12 Thread Vincent Ryan
; is not needed. > > --Sean > > On 08/12/2016 11:07 AM, Vincent Ryan wrote: >> I’ve moved the X.509 check to earlier in the code and reverted the changes >> to the validateChain method. >> Updated webrev is at: >> http://cr.openjdk.java.net/~vinnie/8163503/

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-12 Thread Vincent Ryan
I’ve moved the X.509 check to earlier in the code and reverted the changes to the validateChain method. Updated webrev is at: http://cr.openjdk.java.net/~vinnie/8163503/webrev.02/ > On 10 Aug 2016, at 21:15, Sean Mullan wrote: > > On 08/10/2016 12:39 PM, Vincent Ryan wrote: &g

[9] RFR 6877937: The SunJCE PBKDF2KeyImpl is requiring the MAC instance also be from SunJCE.

2016-08-11 Thread Vincent Ryan
Please review this change to unpin the Mac implementation from the SunJCE provider. Since the Mac is a private field there are no issues regarding Clonable implementations for Mac or its MessageDigest. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-6977937 diff --git a/src/java.base/sha

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Vincent Ryan
ore. >> >> Why not just throw a KeyStoreException in validateChain()? >> >> --Max >> >> On 8/10/2016 2:14, Vincent Ryan wrote: >>> Please review this fix to improve the error handling for attempts to >>> store a Certificate object in PKCS12 keystore. >&g

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Vincent Ryan
ote: > > I thought I've seen this webrev before. > > Why not just throw a KeyStoreException in validateChain()? > > --Max > > On 8/10/2016 2:14, Vincent Ryan wrote: >> Please review this fix to improve the error handling for attempts to store a >> Certific

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-10 Thread Vincent Ryan
> > Regards, > Sean. > > On 09/08/16 19:14, Vincent Ryan wrote: >> Please review this fix to improve the error handling for attempts to store a >> Certificate object in PKCS12 keystore. >> The PKCS12 keystore implementation supports storing only X509Certificate >&

[9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

2016-08-09 Thread Vincent Ryan
Please review this fix to improve the error handling for attempts to store a Certificate object in PKCS12 keystore. The PKCS12 keystore implementation supports storing only X509Certificate objects but the KeyStore API allows Certificate objects. This fix rejects attempts to store non-X.509 certif

Re: RFR 8162739: Create new keytool option to access cacerts file

2016-08-08 Thread Vincent Ryan
Your changes look good to me. Thanks. > On 8 Aug 2016, at 14:46, Weijun Wang wrote: > > Please review the code changes at > > http://cr.openjdk.java.net/~weijun/8162739/webrev.00/ > > A new -cacerts option is added to keytool so there is no need to provide the > full cacerts path on the co

Re: [9] RFR 8157579: com/sun/crypto/provider/Mac/MacClone.java failed on solaris12(sparcv9 and x86)

2016-08-05 Thread Vincent Ryan
Your fix looks good. One comment is that you could trim the Provider[] to exclude the most-preferred provider (at index=0). Thanks. > On 5 Aug 2016, at 03:01, Valerie Peng wrote: > > > Anyone has time to review this fix? The code change is in only one file and > straightforward if you agree

Re: [9] RFR 8161571: Verifying ECDSA signatures permits trailing bytes

2016-07-21 Thread Vincent Ryan
; algorithms can also be checked on some platform if not using provider > option. > > + main0("RSA", 2048, "SHA256withRSA", null); > + main0("DSA", 2048, "SHA256withDSA", null); > > Xuelei > > On 7/20/2016 3:10 AM, Vincent

Re: Code Review Request JDK-8161898 Mark the use of deprecated javax.security.cert APIs with forRemoval=true

2016-07-21 Thread Vincent Ryan
Looks fine to me. Thanks. > On 21 Jul 2016, at 03:32, Xuelei Fan wrote: > > Hi, > > javax.security.cert was deprecated and marked with forRemoval=true in > JDK 9. The use of javax.security.cert APIs should be marked with > forRemoval=true too. > > Please review the update: > > http://cr.o

[9] RFR 8161571: Verifying ECDSA signatures permits trailing bytes

2016-07-19 Thread Vincent Ryan
Please review this fix to apply stricter length checks when verifying public key signatures. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8161571 Webrev: http://cr.openjdk.java.net/~vinnie/8161571/webrev.00/

Re: RFR: 9: 8144559: sun/security/mscapi/SignUsingNONEwithRSA.sh failed intermittently

2016-07-08 Thread Vincent Ryan
Your change looks fine to me - and another shell script zapped. Thanks. > On 7 Jul 2016, at 23:43, Rajan Halade wrote: > > May I request you to review this small patch. I am not able to reproduce the > failure so I enhance the test itself to get rid of shell script. Since > temporary key gene

Re: [9] RFR 8068290: JCK: 2 api/java_security/cert/ tests fail

2016-07-05 Thread Vincent Ryan
I’ve updated the webrev to also check for X509Certificate in both engineSetKeyEntry methods and in engineSetEntry: http://cr.openjdk.java.net/~vinnie/8068290/webrev.01/ > On 4 Jul 2016, at 10:25, Vincent Ryan wrote: > > Good catch Max. I’ll add the check there too. > > &

Re: [9] RFR 8068290: JCK: 2 api/java_security/cert/ tests fail

2016-07-04 Thread Vincent Ryan
Good catch Max. I’ll add the check there too. > On 2 Jul 2016, at 02:04, Wang Weijun wrote: > > Should engineSetKeyEntry make the same check? > > --Max > >> On Jul 2, 2016, at 5:52 AM, Vincent Ryan wrote: >> >> Please review this change to correct two

[9] RFR 8068290: JCK: 2 api/java_security/cert/ tests fail

2016-07-01 Thread Vincent Ryan
Please review this change to correct two failing JCK tests. Bug: https://bugs.openjdk.java.net/browse/JDK-8068290 Webrev: http://cr.openjdk.java.net/~vinnie/8068290/webrev.00/ PKCS12 is the default keystore type in JDK 9 and its implementation stores only X.509 certificates although the KeyStore

Re: RFR 8157730: Mark deprecated java.security.{Identity, IdentityScope, Signer} APIs with forRemoval=true

2016-06-17 Thread Vincent Ryan
> Bernd > > -- > http://bernd.eckenfels.net > > -Original Message----- > From: Sean Mullan > To: Vincent Ryan , OpenJDK > > Sent: Fr., 17 Juni 2016 16:41 > Subject: Re: RFR 8157730: Mark deprecated java.security.{Identity, > IdentityScope, Signer} APIs with forRemoval=t

RFR 8157730: Mark deprecated java.security.{Identity, IdentityScope, Signer} APIs with forRemoval=true

2016-06-17 Thread Vincent Ryan
Three Identity-related classes were deprecated in JDK 1.2. Please review the patch below that marks them as candidates for removal in a future JDK release. See http://openjdk.java.net/jeps/277 for details of the enhanced deprecation annotation. Thanks. diff

Re: RFR 8149521: automatic discovery of LDAP servers with Kerberos authentication

2016-05-10 Thread Vincent Ryan
Looks fine to me Max. Thanks. > On 10 May 2016, at 14:34, Wang Weijun wrote: > > Hi All > > Please take a review at > > http://cr.openjdk.java.net/~weijun/8149521/webrev.00/ > > While the bug report [1] suggests we can fix com.sun.jndi.ldap.ServiceLocator > to emit a trail-dot-less hostna

Re: Code Review Request: 8156502, Use short name of SupportedEllipticCurvesExtension.java

2016-05-08 Thread Vincent Ryan
Your change looks fine to me. Thanks. > On 8 May 2016, at 16:12, Xuelei Fan wrote: > > Hi, > > Please review this class name update: > >http://cr.openjdk.java.net/~xuelei/8156502/webrev.00/ > > The class names of SupportedEllipticCurvesExtension and > SupportedEllipticPointFormatsExtensi

Re: CFV: New Security Group Member: Jamil Nimeh

2016-04-29 Thread Vincent Ryan
Vote: yes > On 29 Apr 2016, at 15:40, Sean Mullan wrote: > > I hereby nominate Jamil Nimeh to Membership in the Security Group. > > Jamil is a member of the Java Security Libraries team at Oracle and has been > an active contributor to the OpenJDK Security Group for approximately two > year

Re: JDK 9 RFR of JDK-8153695: Problem list sun/security/pkcs11/Provider/Login.sh for linux-all

2016-04-06 Thread Vincent Ryan
That looks fine to me. Thanks. > On 7 Apr 2016, at 04:42, Amy Lu wrote: > > sun/security/pkcs11/Provider/Login.sh > This test is known to fail at Linux platform (JDK-8153545). > > Please review the patch to put the test to ProblemList.txt for linux-all > until mentioned issue (JDK-8153545) i

Re: RFR 6483657: MSCAPI provider does not create unique alias names

2016-04-01 Thread Vincent Ryan
Your fix looks good. Thanks. > On 31 Mar 2016, at 16:57, Ivan Gerasimov wrote: > > Hello! > > Could you please help review the fix for this long standing issue? > Windows-MY allows non-unique aliases, but our implementation of KeyStore does > not take it into account. > > To help to deal with

Re: JDK 9 RFR of JDK-8151835: Mark SmallPrimeExponentP.java as intermittently failing

2016-03-14 Thread Vincent Ryan
Looks fine to me. Thanks. > On 14 Mar 2016, at 17:31, joe darcy wrote: > > Hello, > > The test > > sun/security/mscapi/SmallPrimeExponentP.java > > has been seen to timeout intermittently, bug JDK-8151834. > > The sources of the test should be marked accordingly; please review the > corres

Re: [9] review request 8151149: CipherSpi implementation of PBEWithSHA1AndDESede returns key size in bytes

2016-03-07 Thread Vincent Ryan
Thanks Xuelei. > On 7 Mar 2016, at 14:14, Xuelei Fan wrote: > > On 3/7/2016 10:05 PM, Vincent Ryan wrote: >> Please review this fix to PKCS#12 PBE CipherSpi classes to return the cipher >> key size in bits. >> An effective key size of 112 bits is used for Triple

[9] review request 8151149: CipherSpi implementation of PBEWithSHA1AndDESede returns key size in bytes

2016-03-07 Thread Vincent Ryan
Please review this fix to PKCS#12 PBE CipherSpi classes to return the cipher key size in bits. An effective key size of 112 bits is used for Triple DES. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8151149 Webrev: http://cr.openjdk.java.net/~vinnie/8151149/webrev.00/

Re: Code Review Request 8148108 Disable Diffie-Hellman keys less than 1024 bits

2016-03-04 Thread Vincent Ryan
Your fix looks fine. Thanks. > On 4 Mar 2016, at 11:53, Xuelei Fan wrote: > > Hi, > > Please review the update for JDK-8148108: > http://cr.openjdk.java.net/~xuelei/8148108/webrev.00/ > > In this update, it is proposed to restrict the use of DH keys less than > 1024 bits in length in the SS

Re: RFR 8138653: Default key sizes for the AlgorithmParameterGenerator and KeyPairGenerator implementations should be upgraded

2016-03-01 Thread Vincent Ryan
Your fix looks fine. Thanks. > On 1 Mar 2016, at 19:21, Sean Mullan wrote: > > Updated webrev: http://cr.openjdk.java.net/~mullan/webrevs/8138653/webrev.01/ > > The following changes have been made: > > - The default key size for DSA has not been changed (stays at 1024) due to > the high ris

Re: [9] request for review 8149411: PKCS12KeyStore cannot extract AES Secret Keys

2016-02-15 Thread Vincent Ryan
Thanks. > On 15 Feb 2016, at 15:20, Xuelei Fan wrote: > > Looks fine to me. > > Xuelei > > On 2/15/2016 10:54 PM, Vincent Ryan wrote: >> Please review this fix to PKCS12 keystore implementation that removes an >> unnecessary dependency on SecretKeyFac

[9] request for review 8149411: PKCS12KeyStore cannot extract AES Secret Keys

2016-02-15 Thread Vincent Ryan
Please review this fix to PKCS12 keystore implementation that removes an unnecessary dependency on SecretKeyFactory when extracting a secret key. SecretKeySpec is used instead of SecretKeyFactory because it is JCE provider-independent. Webrev: http://cr.openjdk.java.net/~vinnie/8149411/webrev.0

Re: Code Review Request: 8145344: Add SHA1/224 to preferred provider list in java.security for solaris-sparc

2016-02-02 Thread Vincent Ryan
Your fix looks fine to me. Thanks. > On 1 Feb 2016, at 22:37, Anthony Scarpino wrote: > > Hi, > > Could I have a review of the quick change that adds SHA1 and SHA224 to the > solaris-sparc preferred providers list and a minor change to handle > algorithms with two names, like SHA1 and SHA-1.

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

2015-12-01 Thread Vincent Ryan
throw new SSLProtocolException( >> +"Invalid " + type + " extension: too short"); > Can you print the len value here ? > >> +if (remaining != 0) { >> +throw new SSLProtocolException( >> +

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

2015-12-01 Thread Vincent Ryan
Thanks for the additional review comments. Responses in-line below. Updated webrev: http://cr.openjdk.java.net/~vinnie/8144093/webrev.02/ > On 1 Dec 2015, at 01:32, Bradford Wetmore wrote: > > > On 11/29/2015 4:08 PM, Vincent Ryan wrote: > > > Following on from Brad

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

2015-11-30 Thread Vincent Ryan
gt; applicationProtocols field until you confirm the values are valid, and then > assign it to applicationProtocols. > > ExtensionType.java > > 47 new ArrayList(15); > > nit - you can use diamond operator above > > --Sean > > On 11/29/2015 07:08 PM,

Re: Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

2015-11-30 Thread Vincent Ryan
t writeRecord(ByteBuffer[] appData, >int offset, int length, ByteBuffer netData) throws IOException { >... > if (...) { >... >// set connection ALPN value >applicationProtocol = >handshaker.getHandshakeApplicationProtocol(); >... > } > }

Request for review: 8144093: JEP 244/8051498 - TLS Application-Layer Protocol Negotiation Extension

2015-11-29 Thread Vincent Ryan
Hello, Following on from Brad’s recent email, here is the full webrev of the API and the implementation classes for ALPN: http://cr.openjdk.java.net/~vinnie/8144093/webrev.00/ In adds the implementation classes (sun/security/ssl) to the

Re: RFR 8143913: MSCAPI keystore should accept Certificate[] in setEntry()

2015-11-24 Thread Vincent Ryan
Your fix looks good. Thanks. > On 24 Nov 2015, at 14:32, Weijun Wang wrote: > > Please review the fix at > > http://cr.openjdk.java.net/~weijun/8143913/webrev.00/ > > Not everyone passes a X509Certificate[] into KeyStore::setEntry(). > > Thanks > Max

Re: JEP260 -- Impact on SunPKCS11?

2015-11-17 Thread Vincent Ryan
Hello Glen, JCE providers are always accessed via the Java SE public APIs and not directly via sun.* implementation classes. In JDK 9, the SunPKCS11 provider continues to be accessible via those APIs. It’s implementation classes are present in the jdk.crypto.pkcs11 module. Thanks. > On 16 No

Re: RFR 8139436: sun.security.mscapi.KeyStore might load incomplete data

2015-11-15 Thread Vincent Ryan
Yes the source changes look fine. Last week I ran the tests using webrev.4 and all passed. Thanks Christoph for the comprehensive fix. > On 16 Nov 2015, at 00:32, Wang Weijun wrote: > > All tests look fine. > > I suppose Vincent already agreed with the src change. Right? > > For safety, I am

Re: RFR 8139436: sun.security.mscapi.KeyStore might load incomplete data

2015-11-11 Thread Vincent Ryan
139436.2/ > <http://cr.openjdk.java.net/~clanger/webrevs/8139436.2/> > > I removed an unnecessary blank in a method call and updated the copyright > information. I also ran another build and verified with my test case. > > Thanks in advance for taking care. > Christoph > > F

Re: RFR 8139436: sun.security.mscapi.KeyStore might load incomplete data

2015-11-10 Thread Vincent Ryan
Explicitly referencing the Sun JCE provider makes the jdk.crypto.mscapi module depend on the java.base module. And that’s OK. I can sponsor this changeset. > On 10 Nov 2015, at 22:39, Mike StJohns wrote: > > On 11/10/2015 4:12 PM, Langer, Christoph wrote: >> Hi folks, >> >> is there any fee

Re: RFR :8136534: Loading JKS keystore using non-null InputStream results in closed stream

2015-10-14 Thread Vincent Ryan
Looks good to me. > On 14 Oct 2015, at 15:59, Seán Coffey wrote: > > Backporting this fix to jdk8u-dev sources. > > The jdk9 changeset didn't apply cleanly but the fix in essence is the removal > of a try-with-resources block. Tests are green. > > bug report : https://bugs.openjdk.java.net/br

[9] request for review: 8136534: Loading JKS keystore using non-null InputStream results in closed stream

2015-09-16 Thread Vincent Ryan
Please review this JDK 9 fix to stop KeyStore.load from closing the input stream passed to it. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8136534 Webrev: http://cr.openjdk.java.net/~vinnie/8136534/webrev.00/

Re: [9] RFR: 8048622: Enhance tests for PKCS11 keystores with NSS

2015-09-14 Thread Vincent Ryan
Your fix looks fine. Thanks. > On 21 Aug 2015, at 21:34, Artem Smotrakov wrote: > > Hello, > > Please review a couple of changes for PKCS11 tests: > - Added a new test which checks that a keystore can't be loaded with > incorrect password, > found https://bugs.openjdk.java.net/browse/JDK-813

Re: [9] RFR: 8134232: KeyStore.load() throws an IOException with a wrong cause in case of wrong password

2015-09-14 Thread Vincent Ryan
Your fix looks fine Artem. Thanks. > On 11 Sep 2015, at 12:46, Artem Smotrakov wrote: > > Hello, > > Please review this for 9. > > According to [1], KeyStore.load(InputStream, char[]) method should throw an > IOException, and the cause of the IOException should be an > UnrecoverableKeyExcep

Re: RFR 8136425: KeystoreImpl.m using wrong type for cert format

2015-09-12 Thread Vincent Ryan
Your fix looks fine to me. Thanks. > On 12 Sep 2015, at 10:23, Wang Weijun wrote: > > Please take a look at > >http://cr.openjdk.java.net/~weijun/8136425/webrev.00/ > > > It looks like a wrong type is used, kSecFormatX509Cert and >

Re: [9] Urgent RFR 8135099 "9-dev solaris builds failed on 2015-09-04"

2015-09-08 Thread Vincent Ryan
Your fix looks fine to me. Thanks. > On 8 Sep 2015, at 21:33, Valerie Peng wrote: > > Hi, > > I need someone to help reviewing for this bug ASAP: > http://cr.openjdk.java.net/~valeriep/8135099/webrev.00/ > > The build is broken due to the compilation warning introduced in 8130875 > which slip

Re: 8130800: KeyStore.getInstance(File, char[]) does not throw IOE for null password

2015-08-31 Thread Vincent Ryan
Thanks Max. > On 31 Aug 2015, at 10:02, Weijun Wang wrote: > > This looks good. > > Thanks > Max > > On 08/31/2015 04:58 PM, Vincent Ryan wrote: >> Please review this spec change in java.security.KeyStore to clarify that a >> keystore integrity check is n

8130800: KeyStore.getInstance(File, char[]) does not throw IOE for null password

2015-08-31 Thread Vincent Ryan
Please review this spec change in java.security.KeyStore to clarify that a keystore integrity check is not performed when a null password is supplied. Thanks. Webrev: http://cr.openjdk.java.net/~vinnie/8130800/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8130800

[9] Request for review 8134112: api/java_security/KeyStore/index.html#GetInstance2Tests[getInstanceIOE2, getInstanceIOE3] still fail after JDK-8130850 is fixed in b77

2015-08-26 Thread Vincent Ryan
Please review this change to the KeyStore.getInstance(File,LoadStoreParameter) method to correctly handle its LoadStoreParameter argument. It now extracts the embedded password before calling the KeyStore.load(InputStream,char[]) method. This fix corrects a JCK test that had been failing. Thanks

[9] review request for 8132786: java/security/cert/CertPathValidator/OCSP/AIACheck.java fails intermittently

2015-08-19 Thread Vincent Ryan
Please review this fix for an intermittently failing test. It now expects a SocketException or a SocketTimeoutException. Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8132786 Webrev: http://cr.openjdk.java.net/~vinnie/8132786/webrev.00/

  1   2   3   4   5   >