Support for NSS as a libpq TLS backend

The attached patch implements NSS (Network Security Services) [0] with the
required NSPR runtime [1] as a TLS backend for PostgreSQL.

While all sslmodes are implemented and work for the most part, the patch is
*not* ready yet but I wanted to show progress early so that anyone interested
in this can help out with testing and maybe even hacking.

Why NSS?  Well.  It shares no lineage with OpenSSL making it not just an
alternative by fork but a 100% alternative.  It's also actively maintained, is
readily available on many platforms where PostgreSQL is popular and has a FIPS
mode which doesn't require an EOL'd library.  And finally, I was asked nicely
with the promise of a free beverage, an incentive as good as any.


Differences with OpenSSL
------------------------
NSS does not use certificates and keys on the filesystem, it instead uses a
certificate database in which all certificates, keys and CRL's are loaded.  A
set of tools are provided to work with the database, like: certutil, crlutil,
pk12util etc.  We could support plain PEM files as well, and load them into a
database ourselves but so far I've opted for just using what is already in the
database.

This does mean that new GUCs are needed to identify the database.  I've mostly
repurposed the existing ones for cert/key/crl, but had to invent a new one for
the database.  Maybe there should be an entirely new set?  This needs to be
discussed with not only NSS in mind but for additional as-of-yet unknown
backends we might get (SChannel comes to mind).

NSS also supports partial chain validation per default (as do many other TLS
libraries) where OpenSSL does not.  I haven't done anything about that just
yet, thus there is a failing test as a reminder to address it.

The documentation of NSS/NSPR is unfortunately quite poor and often times
outdated or simply nonexisting.  Cloning the repo and reading the source code
is the only option for parts of the API.

Featurewise there might be other things we can make use of in NSS which doesn't
exist in OpenSSL, but for now I've tried to keep them aligned.


Known Bugs and Limitations (in this version of the patch)
---------------------------------------------------------
The frontend doesn't attempt to verify whether the specified CRL exists in the
database or not.  This can be done with pretty much the same code as in the
backend, except that we don't have the client side certificate loaded so we
either need to read it back from the database, or parse a list of all CRLs
(which would save us from having the cert in local memory which generally is a
good thing to avoid).

pgtls_read is calling PR_Recv which works fine for communicating with an NSS
backend cluster, but hangs waiting for IO when communicating with an OpenSSL
backend cluster.  Using PR_Read reverses the situation.  This is probably a
simple bug but I haven't had time to track it down yet.  The below shifts
between the two for debugging.

-         nread = PR_Recv(conn->pr_fd, ptr, len, 0, PR_INTERVAL_NO_WAIT);
+         nread = PR_Read(conn->pr_fd, ptr, len);

Passphrase handling in the backend is broken, more on that under TODO.

There are a few failing tests and a few skipped ones for now, but the majority
of the tests pass.


Testing
-------
In order for the TAP framework to be able to handle backends with different
characteristics I've broken up SSLServer.pm into a set of modules:

    SSL::Server
    SSL::Backend::NSS
    SSL::Backend::OpenSSL

The SSL tests import SSL::Server which in turn imports the appropriate backend
module in order to perform backend specific setup tasks.  The backend used
should be transparent for the TAP code when it comes to switching server certs
etc.

So far I've used foo|bar in the matching regex to provide alternative output,
and SKIP blocks for tests that don't apply.  There might be neater ways to
achieve this, but I was trying to avoid having separate test files for the
different backends.

