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