Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-02-08 Thread Sean Mullan
The update looks good. --Sean On 2/7/17 8:09 PM, Anthony Scarpino wrote: I believe all comments are addressed in the below link http://cr.openjdk.java.net/~ascarpino/8160655/webrev.02/ Everything I didn't comment on inline below was because I hadn't posted an update-to-date webrev.01 at that

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-02-07 Thread Anthony Scarpino
The two changes are connected, removing the key algorithm check in the second change is linked to changing the permits() in the first change. But I agree that this change is unnecessary.. I'm going to revert it back. thanks Tony On 01/26/2017 01:09 PM, Xuelei Fan wrote: DisabledAlgorithmCons

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-02-07 Thread Anthony Scarpino
I believe all comments are addressed in the below link http://cr.openjdk.java.net/~ascarpino/8160655/webrev.02/ Everything I didn't comment on inline below was because I hadn't posted an update-to-date webrev.01 at that time. Tony On 02/06/2017 12:17 PM, Sean Mullan wrote: Hi Tony, Here ar

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-02-06 Thread Sean Mullan
Hi Tony, Here are my comments on the latest webrev: * SignerInfo.java 355 try { 356 JAR_DISABLED_CHECK.permits(digestAlgname, cparams); 357 } catch (CertPathValidatorException e) { 358 throw new SignatureException(e.g

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-31 Thread Anthony Scarpino
I see what you are saying.. It's a simple change that I can make on the on my workspace.. I won't rev the webrev, but here is the change. 377 debug.println(key + ": " + e.getMessage()); Tony On 01/31/2017 09:28 AM, Seán Coffey wrote: Hi Tony, Thanks for the update. I see y

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-31 Thread Seán Coffey
Hi Tony, Thanks for the update. I see your new webrev. I guess my point is that if we're in verbose logging mode, then we should log the message from the exception(.getMessage()) rather than the more (vague) "uses a disabled algorithm" logged message. I see the new code now iterating over th

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-30 Thread Anthony Scarpino
On 01/23/2017 03:27 PM, Anthony Scarpino wrote: Hi, I need a code review of this change that brings more detail constraints checking and control to certpath and jar disabled algorithm Security properties. http://cr.openjdk.java.net/~ascarpino/8160655/webrev/ thanks Tony Updated review http

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-30 Thread Anthony Scarpino
Hi Sean, Actually Sean M and I were talking about that offline on thursday. That file is changing a lot. The three sections you mention have changed a lot, but the general idea is the disabled algorithms are captured and reported after all the checks were done. This is because the we can h

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-30 Thread Seán Coffey
src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java CertPathValidatorException is caught 3 times in new code but we're not printing out the exact algorithm that caused the exception. AFAIK, that should be in the exception message. Would it be possible to use something e.ge

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-26 Thread Sean Mullan
Looks good, mostly minor stuff so far, just have one other file I need more time to review: * java.security Update description of new constraints to match CCC. * PKIXExtendedParameters.java Update class description (it is out-of-date). * CertConstraintParameters.java 2 * Copyright (c) 2016

Re: RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-26 Thread Xuelei Fan
DisabledAlgorithmConstraints.java = public final boolean permits(Set primitives, Key key) { -return checkConstraints(primitives, "", key, null); +try { +permits(new ConstraintsParameters(key.getAlgorithm(), null, key, +

RFR: 8160655 Fix denyAfter and usage types for security properties

2017-01-23 Thread Anthony Scarpino
Hi, I need a code review of this change that brings more detail constraints checking and control to certpath and jar disabled algorithm Security properties. http://cr.openjdk.java.net/~ascarpino/8160655/webrev/ thanks Tony