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: Jacob Champion <pchampion@vmware.com>
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>, "michael@paquier.xyz" <michael@paquier.xyz>, "thomas.munro@gmail.com" <thomas.munro@gmail.com>, "andres@anarazel.de" <andres@anarazel.de>, "sfrost@snowman.net" <sfrost@snowman.net>
Date: 2021-12-15T22:10:14Z
Lists: pgsql-hackers

Attachments


> On 30 Nov 2021, at 20:03, Jacob Champion <pchampion@vmware.com> wrote:
> 
> On Mon, 2021-09-27 at 15:44 +0200, Daniel Gustafsson wrote:
>>> 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.
>> 
>> Please do.
> 
> Didn't get around to it for November, but I'm putting the finishing
> touches on that now.

Cool, thanks!

> While I was looking at the new SAN code (in fe-secure-nss.c,
> pgtls_verify_peer_name_matches_certificate_guts()), I noticed that code
> coverage never seemed to touch a good chunk of it:
> 
>> +        for (cn = san_list; cn != san_list; cn = CERT_GetNextGeneralName(cn))
>> +        {
>> +            char       *alt_name;
>> +            int         rv;
>> +            char        tmp[512];
> 
> That loop can never execute. But I wonder if all of that extra SAN code
> should be removed anyway? There's this comment above it:
> 
>> +	/*
>> +	 * CERT_VerifyCertName will internally perform RFC 2818 SubjectAltName
>> +	 * verification.
>> +	 */
> 
> and it seems like SAN verification is working in my testing, despite
> the dead loop.

Yeah, that's clearly bogus.  I followed the bouncing ball reading NSS code and
from what I can tell the comment is correct.  I removed the dead code, only
realizing after the fact that I might cause conflict with your tree doing so,
in that case sorry.

I've attached a v50 which fixes the issues found by Joshua upthread, as well as
rebases on top of all the recent SSL and pgcrypto changes.

--
Daniel Gustafsson		https://vmware.com/

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