Re: Internal key management system

On Tue, 3 Mar 2020 at 08:49, Cary Huang <cary.huang@highgo.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
statespg_wrap_key takes argument in text data type but in pg_proc.dat and kmgr.c, the function is defined to take
argumentin 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
tryingto mix the 2 terms together when they shouldn't. To my understanding, a key is expressed as binary data that is
actuallyused in the encryption and decryption operations. A user secret, on the other hand, is more like a passphrase,
expressedas 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
usedto encapsulate and uncover cryptographic key materials. The input and output to these 2 functions should all be key
materialsexpressed in bytea data type. In previous email discussion, there was only one key material in discussion,
calledthe master key (generated during initdb and stored in cluster), and this somehow automatically makes people
(includingmyself) associate pg_wrap_key and pg_unwrap_key to be used on this master key and raise a bunch of security
concernsaround it. 
>
> Having read the documentation provided by the patch describing pg_wrap_key and pg_unwrap_key, they seem to serve
anotherpurpose. It states that pg_wrap_key is used to encrypt a user-supplied secret (text) with the master key and
producea wrapped secret while pg_unwrap_key does the opposite, so we can prevent user from having to enter the secret
inplaintext 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
passphraseexpressed in text and pg_wrap_key is merely used to turn the passphrase into a wrapped passphrase expressed
inbytea. 
>
> 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
thereis 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
optionand cluster_passphrase_command is specified during initdb process. The user-provided cluster_passphrase_command
inpostgresql.conf and the cluster_passphrase_command specified during initdb process must match, otherwise, the
databasecluster will not start up. 
>
> The user-provided cluster passphrase is derived into a Key Encryption Key (KEK), which is used to encapsulate the
MasterEncryption Key generated during the initdb process. The encapsulated Master Encryption Key is stored inside the
databasecluster. 
>
> Encryption Key Management System provides several functions to allow users to use the master encryption key to wrap
andunwrap their own encryption secrets during encryption and decryption operations. This feature allows users to
encryptand 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
withthe 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
secretwith pg_wrap_secret() and passes the wrapped encryption secret to the pgcrypto encryption functions. The wrapped
secretcan be stored in the application server or somewhere secured and should be obtained promptly for cryptographic
operationswith 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