Re: Internal key management system

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
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-16 20:24:37
Message-ID: 20201016202437.GA28789@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

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, and pg_encrypt() is documented, but not defined anywhere.

Second, in testing starting/stopping the server, pg_ctl doesn't allow
the cluster_passphrase_command to read from /dev/tty, which I think is a
requirement because the command could likely require a user-supplied
unlock key, even if that is not the actual passphrase, just like ssl
keys. This is because pg_ctl calls setsid() just before calling execl()
to start the server, and setsid() disassociates itself from the
controlling terminal. I think the fix is to remove setsid() from pg_ctl
and add a postmaster flag to call setsid() after it has potentially
called cluster_passphrase_command, and pg_ctl would use that flag.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Attachment Content-Type Size
key.diff text/x-diff 95.5 KB
pass.sh application/x-sh 86 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2020-10-16 20:53:48 Re: upcoming API changes for LLVM 12
Previous Message Anastasia Lubennikova 2020-10-16 20:21:38 Re: Commitfest manager 2020-11