RFR: 8160655 Fix denyAfter and usage types for security properties

[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 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.

In offline discussions this will have to remain for now.


>
> * 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/

I changed the comment in a way that I think is more descriptive.

>
> * 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
>>