Re: Internal key management system - Mailing list pgsql-hackers

Dear Sawada-san

I don't know if my environment or email system is weird, but the V5
patch file is only include simply a changed list.
and previous V4 patch file size was 64kb, but the v5 patch file size was 2kb.
Can you check it?

Best regards.
Moon.

On Tue, Mar 3, 2020 at 5:58 PM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
>
> 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
secretpassphrase expressed in text and pg_wrap_key is merely used to turn the passphrase into a wrapped passphrase
expressedin 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,
ifthere 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
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
secretswith 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
plaintext. One possible use case is to use encryption key management together with pgcrypto. User wraps the user
encryptionsecret with pg_wrap_secret() and passes the wrapped encryption secret to the pgcrypto encryption functions.
Thewrapped secret can be stored in the application server or somewhere secured and should be obtained promptly for
cryptographicoperations 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