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

[Jamil Nimeh]

Oops, missed that one.  Good catch.

--Jamil

On 8/22/19 1:14 PM, Sean Mullan wrote:
> On 8/20/19 12:14 PM, Jamil Nimeh wrote:
>> 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/
>
> 406             // Make a modifiable copy of the CertPathChecker list
>
> This comment is not true anymore, so I would delete it.
>
> Looks good otherwise.
>
> --Sean
>
>>
>> --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
>>>>