Re: Support for NSS as a libpq TLS backend

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Jacob Champion <pchampion(at)vmware(dot)com>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "andrew(dot)dunstan(at)2ndquadrant(dot)com" <andrew(dot)dunstan(at)2ndquadrant(dot)com>, "thomas(dot)munro(at)gmail(dot)com" <thomas(dot)munro(at)gmail(dot)com>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>
Subject: Re: Support for NSS as a libpq TLS backend
Date: 2021-03-24 03:54:32
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Mar 23, 2021 at 12:38:50AM +0100, Daniel Gustafsson wrote:
> Thanks again for reviewing, another version which addresses the remaining
> issues will be posted soon but I wanted to get this out to give further reviews
> something that properly works.

I have been looking at the infrastructure of the tests, patches 0002
(some refactoring) and 0003 (more refactoring with tests for NSS), and
I am a bit confused by its state.

First, I think that the split is not completely clear. For example,
patch 0003 has changes for and, but wouldn't it
be better to have all the refactoring infrastructure only in 0002,
with 0003 introducing only the NSS pieces for its internal data and

+ keyfile => 'server-password',
+ nssdatabase => 'server-cn-only.crt__server-password.key.db',
+ passphrase_cmd => 'echo secret1', and have NSS-related parameters, which
does not look like a clean separation to me as there are OpenSSL tests
that use some NSS parts, and the main scripts should remain neutral in
terms setting contents, including only variables and callbacks that
should be filled specifically for each SSL implementation, no? Aren't
we missing a second piece here with a set of callbacks for the
per-library test paths then?

+ if (defined($openssl))
+ {
+ copy_files("ssl/server-*.crt", $pgdata);
+ copy_files("ssl/server-*.key", $pgdata);
+ chmod(0600, glob "$pgdata/server-*.key") or die $!;
+ copy_files("ssl/root+client_ca.crt", $pgdata);
+ copy_files("ssl/root_ca.crt", $pgdata);
+ copy_files("ssl/root+client.crl", $pgdata);
+ mkdir("$pgdata/root+client-crldir");
+ copy_files("ssl/root+client-crldir/*",
+ }
+ elsif (defined($nss))
+ {
+ RecursiveCopy::copypath("ssl/nss", $pgdata . "/nss") if -e
+ }
This had better be in its own callback, for example.

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Jaime Casanova 2021-03-24 04:28:55 Re: SQL-standard function body
Previous Message Michael Paquier 2021-03-24 03:25:18 Re: Autovacuum worker doesn't immediately exit on postmaster death