Re: Proposed patch for key managment

From: Neil Chen <carpenter(dot)nail(dot)cz(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
Subject: Re: Proposed patch for key managment
Date: 2020-12-15 02:36:56
Message-ID: CAA3qoJ=qtO5JcSBjqFDBT9iKUX9XKmC5bXCrd7rysE+XSMEuTg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, Bruce

I read your question and here are some of my thoughts.

Why is KmgrShmemData a struct, when it only has a single member?
> Are
> all shared memory areas structs?
>

Yes, all areas created by ShmemInitStruct() are structs. I think the
significance of using struct is that it delimits an "area" to store the
KMS related things, which makes our memory management more clear and
unified.

What testing infrastructure should this have?
>
> There are a few shell script I should include to show how to create
> commands. Where should they be stored? /contrib module?

Are people okay with having the feature enabled, but invisible
> since the docs and --help output are missing? When we enable
> ssl_passphrase_command to prompt from the terminal, some of the
> command-line options will be useful.

Since our implementation is not in contrib, I don't think we should put the
script there. Maybe we can refer to postgresql.conf.sample?
Actually, I wonder whether we can add the UDK(user data encryption key) to
the first version of KMS, which can be provided to plug-ins such as
pgsodium. In this way, users can at least use it to a certain extent.

Also, I have tested some KMS functionalities by adding very simple TDE
logic. In the meantime, I found some confusion in the following places:

1. Previously, we added a variable bootstrap_keys_wrap that is used for
encryption during initdb. However, since we save the "wrapped" key, we need
to use a global KEK that can be accessed in boot mode to unwrap it before
use... I don't know if that's good. To make it simple, I modified the
bootstrap_keys_wrap to store the "unwrapped" key so that the encryption
function can get it correctly. (The variable name should be changed
accordingly).

2. I tried to add support for AES_CTR mode, and the code for encrypting
buffer blocks. In the process I found that in pg_cipher_ctx_create, the key
length is declared as "byte". However, in the CryptoKey structure, the
length is stored as "bit", which leads me to use a form similar to
Key->klen / 8 when I call this function. Maybe we should unify the two to
avoid unnecessary confusion.

3. This one is not a problem/bug. I have some doubts about the length of
data encryption blocks. For the page, we do not encrypt the PageHeaderData,
which is 192 bit long. By default, the page size is 8K(65536 bits), so the
length of the data we want to encrypt is 65344 bits. This number can't be
divisible by 128 and 192 keys and 256 bits long keys. Will this cause a
problem?

Here is a simple patch that I wrote with references to Sawada's previous
TDE works, hope it can help you.

Thanks.
--
There is no royal road to learning.
HighGo Software Co.

Attachment Content-Type Size
encrypt_buffer.diff application/octet-stream 29.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2020-12-15 02:38:43 postgres_fdw - cached connection leaks if the associated user mapping/foreign server is dropped
Previous Message yuzuko 2020-12-15 01:46:26 Re: Autovacuum on partitioned table (autoanalyze)