public inbox for [email protected]  
help / color / mirror / Atom feed
From: Stephen Frost <[email protected]>
To: Jacob Champion <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Support for NSS as a libpq TLS backend
Date: Fri, 26 Mar 2021 18:05:40 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Greetings,

* Jacob Champion ([email protected]) wrote:
> On Fri, 2021-03-26 at 15:33 -0400, Stephen Frost wrote:
> > * Jacob Champion ([email protected]) 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.

Ah..  yeah, fair enough.  We could document that we discourage
applications from doing so, but I agree that we'll need to deal with it
since it could happen.

> > > 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.

Yeah, afraid so.

Thanks!

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../[email protected]/2-signature.asc)
  download

view thread (181+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Support for NSS as a libpq TLS backend
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox