[RFR] JDK-8225436 : Stapled OCSPResponses should be added to PKIXRevocationChecker irrespective of revocationEnabled flag

[Jamil Nimeh]

Thanks for the review, Sean.  I've made a couple changes based on your 
recommendations.  I don't create a mutable ArrayList any longer in the 
case where we're making a new PKIXRevocationChecker.  In that one case I 
just directly add it to the PKIXBuilderParameters and if one already 
exists from the immutable CPC list I can just add the responses to it 
and push the whole list back.

http://cr.openjdk.java.net/~jnimeh/reviews/8225436/webrev.02/

--Jamil

On 8/19/19 12:55 PM, Sean Mullan wrote:
> Looks good. There is one case where an unnecessary ArrayList is 
> created (on line 406-7) if revocation is disabled AND we don't find a 
> RevocationChecker -- it would be useful if you could avoid that by 
> iterating over the checkers before creating the array list, since it 
> is probably the more common case. In fact, there is probably some 
> unnecessary copying even before that since the PKIXParameters has 
> already been cloned in engineValidate. We avoid some of that 
> cloning/copying in the certpath provider (see the ValidatorParams 
> class in sun.security.provider.certpath.PKIX) - you could look into 
> that to see if you could use that here - I'll also think about it 
> again when I have a little more time ...
>
> --Sean
>
> On 8/16/19 5:25 PM, Jamil Nimeh wrote:
>> Hello all,
>>
>> This fixes a bug where stapled OCSP responses were being ignored by 
>> the internal Validator in all cases when revocation checking is 
>> disabled. If the TrustManagerFactory is initialized with 
>> CertPathParameters that include a PKIXRevocationChecker, then that 
>> should override the setRevocationEnabled flag and any stapled 
>> responses should be taken into account during path validation.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8225436
>>
>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8225436/webrev.01/
>>
>> Thanks,
>>
>> --Jamil
>>