Horton

Re: Support for NSS as a libpq TLS backend

Jacob Champion <pchampion@vmware.com>

View thread View on postgr.es
From: Jacob Champion <pchampion@vmware.com>
To: "daniel@yesql.se" <daniel@yesql.se>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "hlinnaka@iki.fi" <hlinnaka@iki.fi>, "andrew.dunstan@2ndquadrant.com" <andrew.dunstan@2ndquadrant.com>, "andres@anarazel.de" <andres@anarazel.de>, "thomas.munro@gmail.com" <thomas.munro@gmail.com>, "sfrost@snowman.net" <sfrost@snowman.net>, "michael@paquier.xyz" <michael@paquier.xyz>
Date: 2021-09-21T00:06:15Z
Lists: pgsql-hackers 
On Mon, 2021-07-26 at 15:26 +0200, Daniel Gustafsson wrote:
> > On 19 Jul 2021, at 21:33, Jacob Champion <pchampion@vmware.com> wrote:
> > ..client connections will crash if
> > hostaddr is provided rather than host, because SSL_SetURL can't handle
> > a NULL argument. I'm running with 0002 to fix it for the moment, but
> > I'm not sure yet if it does the right thing for IP addresses, which the
> > OpenSSL side has a special case for.
> 
> AFAICT the idea is to handle it in the cert auth callback, so I've added some
> PoC code to check for sslsni there and updated the TODO comment to reflect
> that.

I dug a bit deeper into the SNI stuff:

> +	server_hostname = SSL_RevealURL(conn->pr_fd);
> +	if (!server_hostname || server_hostname[0] == '\0')
> +	{
> +		/* If SNI is enabled we must have a hostname set */
> +		if (conn->sslsni && conn->sslsni[0])
> +			status = SECFailure;

conn->sslsni can be explicitly set to "0" to disable it, so this should
probably be changed to a check for "1", but I'm not sure that would be
correct either. If the user has the default sslsni="1" and supplies an
IP address for the host parameter, I don't think we should fail the
connection.

> +	if (host && host[0] &&
> +		!(strspn(host, "0123456789.") == strlen(host) ||
> +		  strchr(host, ':')))
> +		SSL_SetURL(conn->pr_fd, host);

It looks like NSS may already have some code that prevents SNI from
being sent for IP addresses, so that part of the guard might not be
necessary. (And potentially counterproductive, because it looks like
NSS can perform verification against the certificate's SANs if you pass
an IP address to SSL_SetURL().)

Speaking of IP addresses in SANs, it doesn't look like our OpenSSL
backend can handle those. That's a separate conversation, but I might
take a look at a patch for next commitfest.

--Jacob

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