Horton

Re: Support for NSS as a libpq TLS backend

Daniel Gustafsson <daniel@yesql.se>

View thread View on postgr.es
From: Daniel Gustafsson <daniel@yesql.se>
To: Michael Paquier <michael@paquier.xyz>
Cc: Jacob Champion <pchampion@vmware.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Andres Freund <andres@anarazel.de>, Postgres hackers <pgsql-hackers@lists.postgresql.org>, Andrew Dunstan <andrew.dunstan@2ndquadrant.com>, Stephen Frost <sfrost@snowman.net>, Thomas Munro <thomas.munro@gmail.com>
Date: 2021-01-19T20:21:41Z
Lists: pgsql-hackers

Attachments

> On 18 Jan 2021, at 08:08, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Tue, Nov 17, 2020 at 04:00:53PM +0100, Daniel Gustafsson wrote:
>> Nice, thanks for the fix!  I've incorporated your patch into the attached v20
>> which also fixes client side error reporting to be more readable.  The SCRAM
>> tests are now also hooked up, albeit with SKIP blocks for NSS, so they can
>> start getting fixed.
> 
> On top of the set of TODO items mentioned in the logs of the patches,
> this patch set needs a rebase because it does not apply.

Fixed in the attached, which also addresses the points raised earlier by Jacob
as well as adds certificates created entirely by NSS tooling as well as initial
cryptohash support.  There is something iffy with these certs (the test fails
on mismatching ciphers and/or signature algorithms) that I haven't been able to
pin down, but to get more eyes on this I'm posting the patch with the test
enabled.  The NSS toolchain requires interactive input which makes the Makefile
a bit hacky, ideas on cleaning that up are appreciated.

> In order to
> move on with this set, I would suggest to extract some parts of the
> patch set independently of the others and have two buildfarm members
> for the MSVC and non-MSVC cases to stress the parts that can be
> committed.  Just seeing the size, we could move on with:
> - The ./configure set, with the change to introduce --with-ssl=openssl. 
> - 0004 for strong randoms.
> - Support for cryptohashes.

I will leave it to others to decide the feasibility of this, I'm happy to slice
and dice the commits into smaller bits to for example separate out the
--with-ssl autoconf change into a non NSS dependent commit, if that's wanted.

> +/*
> + * BITS_PER_BYTE is also defined in the NSPR header files, so we need to undef
> + * our version to avoid compiler warnings on redefinition.
> + */
> +#define pg_BITS_PER_BYTE BITS_PER_BYTE
> +#undef BITS_PER_BYTE
> This could be done separately.

Based on an offlist discussion I believe this was a misunderstanding, but if I
instead misunderstood that feel free to correct me with how you think this
should be done.

> src/sgml/libpq.sgml needs to document PQdefaultSSLKeyPassHook_nss, no?

Good point, fixed.

cheers ./daniel

Commits

  1. Add tab-completion for CREATE FOREIGN TABLE.

  2. Add tap tests for the schema publications.

  3. Move Perl test modules to a better namespace

  4. Adjust configure to insist on Perl version >= 5.8.3.

  5. Simplify code related to compilation of SSL and OpenSSL

  6. Introduce --with-ssl={openssl} as a configure option

  7. Implement support for bulk inserts in postgres_fdw

  8. Fix redundant error messages in client tools

  9. doc: Apply more consistently <productname> markup for OpenSSL

  10. Check ssl_in_use flag when reporting statistics