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

[Sean Mullan]

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.

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