RFR: 8160655 Fix denyAfter and usage types for security properties

Sean Mullan sean.mullan at oracle.com
Mon Feb 6 20:17:50 UTC 2017


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
>



More information about the security-dev mailing list