Re: Internal key management system

From: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, Ahsan Hadi <ahsan(dot)hadi(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Moon, Insung" <tsukiwamoon(dot)pgsql(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Sehrope Sarkuni <sehrope(at)jackdb(dot)com>, cary huang <hcary328(at)gmail(dot)com>, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, Joe Conway <mail(at)joeconway(dot)com>
Subject: Re: Internal key management system
Date: 2020-10-19 03:15:24
Message-ID: CA+fd4k7mMk5CW7U5j9VVHRLuK2dEpTV3tRpn0bPoUDbViu9D5w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 17 Oct 2020 at 05:24, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Fri, Jul 31, 2020 at 04:06:38PM +0900, Masahiko Sawada wrote:
> > > Given that the purpose of the key manager is to help TDE, discussing
> > > the SQL interface part (i.g., the second patch) deviates from the
> > > original purpose. I think we should discuss the design and
> > > implementation of the key manager first and then other additional
> > > interfaces. So I’ve attached a new version patch and removed the
> > > second patch part so that we can focus on only the key manager part.
> > >
> >
> > Since the previous patch sets conflicts with the current HEAD, I've
> > attached the rebased patch set.
>
> I have updated the attached patch and am hoping to move this feature
> forward. The changes I made are:
>
> * handle merge conflicts
> * changed ssl initialization to match other places in our code
> * changed StrNCpy() to strlcpy
> * update the docs

Thank you for updating the patch!

>
> The first three were needed to get it to compile. I then ran some tests
> using the attached shell script as my password script. First, I found
> that initdb called the script twice. The first call worked fine, but
> the second call would accept a password that didn't match the first
> call. This is because there are no keys defined, so there is nothing
> for kmgr_verify_passphrase() to check for passkey verification, so it
> just succeeds. In fact, I can't figure out how to create any keys with
> the patch,

The patch introduces only key management infrastructure but with no
key. Currently, there is no interface to dynamically add a new
encryption key. We need to add the new key ID to internalKeyLengths
array and increase KMGR_MAX_INTERNAL_KEY. The plan was to add a
subsequent patch that adds functionality using encryption keys managed
by the key manager. The patch was to add two SQL functions: pg_wrap()
and pg_unwrap(), along with the internal key wrap key.

IIUC, what to integrate with the key manager is still an open
question. The idea of pg_wrap() and pg_unwrap() seems good but it
still has the problem that the password could be logged to the server
log when the user wraps it. Of course, since the key manager is
originally designed for cluster-wide transparent encryption, TDE will
be one of the users of the key manager. But there was a discussion
it’s better to introduce the key manager first and to make it have
small functions or integrate with existing features such as pgcrypto
because TDE development might take time over the releases. So I'm
thinking to deal with the problem the pg_wrap idea has or to make
pgcrypto use the key manager so that it doesn't require the user to
pass the password as a function argument.

Regards,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2020-10-19 03:16:38 Re: Online checksums verification in the backend
Previous Message Michael Paquier 2020-10-19 02:39:18 Re: Online checksums verification in the backend