Re: Proposed patch for key managment

On Fri, Dec 18, 2020 at 04:36:24PM -0500, Stephen Frost wrote:
> > Starting with the passphrase key wrapper, since it's what's in place now.
> >
> > - cluster_key_challenge_command = 'password_key_wrapper %q'
>
> I do tend to like this idea of having what
> cluster_key_challenge_command, or whatever we call it, expects is an
> actual key and have the command that is run be a separate command that
> takes the passphrase and runs the KDF (key derivation function) on it,
> when a passphrase is what the user wishes to use.
>
> That generally makes more sense to me than having the key derivation
> effort built into the backend which I have a hard time seeing any
> particular reason for, as long we're already calling out to some
> external utility of some kind to get the key.

I have modified the patch to do as you suggested, and as you explained
to me on the phone. :-) Instead of having the server hash the pass
phrase provided by the cluster_passphrase_command, have the
cluster_passphrase_command generate a sha256 hash if the user-provided
input is not already 64 hex bytes. If it is already 64 hex bytes, e.g,
via openssl rand -hex 32, no need to have the server hash that again.
Double-hashing is less secure.

I have modified the attached patch to do that, and the scripts --- all
my tests pass. FYI, I moved hex_decode to src/common so that front-end
could use it, and removed it from ecpg since ecpg can use the common one
now too.

> > Sorry, I wasn't trying to hand wave it away. For automated
> > interactions, like big iron HSMs or cloud KSMs, the difference between
> > 2 operations and 10 operations to start a DB server won't matter. For
> > an admin/operator having to type 10 passwords or get 10 clean
> > thumbprint scans, it would be horrible. My underlying question was, is
> > that toil the only problem to be solved, or is there another reason to
> > get into key combination, key splitting and the related issues which
> > are less documented and less well understood than key wrapping.
>
> I appreciate that you're not trying to hand wave it away but this also
> didn't really answer the actual question- what's the advantage to having
> all of the keys provided by something external rather than having that
> external thing provide just one 'main' key, which we then use to decrypt
> our enveloped keys that we actually use? I can think of some possible
> advantages but I'd really like to know what advantages you're seeing in
> doing that.

I am not going be as kind. Our workflow is:

Desirability -> Design -> Implement -> Test -> Review -> Commit
https://wiki.postgresql.org/wiki/Todo#Development_Process

I have already asked about the first item, and received a reply talking
about the design --- that is not helpful. I only have so much patience
for the "I want my secret decoder ring to glow in the dark" type of
feature additions to this already complex feature. Unless we stay on
Desirability, I will not be replying. If you can't answer the
Desirability question, well, talking about items farther right on the
list is not very helpful.

Now, I will say that your question about how a KMS will use this got me
thinking about how to test this, and that got me to implement the AWS
Secret Manager script, so that we definitely helpful. My point is that
I don't think it is helpful to get into long discussions unless the
Desirability is clearly answered. This is not just related to this
thread --- this kind of jump-over-Desirability has happened a lot in
relation to this feature, so I thought it would be good to clearly state
it now.

> > > This seems like a crux of at least one concern- that the current patch
> > > is deriving the actual KEK from the passphrase and not just taking the
> > > provided value (at least, that's what it looks like from a *very* quick
> > > look into it), and that's the part that I was suggesting that we might
> > > add an option for- to indicate if the cluster passphrase command is
> > > actually returning a passphrase which should be used to derive a key, or
> > > if it's returning a key directly to be used. That does seem to be a
> > > material difference to me and one which we should care about.
> >
> > Yes. Caring about that is the reason I've been making a nuisance of myself.
>
> Right, I think we can agree on this aspect and I've chatted with Bruce
> about it some also. When a direct key can be provided, we should use
> that, and not run it through a KDF. This seems like a particularly
> important case that we should care about even at this early stage.

Agreed, and done in the attached patch. :-) I am thankful for all the
ideas that has helped move this feature forward.

--
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