> On 16 Jul 2020, at 00:16, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> On 12 Jul 2020, at 00:03, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> Thanks for taking a look at the patch, I'll fix up the reported issues Monday
>> at the latest.
>
> A bit of life intervened, but attached is a new version of the patch which
> should work for OpenSSL builds, and have the other issues addressed as well. I
> took the opportunity to clean up the NSS tests to be more like the OpenSSL ones
> to lessen the impact on the TAP testcases. On my Debian box, using the
> standard NSS and NSPR packages, I get 6 failures which are essentially all
> around CRL handling. I'm going to circle back and look at what is missing there.
Taking a look at this, the issue was that I had fat-fingered the Makefile rules
for generating the NSS databases. This is admittedly very messy at this point,
partly due to trying to mimick OpenSSL filepaths/names to minimize the impact
on tests and to keep OpenSSL/NSS tests as "optically" equivalent as I could.
With this, I have one failing test ("intermediate client certificate is
provided by client") which I've left failing since I believe the case should be
supported by NSS. The issue is most likely that I havent figured out the right
certinfo incantation to make it so (Mozilla hasn't strained themselves when
writing documentation for this toolchain, or any part of NSS for that matter).
The failing test when running with OpenSSL also remains, the issue is that the
very first test for incorrect key passphrase fails, even though the server is
behaving exactly as it should. Something in the test suite hackery breaks for
that test but I've been unable to pin down what it is, any help on would be
greatly appreciated.
This version adds support for sslinfo on NSS for most the functions. In the
process I realized that sslinfo never got the memo about SSL support being
abstracted behind an API, so I went and did that as well. This part of the
patch should perhaps be broken out into a separate patch/thread in case it's
deemed interesting regardless of the evetual conclusion on this patch. Doing
this removed a bit of duplication with the backend code, and some errorhandling
moved to be-secure-openssl.c (originally added in d94c36a45ab45). As the
original commit message states, they're mostly code hygiene with belts and
suspenders, but if we deemed them valuable enough for a contrib module ISTM
they should go into the backend as well. Adding a testcase for sslinfo is a
TODO.
Support pg_strong_random, sha2 and pgcrypto has been started, but it's less
trivial as NSS/NSPR requires a lot more initialization and state than OpenSSL,
so it needs a bit more thought.
I've also done a rebase over todays HEAD, a pgindent pass and some cleanup here
and there.
cheers ./daniel