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
