Re: Internal key management system

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, Ahsan Hadi <ahsan(dot)hadi(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Moon, Insung" <tsukiwamoon(dot)pgsql(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Sehrope Sarkuni <sehrope(at)jackdb(dot)com>, cary huang <hcary328(at)gmail(dot)com>, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>
Subject: Re: Internal key management system
Date: 2020-06-14 10:39:07
Message-ID: alpine.DEB.2.22.394.2006141216190.473586@pseudo
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Masahiko-san,

>>> * The external place needs to manage more encryption keys than the
>>> current patch does.
>>
>> Why? If the external place is just a separate process on the same host,
>> probably it would manage the very same amount as what your patch.
>
> In the current patch, the external place needs to manage only one key
> whereas postgres needs to manages multiple DEKs. But with your idea,
> the external place needs to manage both KEK and DEKs.

Hmmm. I do not see a good use case for a "management system" which would
only have to manage a singleton. ISTM that one point of using one KEK is
to allows several DEKs under it. Maybe I have missed something in your
patch, but only one key is a very restricted use case.

>>> Some cloud key management services are charged by the number of active
>>> keys and key operations. So the number of keys postgres requires affects
>>> the charges. It'd be worse if we were to have keys per table.
>>
>> Possibly. Note that you do not have to use a cloud storage paid as a
>> service. However, you could do it if there is an interface, because it
>> would allow postgres to do so if the user wishes that. That is the point
>> of having an interface that can be implemented differently for different
>> use cases.
>
> The same is true for the current patch. The user can get KEK from
> anywhere they want using cluster_passphrase_command.

Yep. Somehow I'm proposing to have a command to get DEKs instead of just
the KEK, otherwise it is not that far.

> But as I mentioned above the number of keys that the user manages
> outside postgres is different.

Yep, and I do not think that "only one key" approach is good. I really
missed something in the patch. From a use case point of view, I thought
that the user could have has many keys has they see fit. Maybe one per
cluser, or database, or table, or a row if for some reason the application
would demand it. I do not think that the pg should decide that, among
other things. That is why I'm constantly refering to a "key identifier",
and in the pseudo code I added a "local id" (typically an int).

>>> * If this approach supports only GET protocol, the user needs to
>>> create encryption keys with appropriate ids in advance so that
>>> postgres can get keys by id. If postgres TDE creates keys as needed,
>>> CREATE protocol would also be required.
>>
>> I'm not sure. ISTM that if there is a KMS to manage keys, it could be its
>> responsability to actually create a key, however the client (pg) would
>> have to request it, basically say "given me a new key for this id".
>>
>> This could even work with a "get" command only, if the KMS is expected to
>> create a new key when asked for a key which does not exists yet. ISTM that
>> the client could (should?) only have to create identifiers for its keys.
>
> Yeah, it depends on KMS, meaning we need different extensions for
> different KMS. A KMS might support an interface that creates key if not
> exist during GET but some KMS might support CREATE and GET separately.

I disagree that it is necessary, but this is debatable. The KMS-side
interface code could take care of that, eg:

if command is "get X"
if (X does not exist in KMS)
create a new key stored in KMS, return it;
else
return KMS-stored key;
...

So you can still have a "GET" only interface which adapts to the "final"
KMS. Basically, the glue code which implements the interface for the KMS
can include some logic to adapt to the KMS point of view.

>>> * If we need only GET protocol, the current approach (i.g.

>> The point of a discussion is basically to present arguments.
>
> My point is the same as Bruce. I'm concerned about the fact that even
> if we introduce this approach the present data could still be stolen
> when a postgres process is compromised.

Yes, sure.

> It seems to me that your approach is extensible and can protect data
> from threats in addition to threats that the current patch can protect
> but it would bring some costs and complexity instead comparing to the
> current patch. I'd like to hear opinions from other hackers in the
> community.

I'd like an extensible design to have anything in core. As I said in an
other mail, if you want to handle a somehow restricted use case, just
provide an external extension and do nothing in core, please. Put in core
something that people with a slightly different use case or auditor can
build on as well. The current patch makes a dozen hard-coded decisions
which it should not, IMHO.

> I think the actual code would help to explain how your approach is not
> complexed.

I provided quite some pseudo code, including some python. I'm not planning
to redevelop the whole thing: my contribution is a review, currently about
the overall design, then if somehow I agree on the design, I would look at
the code more precisely.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josef Šimánek 2020-06-14 12:32:33 [PATCH] Initial progress reporting for COPY command
Previous Message Masahiko Sawada 2020-06-14 10:00:44 Re: Internal key management system