On Wed, Jul 31, 2019 at 5:48 AM Bruce Momjian <br...@momjian.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