On Thu, 25 Apr 2024 17:20:11 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:

> For this PR, I identified TLS tests that can fail due to hard-code 
> certificates expiring. I updated those tests to use certificates that are 
> generated programmatically. This includes adding some helper methods to the 
> CertificateBuilder class to create builder objects with common default 
> values. The builder can be further customized to meet the needs of the test. 
> I also refactored some of the tests to put common functionality in base 
> classes, further reducing duplicated code.
> 
> This PR does not include changes to SSLContextTemplate and the tests that use 
> it. These tests require significant refactoring to incorporate 
> programmatically generated certificates and should be a separate task.

test/jdk/java/security/testlibrary/CertificateBuilder.java line 417:

> 415:      * @throws Exception
> 416:      */
> 417:     public static CertificateBuilder 
> createClientCertificateBuilder(String subjectName, PublicKey publicKey,

Suggest renaming to `newEndEntity()`. I think it would be more useful to pass 
in the type of end-entity certificate you want to create as a parameter, like 
TLS Server, TLS client, code signer, Time Stamp Authority, etc, and then the 
method creates the recommended key usage and extended key usages for each type, 
and maybe some of the extensions as well, although you may need more parameters 
and overloaded methods in those cases. You can use an Enum for the type. See 
`sun.security.validator` classes for similar ideas.

test/jdk/java/security/testlibrary/CertificateBuilder.java line 447:

> 445:      * @throws Exception
> 446:      */
> 447:     public static CertificateBuilder createCACertificateBuilder(String 
> subject, KeyPair caKey,

Suggest renaming method to `newSelfSignedCA()`.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18958#discussion_r1608898347
PR Review Comment: https://git.openjdk.org/jdk/pull/18958#discussion_r1608893502

Reply via email to