The certificate databases can be created with a new nssfiles make target in
src/test/ssl, which use the existing files (and also depend on OpenSSL which I
don't think is a problematic dependency for development environments).  To keep
it simple I've named the certificates in the NSS database after the filenames,
this isn't really NSS best-practices but it makes for an easier time reading
the tests IMO.

If this direction is of interest, extracting into to a separate patch for just
setting up the modules and implementing OpenSSL without a new backend is
probably the next step.


TODO
----
This patch is a work in progress, and there is work left to do, below is a dump
of what is left to fix before this can be considered a full implementation for
review. Most of these items have more documentation in the code comments.

* The split between init and open needs to be revisited, especially in frontend
  where we have a bit more freedom. It remains to be seen if we can do better in
  the backend part.

* Documentation, it's currently not even started

* Windows support.  I've hacked mostly using Debian and have tested versions of
  the patch on macOS, but not Windows so far.

* Figure out how to handle cipher configuration.  Getting a set of ciphers that
  result in a useable socket isn't as easy as with OpenSSL, and policies seem
  much more preferred.  At the very least this needs to be solidly documented.

* The rules in src/test/ssl/Makefile for generating certificate databases can
  probably be generalized into a smaller set of rules based on wildcards.

* The password callback on server-side won't be invoked at server start due to
  init happening in be_tls_open, so something needs to be figured out there.
  Maybe attempt to open the database with a throw-away context in init just to
  invoke the password callback?

* Identify code duplicated between frontend and backend and try to generalize.

* Make sure the handling the error codes correctly in the certificate and auth
  callbacks to properly handle self-signed certs etc.

* Tidy up the tests which are partially hardwired for NSS now to make sure
  there are no regressions for OpenSSL.

* All the code using OpenSSL which isn't the libpq communications parts, like
  pgcrypto, strong_random, sha2, SCRAM et.al

* Review language in code comments and run pgindent et.al

* Settle on a minimum required version.  I've been using NSS 3.42 and NSPR 4.20
  simply since they were the packages Debian wanted to install for me, but I'm
  quite convinced that we can go a bit lower (featurewise we can go much lower
  but there are bugfixes in recent versions that we might want to include).
  Anything lower than a version supporting TLSv1.3 seems like an obvious no-no.


I'd be surprised if this is all, but that's at least a start.  There isn't
really a playbook on how to add a new TLS backend, but I'm hoping to be able to
summarize the required bits and pieces in README.SSL once this is a bit closer
to completion.

My plan is to keep hacking at this to have it reviewable for the 14 cycle, so
if anyone has an interest in NSS, then I would love to hear feedback on how it
works (and doesn't work).

The 0001 patch contains the full NSS support, and 0002 is a fix for the pgstat
abstraction which IMO leaks backend implementation details.  This needs to go
on it's own thread, but since 0001 fails without it I've included it here for
simplicity sake for now.

cheers ./daniel

[0] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSPR

> On 15 May 2020, at 22:46, Daniel Gustafsson <[hidden email]> wrote:

> The 0001 patch contains the full NSS support, and 0002 is a fix for the pgstat
> abstraction which IMO leaks backend implementation details.  This needs to go
> on it's own thread, but since 0001 fails without it I've included it here for
> simplicity sake for now.

The attached 0001 and 0002 are the same patchseries as before, but with the
OpenSSL test module fixed and a rebase on top of the current master.

cheers ./daniel

Another rebase to resolve conflicts with the recent fixes in the SSL tests, as
well as some minor cleanup.

cheers ./daniel

On Fri, Jul 3, 2020 at 11:51 PM Daniel Gustafsson <[hidden email]> wrote:
Hi Daniel,

Thanks for blazing the trail for other implementations to coexist in
the tree.  I see that cURL (another project Daniel works on)
supports a lot of TLS implementations[1].  I recognise 4 other library
names from that table as having appeared on this mailing list as
candidates for PostgreSQL support complete with WIP patches, including
another one from you (Apple Secure Transport).  I don't have strong
views on how many and which libraries we should support, but I was
curious how many packages depend on libss1.1, libgnutls30 and libnss3
in the Debian package repos in my sources.list, and I came up with
OpenSSL = 820, GnuTLS = 342, and NSS = 87.

I guess Solution.pm needs at least USE_NSS => undef for this not to
break the build on Windows.

Obviously cfbot is useless for testing this code, since its build
script does --with-openssl and you need --with-nss, but it still shows
us one thing: with your patch, a --with-openssl build is apparently
broken:

/001_ssltests.pl .. 1/93 Bailout called.  Further testing stopped:
system pg_ctl failed

There are some weird configure-related hunks in the patch:

+  -runstatedir | --runstatedir | --runstatedi | --runstated \
...[more stuff like that]...

-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))

I see the same when I use Debian's autoconf, but not FreeBSD's or
MacPorts', despite all being version 2.69.  That seems to be due to
non-upstreamed changes added by the Debian maintainers (I see the
off_t thing mentioned in /usr/share/doc/autoconf/changelog.Debian.gz).
I think you need to build a stock autoconf 2.69 or run autoconf on a
non-Debian system.

