RE: [RFC PATCH v2] md/dm-crypt - reuse eboiv skcipher for IV generation

[Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Wednesday, August 7, 2019 6:51 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; ebiggers@kernel.org;
> gmazyland@gmail.com
> Subject: Re: [RFC PATCH v2] md/dm-crypt - reuse eboiv skcipher for IV generation
> 
> (trim cc)
> 
> >
> > > > > In your case, we are not dealing with known plaintext attacks,
> > > > >
> > > > Since this is XTS, which is used for disk encryption, I would argue
> > > > we do! For the tweak encryption, the sector number is known plaintext,
> > > > same as for EBOIV. Also, you may be able to control data being written
> > > > to the disk encrypted, either directly or indirectly.
> > > > OK, part of the data into the CTS encryption will be previous ciphertext,
> > > > but that may be just 1 byte with the rest being the known plaintext.
> > > >
> > >
> > > The tweak encryption uses a dedicated key, so leaking it does not have
> > > the same impact as it does in the EBOIV case.
> > >
> > Well ... yes and no. The spec defines them as seperately controllable -
> > deviating from the original XEX definition - but in most practicle use cases
> > I've seen, the same key is used for both, as having 2 keys just increases
> > key  storage requirements and does not actually improve effective security
> > (of the algorithm itself, implementation peculiarities like this one aside
> > :-), as  XEX has been proven secure using a single key. And the security
> > proof for XTS actually builds on that while using 2 keys deviates from it.
> >
> 
> Regardless of all of that, the Linux implementation does in fact use
> separate keys for tweak and data. That is why the key sizes are double
> wrt ordinary AES, i.e., 32 bytes for XTS-AES-128 and 64 bytes for
> XTS-AES-256.
> 
Yes, and rightfully so as the specification *allows* for that, my point was 
merely that it may often be used with both keys being equal, in which case
your original assertion of the tweak key being dedicated doesn't hold.
But it doesn't really matter that much anyway, as I pointed out below.

> > > And a plaintext attack
> > > on the data encryption part of XTS involves knowing the value of the
> > > tweak as well, so you'd have to successfully attack the tweak before
> > > you can attack the data. So while your point is valid, it's definitely
> > > less broken than EBOIV.
> > >
> > For the data encryption, you have a very valid point (which I admit I
> > completely overlooked). For the tweak encryption itself, however ...
> >
> > But even if you would use 2 independent keys, if you first break the
> > tweak key, the tweak becomes known plaintext and you can then continue
> > breaking the data encryption key :-) It's a bit harder, but far from
> > impossible.
> >
> 
> Agreed.
> 
> > > But note that we are basically in agreement here: falling back to
> > > table based AES is undesirable, but for EBOIV it is just much worse
> > > than for other modes.
> > >
> > Much worse than *certain* other modes. It's definitely something that
> > always needs to be in the back of your mind as long as there is some
> > possibility you end up with a not-so-secure implementation.
> >
> 
> Again, agreed, which also happens to be the reason why I am arguing
> that the bare cipher API should not be exposed outside of the crypto
> subsystem at all.
> 
> > > > > > However, in many cases there would actually be a very good reason
> > > > > > In my humble opinion, such insecure table based implementations just
> > > > > > shouldn't exist at all - you can always do better, possibly at the
> > > > > > expense of some performance degradation. Or you should at least have
> > > > > > some flag  available to specify you have some security requirements
> > > > > > and such an implementation is not an acceptable response.
> > > > > >
> > > > >
> > > > > We did some work to reduce the time variance of AES: there is the
> > > > > aes-ti driver, and there is now also the AES library, which is known
> > > > > to be slower than aes-generic, but does include some mitigations for
> > > > > cache timing attacks.
> > > > >
> > > > > Other than that, I have little to offer, given that the performance vs
> > > > > security tradeoffs were decided long before security became a thing
> > > > > like it is today, and so removing aes-generic is not an option,
> > > > > especially since the scalar alternatives we have are not truly time
> > > > > invariant either.
> > > > >
> > > > Replacing aes-generic with a truly time-invariant implementation could
> > > > be an option.
> > >
> > > If you can find a truly time-invariant C implementation of AES that
> > > isn't orders of magnitude slower than aes-generic, I'm sure we can
> > > merge it.
> > >
> > I guess the "orders of a magnitude slower" thing is the catch here :-)
> >
> > But from my perspective, crypto performance is irrelevant if it is not
> > secure at all. (after all, it's crypto, so the *intent* is security)
> > Of course there's gradation in security levels, but timing-attack
> > resistance really is the lowest of the lowest IMHO.
> >
> 
> I am not disagreeing with that at all. This is actually one of the
> main reasons for my work on refactoring the way AES is being used in
> the kernel: for historical reasons, many drivers have a hard
> dependency on CONFIG_CRYPTO_AES, and pull in the aes-generic driver,
> and so at the moment, it is not possible to disable it. Going forward,
> I'd like to refine this further so that aes-generic can be replaced by
> aes-ti.
> 
> > > > Or selecting aes-generic only if some (new) "allow_insecure"
> > > > flag is set on the cipher request. (Obviously, you want to default to
> > > > secure, not insecure. Speaking as someone who earns his living doing
> > > > security :-)
> > > >
> > >
> > > We all do. But we all have different use cases to worry about, and
> > > different experiences and backgrounds :-)
> > >
> > > The main problem is that banning aes-generic is a bit too rigorous
> > > imo. It highly depends on whether there is known plaintext and whether
> > > there are observable latencies in the first place.
> > >
> > Agree on the banning part, but it would be good if you could be *certain*
> > somehow that you don't end up with it. For certain use cases, a (much)
> > slower, but more, secure implementation may be the better choice. As you
> > already discovered by yourself.
> >
> 
> See above. In the future, I'd like to ensure that aes-generic does not
> have to be built into the core kernel (which will be possible once
> existing core kernel users that require AES switch to the AES library
> or stop using AES altogether). That way, the module can be blacklisted
> or omitted from the kernel build altogether (as well as other modules
> that reimplement that same algorithm using arch specific assembly
> code)
> 
Sound like you are actually already busy trying to get rid of aes-generic
usage, so in that case, I should let you get back to work :-)


Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com