code review 7011497: new CertPathValidatorException.BasicReason enum constant for constrained algorithm

[Sean Mullan]

Hi Andrew,

Did you consider using the existing X509CertSelector class to match on the 
authority key identifier? I actually think this should work, and it will avoid 
having to create the AKIDMatchState class. Take a look at the 
ForwardBuilder.getMatchingCACerts method towards the end, where it gets the 
AuthorityKeyIdentifierExtension. Can you create a similar X509CertSelector to 
select the proper trust anchor?

--Sean


On 1/14/11 12:32 PM, Xuelei Fan wrote:
> On 1/15/2011 1:30 AM, Xuelei Fan wrote:
>> Hi Sean,
>>
>> webrev:
> http://cr.openjdk.java.net/~xuelei/7011497/webrev/
>
>> Would you please review the update again. I integrate the fix for
>> 7011497 and 7012357 together.
>>
>> Comparing with previous webrev, the following updates are unchanged:
>> src/share/classes/java/security/cert/CertPathValidatorException.java
>> src/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
>> src/share/classes/sun/security/validator/SimpleValidator.java
>> other test files.
>>
>>
>> The following are new changes for CR 7012357:
>> src/share/classes/sun/security/provider/certpath/ForwardBuilder.java
>> src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java
>> test/sun/security/provider/certpath/DisabledAlgorithms/CPValidatorEndEntity.java
>>
>>
>> Thanks,
>> Xuelei
>>
>> On 1/14/2011 11:10 AM, Xuelei Fan wrote:
>>> We don't checking the SKID and AKID during searching for the trust anchor.
>>>
>>> I have filled a new CR for the issue, 7012357, Improve trust anchor
>>> searching method during cert path validation.
>>>
>>> I will have this commented out block in CPValidatorEndEntity.java. I
>>> will use this test case for CR 7012357.
>>>
>>> Thanks,
>>> Xuelei
>>>
>>> On 1/14/2011 12:44 AM, Xuelei Fan wrote:
>>>> I just realized, if subject KID and issuer KID works, the cert path
>>>> validation should be able to find the proper trust anchor.  I will look
>>>> into the issue tomorrow.
>>>>
>>>> Xuelei
>>>>
>>>> On 1/14/2011 12:27 AM, Xuelei Fan wrote:
>>>>> On 1/14/2011 12:05 AM, Sean Mullan wrote:
>>>>>> On 1/13/11 6:38 AM, Xuelei Fan wrote:
>>>>>>> Hi Sean,
>>>>>>>
>>>>>>> Would you please review the fix for CR 7011497?
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~xuelei/7011497/webrev/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Xuelei
>>>>>>
>>>>>> CPValidatorEndEntity.java:
>>>>>>
>>>>>>   307         /* coment out useless trust anchor
>>>>>>   308         is = new
>>>>>> ByteArrayInputStream(trustAnchor_SHA1withRSA_512.getBytes());
>>>>>>   309         cert = cf.generateCertificate(is);
>>>>>>   310         anchor = new TrustAnchor((X509Certificate)cert, null);
>>>>>>   311         anchors.add(anchor);
>>>>>>   312         */
>>>>>>
>>>>>> Why do you leave this code in with this comment?
>>>>>>
>>>>> If I have this block. The cert path validation cannot find the proper
>>>>> trust anchor. As there are two trusted certificates, they are almost the
>>>>> same except the key size (one key size is 1024, another one is 512).
>>>>>
>>>>> In cert path validation, once a trust anchor found, if the signature is
>>>>> not valid, I think no more effort to test more trust anchors.
>>>>>
>>>>> I was wondering whether it is worthy to try more trust anchors. It's
>>>>> expensive!
>>>>>
>>>>> Thanks for the review.
>>>>>
>>>>> Xuelei
>>>>>
>>>>>> Otherwise, looks good.
>>>>>>
>>>>>> --Sean
>>>>>
>>>>
>>>
>>
>