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

On Wed, Jul 31, 2019 at 5:48 AM Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Tue, Jul 30, 2019 at 10:14:14AM -0400, Sehrope Sarkuni wrote:
> > > 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.
>
> I had more time to think about the complexity of adding relfilenode to
> the IV. Since relfilenode is only unique within a database/tablespace,
> we would need to have pg_upgrade preserve database/tablespace oids
> (which I assume are the same as the directory and tablespace symlinks).
> Then, to decode a page, you would need to look up those values. This is
> in addition to the new complexity of CREATE DATABASE and moving files
> between tablespaces. I am also concerned that crash recovery operations
> and cluster forensics and repair would need to also deal with this.
>
> I am not even clear if pg_upgrade preserving relfilenode is possible ---
> when we wrap the relfilenode counter, does it start at 1 or at the
> first-user-relation-oid? If the former, it could conflict with oids
> assigned to new system tables in later major releases. Tying the
> preservation of relations to two restrictions seems risky.
>
> Using just the page LSN and page number allows a page to be be
> decrypted/encrypted independently of its file name, tablespace, and
> database, and I think that is a win for simplicity. Of course, if it is
> insecure we will not do it.
>
> I am thinking for the heap/index IV, it would be:
>
> uint64 lsn;
> unint32 page number;
> /* only uses 11 bits for a zero-based CTR counter for 32k pages */
> uint32 counter;
>

+1
IIUC since this would require to ensure uniqueness by using key+IV we
need to use different keys for different relations. Is that right?

> and for WAL it would be:
>
> uint64 segment_number;
> uint32 counter;
> /* guarantees this IV doesn't match any relation IV */
> uint32 2^32-1 /* all 1's */

I would propose to include the page number within a WAL segment to IV
so that we can encrypt each WAL page with the counter always starting
from 0. And if we use different encryption keys for tables/indexes and
WAL I think we don't need 2^32-1.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center