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

[Xuelei Fan]

Ok to me.

Xuelei

> On Feb 17, 2014, at 9:57 PM, Sean Mullan <sean.mullan at oracle.com> wrote:
> 
>> 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
>> 
>