Re: Internal key management system

On Wed, Oct 28, 2020 at 05:16:32PM +0800, Craig Ringer wrote:
> On Wed, Oct 28, 2020 at 12:02 PM Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>
> wrote:
>
> On Wed, Oct 28, 2020 at 9:43 AM Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >
>
> I don't know much about how to hook into that stuff so if you have an
> idea, I am all ears.
>
>
> Yeah, I have a reasonable idea. The main thing will be to re-read the patch
> and put it into more concrete terms, which I'll try to find time for soon.
> I need to find time to craft a proper demo that uses a virtual hsm, and can
> also demonstrate how to use the host TPM or a Yubikey using the simple
> openssl engine interfaces or a URI.
>
>
> Do you have this in a public git tree anywhere? If not please consider using
> "git format-patch -v 1 -1" or similar to generate it, so I can "git am" the
> patch.

I have made a github branch, and will keep it updated:

https://github.com/bmomjian/postgres/tree/key

I am also attaching an updated patch.

> A few comments on the patch as I read through. Some will be addressed by the
> quick PoC I'm preparing for pluggable key derivation, some won't. In no
> particular order:
>
> * The term KEK only appears in the docs; where it appears in the sources it's
> lower case. I suggest making "KEK" grep-able in the sources.

Fixed.

> * BootStrapKmgr() says to call once on "system install" . I suggest "initdb".

Done.

> * The jumble of #ifdef FRONTEND in src/common/kmgr_utils.c shouldn't remain in
> the final patch if possible. This may require some "static inline" wrappers or
> helpers.

I can do this if you give me more details.

> * PgKeyWrapCtx.key is unused and should probably be deleted

Removed.

> * HMAC support should go into PgCipherCtx so that we can have a single
> PgCipherCtx that supports cipher ops, cipher+HMAC ops, or just HMAC ops
> * PgKeyWrapCtx.cipherctx should then be supplemented with a hmacctx. It should
> be legal to set cipherctx and hmacctx to the same value, since in some cases
> the it won't be easy to initialize the backing implementation separately for
> key and HMAC.

Sorry, I don't know how to do the above items.

> FORK?
> ----
>
> One possible problem with this is that we should not assume we can perform KEK
> operations in postmaster children, since there's no guarantee we can use
> whatever sits behind a PgCipherCtx after fork(). But AFAICS the KEK doesn't
> live beyond the various kmgr_ operations as it is, so there's no reason it
> should ever have to be carried over a fork() anyway.

Yes, I think so.

> * the kmgr appears to need to be able to work in frontend code (?)
> * for key rotation we need to be able to change KEKs, and possibly KEK
> acquisition methods, at runtime

We might need to change the KEK using a command-line tool so we can more
easily prompt for the new KEK.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee