Re: Support for NSS as a libpq TLS backend

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>
Subject: Re: Support for NSS as a libpq TLS backend
Date: 2020-07-10 05:10:59
Message-ID: CA+hUKGLpx6KZJOf=iPQNs_aJjJqVKWM4MuDUES_nUXpfk9ysoA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 3, 2020 at 11:51 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> > On 25 Jun 2020, at 17:39, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> >> On 15 May 2020, at 22:46, Daniel Gustafsson <daniel(at)yesql(dot)se> 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.
>
> Another rebase to resolve conflicts with the recent fixes in the SSL tests, as
> well as some minor cleanup.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2020-07-10 05:11:19 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Previous Message Andrey V. Lepikhov 2020-07-10 04:28:44 Re: POC: postgres_fdw insert batching