[dm-devel] xts fuzz testing and lack of ciphertext stealing support

[Ard Biesheuvel]

On Wed, 17 Jul 2019 at 19:28, Eric Biggers <ebiggers at kernel.org> wrote:
>
> On Wed, Jul 17, 2019 at 05:09:31PM +0000, Horia Geanta wrote:
> > On 7/17/2019 1:16 AM, Eric Biggers wrote:
> > > Hi Horia,
> > >
> > > On Tue, Jul 16, 2019 at 05:46:29PM +0000, Horia Geanta wrote:
> > >> Hi,
> > >>
> > >> With fuzz testing enabled, I am seeing xts(aes) failures on caam drivers.
> > >>
> > >> Below are several failures, extracted from different runs:
> > >>
> > >> [    3.921654] alg: skcipher: xts-aes-caam encryption unexpectedly succeeded on test vector "random: len=40 klen=64"; expected_error=-22, cfg="random: inplace use_finup nosimd src_divs=[57.93%@+11, 37.18%@+164, <reimport>0.68%@+4, 0.50%@+305, 3.71%@alignmask+3975]"
> > >>
> > >> [    3.726698] alg: skcipher: xts-aes-caam encryption unexpectedly succeeded on test vector "random: len=369 klen=64"; expected_error=-22, cfg="random: inplace may_sleep use_digest src_divs=[100.0%@alignmask+584]"
> > >>
> > >> [    3.741082] alg: skcipher: xts-aes-caam encryption unexpectedly succeeded on test vector "random: len=2801 klen=64"; expected_error=-22, cfg="random: inplace may_sleep use_digest src_divs=[100.0%@+6] iv_offset=18"
> > >>
> > >> It looks like the problem is not in CAAM driver.
> > >> More exactly, fuzz testing is generating random test vectors and running
> > >> them through both SW generic (crypto/xts.c) and CAAM implementation:
> > >> -SW generic implementation of xts(aes) does not support ciphertext stealing
> > >> and throws -EINVAL when input is not a multiple of AES block size (16B)
> > >> -caam has support for ciphertext stealing, and allows for any input size
> > >> which results in "unexpectedly succeeded" error messages.
> > >>
> > >> Any suggestion how this should be fixed?
> > >>
> > >> Thanks,
> > >> Horia
> > >
> > > I don't think drivers should allow inputs the generic implementation doesn't,
> > > since those inputs aren't tested by the crypto self-tests (so how do you know
> > How about implementation adding static test vectors and using them to validate
> > the standard feature set that's not supported by the generic implementation?
> >
> > > it's even correct?), and people could accidentally rely on the driver-specific
> > > behavior and then be unable to migrate to another platform or implementation.
> > >
> > People could also *intentionally* rely on a driver offering an implementation
> > that is closer to the spec / standard.
> >
> > > So for now I recommend just updating the caam driver to return -EINVAL on XTS
> > > inputs not evenly divisible by the block size.
> > >
> > I was hoping for something more constructive...
> >
> > > Of course, if there are actual use cases for XTS with ciphertext stealing in the
> > > kernel, we could add it to all the other implementations too.  But I'm not aware
> > > of any currently.  Don't all XTS users in the kernel pass whole blocks?
> > >
> > That's my guess too.
> >
> > What about user space relying on offloading and xts working
> > according to the spec?
> >
>
> Sure, AF_ALG users could expect ciphertext stealing to work.  I don't know of
> any actual examples of people saying they want it, but it's possible.
>
> My point is simply that we add this, we need to find a way to support it in all
> implementations.  It's not helpful to add it to only one specific driver, as
> then it's inconsistent and is untestable with the common tests.
>

IIRC there are different ways to implement CTS, and the cts template
we have in the kernel (which is used for CBC in some cases) implements
the flavor where the last two blocks are reordered if the input size
is an exact multiple of the block size. If your CTS implementation
does not implement this reordering (which seems to be the case since
it is compatible with plain XTS), it implements CTS in an incompatible
way.

Since the kernel does not support CTS for XTS any way, and since no
AF_ALG users can portably rely on this, I agree with Eric that the
only sensible way to address this is to disable this functionality in
the driver.