[9] RFR: 8134708: Certpath validation fails to load certs and CRLs if AIA and CRLDP extensions point to LDAP resources

[Seán Coffey]

Looks good to me Artem. Thanks for modifying!

Regards,
Sean.

On 08/09/2015 18:00, Artem Smotrakov wrote:
> Hi Sean,
>
> I updated LDAPCertStore, please take a look
>
> http://cr.openjdk.java.net/~asmotrak/8134708/webrev.02/
>
> - updated exception messages
> - fixed typos
> - added @Override annotations
> - removed unused field
> - removed unused imports
>
> Artem
>
> On 09/02/2015 01:23 PM, Seán Coffey wrote:
>> Hi Artem,
>>
>> I'll let the main review to other reviewers but while we're here, can 
>> you consider improving the original exception message that was seen 
>> in this issue ?
>> In LDAPCertStore constructor :
>>
>>         } else {
>>             throw new InvalidAlgorithmParameterException(
>>                 "parameters must be either LDAPCertStoreParameters or " +
>>                 "URICertStoreParameters");
>>         }
>>
>> Can we print the instance type of the 'params' variable in the 
>> exception message ? params.getClass().getName() should be sufficient.
>>
>> I see 2-3 other exceptions in LDAPCertStore that could be improved 
>> there also. If you can change them, that would be great  - otherwise 
>> we can follow up with  enhancement request.
>>
>>             if (!u.getScheme().equalsIgnoreCase("ldap")) {
>>                 throw new InvalidAlgorithmParameterException(
>>                 "Only LDAP URIs are supported for LDAP Certore");
>>
>> Let's print the scheme received!
>>
>>
>>         } else if (!(selector instanceof X509CertSelector)) {
>>             throw new CertStoreException("need X509CertSelector to 
>> find certs");
>>
>> this code occurs twice. Let's print the selector class received.
>>
>> Regards,
>> Sean.
>> On 02/09/15 00:15, Artem Smotrakov wrote:
>>> Hello,
>>>
>>> Please review this fix for 9.
>>>
>>> Certpath validation fails to load certs and CRLs if AIA and CRLDP 
>>> extensions point to LDAP resources. This happens because 
>>> LDAPCertStore accepts only instances of LDAPCertStoreParameters and 
>>> URICertStoreParameters classes, but 
>>> sun.security.provider.certpath.URICertStore uses an inner static 
>>> URICertStoreParameters class. Please see details in the bug.
>>>
>>> This fix removes URICertStore.URICertStoreParameters class, and 
>>> updates URICertStore and DistributionPointFetcher to use new 
>>> java.security.cert.URICertStoreParameters class.
>>>
>>> A regression test starts a local name service which logs requested 
>>> host names. The test checks that host names from AIA and CRLDP 
>>> extensions were loaded and requested to resolve during certpath 
>>> validation.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8134708
>>> Webrev: http://cr.openjdk.java.net/~asmotrak/8134708/webrev.01/
>>>
>>> Artem
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20150908/47493d04/attachment.htm>