Code review request: 8011402: Move blacklisting certificate logic from hard code to data
Erik Joelsson
erik.joelsson at oracle.com
Fri Sep 6 14:07:47 UTC 2013
Hello Max,
I couldn't find the link to the review but I'm guessing this is the one:
http://cr.openjdk.java.net/~weijun/8011402/webrev.00/
On 2013-09-06 15:30, 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.
>
> 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?
>
I do not think so.
> 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?
>
We could put this in a common file, in that case it would be
common/makefiles/MakeBase.gmk. However, this macro does more than just
cat files. It sorts and removes duplicates too. In this case, since it's
used only once, I would skip the macro def and just inline the recipe.
Also note that prep-target doesn't exist anymore. What you want is probably:
$(MKDIR) -p $(@D)
> 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?
Yes:
BLACKLISTED_CERTS_SRCS := /path/to/open/certs
ifndef OPENJDK
BLACKLISTED_CERTS_SRCS += /path/to/closed/certs
endif
$(BLACKLISTED_CERTS_DEST): $(BLACKLISTED_CERTS_SRCS)
<do stuff>
/Erik
>
> Thanks
> Max
More information about the security-dev
mailing list