Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1r0rYc-008DUP-OF for pgsql-hackers@arkaria.postgresql.org; Wed, 08 Nov 2023 23:01:58 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1r0rYb-008TGy-1n for pgsql-hackers@arkaria.postgresql.org; Wed, 08 Nov 2023 23:01:57 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1r0rYa-008TGq-Lm for pgsql-hackers@lists.postgresql.org; Wed, 08 Nov 2023 23:01:56 +0000 Received: from mail-lf1-x133.google.com ([2a00:1450:4864:20::133]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1r0rYV-005R5w-Rn for pgsql-hackers@postgresql.org; Wed, 08 Nov 2023 23:01:55 +0000 Received: by mail-lf1-x133.google.com with SMTP id 2adb3069b0e04-507bd644a96so305185e87.3 for ; Wed, 08 Nov 2023 15:01:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=crunchydata.com; s=google; t=1699484510; x=1700089310; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=+lKHbPjy1AuQP+dya6bv9AgX2Yag53CgWNIRbo7tL1E=; b=cOfQYXyBW+Lqnshif3T65D7bHmH36RqwGo8khv4GbJVqqXnGac0+r+l2U/x6+/xuVO A9Mr47YYdKj2Gw6p9z5rb3Ji+6sij3Arh3nD0ZC0EGdCoCWBCgyM12L4z3V0u5VvMna/ wiK9SzdSpwY30ujXJyyoVW8jQn1ywi4phAcZqFtF6fvxVvmQyMkADyy4q35tUOLn5eHu a7vLp3MI35LqyOj4l8NCkkBUdo+xMo4uMpEUWmYMV6jEmyfwFSErTJK9PdUGZmjftVHo MJ0KUeP+5hFXKzvW3UtzDYk/1Ik1wSarQTYoQhizpdEa+ja7upRhEscYtjgazyzXUWy3 RTZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699484510; x=1700089310; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+lKHbPjy1AuQP+dya6bv9AgX2Yag53CgWNIRbo7tL1E=; b=OgDmrpt0MyFvpILtKkb+ADClmu+Mvss93/3xNXTnXlOM6BjEYkcn3sj3jZSiGESAvR 4njhduod/+EEvTUHKAQNyonzXoTYyV4W7c2cmUCAa5cUlKzDaQLFjdBjgRDOuO9TJQpF foARvUIj97+OHktTJSsoEbaABzVH2cyhYLP0GMaKE4sO9qULGKzFkrVXsCFWNIDh+Xdw Jqq4uspqz59/nGRTHJdCHWotiq/8h+LNttaufwia7yL7j1hT9btExEhPtV1+5RBeivCD QlYydoR9tHE/31x81KyU969ZXIpuE+JszSmOpm4Yp0ZIYBGfq+566NXDUfoGYJEFcbJI yhxQ== X-Gm-Message-State: AOJu0YwYatibslIiALUgTyOFrgy2LN2n9MoVgVTuMi3NonwNERZ0nQFP MEwHJG3I/HXk/RGPfTmyiCMTTAdk172ZvCDquaeHm43c+sgXMkkSD8M= X-Google-Smtp-Source: AGHT+IEiD1AMg5Exi2/BUEk8x+nh8A3a5fWv4B1yjjLtvkklRRVJ4o1ItFG3YTQFB+gAQC+WHD1ndv8OdMds/JhWmoQ= X-Received: by 2002:a05:6512:220d:b0:507:a089:caf4 with SMTP id h13-20020a056512220d00b00507a089caf4mr3075263lfu.60.1699484510238; Wed, 08 Nov 2023 15:01:50 -0800 (PST) MIME-Version: 1.0 References: <20231103023228.gz32vjx67jwzfh6h@alap3.anarazel.de> <20231107234932.ibhrwnm2i7255ept@awork3.anarazel.de> In-Reply-To: <20231107234932.ibhrwnm2i7255ept@awork3.anarazel.de> From: David Christensen Date: Wed, 8 Nov 2023 17:01:38 -0600 Message-ID: Subject: Re: Moving forward with TDE [PATCH v3] To: Andres Freund Cc: Stephen Frost , vignesh C , Aleksander Alekseev , PostgreSQL-development Content-Type: multipart/alternative; boundary="00000000000088cee40609ac1260" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000088cee40609ac1260 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Nov 7, 2023 at 5:49=E2=80=AFPM Andres Freund w= rote: > Hi, > > On 2023-11-06 09:56:37 -0500, Stephen Frost wrote: > > * Andres Freund (andres@anarazel.de) wrote: > > > I still am quite quite unconvinced that using the LSN as a nonce is a > good > > > design decision. > > > > This is a really important part of the overall path to moving this > > forward, so I wanted to jump to it and have a specific discussion > > around this. I agree that there are downsides to using the LSN, some o= f > > which we could possibly address (eg: include the timeline ID in the IV)= , > > but others that would be harder to deal with. > > > The question then is- what's the alternative? > > > > One approach would be to change the page format to include space for an > > explicit nonce. I don't see the community accepting such a new page > > format as the only format we support though as that would mean no > > pg_upgrade support along with wasted space if TDE isn't being used. > > Right. > Hmm, if we /were/ to introduce some sort of page format change, Couldn't that be a use case for modifying the pd_version field? Could v4 pages be read in and written out as v5 pages with different interpretations? > > Ideally, we'd instead be able to support multiple page formats where > > users could decide when they create their cluster what features they > > want- and luckily we even have such an effort underway with patches > > posted for review [1]. > > I think there are some details wrong with that patch - IMO the existing > macros > should just continue to work as-is and instead the places that want the > more > narrow definition should be moved to the new macros and it changes places > that > should continue to use compile time constants - but it doesn't seem like = a > fundamentally bad idea to me. I certainly like it much better than makin= g > the > page size runtime configurable. > There had been some discussion about this WRT renaming macros and the like (constants, etc)=E2=80=94I think a new pass eliminating the variable blocks= ize pieces and seeing if we can minimize churn here is worthwhile, will take a look and see what the minimally-viable set of changes is here. > (I'll try to reply with the above points to [1]) > > > > Certainly, with the base page-special-feature patch, we could have an > option > > for users to choose that they want a better nonce than the LSN, or we > could > > bundle that assumption in with, say, the authenticated-encryption featu= re > > (if you want authenticated encryption, then you want more from the > > encryption system than the basics, and therefore we presume you also > want a > > better nonce than the LSN). > > I don't think we should support using the LSN as a nonce if we have an > alternative. The cost and complexity overhead is just not worth it. Yes, > it'll be harder for users to migrate to encryption, but adding complexity > elsewhere in the system to get an inferior result isn't worth it. > From my read, XTS (which I'd see as inferior to authenticated encryption, but better than some other options) could use LSN as an IV without leakage concerns, perhaps mixing in the BlockNumber as well. If we are going to allow multiple encryption types, I think we may need to consider that needs for IVs may be different, so this may need to be something that is selectable per encryption type. I am unclear how much of a requirement this is, but seems like having a design supporting this to be pluggable=E2=80=94even if a static lookup tabl= e internally for encryption type, block length, IV source, etc=E2=80=94seems = the most future proof if we had to retire an encryption method or prevent creation of specific methods, say. > > Another approach would be a separate fork, but that then has a number o= f > > downsides too- every write has to touch that too, and a single page of > > nonces would cover a pretty large number of pages also. > > Yea, the costs of doing so is nontrivial. If you were trying to implement > encryption on the smgr level - which I doubt you should but am not certai= n > about - my suggestion would be to interleave pages with metadata like > nonces > and AEAD with the data pages. I.e. one metadata page would be followed by > (BLCKSZ - SizeOfPageHeaderData) / (sizeof(nonce) + sizeof(AEAD)) > pages containing actual relation data. That way you still get decent > locality > during scans and writes. > Hmm, this is actually an interesting idea, I will think about this a bit. > Relation forks were a mistake, we shouldn't use them in more places. > > > I think it'd be much better if we also encrypted forks, rather than just > the > main fork... > I believe the existing code should just work by modifying the PageNeedsToBeEncrypted macro; I will test that and see if anything blows up. David --00000000000088cee40609ac1260 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue, Nov 7, 2023 at 5:49=E2=80=AFPM An= dres Freund <andres@anarazel.de> wrote:
Hi,

On 2023-11-06 09:56:37 -0500, Stephen Frost wrote:
> * Andres Freund (
andres@anarazel.de) wrote:
> > I still am quite quite unconvinced that using the LSN as a nonce = is a good
> > design decision.
>
> This is a really important part of the overall path to moving this
> forward, so I wanted to jump to it and have a specific discussion
> around this.=C2=A0 I agree that there are downsides to using the LSN, = some of
> which we could possibly address (eg: include the timeline ID in the IV= ),
> but others that would be harder to deal with.

> The question then is- what's the alternative?
>
> One approach would be to change the page format to include space for a= n
> explicit nonce.=C2=A0 I don't see the community accepting such a n= ew page
> format as the only format we support though as that would mean no
> pg_upgrade support along with wasted space if TDE isn't being used= .

Right.

Hmm, if we /were/ to introduce s= ome sort of page format change, Couldn't that be a use case for modifyi= ng the pd_version field?=C2=A0 Could v4 pages be read in and written out as= v5 pages with different interpretations?
=C2=A0
> Ideally, we'd instead be able to support multiple page formats whe= re
> users could decide when they create their cluster what features they > want- and luckily we even have such an effort underway with patches > posted for review [1].

I think there are some details wrong with that patch - IMO the existing mac= ros
should just continue to work as-is and instead the places that want the mor= e
narrow definition should be moved to the new macros and it changes places t= hat
should continue to use compile time constants - but it doesn't seem lik= e a
fundamentally bad idea to me.=C2=A0 I certainly like it much better than ma= king the
page size runtime configurable.

There h= ad been some discussion about this WRT renaming macros and the like (consta= nts, etc)=E2=80=94I think a new pass eliminating the variable blocksize pie= ces and seeing if we can minimize churn here is worthwhile, will take a loo= k and see what the minimally-viable set of changes is here.
=C2= =A0
(I'll try to reply with the above points to [1])


> Certainly, with the base page-special-feature patch, we could have an = option
> for users to choose that they want a better nonce than the LSN, or we = could
> bundle that assumption in with, say, the authenticated-encryption feat= ure
> (if you want authenticated encryption, then you want more from the
> encryption system than the basics, and therefore we presume you also w= ant a
> better nonce than the LSN).

I don't think we should support using the LSN as a nonce if we have an<= br> alternative. The cost and complexity overhead is just not worth it.=C2=A0 Y= es,
it'll be harder for users to migrate to encryption, but adding complexi= ty
elsewhere in the system to get an inferior result isn't worth it.

From my read, XTS (which I'd see as infe= rior to authenticated encryption, but better than some other options) could= use LSN as an IV without leakage concerns, perhaps mixing in the BlockNumb= er as well.=C2=A0 If we are going to allow multiple encryption types, I thi= nk we may need to consider that needs for IVs may be different, so this may= need to be something that is selectable per encryption type.=C2=A0=C2=A0

I am unclear how much of a requirement this is, but= seems like having a design supporting this to be pluggable=E2=80=94even if= a static lookup table internally for encryption type, block length, IV sou= rce, etc=E2=80=94seems the most future proof if we had to retire an encrypt= ion method or prevent creation of specific methods, say.
=C2=A0
> Another approach= would be a separate fork, but that then has a number of
> downsides too- every write has to touch that too, and a single page of=
> nonces would cover a pretty large number of pages also.

Yea, the costs of doing so is nontrivial. If you were trying to implement encryption on the smgr level - which I doubt you should but am not certain<= br> about - my suggestion would be to interleave pages with metadata like nonce= s
and AEAD with the data pages. I.e. one metadata page would be followed by =C2=A0 (BLCKSZ - SizeOfPageHeaderData) / (sizeof(nonce) + sizeof(AEAD))
pages containing actual relation data.=C2=A0 That way you still get decent = locality
during scans and writes.

Hmm, this is a= ctually an interesting idea, I will think about this a bit.
=C2= =A0
Relation forks were a mistake, we shouldn't use them in more places.

I think it'd be much better if we also encrypted forks, rather than jus= t the
main fork...

I believe the existing cod= e should just work by modifying the=C2=A0PageNeedsToBeEncrypted macro; I wi= ll test that and see if anything blows up.

David

--00000000000088cee40609ac1260--