Re: XTS cipher mode for cluster file encryption

Greetings,

* Bruce Momjian (bruce(at)momjian(dot)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 written 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.)
> >
> > Note: Maybe there are reasons why the checksum needs to be calculated before
> > encryption, not sure.
>
> 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 doesn't need in
> > > > > > additional Nonce/Auth Code.
> > > > >
> > > > > Sure, in which case it's something that could potentially be added 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 to try
> > > > > and get everything in all at once.
> > > >
> > > > And among others Adiantum looks best: it is fast even without hardware
> > > > acceleration, it provides whole block encryption (ie every bit depends
> > > > on every bit) and it doesn't bound to plain-text format.
> > >
> > > And it could still be added later as another option if folks really want
> > > 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 certainly
> > > be simpler. Trying to do everything from the start will make this very
> > > 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 things
> > are not coupled too much. And it's not like the encryption methods are
> > expected to be super difficult.
>
> 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