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 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 the permittedAlgs Map - that will
certainly benefit logging.

regards,
Sean.

On 30/01/2017 18:21, Anthony Scarpino wrote:
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 have multiple
signatures and one of them maybe allowed.  Throwing an exception on
the first failure would not pick up a possible second signature that
was allowed.

thanks

Tony

On 01/30/2017 03:31 AM, Seán Coffey wrote:
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.getMessage() call to print more detail ? You'd have to check
for null also.

 371                 } catch(CertPathValidatorException e) {
 372                     if (debug != null) {
 373                         debug.println(key + " uses a disabled
algorithm.");
 374                     }

Spacing issue on line 371 of same file :

371                 } catch(CertPathValidatorException e) {

Regards,
Sean.

On 26/01/17 21:57, Sean Mullan wrote:
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, 2017 Oracle and/or its affiliates. All rights
reserved.

Should be a comma after 2017.

* AlgorithmChecker.java

278         String currSigAlg =
((X509Certificate)cert).getSigAlgName();

Just use x509Cert.getSigAlgName() instead

* SignatureFileVerifier.java

294         Timestamp[] timestamp = new Timestamp[newSigners.length];

"timestamps" would be more clear as a variable name

299                 System.out.println("Timestamp[" + (i - 1) + "] =
" +

debug.println

--Sean

On 1/23/17 6: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




Reply via email to