I installed libnss3-dev on my Debian box and then configure had
trouble locating and understanding <ssl.h>, until I added
--with-includes=/usr/include/nss:/usr/include/nspr.  I suspect this is
supposed to be done with pkg-config nss --cflags somewhere in
configure (or alternatively nss-config --cflags, nspr-config --cflags,
I don't know, but we're using pkg-config for other stuff).

I installed the Debian package libnss3-tools (for certutil) and then,
in src/test/ssl, I ran make nssfiles (I guess that should be
automatic?), and make check, and I got this far:

Test Summary Report
-------------------
t/001_ssltests.pl (Wstat: 3584 Tests: 93 Failed: 14)
  Failed tests:  14, 16, 18-20, 24, 27-28, 54-55, 78-80
                91
  Non-zero exit status: 14

You mentioned some were failing in this WIP -- are those results you expect?

[1] https://curl.haxx.se/docs/ssl-compared.html

The list on that URL is also just a selection, the total count is 10 (not
counting OpenSSL forks) IIRC, after axing support for a few lately.  OpenSSL
clearly has a large mindshare but the gist of it is that there exist quite a
few alternatives each with their own strengths.

> I recognise 4 other library
> names from that table as having appeared on this mailing list as
> candidates for PostgreSQL support complete with WIP patches, including
> another one from you (Apple Secure Transport).  I don't have strong
> views on how many and which libraries we should support,

I think it's key to keep in mind *why* it's relevant to provide options in the
first place, after all, as they must be 100% interoperable one can easily argue
for a single one being enough.  We need to to look at what they offer users on
top of just a TLS connection, like: managed certificate storage like for
example macOS Keychains, FIPS certification, good platform availability and/or
OS integration etc.  If all a library offers is "not being OpenSSL" then it's
not clear that we're adding much value by spending the cycles to support it.

My personal opinion is that we should keep it pretty limited, not least to
lessen the burden of testing and during feature development.  Supporting a new
library comes with requirements on both the CFBot as well as the buildfarm, not
to mention on developers who dabble in that area of the code.  The goal should
IMHO be to make it trivial for every postgres installation to use TLS
regardless of platform and experience level with the person installing it.

The situation is a bit different for curl where we have as a goal to provide
enough alternatives such that every platform can have a libcurl/curl more or
less regardless of what it contains.  As a consequence, we have around 80 CI
jobs to test each pull request to provide ample coverage.  Being a kitchen-
sink is really hard work.

> but I was
> curious how many packages depend on libss1.1, libgnutls30 and libnss3
> in the Debian package repos in my sources.list, and I came up with
> OpenSSL = 820, GnuTLS = 342, and NSS = 87.

I don't see a lot of excitement over GnuTLS lately, but Debian shipping it due
to (I believe) licensing concerns with OpenSSL does help it along.  In my
experience, platforms with GnuTLS easily available also have OpenSSL easily
available.

> I guess Solution.pm needs at least USE_NSS => undef for this not to
> break the build on Windows.

Thanks, I'll fix (I admittedly haven't tried this at all on Windows yet).

> Obviously cfbot is useless for testing this code, since its build
> script does --with-openssl and you need --with-nss,

Right, this is a CFBot problem with any patch that require specific autoconf
flags to be excercised.  I wonder if we can make something when we do CF app
integration which can inject flags to a Travis pipeline in a safe manner?

> but it still shows
> us one thing: with your patch, a --with-openssl build is apparently
> broken:
>
> /001_ssltests.pl .. 1/93 Bailout called.  Further testing stopped:
> system pg_ctl failed

Humm ..  I hate to say "it worked on my machine" but it did, but my TLS
environment is hardly standard.  Sorry for posting breakage, most likely this
is a bug in the new test module structure that the patch introduce in order to
support multiple backends for src/tests/ssl.  I'll fix.

Sigh, yes that's a Debianism that slipped through, again sorry about that.

> I installed libnss3-dev on my Debian box and then configure had
> trouble locating and understanding <ssl.h>, until I added
> --with-includes=/usr/include/nss:/usr/include/nspr.  I suspect this is
> supposed to be done with pkg-config nss --cflags somewhere in
> configure (or alternatively nss-config --cflags, nspr-config --cflags,
> I don't know, but we're using pkg-config for other stuff).

