On Mon, Jul 29, 2019 at 8:35 PM Bruce Momjian <bruce@momjian.us> wrote:
On Sun, Jul 28, 2019 at 10:33:03PM -0400, Bruce Momjian wrote: > I am thinking of writing some Assert() code that checks that all buffers > using a single LSN are from the same relation (and therefore different > page numbers). I would do it by creating a static array, clearing it on > XLogBeginInsert(), adding to it for each XLogInsert(), then checking on > PageSetLSN() that everything in the array is from the same file. Does > that make sense?
So, I started looking at how to implement the Assert checks and found that Heikki has already added (in commit 2c03216d83) Assert checks to avoid duplicate block numbers in WAL. I just added the attached patch to check that all RelFileNodes are the same.
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.
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.
I ran the regression tests with asserts on and got no failures, so I think we are good.
It's not strictly required but it also doesn't hurt that LSN is unique per-relation so that's still good news!
Might be useful for something down the road like a separate stream of MACs computed per-LSN.