On Tue, Jul 30, 2019 at 07:44:20AM -0400, Sehrope Sarkuni wrote:
> On Mon, Jul 29, 2019 at 8:35 PM Bruce Momjian <[email protected]> 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/[email protected]
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 <[email protected]> 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
