On Tue, Jul 30, 2019 at 07:44:20AM -0400, Sehrope Sarkuni wrote: > On Mon, Jul 29, 2019 at 8:35 PM Bruce Momjian <br...@momjian.us> wrote: > From the patch: > > /* > ! * The initialization vector (IV) is used for page-level > ! * encryption. We use the LSN and page number as the IV, and IV > ! * values must never be reused since it is insecure. It is safe > ! * to use the LSN on multiple pages in the same relation since > ! * the page number is part of the IV. It is unsafe to reuse the > ! * LSN in different relations because the page number might be > ! * the same, and hence the IV. Therefore, we check here that > ! * we don't have WAL records for different relations using the > ! * same LSN. > ! */ > > 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.2bxakegiqafxg...@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. 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. > 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. -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c new file mode 100644 index 3ec67d4..57f4d71 *** a/src/backend/access/transam/xloginsert.c --- b/src/backend/access/transam/xloginsert.c *************** XLogResetInsertion(void) *** 208,213 **** --- 208,215 ---- /* * Register a reference to a buffer with the WAL record being constructed. * This must be called for every page that the WAL-logged operation modifies. + * Because of page-level encryption, You cannot reference more than one + * RelFileNode in a WAL record; Assert checks for that. */ void XLogRegisterBuffer(uint8 block_id, Buffer buffer, uint8 flags) *************** XLogRegisterBuffer(uint8 block_id, Buffe *** 235,241 **** /* * Check that this page hasn't already been registered with some other ! * block_id. */ #ifdef USE_ASSERT_CHECKING { --- 237,243 ---- /* * Check that this page hasn't already been registered with some other ! * block_id, and check for different RelFileNodes in the same WAL record. */ #ifdef USE_ASSERT_CHECKING { *************** XLogRegisterBuffer(uint8 block_id, Buffe *** 248,256 **** --- 250,274 ---- if (i == block_id || !regbuf_old->in_use) continue; + /* check for duplicate block numbers */ Assert(!RelFileNodeEquals(regbuf_old->rnode, regbuf->rnode) || regbuf_old->forkno != regbuf->forkno || regbuf_old->block != regbuf->block); + + /* + * The initialization vector (IV) is used for page-level + * encryption. We use the LSN and page number as the IV, and IV + * values must never be reused since it is insecure. It is safe + * to use the LSN on multiple pages in the same relation since + * the page number is part of the IV. It is unsafe to reuse the + * LSN in different relations because the page number might be + * the same, and hence the IV. Therefore, we check here that + * we don't have WAL records for different relations using the + * same LSN. We only encrypt MAIN_FORKNUM files. + */ + Assert(RelFileNodeEquals(regbuf_old->rnode, regbuf->rnode) || + regbuf_old->forkno != MAIN_FORKNUM || + regbuf->forkno != MAIN_FORKNUM); } } #endif