Re: Key management with tests

Hi Stephen,

On Tue, Jan 12, 2021 at 10:47 AM Stephen Frost <sfrost@snowman.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.