On Fri, 2021-01-29 at 13:57 +0100, Daniel Gustafsson wrote:
> > On 21 Jan 2021, at 06:21, Michael Paquier <michael@paquier.xyz> wrote:
> > I really need to study more the choide of the options chosen for
> > NSS_InitContext()... But based on the docs I can read on the matter I
> > think that saving nsscontext in pg_cryptohash_ctx is right for each
> > cryptohash built.
>
> It's a safe but slow option, NSS wasn't really made for running a single crypto
> operation. Since we are opening a context which isn't backed by an NSS
> database we could have a static context, which indeed speeds up processing a
> lot. The problem with that is that there is no good callsite for closing the
> context as the backend is closing down. Since you are kneedeep in the
> cryptohash code, do you have any thoughts on this? I've included 0008 which
> implements this, with a commented out dummy stub for cleaning up.
>
> Making nss_context static in cryptohash_nss.c is
> appealing but there is no good option for closing it there. Any thoughts on
> how to handle global contexts like this?
I'm completely new to this code, so take my thoughts with a grain of
salt...
I think the bad news is that the static approach will need support for
ENABLE_THREAD_SAFETY. (It looks like the NSS implementation of
pgtls_close() needs some thread support too?)
The good(?) news is that I don't understand why OpenSSL's
implementation of cryptohash doesn't _also_ need the thread-safety
code. (Shouldn't we need to call CRYPTO_set_locking_callback() et al
before using any of its cryptohash implementation?) So maybe we can
implement the same global setup/teardown API for OpenSSL too and not
have to one-off it for NSS...
--Jacob
