From 7638ec548a2b36fb40830fd1de73d9074920350d Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Mon, 7 Apr 2025 20:17:32 +0200 Subject: [PATCH] Make comment about all-zero pages less scary The comment about all-zero pages created by smgrzeroextend() sounded much scarier than the reality. In fact trying to encrypt these all-zero pages might not only be a waste of CPU cycles but also could decrease security by making us re-use the same IV first with all zeros and then with the actual data. And the extra amount of protection we gain from encrypting them is minuscule since they are only added at the end of the table, soon overwritten and only gives the attacker a very slightly more accurate table size. --- contrib/pg_tde/src/smgr/pg_tde_smgr.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/contrib/pg_tde/src/smgr/pg_tde_smgr.c b/contrib/pg_tde/src/smgr/pg_tde_smgr.c index 0decca0e95a..1a3b9b3a3da 100644 --- a/contrib/pg_tde/src/smgr/pg_tde_smgr.c +++ b/contrib/pg_tde/src/smgr/pg_tde_smgr.c @@ -164,24 +164,19 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, BlockNumber bn = blocknum + i; unsigned char iv[16]; + /* + * Detect unencrypted all-zero pages written by smgrzeroextend() by + * looking at the first 32 bytes of the page. + * + * Not encrypting all-zero pages is safe because they are only written + * at the end of the file when extending a table on disk so they tend + * to be short lived plus they only leak a slightly more accurate + * table size than one can glean from just the file size. + */ for (int j = 0; j < 32; ++j) { if (((char **) buffers)[i][j] != 0) { - /* - * Postgres creates all zero blocks in an optimized route, - * which we do not try - */ - /* to encrypt. */ - /* - * Instead we detect if a block is all zero at decryption - * time, and - */ - /* leave it as is. */ - /* - * This could be a security issue later, but it is a good - * first prototype - */ allZero = false; break; }