RFR: 8160655 Fix denyAfter and usage types for security properties

[Seán Coffey]

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