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.getMessage(), e);
359 }
It's a little odd this throws a CPVE because there is no certificate
being checked here.
* AlgorithmChecker.java
278 String currSigAlg = ((X509Certificate)cert).getSigAlgName();
Just use x509cert.getSigAlgName() instead.
* SignatureFileVerifier.java
145 System.err.println("key = " + name + " == name " + name);
leftover debug?
265 // Algorithms that have been checked if they are weak.
266 private Map<String, Boolean> permittedAlgs= new HashMap<>();
267 // TSA timestamp of signed jar. The newest timestamp is used.
If there was
268 // no TSA timestamp used when signed, current time is used
("null").
269 private Timestamp timestamp = null;
can you move these lines up to the top of the class where other
variables are declared?
// Algorithms that have been checked if they are weak.
266 private Map<String, Boolean> permittedAlgs= new HashMap<>();
It might be better to have a Map<String, List<String>> where the key is
the header (*-DIGEST*) and the value is a list of weak algs. This way
you could avoid the need for the getWeakAlgorithms method and just get
the weak list of algs out of the map directly. An empty list or null
value would indicate the header is ok.
302 /* Look for the latest timestamp in the signature block */
s/the latest timestamp/the latest timestamp or no timestamp/
* java.security
Missing spaces between words on lines 646-648 (ex: algorithmin)
* Main.java
640 void rey verifyJar(String jarName)
typo?
* PKIXExtendedParameters.java
Update class description to reflect this contains more than a timestamp.
--Sean
On 1/30/17 6:57 PM, Anthony Scarpino wrote:
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://cr.openjdk.java.net/~ascarpino/8160655/webrev.01/
It address the comments and has a different SignatureFileVerifier.java
Tony
