Re: Proposed patch for key managment - Mailing list pgsql-hackers

On Sat, 5 Dec 2020 at 11:08, Bruce Momjian <bruce@momjian.us> wrote:
>
> On Wed, Dec  2, 2020 at 04:38:14PM -0500, Bruce Momjian wrote:
> > If most people approve of this general approach, and the design
> > decisions made, I would like to apply this in the next few weeks, but
> > this brings complications.  The syntax added by this commit might not
> > provide a useful feature until PG 15, so how do we hide it from users.
> > I was thinking of not applying the doc changes (or commenting them out)
> > and commenting out the --help output.
>
> Here is an updated patch to handle the new hash API introduced by
> commit 87ae9691d2.
>

Thank you for updating the patch and moving forward!

I've tried to use this patch on my environment (FreeBSD 12.1) but it
doesn't work. I got the following error:

$ bin/initdb -D data -E UTF8 --no-locale -c'echo
"1234567890123456789012345678901234567890123456789012345678901234567890"'
The files belonging to this database system will be owned by user "masahiko".
This user must also own the server process.

The database cluster will be initialized with locale "C".
The default text search configuration will be set to "english".

Data page checksums are disabled.
Cluster file encryption is enabled.

creating directory data ... ok
creating subdirectories ... ok
selecting dynamic shared memory implementation ... posix
selecting default max_connections ... 100
selecting default shared_buffers ... 128MB
selecting default time zone ... Japan
creating configuration files ... ok
running bootstrap script ... child process was terminated by signal
10: Bus error
initdb: removing data directory "data"

The backtrace is:

(lldb) bt
* thread #1, name = 'postgres', stop reason = signal SIGBUS: hardware error
    frame #0: 0x0000000000d3b074
postgres`ResourceArrayRemove(resarr=0x7f7f7f7f7f7f80df,
value=34383251232) at resowner.c:308:23
    frame #1: 0x0000000000d3c0ef
postgres`ResourceOwnerForgetCryptoHash(owner=0x7f7f7f7f7f7f7f7f,
handle=34383251232) at resowner.c:1421:7
    frame #2: 0x0000000000d77c54
postgres`pg_cryptohash_free(ctx=0x000000080166c720) at
cryptohash_openssl.c:228:3
  * frame #3: 0x0000000000d6c065
postgres`kmgr_derive_keys(passphrase="1234567890123456789012345678901234567890123456789012345678901234567890\n",
passlen=71, enckey="\x01d

\x17(\x93\xa8id\xae\xa5\x86\x84\xb9Y>\xa84\x16\U00000085\U0000009d'\r\x11123456789012345678901234567890
1234567890123456789012345678901234567890\n",

mackey="0\x1f\xb4_eg\x95\x02\x9e2\x0e\xba\t\b^\x18\x90U\xa0\x8e\xaei\x01oYJL\xa4\xb3E\x97a,\x06h4\x9fX\x9eS\xb2\x81%^d\xa4\x01d

\x17(\x93\xa8id\xae\xa5\x86\x84\xb9Y>\xa84\x16\U00000085\U0000009d'\r\x1112345678901234567890123456789
01234567890123456789012345678901234567890\n") at kmgr_utils.c:239:2
    frame #4: 0x0000000000d60810 postgres`BootStrapKmgr at kmgr.c:93:2
    frame #5: 0x0000000000647fa1 postgres`BootStrapXLOG at xlog.c:5359:3
    frame #6: 0x000000000066f034 postgres`AuxiliaryProcessMain(argc=7,
argv=0x00007fffffffcdb8) at bootstrap.c:450:4
    frame #7: 0x00000000008e9cfb postgres`main(argc=8,
argv=0x00007fffffffcdb0) at main.c:201:3
    frame #8: 0x000000000051010f postgres`_start(ap=<unavailable>,
cleanup=<unavailable>) at crt1.c:76:7

Investigating the issue, it seems we need to initialize the context
and the state with 0. The following change fixes this issue.

diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c
index e5233daab6..a45c86fa67 100644
--- a/src/common/cryptohash_openssl.c
+++ b/src/common/cryptohash_openssl.c
@@ -81,6 +81,8 @@ pg_cryptohash_create(pg_cryptohash_type type)
        return NULL;
    }

+   memset(ctx, 0, sizeof(pg_cryptohash_ctx));
+   memset(state, 0, sizeof(pg_cryptohash_state));
    ctx->data = state;
    ctx->type = type;

Regards,

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