Re: storing an explicit nonce

On Tue, May 25, 2021 at 03:09:03PM -0400, Robert Haas wrote:
> On Tue, May 25, 2021 at 2:45 PM Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Well, if we create a separate nonce counter, we still need to make sure
> > it doesn't go backwards during a crash, so we have to WAL log it
>
> I think we don't really need a global counter, do we? We could simply
> increment the nonce every time we write the page. If we want to avoid
> using the same IV for different pages, then 8 bytes of the nonce could
> store a value that's different for every page, and the other 8 bytes
> could store a counter. Presumably we won't manage to write the same
> page more than 2^64 times, since LSNs are limited to be <2^64, and
> those are consumed more than 1 byte at a time for every change to any
> page anywhere.

The issue we had here is what do you use as a special value for each
relation? Where do you store it if it is not computed? You can use a
global counter for the per-page nonce that doesn't change when the page
is updated, but that would still need to be a global counter.

Also, when you change hint bits, either you don't change the nonce/LSN,
and don't recrypt the page (and the hint bit changes are visible), or
you change the nonce and reencrypt the page, and you are then WAL
logging the page. I don't see how having a nonce different from the LSN
helps here.

> > The buffer encryption overhead is 2-4%, and WAL encryption is going to
> > add to that, so I thought hint bit logging overhead would be minimal
> > in comparison.
>
> I think it depends. If buffer evictions are rare, then it won't matter
> much. But if they are common, then using the LSN as the nonce will add
> a lot of overhead.

Well, see above. A separate nonce somewhere else doesn't help much, as
I see it.

> > Have you looked at the code, specifically EncryptPage():
> >
> > https://github.com/postgres/postgres/compare/bmomjian:cfe-11-gist..bmomjian:_cfe-12-rel.patch
> >
> > + if (!relation_is_permanent && !is_gist_page_or_similar)
> > + PageSetLSN(page, LSNForEncryption(relation_is_permanent));
> >
> >
> > It assigns an LSN to unlogged pages. As far as the buffer manager
> > seeing fake LSNs that already happens for GiST indexes, so I just built
> > on that --- seemed to work fine.
>
> I had not, but I don't see why this issue is specific to GiST rather
> than common to every kind of unlogged and temporary relation.
>
> > I have to ask why we should consider adding it to the special space,
> > since my current version seems fine, and has minimal code impact, and
> > has some advantages over using the special space. Is it because of the
> > WAL hint overhead, or for a cleaner API, or something else?
>
> My concern is about the overhead, and also the code complexity. I
> think that making sure that the LSN gets changed in all cases may be
> fairly tricky.

Please look over the patch to see if I missed anything --- for me, it
seemed quite clear, and I am not an expert in that area of the code.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

If only the physical world exists, free will is an illusion.