Re: Internal key management system

On Tue, 3 Mar 2020 at 08:49, Cary Huang <cary(dot)huang(at)highgo(dot)ca> wrote:
>
> Hi Masahiko
> Please see below my comments regarding kms_v4.patch that you have shared earlier.

Thank you for reviewing this patch!

>
> (1)
> There is a discrepancy between the documentation and the actual function definition. For example in func.sgml, it states pg_wrap_key takes argument in text data type but in pg_proc.dat and kmgr.c, the function is defined to take argument in bytea data type.
>
> ===> doc/src/sgml/func.sgml
> + <entry>
> + <indexterm>
> + <primary>pg_wrap_key</primary>
> + </indexterm>
> + <literal><function>pg_wrap_key(<parameter>data</parameter> <type>text</type>)</function></literal>
> + </entry>
> + <entry>
> + <type>bytea</type>
> + </entry>
>
> ===> src/include/catalog/pg_proc.dat
> +{ oid => '8201', descr => 'wrap the given secret',
> + proname => 'pg_wrap_key',
> + provolatile => 'v', prorettype => 'bytea',
> + proargtypes => 'bytea', prosrc => 'pg_wrap_key' },
>
> ===> src/backend/crypto/kmgr.c
> +Datum
> +pg_wrap_key(PG_FUNCTION_ARGS)
> +{
> + bytea *data = PG_GETARG_BYTEA_PP(0);
> + bytea *res;
> + int datalen;
> + int reslen;
> + int len;

Fixed.

> +
>
> (2)
> I think the documentation needs to make clear the difference between a key and a user secret. Some parts of it are trying to mix the 2 terms together when they shouldn't. To my understanding, a key is expressed as binary data that is actually used in the encryption and decryption operations. A user secret, on the other hand, is more like a passphrase, expressed as string, that is used to derive a encryption key for subsequent encryption operations.
>
> If I just look at the function names "pg_wrap_key" and "pg_unwrap_key", I immediately feel that these functions are used to encapsulate and uncover cryptographic key materials. The input and output to these 2 functions should all be key materials expressed in bytea data type. In previous email discussion, there was only one key material in discussion, called the master key (generated during initdb and stored in cluster), and this somehow automatically makes people (including myself) associate pg_wrap_key and pg_unwrap_key to be used on this master key and raise a bunch of security concerns around it.
>
> Having read the documentation provided by the patch describing pg_wrap_key and pg_unwrap_key, they seem to serve another purpose. It states that pg_wrap_key is used to encrypt a user-supplied secret (text) with the master key and produce a wrapped secret while pg_unwrap_key does the opposite, so we can prevent user from having to enter the secret in plaintext when using pgcrypto functions.
>
> This use case is ok but user secret is not really a cryptographic key material is it? It is more similar to a secret passphrase expressed in text and pg_wrap_key is merely used to turn the passphrase into a wrapped passphrase expressed in bytea.
>
> If I give pg_wrap_key with a real key material expressed in bytea, I will not be able to unwrap it properly:
>
> Select pg_unwrap_key (pg_wrap_key(E'\\x2b073713476f9f0e761e45c64be8175424d2742e7d53df90b6416f1d84168e8a') );
>
> pg_unwrap_key
> ----------------------------------------------
> +\x077\x13Go\x0Ev\x1EEK\x17T$t.}SߐAo\x1D\x16
> (1 row)
>
> Maybe we should rename these SQL functions like this to prevent confusion.
> => pg_wrap_secret (takes a text, returns a bytea)
> => pg_unwrap_secret(takes a bytea, returns a text)

Agreed to change argument types. User secret will be normally text
password as we do with pgcrypto. So probably these functions can cover
most cases. I changed the function name to pg_wrap and pg_unwrap
because these functions generically wrap and unwrap the given data.

>
> if there is a use case for users to encapsulate key materials then we can define 2 more wrap functions for these, if there is no use case, don't bother:
> => pg_wrap_key (takes a bytea, returns a bytea)
> => pg_unwrap_key (takes a bytea, returns a bytea)

+1. Like pgcrypto has both pgp_sym_encrypt_bytea and pgp_sym_encrypt,
maybe we can have such functions.

>
> (3)
> I would rephrase "chapter 32: Encryption Key Management Part III. Server Administration" documentation like this:
>
> =====================
> PostgreSQL supports Encryption Key Management System, which is enabled when PostgreSQL is built with --with-openssl option and cluster_passphrase_command is specified during initdb process. The user-provided cluster_passphrase_command in postgresql.conf and the cluster_passphrase_command specified during initdb process must match, otherwise, the database cluster will not start up.
>
> The user-provided cluster passphrase is derived into a Key Encryption Key (KEK), which is used to encapsulate the Master Encryption Key generated during the initdb process. The encapsulated Master Encryption Key is stored inside the database cluster.
>
> Encryption Key Management System provides several functions to allow users to use the master encryption key to wrap and unwrap their own encryption secrets during encryption and decryption operations. This feature allows users to encrypt and decrypt data without knowing the actual key.
> =====================
>
> (4)
> I would rephrase "chapter 32.2: Wrap and Unwrap user secret" documentation like this: Note that I rephrase based on (2) and uses pg_(un)wrap_secret instead.
>
> =====================
> Encryption key management System provides several functions described in Table 9.97, to wrap and unwrap user secrets with the Master Encryption Key, which is uniquely and securely stored inside the database cluster.
>
> These functions allow user to encrypt and decrypt user data without having to provide user encryption secret in plain text. One possible use case is to use encryption key management together with pgcrypto. User wraps the user encryption secret with pg_wrap_secret() and passes the wrapped encryption secret to the pgcrypto encryption functions. The wrapped secret can be stored in the application server or somewhere secured and should be obtained promptly for cryptographic operations with pgcrypto.
> [same examples follow after...]
> =====================

Thank you for suggesting the updated sentences. I've updated the docs
based on your suggestions.

Regards,

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