[14] RFR 8162628: Migrate cacerts keystore from JKS

[Weijun Wang]

> On Aug 14, 2019, at 3:23 AM, Michael Osipov <1983-01-06 at gmx.net> wrote:
> 
> Am 2019-08-13 um 18:23 schrieb Weijun Wang:
>> Please take a preliminary review at
>> 
>>    https://cr.openjdk.java.net/~weijun/8162628/webrev.00
>> 
>> There is no test yet. I mainly want you to see if this is doable and whether there can be any unexpected compatibility impact.
>> 
>> So, the major points are:
>> 
>> 1. Invent a new KeyStore type named "PEM", which is a stack of PEM-format certificates. It only support X.509 certificates and is read-only (at the moment).
>> 2. Migrate lib/security/cacerts to this format.
>> 
>> Some details:
>> 
>> 1. JKS/PKCS12/PEM is now aliases to each other, which means you can load a PKCS12 keystore using KeyStore.getInstance("pem"). This is an expansion of the former JKS/PKCS12 dual type.
>> 2. PEM supports engineProbe(), and returns true as long as the first 5 bytes are readable ASCII. This is because people might put comment before "-----BEGIN CERT-----".
>> 3. @attr can be added into comment as attributes in the comment area. cacerts will contain "@alias: aliasname". I'm still using the "[jdk]" label in the alias for jdkCA recognition.
> 
> My comments:
> GenerateCacerts:
> * Why not use Files#newBufferedWriter(Path)?

I like println.

> * How does the input look like? Do we only have base 64 data?

Here is an example:

   https://hg.openjdk.java.net/jdk/jdk/file/29ab1f3bd353/make/data/cacerts/actalisauthenticationrootca

>  It might be really helpful to write at least the DN of the cert
>  for analysis purposes. I have written some decent Java code with does
> this.
>  Will to share if you want to.

Thanks. The input already includes something.

> KeyStoreDelegator:
> * Please avoid iterating a list like an array and using List#get(int).

But I have 2 lists. It is a Pair but Java does not have it, and I don't like Map::Entry very much.

> PemKeyStore:
> * It pretty much looks like you don't support private keys. Do you intend
>  to deliver that in a seperate issue?

To be determined, but no requirement now. Also, I don't want to invent a competitor for pkcs12.

> * engineStore(): Why is that not supported? Without password of course.

No requirement. I also don't want to see people losing their comments.

> * engineLoad():
> ** Why not use Reader r = new Buffered...?

readLine() is only in BufferedReader.

> ** split(":\\s*", 2): expect the unexpected: "@", kv might be null.

jshell> "@".substring(1).split(":\\s*", 2)
$1 ==> String[1] { "" }

> ** attr.add(new PemAttribute(kv[0], kv.length == 2 ? kv[1] : "true"));
>   This one is for adding arbitrary attributes, but not alias?

Yes, though I don't have a usage now.

> 
> I will also pass a pretty large cacerts with public CA and our CAs and
> see wether your parser doesn't choke on it.

PEM is certainly slower than JKS because of text reading and de-Base64. I'll see if I can make any enhancement.

In your other mail, you asked about aliases. If there is no "@alias", it will be the finderprint of the cert.

Thanks,
Max

> 
> Michael