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

[Andrew John Hughes]

On 29/01/2020 16:26, Severin Gehwolf wrote:
> 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".

Yeah, it was included as is in the original patch.

Updated in
https://cr.openjdk.java.net/~andrew/openjdk8/8193255/jdk/webrev.02/

> 
> 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.

Yeah, that was missed because the others are copyright changes in
existing files in the original backport, while GenerateCacerts.java is a
new file. Updated now in the webrev above.

> 
>   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.
> 

What's intentional? Sorry, you've lost me here.

Yes, 8225392 will follow. It makes the cacerts files comparable again.
At present, they are generated with the current date.

snip...

> 
>> 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.

8193255 is included in this webrev and I intend to list it in the
commit. My point was that it's impossible to build anything without
that, as neither bootstrap JDK has Path.of.

Let me know if this is now ok with these minor changes,
Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew