Re: Support for NSS as a libpq TLS backend

On Fri, 2021-03-26 at 15:33 -0400, Stephen Frost wrote:
> * Jacob Champion (pchampion@vmware.com) wrote:
> > Databases that are opened *after* the first one are given their own
> > separate slots. [...]
> 
> This is more-or-less what we would want though, right..?  If a user asks
> for a connection with ssl_database=blah and sslcert=whatever, we'd want
> to open database 'blah' and search (just) that database for cert
> 'whatever'.  We could possibly offer other options in the future but
> certainly this would work and be the most straight-forward and expected
> behavior.

Yes, but see below.

> > If you SECMOD_OpenUserDB() a database that is identical to the first
> > (internal) database, NSS deduplicates for you and just returns the
> > internal slot. Which seems like it's helpful, except you're not
> > allowed to close that database, and you have to know not to close it
> > by checking to see whether that slot is the "internal key slot". It
> > appears to remain open until NSS is shut down entirely.
> 
> Seems like we shouldn't do that and should just use SECMOD_OpenUserDB()
> for opening databases.

We don't have control over whether or not this happens. If the
application embedding libpq has already loaded the database into the
internal slot via its own NSS initialization, then when we call
SECMOD_OpenUserDB() for that same database, the internal slot will be
returned and we have to handle it accordingly.

It's not a huge amount of work, but it is magic knowledge that has to
be maintained, especially in the absence of specialized clientside
tests.

> > But if you open a database that is *not* the magic internal database,
> > and then open a duplicate of that one, NSS creates yet another new slot
> > for the duplicate. So SECMOD_OpenUserDB() may or may not be a resource
> > hog, depending on the global state of the process at the time libpq
> > opens its first connection. We won't be able to control what the parent
> > application will do before loading us up.
> 
> I would think we'd want to avoid re-opening the same database multiple
> times, to avoid the duplicate slots and such.  If the application code
> does it themselves, well, there's not much we can do about that, but we
> could at least avoid doing so in *our* code.  I wouldn't expect us to be
> opening hundreds of databases either and so keeping a simple list around
> of what we've opened and scanning it seems like it'd be workable.  Of
> course, this could likely be improved in the future but I would think
> that'd be good for an initial implementation.
> 
> [...]
> 
> > It also doesn't look like any of the SECMOD_* machinery that we're
> > looking at is thread-safe, but I'd really like to be wrong...
> 
> That's unfortuante but solvable by using our own locks, similar
> to what's done in fe-secure-openssl.c.

Yeah. I was hoping to avoid implementing our own locks and refcounts,
but it seems like it's going to be required.

--Jacob