On Mon, Mar 4, 2024 at 6:23 AM Daniel Gustafsson <dan...@yesql.se> wrote:
> > On 12 Sep 2023, at 21:40, Jacob Champion <jchamp...@timescale.com> wrote:
Sorry for the long delay!
> >> + <function>ssl_client_get_notbefore() returns text</function>
> >> ...> + <function>ssl_client_get_notafter() returns text</function>
> >
> > I think this should say timestamptz rather than text? Ditto for the
> > pg_stat_ssl documentation.
> >
> > Speaking of which: is the use of `timestamp` rather than `timestamptz`
> > in pg_proc.dat intentional? Will that cause problems with comparisons?
>
> It should be timestamptz, it was a tyop on my part. Fixed.
Looks like sslinfo--1.2--1.3.sql is also declaring the functions as
timestamp rather than timestamptz, which is breaking comparisons with
the not_before/after columns. It might also be nice to rename
ASN1_TIME_to_timestamp().
Squinting further at the server backend implementation, should that
also be using TimestampTz throughout, instead of Timestamp? It all
goes through float8_timestamptz at the end, so I guess it shouldn't
have a material impact, but it's a bit confusing.
> Thanks for reviewing, the attached v8 contains the fixes from this review
> along
> with a fresh rebase and some attempts at making tests more stable in the face
> of timezones by casting to date.
In my -08 timezone, the date doesn't match what's recorded either
(it's my "tomorrow"). I think those probably just need to be converted
to UTC explicitly? I've attached a sample diff on top of v8 that
passes tests on my machine.
--Jacob
diff --git a/contrib/sslinfo/sslinfo--1.2--1.3.sql
b/contrib/sslinfo/sslinfo--1.2--1.3.sql
index 9d64d2bfa4..424a11afe4 100644
--- a/contrib/sslinfo/sslinfo--1.2--1.3.sql
+++ b/contrib/sslinfo/sslinfo--1.2--1.3.sql
@@ -3,10 +3,10 @@
-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "CREATE EXTENSION sslinfo" to load this file. \quit
-CREATE FUNCTION ssl_client_get_notbefore() RETURNS timestamp
+CREATE FUNCTION ssl_client_get_notbefore() RETURNS timestamptz
AS 'MODULE_PATHNAME', 'ssl_client_get_notbefore'
LANGUAGE C STRICT PARALLEL RESTRICTED;
-CREATE FUNCTION ssl_client_get_notafter() RETURNS timestamp
+CREATE FUNCTION ssl_client_get_notafter() RETURNS timestamptz
AS 'MODULE_PATHNAME', 'ssl_client_get_notafter'
LANGUAGE C STRICT PARALLEL RESTRICTED;
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 490c48a7bb..90a4230413 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -740,10 +740,10 @@ command_like(
"$common_connstr user=ssltestuser sslcert=ssl/client.crt "
. sslkey('client.key'),
'-c',
- "SELECT
ssl,version,cipher,bits,client_dn,client_serial,issuer_dn,not_before::date,not_after::date
FROM pg_stat_ssl WHERE pid = pg_backend_pid()"
+ "SELECT
ssl,version,cipher,bits,client_dn,client_serial,issuer_dn,not_before AT TIME
ZONE 'UTC' AS not_before,not_after AT TIME ZONE 'UTC' AS not_after FROM
pg_stat_ssl WHERE pid = pg_backend_pid()"
],
qr{^ssl,version,cipher,bits,client_dn,client_serial,issuer_dn,not_before,not_after\r?\n
-
^t,TLSv[\d.]+,[\w-]+,\d+,/?CN=ssltestuser,$serialno,/?\QCN=Test CA for
PostgreSQL SSL regression test client certs,2023-06-29,2050-01-01\E\r?$}mx,
+
^t,TLSv[\d.]+,[\w-]+,\d+,/?CN=ssltestuser,$serialno,/?\QCN=Test CA for
PostgreSQL SSL regression test client certs,2023-06-29 01:01:01,2050-01-01
01:01:01\E\r?$}mx,
'pg_stat_ssl with client certificate');
# client key with wrong permissions
diff --git a/src/test/ssl/t/003_sslinfo.pl b/src/test/ssl/t/003_sslinfo.pl
index 587c0e2dce..4df3a941b5 100644
--- a/src/test/ssl/t/003_sslinfo.pl
+++ b/src/test/ssl/t/003_sslinfo.pl
@@ -167,15 +167,15 @@ is($result, 't', "ssl_issuer_field() for commonName");
$result = $node->safe_psql(
"certdb",
- "SELECT ssl_client_get_notbefore()::date = not_before::date, "
- . "not_before::date = '2023-06-29' FROM pg_stat_ssl WHERE pid =
pg_backend_pid();",
+ "SELECT ssl_client_get_notbefore() = not_before, "
+ . "not_before AT TIME ZONE 'UTC' = '2023-06-29 01:01:01' FROM
pg_stat_ssl WHERE pid = pg_backend_pid();",
connstr => $common_connstr);
is($result, 't|t', "ssl_client_get_notbefore() for not_before timestamp");
$result = $node->safe_psql(
"certdb",
- "SELECT ssl_client_get_notafter()::date = not_after::date, "
- . "not_after::date = '2050-01-01' FROM pg_stat_ssl WHERE pid =
pg_backend_pid();",
+ "SELECT ssl_client_get_notafter() = not_after, "
+ . "not_after AT TIME ZONE 'UTC' = '2050-01-01 01:01:01' FROM
pg_stat_ssl WHERE pid = pg_backend_pid();",
connstr => $common_connstr);
is($result, 't|t', "ssl_client_get_notafter() for not_after timestamp");
