Re: Key management with tests - Mailing list pgsql-hackers

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:
> On Thu, Jan  7, 2021 at 04:08:49PM -0300, Álvaro Herrera wrote:
> > On 2021-Jan-07, Bruce Momjian wrote:
> >
> > > All the tests pass now.  The current src/test directory is 19MB, and
> > > adding these tests takes it to 23MB, or a 20% increase.  That seems like
> > > a lot.  It is testing 128-bit and 256-bit keys --- should we do fewer
> > > tests, or just test 256, or use gzip to compress the tests by 50%?
> > > (Does every platform have gzip?)
> >
> > So the tests are about 95% of the patch ... do we really need that many
> > tests?
>
> No, I don't think so.  Stephen imported the entire NIST test suite.  It
> was so comperhensive, it detected several OpenSSL bugs for zero-length
> strings, which I already reported, but we would never be encrypting
> zero-length strings, so there wasn't a lot of value to it.

I ran the entire test suite locally to ensure everything worked, but I
didn't actually include all of it in the PR which you merged- I had
already reduced it quite a bit by removing all 'additional
authenticated data' test cases (which the tests will automatically skip
and which we haven't implemented support for in the common library
wrappers) and by removing the 192-bit cases.  This reduced the overall
test set by about 2/3rd's or so, as I recall.

> Anyway, I think we need to figure out how to trim.  The first part would
> be to figure out whether we need 128 _and_ 256-bit tests, and then see
> what items are really useful.  Stephen, do you have any ideas on that?
> We currently have 10296 tests, and I think we could get away with 100.

Yeah, it's probably still too much, but I don't have any particularly
justifiable suggestions as to exactly what we should remove or what we
should keep.

Perhaps it'd make sense to try and cover the cases that are more likely
to be issues between our wrapper functions and OpenSSL, and not stress
too much about constantly testing cases that should really be up to
OpenSSL.  As such, I'd propose:

- Add back in some 192-bit tests, so we cover all three bit lengths.
- Add back in some additional authenticated test cases, just to make
  sure that, until/unless we implement support, the test code properly
  skips over those.
- Keep tests for various length plaintext/ciphertext (including 0-byte
  cases, so we make sure those work, since they really should).
- Keep at least one test for each length of tag that's included in the
  test suite.

I'm not sure how many tests we'd end up with from that, but my swag /
gut feeling is that it'd probably be on the order of 100ish and a small
enough set that it won't dwarf the rest of the patch.

Would be nice if we had a way for some buildfarm animal or something to
pull in the entire suite and test it, imv..  If anyone wants to
volunteer, I'd be happy to explain how to make that happen (it's not
hard though- download/unzip the files, drop them in the directory,
update the test script to add all the files into the array).

Thanks,

Stephen