Code review request: 8011402: Move blacklisting certificate logic from hard code to data

Weijun Wang weijun.wang at oracle.com
Mon Sep 9 07:15:43 UTC 2013



On 9/7/13 1:26 AM, Sean Mullan wrote:
>>    8011402: Move blacklisting certificate logic from hard code to data
>>
>
> X509CertImpl:
>
> Might it not be better to store the fingerprints in
> UntrustedCertificates in a WeakHashMap (using the Certificate as a key)?
> This way we don't need to add public mutator methods to this immutable
> (for the most part) class. If you agree, we should also change
> Certificate.hashCode to cache the hashcode instead of calculating it
> every time.

I'll do it.

>
> UntrustedCertificates:
>
> [65] We should log the exception (could be a parsing error, so we would
> want to know that)

You mean for -Djava.security.debug=certpath?

>
> BlacklistedCertsConverter:
>
> I'm a little concerned that this tool re-writes the blacklisted.certs
> file each time, as a mistake could wipe out previous entries. I would
> prefer if it just appended to the existing file. I would suggest that
> the input be a file containing a single PEM encoded cert, and that the
> tool append the hash to the blacklist.certs file, and the PEM cert to
> the blacklist.pem file.

There are several reasons I chose the current approach:

1. A mistake won't wipe out previous entries, the file is in source code 
control.

2. It still allows people adding comments to the blacklisted.certs.pem file.

3. The tool will also be used to generate the file in the closed repo. 
Either it will need extra arguments for files it appends to or it must 
be called in the same directory. Either looks like extra burden.

4. It allows re-order of blacklisted.certs.pem file.

5. It allows the developer running the tool again and again, but not 
something like

    false run -> revert -> re-run.

Thanks
Max



More information about the security-dev mailing list