Re: Internal key management system

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.