Re: Support for NSS as a libpq TLS backend

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Jacob Champion <pchampion(at)vmware(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: Support for NSS as a libpq TLS backend
Date: 2021-01-19 20:21:41
Message-ID: A9C4ADFC-D86B-414A-AF98-7FC14D482198@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 18 Jan 2021, at 08:08, Michael Paquier <michael(at)paquier(dot)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

Attachment Content-Type Size
v21-0001-NSS-Frontend-Backend-and-build-infrastructure.patch application/octet-stream 113.9 KB
v21-0002-NSS-Testharness-updates.patch application/octet-stream 56.9 KB
v21-0003-NSS-pg_strong_random-support.patch application/octet-stream 3.8 KB
v21-0004-NSS-Documentation.patch application/octet-stream 19.8 KB
v21-0005-NSS-contrib-modules.patch application/octet-stream 29.9 KB
v21-0006-NSS-cryptohash-support.patch application/octet-stream 6.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-01-19 20:23:50 Re: Support for NSS as a libpq TLS backend
Previous Message Tom Lane 2021-01-19 20:21:03 Re: Add primary keys to system catalogs