Re: Proposed patch for key managment

Thanks, Bruce

On Sat, 19 Dec 2020 at 16:58, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
...
>
> To enable the direct injection of keys into the server, we would need a
> new command for this, since trying to make the passphrase command do
> this will lead to unnecessary complexity. The passphrase command should
> do one thing, and you can't have it changing its behavior based on
> parameters, since the parameters are for the script to process, not to
> change behavior of the server.
>
> If we wanted to do this, we would need a new command, and one of the
> parameters would be %k or something that would identify the key number
> we want to retrieve from the KMS. Stephen pointed out that we could
> still validate that key; the key would not be stored wrapped in the
> file system, but we could store an HMAC in the file system, and use that
> for validation.
>

I think that this is where we have ended up talking at cross-purposes.
My idea (after some refining based on Stephen's feedback) is that
there should be only this new command (the cluster key command) and
that the program for unwrapping stored keys should be called this way.
As could a program to contact an HSM, etc. This moves the generating
and wrapping functionality out of the core Postgres processes, making
it far easier to add alternatives. I see this approach was discussed
on the email thread you linked to, but I can't see where or how a
conclusion was reached about it...

>
> On other interesting approach would be to allow the server to call out
> for a KMS when it needs to generate the initial data keys that are
> wrapped by the passphrase; this is in contrast to calling the KMS
> everytime it needs the data keys.
>
...
> ...The data keys are generated using this random value
> code:
>
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/port/pg_strong_random.c;hb=HEAD
>
> so someone would have to explain why generating this remotely in a KMS
> is superior, not just based on company policy.
>

The pg_strong_random() function uses OpesnSSL's RAND_bytes() where
available. With appropriate configuration of an OpenSSL engine
supplying an alternative RAND, this could be handed off to a TRNG.

This is an example of the other option for providing flexibility to
support specific key generation, wrapping, ... requirements - handing
the operations off to a library like OpenSSL or nss tls which, in
turn, can use pluggable providers for these functions. FWIW, the
proprietary DBMSs use a pluggable engine approach to meet requirements
in this category.

>
> We should create code for the general use-case, which is currently
> passphrase-wrapped system-generated data keys, and then get feedback on
> what else we need. However, I should point out that the community is
> somewhat immune to the "my company needs this" kind of argument without
> logic to back it up, though sometimes I think this entire feature is in
> that category. ...

Yeah. Security in general, and crypto in particular, are often
"because someone told me to" requirements.

>
> My final point is that we can find ways to do what you are suggesting as
> an addition to what we are adding now. What we need is clear
> justification of why these additional features are needed. Requiring
> the use of a true random number generator is a valid argument, but we
> need to figure out, once this is implemented, who really wants that, and
> how to implement it cleanly.
>

Clean interfaces would be either "above" the database calling out to
commands in user-land or "below" the database in the abstractions
which OpenSSL, nss tls and friends provide. Since the conclusion seems
already to have been reached that the keyring should be inside the
core process and only the passphrase should pop out above, I'll review
the patch in this vein and see if I can suggest any routes to carving
out more manageable subsets of the patch.

"...once this is implemented..." changes become a lot more difficult.
Particularly when the changes would affect what are regarded as the
database's on-disk files. Which I suspect is a contributing factor to
the level of engagement surrounding this feature.

Regards
Alastair