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

[Sean Mullan]

On 09/06/2013 09:30 AM, Weijun Wang wrote:
> Hi Sean
>
> Please review the code changes at
>
>    8011402: Move blacklisting certificate logic from hard code to data
>
> Hard coded blacklisted certificates are moved out of the class file and
> now inside a data file. Furthermore, only their fingerprints are
> released in the JRE. The makefile covers blacklist files in both open
> and closed repo.

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.

UntrustedCertificates:

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

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.

--Sean

> No regression test, cleanup.
>
> *build-dev*, I am not an export of Makefile, and I have some questions:
>
> 1. I create a new macro (or function?) called cat-files. Its only
> difference from install-file is that it needs to deal with two inputs.
> Do we already have a similar macro somewhere?
>
> 2. cat-files is defined inside CopyFiles.gmk right beside its usage. Do
> you think it's better to define it in a common file?
>
> 3. Most important: it only works if both $(BLACKLISTED_CERTS_SRC_OPEN)
> and $(BLACKLISTED_CERTS_SRC_CLOSED) already exists. Currently there is
> no closed blacklist, but I still have to create an empty file there.
> Otherwise, there will be
>
> make[2]: *** No rule to make target
> `/space/repos/jdk8/tl/jdk/src/closed/share/lib/security/blacklisted.certs',
> needed by
> `/space/repos/jdk8/tl/build/macosx-x86_64-normal-server-release/jdk/lib/security/blacklisted.certs'.
>   Stop.
>
> Is there a way to make it work without adding that empty file?
>
> Thanks
> Max