Re: [PATCH] test/ssl: rework the sslfiles Makefile target

On Wed, 2021-07-28 at 00:24 +0200, Daniel Gustafsson wrote:
> > On 4 Mar 2021, at 01:03, Jacob Champion <pchampion(at)vmware(dot)com> wrote:
> > Andrew pointed out elsewhere [1] that it's pretty difficult to add new
> > certificates to the test/ssl suite without blowing away the current
> > state and starting over. I needed new cases for the NSS backend work,
> > and ran into the same pain, so here is my attempt to improve the
> > situation.
>
> Thanks for working on this, I second the pain cited. I've just started to look
> at this, so only a few comments thus far.
>
> > The unused server-ss certificate has been removed entirely.
>
> Nice catch, this seems to have been unused since the original import of the SSL
> test suite. To cut down scope of the patch (even if only a small bit) I
> propose to apply this separately first, as per the attached.

LGTM.

> > - Serial number collisions are less likely, thanks to Andrew's idea to
> > use the current clock time as the initial serial number in a series.
>
> +my $serialno = `openssl x509 -serial -noout -in ssl/client.crt`;
> +$serialno =~ s/^serial=//;
> +$serialno = hex($serialno); # OpenSSL prints serial numbers in hexadecimal
>
> Will that work on Windows? We don't currently require the openssl binary to be
> in PATH unless one wants to rebuild sslfiles (which it is quite likely to be
> but there should at least be errorhandling covering when it's not).

Hm, that's a good point. It should be easy enough for me to add a
fallback if the invocation fails; I'll give it a shot tomorrow.

> > - I am making _heavy_ use of GNU Make-isms, which does not improve
> > long-term maintainability.
>
> GNU Make is already a requirement, I don't see this shifting the needle in any
> direction.

As long as the .SECONDARYEXPANSION magic is clear enough to others, I'm
happy.

Thanks!
--Jacob