RFR: 8160655 Fix denyAfter and usage types for security properties

Anthony Scarpino anthony.scarpino at oracle.com
Wed Feb 1 00:40:45 UTC 2017


I see what you are saying.. It's a simple change that I can make on the 
on my workspace.. I won't rev the webrev, but here is the change.

377                   debug.println(key + ":  " + e.getMessage());

Tony

On 01/31/2017 09:28 AM, Seán Coffey wrote:
> 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
>>>
>>
>




More information about the security-dev mailing list