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

Reply via email to