Re: Key management with tests

Hi Stephen,

On Tue, Jan 12, 2021 at 10:47 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:

>
> This is an interesting question but ultimately I don't think we should
> be looking at this from the perspective of allowing arbitrary changes to
> the page format. The challenge is that much of the page format, today,
> is defined by a C struct and changing the way that works would require a
> great deal of code to be modified and turn this into a massive effort,
> assuming we wish to have the same compiled binary able to work with both
> unencrypted and encrypted clusters, which I do believe is a requirement.
>
> The thought that I had was to, instead, try to figure out if we could
> fudge some space by, say, putting a 128-bit 'hole' at the end of the
> page and just move pd_special back, effectively making the page seem
> 'smaller' to all of the code that uses it, except for the code that
> knows how to do the decryption. I ran into some trouble with that but
> haven't quite sorted out what happened yet. Other ideas would be to put
> it before pd_special, or maybe somewhere else, but a lot depends on the
> code's expectations.
>
>
I agree that we should not make too many changes to affect the use of
unencrypted clusters. But as a personal opinion only, I don't think it's a
good idea to add some "implicit" tricks. To provide an inspiration, can we
add a flag to mark whether the page format has been changed:

--- a/src/include/storage/bufpage.h
+++ b/src/include/storage/bufpage.h
@@ -181,8 +185,9 @@ typedef PageHeaderData *PageHeader;
#define PD_PAGE_FULL 0x0002 /* not enough free space for new tuple? */
#define PD_ALL_VISIBLE 0x0004 /* all tuples on page are visible to
* everyone */
+#define PD_PAGE_ENCRYPTED 0x0008 /* Is page encrypted? */

-#define PD_VALID_FLAG_BITS 0x0007 /* OR of all valid pd_flags bits */
+#define PD_VALID_FLAG_BITS 0x000F /* OR of all valid pd_flags bits */

/*
* Page layout version number 0 is for pre-7.3 Postgres releases.
@@ -389,6 +394,13 @@ PageValidateSpecialPointer(Page page)
#define PageClearAllVisible(page) \
(((PageHeader) (page))->pd_flags &= ~PD_ALL_VISIBLE)

+#define PageIsEncrypted(page) \
+ (((PageHeader) (page))->pd_flags & PD_PAGE_ENCRYPTED)
+#define PageSetEncrypted(page) \
+ (((PageHeader) (page))->pd_flags |= PD_PAGE_ENCRYPTED)
+#define PageClearEncrypted(page) \
+ (((PageHeader) (page))->pd_flags &= ~PD_PAGE_ENCRYPTED)
+
#define PageIsPrunable(page, oldestxmin) \
( \
AssertMacro(TransactionIdIsNormal(oldestxmin)), \

In this way, I think it has little effect on the unencrypted cluster, and
we can also modify the page format as we wish. Of course, it's also
possible that I didn't understand your design correctly, or there's
something wrong with my idea. :D

--
There is no royal road to learning.
HighGo Software Co.