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

[Artem Smotrakov]

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