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

[Sean Mullan]

On 02/14/2014 08:52 PM, Xuelei Fan wrote:
> AdaptableX509CertSelector.java
> ==============================
> 1. for boolean value checking, I normally use
>     if (boolean-value) or if (!boolean-value)

I usually do too, but "== false" seemed more readable to me in this 
code, perhaps because it is immediately returning false afterwards. 
Anyway, it's not a big deal either way to me, so I'll change it to "!".

> 2. AdaptableX509CertSelector.match(Certificate)
>     KID can be used to facilitate the path building.  I would suggest
> check the SKID and SN at first, and then call super.match().

Good point, this will filter out non-matching certs faster. Changed.

> 3. AdaptableX509CertSelector.clone()
>     Also want to clone "serial" variabl?

Not necessary, it's immutable.

> 4. absence of SKID
>     Per section 4.2.1.2, RFC 5280:
>
>     To facilitate certification path construction, this extension MUST
>     appear in all conforming CA certificates, that is, all certificates
>     including the basic constraints extension (Section 4.2.1.9) where the
>     value of cA is TRUE.  In conforming CA certificates, the value of the
>     subject key identifier MUST be the value placed in the key identifier
>     field of the authority key identifier extension (Section 4.2.1.1) of
>     certificates issued by the subject of this certificate.
>
>     Not a big concerns, I was just wondering we may still want to check
> KID although the serial number checking may be optional.  This update
> allow the absence of subject key ID extension.

Yes I was wondering about that, but I am just preserving the previous 
behavior: see lines 167-170 -- which I assume was done for a good reason.

--Sean

>
> Otherwise, looks fine to me.
>
> Xuelei
>
>
> On 2/13/2014 9:04 PM, 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
>