I had another look at this today and I think this patch is in pretty good
shape, below are a few comments on this revision:
- 'sslinfo--1.2.sql',
+ 'sslinfo--1.2--1.3.sql',
+ 'sslinfo--1.3.sql',
The way we typically ship extensions in contrib/ is to not create a new base
version .sql file for smaller changes like adding a few functions. For this
patch we should keep --1.2.sql and instead supply a 1.2--1.3.sql with the new
functions.
+ <structfield>not_befoer</structfield> <type>text</type>
s/not_befoer/not_before/
+ errmsg("failed to convert tm to timestamp")));
I think this error is too obscure for the user to act on, what we use elsewhere
is "timestamp out of range" and I think thats more helpful. I do wonder if
there is ever a legitimate case when this can fail while still having a
authenticated client connection?
+ ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,/?CN=ssltestuser,$serialno,/?\QCN=Test CA for PostgreSQL SSL regression test client
certs\E,\Q2021-03-0322:12:07\E,\Q2048-07-19 22:12:07\E\r?$}mx,
This test output won't actually work for testing against, it works now because
the dates match the current set of certificates, but the certificates can be
regenerated with `cd src/test/ssl && make -f sslfiles.mk` and that will change
the not_before/not_after dates. In order to have stable test data we need to
set fixed end/start dates and reissue all the client certs.
+ "SELECT ssl_client_get_notbefore() = not_before FROM pg_stat_ssl WHERE pid = pg_backend_pid();",
+ connstr => $common_connstr);
+is($result, 't', "ssl_client_get_notbefore() for not_before timestamp");
While this works, it will fail to catch if we have the same bug in both sslinfo
and the backend. With stable test data we can add the actual date in the mix
and verify that both timestamps are equal and that they match the expected
date.
I have addressed the issues above in a new v5 patchset which includes a new
patch for setting stable validity on the test certificates (the notBefore time
was arbitrarily chosen to match the date of opening up the tree for v17 - we
just need a date in the past). Your two patches are rolled into a single one
with a commit message added to get started on that part as well.
--
Daniel Gustafsson