On Tue, 3 Jun 2025 17:29:13 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> As a constructor, the code would look like this
>>
>>
>> public CertificateBuilder(String subjectName,
>> PublicKey publicKey, PublicKey caKey,
>> KeyUsage... keyUsages)
>> throws CertificateException, IOException {
>> factory = CertificateFactory.getInstance("X.509");
>> SecureRandom random = new SecureRandom();
>>
>> boolean [] keyUsage = new boolean[KeyUsage.values().length];
>> for (KeyUsage ku : keyUsages) {
>> keyUsage[ku.ordinal()] = true;
>> }
>>
>> this.subjectName = new X500Principal(subjectName);
>> this.publicKey = Objects.requireNonNull(publicKey, "Caught null
>> public key");
>> notBefore = (Date)Date.from(Instant.now().minus(1,
>> ChronoUnit.HOURS));
>> notAfter = Date.from(Instant.now().plus(1, ChronoUnit.HOURS));
>> serialNumber = BigInteger.valueOf(random.nextLong(1000000)+1);
>> byte[] keyIdBytes = new KeyIdentifier(publicKey).getIdentifier();
>> Extension e = new SubjectKeyIdentifierExtension(keyIdBytes);
>> extensions.put(e.getId(), e);
>>
>> KeyIdentifier kid = new KeyIdentifier(caKey);
>> e = new AuthorityKeyIdentifierExtension(kid, null, null);
>> extensions.put(e.getId(), e);
>>
>> if (keyUsages.length != 0) {
>> e = new KeyUsageExtension(keyUsage);
>> extensions.put(e.getId(), e);
>> }
>> }
>>
>>
>>
>>> it also means I can't use this method to to create a certificate with a
>>> longer, or shorter validity period
>>
>> There are methods to set the not-before and not-after fields. Any field set
>> in this static method can be changed:
>>
>> `
>> newCertificateBuilder(...).notAfter(Date.from(Instant.now().plus(10,
>> TimeUnit.YEARS)));
>> `
>>
>> I don't like using `Date` here and would be happy to overload it to take
>> `Instant` as well.
>
>> As a constructor, the code would look like this
>
> <snip>
>
> Ah, I see. Well technically you could call the set methods from the ctor but
> you would get `this-escape` compiler warnings, which you probably want to
> avoid.
>
>> > it also means I can't use this method to to create a certificate with a
>> > longer, or shorter validity period
>>
>> There are methods to set the not-before and not-after fields. Any field set
>> in this static method can be changed:
>>
>> `newCertificateBuilder(...).notAfter(Date.from(Instant.now().plus(10,
>> TimeUnit.YEARS)));`
>>
>> I don't like using `Date` here and would be happy to overload it to take
>> `Instant` as well.
>
> Yes, but I don't think the static method which is generically named
> `newCertificateBuilder` should be hard-coding a one hour validity period,
> that detail should be either in a different method (my preference) or this
> method should be named more clearly like `oneHourCertificateBuilder`. I
> realize this is just a test method, but this is a nicely designed API that
> has been very useful so I would prefer if we keep the flexibility.
I removed the default one-hour validity from the builder method and created a
`setOneHourValidity()` method.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23700#discussion_r2303642791
