Review Request for JDK-8025708 : Certificate Path Building problem with AKI serial number

[Jason Uh]

On 2/14/14 7:17 AM, Sean Mullan wrote:
> On 02/13/2014 10:04 PM, Jason Uh wrote:
>> Hi Sean,
>>
>> Looks good to me, but I'm not an official Reviewer.
>>
>> I have a couple of questions, though.
>
> Sure.
>
>> 1. This isn't a part of your change, but shouldn't the comment on line
>> 200 of AdaptableX509CertSelector.java read "As for version 3,..." and
>> not "As for version 2,..."?
>
> Yes, good catch. I changed it.
>
>> 2. Just curious, any reason why this wasn't just fixed with
>>
>>      void parseAuthorityKeyIdentifierExtension(
>>              AuthorityKeyIdentifierExtension akidext) throws
>> IOException {
>>        + super.setSubjectKeyIdentifier(null);
>>        + super.setSerialNumber(null);
>>
>>          if (akidext != null) { ... }
>>
>> in AdaptableX509CertSelector.java?
>
> I did initially consider that fix, but the problem is the
> subjectKeyIdentifier and serialNumber matching rules are slightly
> different for X509CertSelector. In X509CertSelector the
> subjectKeyIdentifier match fails if the certificate has no subject key
> identifier, whereas in AdaptableX509CertSelector it passes. With
> serialNumber, the match is only performed in AdaptableX509CertSelector
> if the certificate version is < 3.
>
> The previous code for AdaptableX509CertSelector had to workaround that
> by setting these criteria to null in certain cases to trick the
> superclass matching rules to pass. See lines 167-171 and 182-184. After
> reviewing this code, I came to the conclusion that it was much cleaner
> to always override the matching rules for these criteria in
> AdaptableX509CertSelector and never call the superclass methods to set
> those criteria (which is why they are overridden to throw
> IllegalArgumentExc).
>
> Hope this makes sense.

Ah, I see. Agreed! Thanks for the explanation.

Jason

> --Sean
>
>
>
>>
>> Thanks!
>>
>> Jason
>>
>> On 2/13/14 5:04 AM, Sean Mullan wrote:
>>> See: http://cr.openjdk.java.net/~mullan/webrevs/8025708/webrev/
>>>
>>> This fixes a problem with the PKIX CertPathBuilder where it wasn't able
>>> to build a path when the Authority Key Identifier extension of an
>>> intermediate CA cert did not contain a serial number field, and the end
>>> entity cert did.
>>>
>>> The problem was in the AdaptableX509CertSelector class. It was reusing
>>> this selector without re-initializing certain fields. I changed the
>>> implementation of this class so that it doesn't have this issue anymore.
>>>
>>> Thanks,
>>> Sean
>>
>