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.saw...@2ndquadrant.com> wrote:
>
> On Tue, 3 Mar 2020 at 08:49, Cary Huang <cary.hu...@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 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
