Re: Support for NSS as a libpq TLS backend

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Jacob Champion <pchampion(at)vmware(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "andrew(dot)dunstan(at)2ndquadrant(dot)com" <andrew(dot)dunstan(at)2ndquadrant(dot)com>, "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>, "thomas(dot)munro(at)gmail(dot)com" <thomas(dot)munro(at)gmail(dot)com>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, "sfrost(at)snowman(dot)net" <sfrost(at)snowman(dot)net>
Subject: Re: Support for NSS as a libpq TLS backend
Date: 2021-12-15 22:10:14
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 30 Nov 2021, at 20:03, Jacob Champion <pchampion(at)vmware(dot)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

Attachment Content-Type Size
v50-0010-nss-Build-infrastructure.patch application/octet-stream 24.5 KB
v50-0009-nss-Support-NSS-in-cryptohash.patch application/octet-stream 6.1 KB
v50-0008-nss-Support-NSS-in-sslinfo.patch application/octet-stream 9.6 KB
v50-0007-nss-Support-NSS-in-pgcrypto.patch application/octet-stream 78.2 KB
v50-0006-nss-Documentation.patch application/octet-stream 35.6 KB
v50-0005-nss-pg_strong_random-support.patch application/octet-stream 2.0 KB
v50-0004-test-check-for-empty-stderr-during-connect_ok.patch application/octet-stream 3.0 KB
v50-0003-nss-Add-NSS-specific-tests.patch application/octet-stream 64.1 KB
v50-0002-Refactor-SSL-testharness-for-multiple-library.patch application/octet-stream 12.9 KB
v50-0001-nss-Support-libnss-as-TLS-library-in-libpq.patch application/octet-stream 103.2 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2021-12-15 22:13:10 Re: pg_upgrade should truncate/remove its logs before running
Previous Message Bossart, Nathan 2021-12-15 22:09:59 Re: Add index scan progress to pg_stat_progress_vacuum