RFR: 8325766: Review seclibs tests for cert expiry [v3]

[Matthew Donovan]

On Thu, 23 May 2024 18:23:28 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> I renamed the method. 
>> 
>> I don't want to over-generalize the code when I don't know what we'll need/want in the future. The tests in this PR just create CA and end-entity certs and with a couple exceptions, the tests in this PR are all of the tests that fail when the system clock is set to 2050. The exceptions are also client/server tests that don't need code signer and TSA certificates. 
>> 
>> When considering your comment, I went through the tests in this PR and found and removed some additional redundancy.
>
> The problem with generalizing an end-entity certificate is that there is not much you can generalize other than the cA field of the BasicConstraints extension being false (or not including the BC extension). Right now the newEndEntity() method looks like it is for TLS server certificates based on the KeyUsage extension (but it is also missing the KeyAgreement bit). But you could go further and add the EKU extension with the TLS server bit set. It shouldn't be used for TLS client certificates because they would have different KU extension values. Same for code signing end entity certificates. If you don't make it more specific, I feel that it is likely to be used to create certificates incorrectly.
> 
> So if you only need to create TLS server certs, I suggest renaming this method to newTLSServer() to make it clear it is for TLS server certs only. 
> 
> If a test needs to create certs which don't conform to a specific profile or have invalid fields, then it is probably better off calling CertificateBuilder methods itself rather than trying to create a helper method that satisfies all types of use cases.

The intent of these methods was not to create full certificates to satisfy lots of use cases but to create a CertificateBuilder object with the common fields (subject, serial number, validity, etc.) set. 

I see what you're saying about some of the extensions and created `newServerCertificateBuilder` and `newClientCertificateBuilder` methods that set key usage and the EKU extension for each.

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

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