Yeah, that's a good point, I should fix that.  Having a metric ton of TLS
libraries in various versions around in my environment I've been Stockholm
Syndromed to --with-includes to the point where I didn't even think to run
without it. It should clearly be as easy to use as OpenSSL wrt autoconf.

> I installed the Debian package libnss3-tools (for certutil) and then,
> in src/test/ssl, I ran make nssfiles (I guess that should be
> automatic?)

Yes, it needs to run automatically for NSS builds on make check.

I'm not on my dev box at the moment, and I don't remember off the cuff, but
that sounds higher than I remember.  I wonder if I fat-fingered the regexes in
the last version?

Thanks for taking a look at the patch, I'll fix up the reported issues Monday
at the latest.

cheers ./daniel

On Fri, Jul 10, 2020 at 5:10 PM Thomas Munro <[hidden email]> wrote:
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>
> I see the same when I use Debian's autoconf, but not FreeBSD's or
> MacPorts', despite all being version 2.69.  That seems to be due to
> non-upstreamed changes added by the Debian maintainers (I see the
> off_t thing mentioned in /usr/share/doc/autoconf/changelog.Debian.gz).

By the way, Dagfinn mentioned that these changes were in fact
upstreamed, and happened to be beta-released today[1], and are due out
in ~3 months as 2.70.  That'll be something for us to coordinate a bit
further down the road.

[1] https://lists.gnu.org/archive/html/autoconf/2020-07/msg00006.html

On 5/15/20 4:46 PM, Daniel Gustafsson wrote:
>
> My plan is to keep hacking at this to have it reviewable for the 14 cycle, so
> if anyone has an interest in NSS, then I would love to hear feedback on how it
> works (and doesn't work).


I'll be happy to help, particularly with Windows support and with some
of the callback stuff I've had a hand in.


cheers


andrew



--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

> On 12 Jul 2020, at 00:03, Daniel Gustafsson <[hidden email]> wrote:

> Thanks for taking a look at the patch, I'll fix up the reported issues Monday
> at the latest.

A bit of life intervened, but attached is a new version of the patch which
should work for OpenSSL builds, and have the other issues addressed as well.  I
took the opportunity to clean up the NSS tests to be more like the OpenSSL ones
to lessen the impact on the TAP testcases.  On my Debian box, using the
standard NSS and NSPR packages, I get 6 failures which are essentially all
around CRL handling. I'm going to circle back and look at what is missing there.

This version also removes the required patch for statistics reporting as that
has been committed in 6a5c750f3f72899f4f982f921d5bf5665f55651e.

cheers ./daniel

That would be fantastic, thanks!  The password callback handling is still a
TODO so feel free to take a stab at that since you have a lot of context on
there.

For Windows, I've include USE_NSS in Solution.pm as Thomas pointed out in this
thread, but that was done blind as I've done no testing on Windows yet.

cheers ./daniel

Taking a look at this, the issue was that I had fat-fingered the Makefile rules
for generating the NSS databases.  This is admittedly very messy at this point,
partly due to trying to mimick OpenSSL filepaths/names to minimize the impact
on tests and to keep OpenSSL/NSS tests as "optically" equivalent as I could.

With this, I have one failing test ("intermediate client certificate is
provided by client") which I've left failing since I believe the case should be
supported by NSS.  The issue is most likely that I havent figured out the right
certinfo incantation to make it so (Mozilla hasn't strained themselves when
writing documentation for this toolchain, or any part of NSS for that matter).

The failing test when running with OpenSSL also remains, the issue is that the
very first test for incorrect key passphrase fails, even though the server is
behaving exactly as it should.  Something in the test suite hackery breaks for
that test but I've been unable to pin down what it is, any help on would be
greatly appreciated.

This version adds support for sslinfo on NSS for most the functions.  In the
process I realized that sslinfo never got the memo about SSL support being
abstracted behind an API, so I went and did that as well.  This part of the
patch should perhaps be broken out into a separate patch/thread in case it's
deemed interesting regardless of the evetual conclusion on this patch.  Doing
this removed a bit of duplication with the backend code, and some errorhandling
moved to be-secure-openssl.c (originally added in d94c36a45ab45).  As the
original commit message states, they're mostly code hygiene with belts and
suspenders, but if we deemed them valuable enough for a contrib module ISTM
they should go into the backend as well.  Adding a testcase for sslinfo is a
TODO.

Support pg_strong_random, sha2 and pgcrypto has been started, but it's less
trivial as NSS/NSPR requires a lot more initialization and state than OpenSSL,
so it needs a bit more thought.

I've also done a rebase over todays HEAD, a pgindent pass and some cleanup here
and there.

cheers ./daniel

On 7/15/20 6:18 PM, Daniel Gustafsson wrote:
OK, here is an update of your patch that compiles and runs against NSS
under Windows (VS2019).


In addition to some work that was missing in src/tools/msvc, I had to
make a few adjustments, including:


  * strtok_r() isn't available on Windows. We don't use it elsewhere in
    the postgres code, and it seemed unnecessary to have reentrant calls
    here, so I just replaced it with equivalent strtok() calls.
  * We were missing an NSS implementation of
    pgtls_verify_peer_name_matches_certificate_guts(). I supplied a
    dummy that's enough to get it building cleanly, but that needs to be
    filled in properly.


There is still plenty of work to go, but this seemed a sufficient
milestone to report progress on.



cheers


andrew



--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 7/31/20 4:44 PM, Andrew Dunstan wrote:
OK, this version contains pre-generated nss files, and passes a full
buildfarm run including the ssl test module, with both openssl and NSS.
That should keep the cfbot happy :-)


cheers


andrew


--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 8/3/20 12:46 PM, Andrew Dunstan wrote: rebased on current master.


cheers


andrew


--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

> On 3 Aug 2020, at 21:18, Andrew Dunstan <[hidden email]> wrote:
> On 8/3/20 12:46 PM, Andrew Dunstan wrote:
>> On 7/31/20 4:44 PM, Andrew Dunstan wrote:

>>> OK, here is an update of your patch that compiles and runs against NSS
>>> under Windows (VS2019).

Out of curiosity since I'm not familiar with Windows, how hard/easy is it to
install NSS for the purpose of a) hacking on postgres+NSS and b) using postgres
with NSS as the backend?

