RFR: [8u] JDK-8193255: Root Certificates should be stored in text format and assembled at build time

[Severin Gehwolf]

Hi Andrew,

On Wed, 2020-01-29 at 07:42 +0000, Andrew John Hughes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8193255
> Webrevs:
>   Root: https://cr.openjdk.java.net/~andrew/openjdk8/8193255/root/

This looks trivially fine.

>   JDK: https://cr.openjdk.java.net/~andrew/openjdk8/8193255/jdk/

make/data/cacerts:
  Diffed files with revision 61398c1b7487 of jdk11u. No differences.
  Looks good.

make/CopyFiles.gmk:

  +# CACERTS_FILE is optionally set in configure to override the default cacerts
  +# which is otherwise generated in Gendata-java.base.gmk

  "Gendata-java.base.gmk" does not exist in the 8u backport. Please    
  change that to "GenerateData.gmk".

make/GenerateData.gmk:
  No comments. Looks fine.

make/Tools.gmk:
  No comments. Looks fine.

src/share/lib/security/cacerts:
  Deleting a binary blob. Yay! :)

make/src/classes/build/tools/generatecacerts/GenerateCacerts.java:

  You've updated copyright years in other files to 2020 (over 2019).
  Perhaps that should be done in GenerateCacerts.java too? No need
  for a separate webrev for this.

  50                 if (!fName.equals("README")) {
  51                     String alias = fName + " [jdk]";
  52                     try (InputStream fis = Files.newInputStream(p)) {
  53                         ks.setCertificateEntry(alias, cf.generateCertificate(fis));
  54                     }
  55                 }

  So judging by your explanation below this is intentional. Strictly speaking,
  doing a backport of JDK-8225392 would need to be aware of that. This should be
  OK as we need to account for JDK 7 bootstrap too and you said you are going to
  do JDK-8225392 backport as well.

> This change gets rid of the binary cacerts file held in the repository
> and instead generates it from individual text certificates at build time.

Great!

> For 8u, it's as much a rewrite as a backport, due to changes in the
> build system and the different requirements for build tools (they need
> to work with 10 & 11 in OpenJDK 11, but 7 & 8 in OpenJDK 8)
> 
> The original JDK-8193255 also contains a number of changes to makefiles
> which are really worthy of a separate bug and don't relate to this
> change. These are changes to make/gendata/GendataBlacklistedCerts.gmk,
> make/gendata/GendataBreakIterator.gmk and
> make/gendata/GendataPublicSuffixList.gmk which replace BUILD_TOOLS with
> BUILD_TOOLS_JDK. This is apparently a change that should have been made
> when the repositories were unified [0], and it has little relation to
> the cacerts change, other than the issue was discovered while it was
> being developed. These changes aren't included in the backport,
> especially as two of these makefiles don't even exist in 8u.

OK. Thanks for the detective work. Agreed.

> Backporting
> JDK-8047765 to generate blacklisted.certs may be worthwhile at some
> point, though.
> 
> The makefile changes that are retained have been adapted to the style
> used in 8u, but are in essence the same, though occasionally in
> different locations. The default value for CACERTS_FILE is set by
> configure in 8u, so that is removed in the top-level repository (the
> empty value being used to trigger generation)
> 
> The build tool, GenerateCacerts.java, has been modified to avoid the use
> of lambdas and the functional API from 8u, so it can be built using a
> Java 7 JDK. I actually think this makes the code more readable, avoiding
> some duplication and the need to wrap an exception in a RuntimeException.

That's fine with me.

> JDK-8235142: "JDK-8193255 backport broke bootstrap with JDK 10" is
> effectively included here as removing the use of Path.of() is essential
> when the bootstrap JDKs are either 7 or 8.

Yes, we don't need JDK-8235142 in jdk8u in this case.

> Building with this patch produces a cacerts file containing the same
> certificates as from a build of 8u242, with the exception of the Amazon
> & LuxTrust certificates yet to be backported:
> 
> $ keytool -list -keystore
> /home/andrew/builder/8u/jdk/lib/security/cacerts > 8u.list
> $ keytool -list -keystore
> /home/andrew/builder/8u-dev/jdk/lib/security/cacerts > 8u-dev.list
> $ grep 'trustedCertEntry' 8u-dev.list | cut -d ' ' -f 1|sort > 8u-dev.sort
> $ grep 'trustedCertEntry' 8u.list | cut -d ' ' -f 1|sort > 8u.sort
> $ diff -u 8u.sort 8u-dev.sort --- 8u.sort	2020-01-29 06:24:17.108785705
> +0000
> +++ 8u-dev.sort	2020-01-29 06:24:11.636869008 +0000
> @@ -6,10 +6,6 @@
>  affirmtrustnetworkingca
>  affirmtrustpremiumca
>  affirmtrustpremiumeccca
> -amazonrootca1
> -amazonrootca2
> -amazonrootca3
> -amazonrootca4
>  baltimorecybertrustca
>  buypassclass2ca
>  buypassclass3ca
> @@ -54,7 +50,6 @@
>  identrustpublicca
>  keynectisrootca
>  letsencryptisrgx1
> -luxtrustglobalroot2ca
>  luxtrustglobalrootca
>  quovadisrootca
> 
> Once this is reviewed & approved, we'll follow this up with JDK-8225392
> and as-is text based backports of the Amazon & LuxTrust certificates
> that were added as binaries in 8u242 only.

That's a good plan!

Thanks,
Severin

> [0]
> https://mail.openjdk.java.net/pipermail/security-dev/2019-May/020044.html