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