Re: Support for NSS as a libpq TLS backend

From: Jacob Champion <pchampion(at)vmware(dot)com>
To: "daniel(at)yesql(dot)se" <daniel(at)yesql(dot)se>
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>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, "thomas(dot)munro(at)gmail(dot)com" <thomas(dot)munro(at)gmail(dot)com>, "sfrost(at)snowman(dot)net" <sfrost(at)snowman(dot)net>, "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>
Subject: Re: Support for NSS as a libpq TLS backend
Date: 2021-06-15 23:50:14
Message-ID: 4ec146256e31afa0542f9fa970ec258c5f1a5f98.camel@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 2021-06-16 at 00:08 +0200, Daniel Gustafsson wrote:
> > On 15 Jun 2021, at 00:15, Jacob Champion <pchampion(at)vmware(dot)com> wrote:
> > Attached is a quick patch; does it work on your machine?
>
> It does, thanks! I've included it in the attached v37 along with a few tiny
> non-functional improvements in comment spelling etc.

Great, thanks!

I've been tracking down reference leaks in the client. These open
references prevent NSS from shutting down cleanly, which then makes it
impossible to open a new context in the future. This probably affects
other libpq clients more than it affects psql.

The first step to fixing that is not ignoring failures during NSS
shutdown, so I've tried a patch to pgtls_close() that pushes any
failures through the pqInternalNotice(). That seems to be working well.
The tests were still mostly green, so I taught connect_ok() to fail if
any stderr showed up, and that exposed quite a few failures.

I am currently stuck on one last failing test. This leak seems to only
show up when using TLSv1.2 or below. There doesn't seem to be a
substantial difference in libpq code coverage between 1.2 and 1.3, so
I'm worried that either 1) there's some API we use that "requires"
cleanup, but only on 1.2 and below, or 2) there's some bug in my
version of NSS.

Attached are a few work-in-progress patches. I think the reference
cleanups themselves are probably solid, but the rest of it could use
some feedback. Are there better ways to test for this? and can anyone
reproduce the TLSv1.2 leak?

--Jacob

Attachment Content-Type Size
0001-nss-don-t-ignore-failures-during-context-shutdown.patch.txt text/plain 1.3 KB
0002-test-check-for-empty-stderr-during-connect_ok.patch.txt text/plain 3.6 KB
0003-nss-clean-up-leaked-resources.patch.txt text/plain 3.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2021-06-15 23:59:55 Re: disfavoring unparameterized nested loops
Previous Message Tom Lane 2021-06-15 23:26:25 Re: Improving isolationtester's data output