Thread: pgsql: Refactor the sslfiles Makefile target for ease of use
Refactor the sslfiles Makefile target for ease of use The Makefile handling of certificate and keypairs used for TLS testing had become quite difficult to work with. Adding a new cert without the need to regenerate everything was too complicated. This patch refactors the sslfiles make target such that adding a new certificate requires only adding a .config file, adding it to the top of the Makefile, and running make sslfiles. Improvements: - Interfile dependencies should be fixed, with the exception of the CRL dirs. - New certificates have serial numbers based on the current time, reducing the chance of collision. - The CA index state is created on demand and cleaned up automatically at the end of the Make run. - *.config files are now self-contained; one certificate needs one config file instead of two. - Duplication is reduced, and along with it some unneeded code (and possible copy-paste errors). - all configuration files underneath the conf/ directory. The target is moved to its own makefile in order to avoid colliding with global make settings. Author: Jacob Champion <pchampion@vmware.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/d15a9838344ba090e09fd866abf913584ea19fb7.camel@vmware.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/b4c4a00eada3c512e819e9163114a5ad1606bc7e Modified Files -------------- src/test/ssl/Makefile | 167 +------------- src/test/ssl/README | 4 +- src/test/ssl/{ => conf}/cas.config | 10 +- src/test/ssl/{ => conf}/client-dn.config | 1 - src/test/ssl/conf/client-revoked.config | 13 ++ src/test/ssl/{ => conf}/client.config | 1 - src/test/ssl/{ => conf}/client_ca.config | 5 + src/test/ssl/{ => conf}/root_ca.config | 1 + .../ssl/{ => conf}/server-cn-and-alt-names.config | 0 src/test/ssl/{ => conf}/server-cn-only.config | 3 +- .../{ => conf}/server-multiple-alt-names.config | 0 src/test/ssl/{ => conf}/server-no-names.config | 5 +- src/test/ssl/{ => conf}/server-revoked.config | 3 +- .../ssl/{ => conf}/server-single-alt-name.config | 0 src/test/ssl/{ => conf}/server_ca.config | 5 + src/test/ssl/ssl/both-cas-1.crt | 86 ++++---- src/test/ssl/ssl/both-cas-2.crt | 86 ++++---- src/test/ssl/ssl/client+client_ca.crt | 65 +++--- src/test/ssl/ssl/client-crldir/9bb9e3c3.r0 | 18 +- src/test/ssl/ssl/client-dn.crt | 34 +-- src/test/ssl/ssl/client-revoked.crt | 31 +-- src/test/ssl/ssl/client.crl | 18 +- src/test/ssl/ssl/client.crt | 31 +-- src/test/ssl/ssl/client_ca.crt | 34 +-- src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0 | 18 +- src/test/ssl/ssl/root+client-crldir/a3d11bff.r0 | 16 +- src/test/ssl/ssl/root+client.crl | 34 +-- src/test/ssl/ssl/root+client_ca.crt | 52 ++--- src/test/ssl/ssl/root+server-crldir/a3d11bff.r0 | 16 +- src/test/ssl/ssl/root+server-crldir/a836cc2d.r0 | 18 +- src/test/ssl/ssl/root+server.crl | 34 +-- src/test/ssl/ssl/root+server_ca.crt | 52 ++--- src/test/ssl/ssl/root.crl | 16 +- src/test/ssl/ssl/root_ca.crt | 18 +- src/test/ssl/ssl/server-cn-and-alt-names.crt | 36 +-- src/test/ssl/ssl/server-cn-only.crt | 33 +-- src/test/ssl/ssl/server-crldir/a836cc2d.r0 | 18 +- src/test/ssl/ssl/server-multiple-alt-names.crt | 36 +-- src/test/ssl/ssl/server-no-names.crt | 32 +-- src/test/ssl/ssl/server-revoked.crt | 33 +-- src/test/ssl/ssl/server-single-alt-name.crt | 34 +-- src/test/ssl/ssl/server.crl | 18 +- src/test/ssl/ssl/server_ca.crt | 34 +-- src/test/ssl/sslfiles.mk | 244 +++++++++++++++++++++ src/test/ssl/t/001_ssltests.pl | 17 +- 45 files changed, 778 insertions(+), 652 deletions(-)
> On 19 Oct 2021, at 20:17, Daniel Gustafsson <dgustafsson@postgresql.org> wrote: > > Refactor the sslfiles Makefile target for ease of use This made prairiedog fall over in the SSL tests [0], with what seems a pretty straightforward reason: 32- vs 64-bit Perl. Integer overflow in hexadecimal number at t/001_ssltests.pl line 493. Illegal hexadecimal digit ' ' ignored at t/001_ssltests.pl line 493. Hexadecimal number > 0xffffffff non-portable at t/001_ssltests.pl line 493. The conversion of the serialnumber from hex to base10 requires a 64-bit capable Perl, and my guess is that prairiedog has a 32-bit Perl. I installed 5.8.3 in 64-bit mode and verified that it does work there (using the perlbrew recipe in 3eb1f4d097454 which is fantastic). Attached diff passes tests for me, but I don't have a 32-bit system handy so I prefer some more eyes on it before pushing. The most portable way I could find to determine 32/64 bit in Perl was to use Config and check ivsize. It works in my 5.8.3 installation, but adding Andrew on CC to get a Perl expert opinion on that. -- Daniel Gustafsson https://vmware.com/ [0] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prairiedog&dt=2021-10-20%2005%3A32%3A46
Attachment
Daniel Gustafsson <daniel@yesql.se> writes:
> The conversion of the serialnumber from hex to base10 requires a 64-bit capable
> Perl, and my guess is that prairiedog has a 32-bit Perl.
Yup, it does.
regards, tom lane
> On 20 Oct 2021, at 15:33, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Daniel Gustafsson <daniel@yesql.se> writes: >> The conversion of the serialnumber from hex to base10 requires a 64-bit capable >> Perl, and my guess is that prairiedog has a 32-bit Perl. > > Yup, it does. Thanks for confirming, then my fix is attacking the right problem. Unless there are objections raised to using Config I'll push this in the morning when I can keep an eye on the buildfarm. -- Daniel Gustafsson https://vmware.com/
Hi,
On 2021-10-19 18:17:34 +0000, Daniel Gustafsson wrote:
> Refactor the sslfiles Makefile target for ease of use
>
> The Makefile handling of certificate and keypairs used for TLS testing
> had become quite difficult to work with. Adding a new cert without the
> need to regenerate everything was too complicated. This patch refactors
> the sslfiles make target such that adding a new certificate requires
> only adding a .config file, adding it to the top of the Makefile, and
> running make sslfiles.
This fails during 'make clean' in a vpath build, because sslfiles.mk cannot be
found. Probably just needs
diff --git i/src/test/ssl/Makefile w/src/test/ssl/Makefile
index 3592c0d184f..cb24e31c6e2 100644
--- i/src/test/ssl/Makefile
+++ w/src/test/ssl/Makefile
@@ -19,11 +19,11 @@ export with_ssl
# with settings in Makefile.global.
.PHONY: sslfiles sslfiles-clean
sslfiles sslfiles-clean:
- $(MAKE) -f sslfiles.mk $@
+ $(MAKE) -f $(srcdir)/sslfiles.mk $@
clean distclean maintainer-clean:
rm -rf tmp_check
- $(MAKE) -f sslfiles.mk $@
+ $(MAKE) -f $(srcdir)/sslfiles.mk $@
# Doesn't depend on sslfiles because we don't rebuild them by default
check:
Greetings,
Andres Freund
> On 26 Oct 2021, at 19:41, Andres Freund <andres@anarazel.de> wrote: > > Hi, > > On 2021-10-19 18:17:34 +0000, Daniel Gustafsson wrote: >> Refactor the sslfiles Makefile target for ease of use >> >> The Makefile handling of certificate and keypairs used for TLS testing >> had become quite difficult to work with. Adding a new cert without the >> need to regenerate everything was too complicated. This patch refactors >> the sslfiles make target such that adding a new certificate requires >> only adding a .config file, adding it to the top of the Makefile, and >> running make sslfiles. > > This fails during 'make clean' in a vpath build, because sslfiles.mk cannot be > found. Probably just needs That seems reasonable, I will look into it and fix it. Sorry about that. -- Daniel Gustafsson https://vmware.com/
> On 26 Oct 2021, at 19:41, Andres Freund <andres@anarazel.de> wrote: > This fails during 'make clean' in a vpath build, because sslfiles.mk cannot be > found. Probably just needs It does in fact need that diff, but sslfiles.mk also needs to be taught about the current $(srcdir) to properly function under VPATH for "make sslfiles". Since it, by design, doesn't include Makefile.global it must be passed in during invocation. Since you are the VPATH expert, does the attached look sane? It passes for me locally but I almost never use VPATH to another set of eyes would be good. -- Daniel Gustafsson https://vmware.com/
Attachment
> On 27 Oct 2021, at 16:37, Daniel Gustafsson <daniel@yesql.se> wrote: > >> On 26 Oct 2021, at 19:41, Andres Freund <andres@anarazel.de> wrote: > >> This fails during 'make clean' in a vpath build, because sslfiles.mk cannot be >> found. Probably just needs > > It does in fact need that diff, but sslfiles.mk also needs to be taught about > the current $(srcdir) to properly function under VPATH for "make sslfiles". Discussing this off-list with Jacob, and dabbling some more with it, this needs more thought. It's not clear what make sslfiles should do in a VPATH build, and where, since the artefacts are both generated and distributed. I've pushed the small fix to ensure "make clean" works as that's a much more common usecase, to allow for time to discuss "make sslfiles" on -hackers. -- Daniel Gustafsson https://vmware.com/
i, On 2021-10-27 21:57:42 +0200, Daniel Gustafsson wrote: > > On 27 Oct 2021, at 16:37, Daniel Gustafsson <daniel@yesql.se> wrote: > > > >> On 26 Oct 2021, at 19:41, Andres Freund <andres@anarazel.de> wrote: > > > >> This fails during 'make clean' in a vpath build, because sslfiles.mk cannot be > >> found. Probably just needs > > > > It does in fact need that diff, but sslfiles.mk also needs to be taught about > > the current $(srcdir) to properly function under VPATH for "make sslfiles". I think that may previously also not have worked. I mostly care about getting make clean to work again... > Discussing this off-list with Jacob, and dabbling some more with it, this needs > more thought. It's not clear what make sslfiles should do in a VPATH build, > and where, since the artefacts are both generated and distributed. It's imo pretty clear that it'd need to put the files in the source directory. > I've pushed the small fix to ensure "make clean" works as that's a much more > common usecase, to allow for time to discuss "make sslfiles" on -hackers. Cool! Greetings, Andres Freund