Re: Internal key management system

On Wed, 3 Jun 2020 at 08:30, Cary Huang <cary.huang@highgo.ca> wrote:
>
> Hi
>
>
>
> I took a step back today and started to think about the purpose of internal KMS and what it is supposed to do, and
howit compares to external KMS. Both are intended to manage the life cycle of encryptions keys including their
generation,protection, storage and rotation. External KMS, on the other hand, is a more centralized but expensive way
tomanage encryption life cycles and many deployment actually starts with internal KMS and later migrate to external
one.
>
>
>
> Anyhow, the design and implementation of internal KMS should circle around these stages of key lifecycle.
>
>
>
> 1. Key Generation -  Yes, internal KMS module generates keys using pseudo random function, but only the keys for TDE
and SQL level keys. Users cannot request new key generation 
> 2. Key Protection - Yes, internal KMS wrap all keys with KEK and HMAC hash derived from a cluster passphrase
> 3. Key Storage - Yes, the wrapped keys are stored in the cluster
> 4. Key Rotation - Yes, internal KMS has a SQL method to swap out cluster passphrase, which rotates the KEK and HMAC
key
>
>
>
> I am saying this, because I want to make sure we can all agree on the scope of internal KMS. Without clear scope,
thisKMS development will seem to go on forever. 
>
>

Yes, the internal KMS is not an alternative to external KMS such as
AWS KMS, SafeNet Key Secure, and Vault, but a PostgreSQL internal
component that can work with these external solutions (via
cluster_passphrase_command). It's the same position as our other
management components such as bufmgr, smgr, and lmgr.

I agree with this scope. It manages only encryption keys used by PostgreSQL.

>
> In this patch, the internal KMS exposes pg_encrypt() and pg_decrypt() (was pg_wrap() and pg_unwrap() before) to the
userto turn a clear text password into some sort of key material based on the SQL level key generated at initdb. This
isused so the user does not have to provide clear text password to pgp_sym_encrypt() provided by pgcrypto extension.
Theintention is good, I understand, but I don't think it is within the scope of KMS and it is definitely not within the
scopeof TDE either. 
>

I agree that neither pg_encrypt() nor pg_decrypt() is within the scope
of KMS and TDE. That's why I've split the patch, and that's why I
renamed to pg_encrypt() and pg_decrypt() to clarify the purpose of
these functions is not key management. Key wrapping and unwrapping is
one of the usages of these functions.

I think we can use the internal KMS for several purposes. It can
manage encryption keys not only for cluster-wide TDE but also, for
example, for column-level TDE and encryption SQL functions.
pg_encrypt() and pg_decrypt() are one example of the usage of the
internal KMS. Originally since we thought KMS and TDE are not
introduced at the same release, the idea is come up with so that users
can use KMS functionality with some interface. Therefore these SQL
functions are not within the scope of KMS and it should be fine with
introducing the internal KMS having 0 keys.

> Even if the password can be passed into pgp_sym_encrypt() securely by using pg_decrypt() function, the
pgp_sym_encrypt()still will have to take this password and derive into an encryption key using algorithms that internal
KMSdoes not manage currently. This kind of defeats the purpose of internal KMS. So simply using pg_encrypt() and
pg_decrypt()is not really a solution to pgcrypto's limitation. 

Yeah, when using pgcrypto, user must manage their encryption keys. The
internal KMS doesn't help that because it manages only keys internally
used. What pg_encrypt() and pg_decrypt() can help is only to hide the
password from server logs.

> This should be in another topic/project that is aimed to improve pgcrypto by integrating it with the internal KMS,
similarto TDE where it also has to integrate with the internal KMS later. 
>

Agreed.

> So for internal KMS, the only cryptographic functions needed for now is kmgr_wrap_key() and kmgr_unwrap_key() to
encapsulateand restore the encryptions keys to satisfy the "key protection" life cycle stage. I don't think
pg_encrypt()and pg_decrypt() should be part of internal KMS. 
>

Agreed.

>
> Anyways, I have also reviewed the patch and have a few comments below:
>
>
>
> (1)
>
> The ciphering algorithm in my opinion should contain the algorithm name, key length and block cipher mode, which is
similarto openssl's definition. 
>
>
>
> Instead of defining a cipher as  PG_CIPHER_AES_CBC, and have key length as separate parameter, I would define them as
>
> #define PG_CIPHER_AES128_CBC 0
>
> #define PG_CIPHER_AES256_CBC 1
>
> #define PG_CIPHER_AES128_CTR 2
>
> #define PG_CIPHER_AES256_CTR 3
>

Agreed. I was concerned that we will end up having many IDs in the
future for example when porting pgcrypto functions into core but I'm
okay with that change.

>
>
> I know PG_CIPHER_AES128_CTR and PG_CIPHER_AES256_CTR are not being used now as these are for the TDE in future, but
mightas well list them here because this KMS is made to work specifically for TDE as I understand. 
>
> -----------------------------------------------------------------------------------------------------------
>
> /*
>
>  * Supported symmetric encryption algorithm. These identifiers are passed
>
>  * to pg_cipher_ctx_create() function, and then actual encryption
>
>  * implementations need to initialize their context of the given encryption
>
>  * algorithm.
>
>  */
>
> #define PG_CIPHER_AES_CBC                        0
>
> #define PG_MAX_CIPHER_ID                        1
>
> -----------------------------------------------------------------------------------------------------------
>
>
>
> (2)
>
> If the cipher algorithms are defined like (1), then there is no need to pass key length as argument to
ossl_cipher_ctx_create()function because it already knows the key length based on the cipher definition. Less argument
thebetter. 

Agreed.


Regards,

--
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services