Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS)

On Tue, Jul 30, 2019 at 8:16 AM Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Tue, Jul 30, 2019 at 07:44:20AM -0400, Sehrope Sarkuni wrote:
> > If each relation file has its own derived key, the derived TDEK for that
> > relation file, then there is no issue with reusing the same IV = LSN ||
> Page
> > Number. The TDEKs will be different so Key + IV will never collide.
>
> So, this email explains that we are considering not using the
> relfilenode/tablepsace/database to create a derived key per relation,
> but us the same key for all relaions because the IV will be unique per
> page across all relations:
>
>
> https://www.postgresql.org/message-id/20190729134442.2bxakegiqafxgj6u@momjian.us
>
> There is talk of using a derived key with a contant to make sure all
> heap/index files use a different derived key than WAL, but I am not
> sure. This is related to whether WAL IV and per-heap/index IV can
> collide.
>

Ah, I read that to imply that derived keys were a must. Specifically this
piece at the end:

From Joe's email on 2019-07-13 18:41:34:
>> Based on all of that I cannot find a requirement that we use more than
one key per database.
>>
>> But I did find that files in an encrypted file system are encrypted with
derived keys from a master key, and I view this as analogous to what we
are doing.

I read that as the "one key per database" is the MDEK.

And read the part about derived keys as referring to separate derived keys
for relations. Perhaps I misread and it was referring to different keys for
WAL vs pages.

> There are other emails in the thread that also discuss the topic. The
> issue is that we add a lot of complexity to other parts of the system,
> (e.g. pg_upgrade, CREATE DATABASE, moving relations between tablespaces)
> to create a derived key, so we should make sure we need it before we do
> it.
>

Yes it definitely complicates things both on the derivation and potential
additional storage for the salts (they're small and fixed size, but you
still need to put it somewhere).

I think key rotation for TDEK will be impossible without some stored salt
and per-relation derived key. It might not be needed in a first cut though
as the "default salt" could be no salt or a place holder of all zeroes.
Even if the rotation itself is out of scope for a first pass the potential
to eventually add it should be there.

Should keep in mind that because we do not have a MAC on the encrypted
pages we'll need to know which derived key to use. We can't try multiple
options to see which is correct as any key would "succeed" with garbage
decrypted data.

> > In general it's fine to use the same IV with different keys. Only reuse
> of Key
> > + IV is a problem and the entire set of possible counter values (IV + 0,
> IV +
> > 1, ...) generated with a key must be unique. That's also why we must
> leave at
> > least log2(PAGE_SIZE / AES_BLOCK_SIZE) bits at the end of the IV to be
> filled
> > in with 0, 1, 2, ... for each 16-byte AES-block on the page. If our
> per-page IV
> > prefix used any of those bits then the counter could overflow into the
> next
> > page's IV's range.
>
> Agreed.
>
> Attached is an updated patch that checks only main relation forks, which
> I think are the only files we are going ot encrypt, and it has better
> comments.
>

Okay that makes sense in the context of using a single key and relying on
the LSN based IV to be unique.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/