Date: Mon, 1 Nov 2021 14:24:36 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Bruce Momjian <bruce@momjian.us>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Yura Sokolov <y.sokolov@postgrespro.ru>, Sasasu <i@sasa.su>,
	Robert Haas <robertmhaas@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: XTS cipher mode for cluster file encryption
Message-ID: <20211101182436.GW20998@tamriel.snowman.net>
References: <46bc5203-0a3c-0426-e69f-5f2997648b35@sasa.su>
 <20211020122407.GW20998@tamriel.snowman.net>
 <d85f5865-1f5a-618e-7d92-92914a0f26b1@sasa.su>
 <20211021172812.GZ20998@tamriel.snowman.net>
 <48020c9811b72499aa5c4c4584b34ed33b75d1b0.camel@postgrespro.ru>
 <20211025161227.GE20998@tamriel.snowman.net>
 <741fa2d3752cc9ef958bb36c495fe952459eaff2.camel@postgrespro.ru>
 <20211026194330.GN20998@tamriel.snowman.net>
 <f51aad97-5f3d-0a61-69d9-2c297ea934e1@enterprisedb.com>
 <20211026213930.GA8607@momjian.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="/z4P6UMu7uOEsbeQ"
Content-Disposition: inline
In-Reply-To: <20211026213930.GA8607@momjian.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20211101182436.GW20998%40tamriel.snowman.net>
Precedence: bulk


--/z4P6UMu7uOEsbeQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Bruce Momjian (bruce@momjian.us) wrote:
> On Tue, Oct 26, 2021 at 11:11:39PM +0200, Tomas Vondra wrote:
> > BTW I'm not sure what the existing patches do, but I wonder if we should
> > calculate the checksum before or after encryption. I'd say it should be
> > after encryption, because checksums were meant as a protection against
> > issues at the storage level, so the checksum should be on what's writte=
n to
> > storage, and it'd also allow offline verification of checksums etc. (Of
> > course, that'd make the whole idea of relying on our checksums even more
> > futile.)
> >=20
> > Note: Maybe there are reasons why the checksum needs to be calculated b=
efore
> > encryption, not sure.
>=20
> Yes, these are the tradeoffs --- allowing offline checksum checking
> without requiring the key vs. giving _some_ integrity checking and
> requiring the key.

I'm in favor of calculating the checksum before encrypting as that will
still catch the storage level bit-flips that it was implemented to
address in the first place and will also make it so that we're very
likely to realize we have an incorrect key before trying to do anything
with the page.  That it might also serve as a deterrent against
attackers trying to randomly flip bits in a page has perhaps some value
but without a cryptographic-level hash it isn't really enough to prevent
against an attacker who has write access to a page.

Any tools which include checking the checksum on pages already have to
deal with clusters where checksums aren't enabled anyway and I wouldn't
expect it to generally be an issue for tools which want to validate
checksums on an encrypted cluster to be able to have the appropriate
key(s) necessary for doing so and to be able to perform the decryption
to do the check.  We can certainly make pg_checksums do this and I don't
see it as an issue for pgbackrest, as two examples that I've
specifically thought about.

> > > > > > - like XTS it doesn't need to change plain text format and does=
n't need in
> > > > > >    additional Nonce/Auth Code.
> > > > >=20
> > > > > Sure, in which case it's something that could potentially be adde=
d later
> > > > > as another option in the future.  I don't think we'll always have=
 just
> > > > > one encryption method and it's good to generally think about what=
 it
> > > > > might look like to have others but I don't think it makes sense t=
o try
> > > > > and get everything in all at once.
> > > >=20
> > > > And among others Adiantum looks best: it is fast even without hardw=
are
> > > > acceleration, it provides whole block encryption (ie every bit depe=
nds
> > > > on every bit) and it doesn't bound to plain-text format.
> > >=20
> > > And it could still be added later as another option if folks really w=
ant
> > > it to be.  I've outlined why it makes sense to go with XTS first but I
> > > don't mean that to imply that we'll only ever have that.  Indeed, once
> > > we've actually got something, adding other methods will almost certai=
nly
> > > be simpler.  Trying to do everything from the start will make this ve=
ry
> > > difficult to accomplish though.
> ...
> > So maybe the best thing is simply to roll with both - design the whole
> > feature in a way that allows selecting the encryption scheme, with two
> > options. That's generally a good engineering practice, as it ensures th=
ings
> > are not coupled too much. And it's not like the encryption methods are
> > expected to be super difficult.
>=20
> I am not in favor of adding additional options to this feature unless we
> can explain why users should choose one over the other.  There is also
> the problem of OpenSSL not supporting Adiantum.

I can understand the general idea that we should be sure to engineer
this in a way that multiple methods can be used, as surely one day folks
will say that AES128 isn't acceptable any more.  In terms of what we'll
do from the start, I would think providing the options of AES128 and
AES256 would be good to ensure that we have the bits covered to support
multiple methods and I don't think that would put us into a situation of
having to really explain which to use to users (we don't for pgcrypto
anyway, as an example).  I agree that we shouldn't be looking at adding
in a whole new crypto library for this though, that's a large and
independent effort (see the work on NSS happening nearby).

Thanks,

Stephen

--/z4P6UMu7uOEsbeQ
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJhgDDjAAoJEO1sijiDR2RVfIwP+gMuiue0BpUGec/R2Rat02Jo
wL3nl6YLul3cqdIbqiJlS3/Nob27BfljnReskuVnL/123Lh5wa/R4wmR0iTWfdxC
Li9HQy8uqdUjrq+fRRmVLpqWvnfHepu3kIPV8B+E3zHo2p365tU2KR8NJWKxuRhu
D5+TQyNiXNHSI7E3WhhrdfWvEhbIEaWJ8zgdntxvYXsPLlbEu5f8e+J5N2tGvySg
4D74HBs3ZJitglojkmYY+DPFzxX9CuaWs0koOqWtTNHUJctQrnGKgGGuCktuc1As
+aYZ8t3D21SDHIv5bUod2Rrfo7L/CrzCN5C25XtGp5lK2tnPvBB/wcWy0pCSRCMZ
kPZzqLEn0NLSe4zpEmLJ2a9VnDC93lPIFFJPcqYNb3h1mO7M+JMwgrVXS0UWY6Vp
rj/0zlXvicR56RiEPECzdJZUw0w7vauvTL9WeofdaLvJqO2BoICo10nJB0JVzasL
oUdzqY2mysDZpnYOSeWfZGxCFhHmucUtI5JD8/syxmWe4j65mWAwm54O+DjsrgQ6
ACCS/x4FRO/iDrU5Brv8fccJkSlEUAMzxEU/VPTrgnyaQ4kHAF6J+k5IBdruEgWw
W8y/NLjqBjLOnk3GEZGXc+kEyXOFPTMBXgrSyF1oCVba/KhPrHDEbaEh8wHC3OyT
IO6yILefVmyrbULXUrIg
=u8sl
-----END PGP SIGNATURE-----

--/z4P6UMu7uOEsbeQ--