>>>  * strtok_r() isn't available on Windows. We don't use it elsewhere in
>>>    the postgres code, and it seemed unnecessary to have reentrant calls
>>>    here, so I just replaced it with equivalent strtok() calls.

Fair enough, that makes sense.

>>>  * We were missing an NSS implementation of
>>>    pgtls_verify_peer_name_matches_certificate_guts(). I supplied a
>>>    dummy that's enough to get it building cleanly, but that needs to be
>>>    filled in properly.

Interesting, not sure how I could've missed that one.

>> OK, this version contains pre-generated nss files, and passes a full
>> buildfarm run including the ssl test module, with both openssl and NSS.
>> That should keep the cfbot happy :-)

Exciting, thanks a lot for helping out on this!  I've started to look at the
required documentation changes during vacation, will hopefully be able to post
something soon.

cheers ./daniel

On 8/4/20 5:42 PM, Daniel Gustafsson wrote:
>> On 3 Aug 2020, at 21:18, Andrew Dunstan <[hidden email]> wrote:
>> On 8/3/20 12:46 PM, Andrew Dunstan wrote:
>>> On 7/31/20 4:44 PM, Andrew Dunstan wrote:
>>>> OK, here is an update of your patch that compiles and runs against NSS
>>>> under Windows (VS2019).
> Out of curiosity since I'm not familiar with Windows, how hard/easy is it to
> install NSS for the purpose of a) hacking on postgres+NSS and b) using postgres
> with NSS as the backend?




I've laid out the process at
https://www.2ndquadrant.com/en/blog/nss-on-windows-for-postgresql-development/


>>> OK, this version contains pre-generated nss files, and passes a full
>>> buildfarm run including the ssl test module, with both openssl and NSS.
>>> That should keep the cfbot happy :-)
> Exciting, thanks a lot for helping out on this!  I've started to look at the
> required documentation changes during vacation, will hopefully be able to post
> something soon.
>


Good. Having got the tests running cleanly on Linux, I'm now going back
to work on that for Windows.


After that I'll look at the hook/callback stuff.


cheers


andrew



--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services