RE: XTS template wrapping question

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

> -----Original Message-----
> From: Eric Biggers <ebiggers@kernel.org>
> Sent: Friday, August 9, 2019 7:07 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; davem@davemloft.net
> Subject: Re: XTS template wrapping question
> 
> On Fri, Aug 09, 2019 at 03:06:23PM +0000, Pascal Van Leeuwen wrote:
> > > -----Original Message-----
> > > From: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > Sent: Friday, August 9, 2019 4:18 PM
> > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>; linux-crypto@vger.kernel.org;
> > > herbert@gondor.apana.org.au; davem@davemloft.net; Eric Biggers <ebiggers@kernel.org>
> > > Subject: RE: XTS template wrapping question
> > >
> > > > -----Original Message-----
> > > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf
> > > Of
> > > > Pascal Van Leeuwen
> > > > Sent: Friday, August 9, 2019 1:39 PM
> > > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; davem@davemloft.net;
> Eric
> > > > Biggers <ebiggers@kernel.org>
> > > > Subject: XTS template wrapping question
> > > >
> > > > Herbert, Eric,
> > > >
> > > > While working on the XTS template, I noticed that it is being used
> > > > (e.g. from testmgr, but also when explictly exported from other drivers)
> > > > as e.g. "xts(aes)", with the generic driver actually being
> > > > "xts(ecb(aes-generic))".
> > > >
> > > > While what I would expect would be "xts(ecb(aes))", the reason being
> > > > that plain "aes" is defined as a single block cipher while the XTS
> > > > template actually efficiently wraps an skcipher (like ecb(aes)).
> > > > The generic driver reference actually proves this point.
> > > >
> > > > The problem with XTS being used without the ecb template in between,
> > > > is that hardware accelerators will typically advertise an ecb(aes)
> > > > skcipher and the current approach makes it impossible to leverage
> > > > that for XTS (while the XTS template *could* actually do that
> > > > efficiently, from what I understand from the code ...).
> > > > Advertising a single block "aes" cipher from a hardware accelerator
> > > > unfortunately defeats the purpose of acceleration.
> > > >
> > > > I also wonder what happens if aes-generic is the only AES
> > > > implementation available? How would the crypto API know it needs to
> > > > do "xts(aes)" as "xts(ecb(aes))" without some explicit export?
> > > > (And I don't see how xts(aes) would work directly, considering
> > > > that only seems to handle single cipher blocks? Or ... will
> > > > the crypto API actually wrap some multi-block skcipher thing
> > > > around the single block cipher instance automatically??)
> > > >
> > > Actually, the above was based on observations from testmgr, which
> > > doesn't seem to test xts(safexcel-ecb-aes) even though I gave that
> > > a very high .cra_priority as well as that what is advertised under
> > > /proc/crypto, which does not include such a thing either.
> > >
> > > However, playing with tcrypt mode=600 shows some interesting
> > > results:
> > >
> > > WITHOUT the inside-secure driver loaded, both LRW encrypt and
> > > decrypt run on top of ecb-aes-aesni as you would expect.
> > > Both xts encrypt and decrypt give a "failed to load transform"
> > > with an error code of -80. Strange ... -80 = ELIBBAD??
> > > (Do note that the selftest of xts(aes) using xts-aesni worked
> > > just fine according to /proc/crypto!)
> > >
> > > WITH the inside-secure driver loaded, NOT advertising xts(aes)
> > > itself and everything at cra_priority of 300: same (expected).
> > >
> > > WITH the inside-secure driver loaded, NOT advertising xts(aes)
> > > itself and everything safexcel at cra_priority of 2000:
> > > LRW decrypt now runs on top of safexcel-ecb-aes, but LRW
> > > encrypt now runs on top of aes-generic? This makes no sense as
> > > the encrypt datapath structure is the same as for decrypt so
> > > it should run just fine on top of safexcel-ecb-aes. And besides
> > > that, why drop from aesni all the way down to aes-generic??
> > > xts encrypt and decrypt still give the -80 error, while you
> > > would expect that to now run using the xts wrapper around
> > > safexcel-ecb-aes (but no way to tell if that's happening).
> > >
> > > WITH the inside-secure driver loaded, advertising xts(aes)
> > > itself and everything at cra_priority of 2000:
> > > still the same LRW assymmetry as mentioned above, but
> > > xts encrypt and decrypt now work fine using safexcel-aes-xts
> > >
> > > Conclusions from the above:
> > >
> > > - There's something fishy with the selection of the underlying
> > >   AES cipher for LRW encrypt (but not for LRW decrypt).
> > >
> > Actually, this makes no sense at all as crypto_skcipher_alloc
> > does not even see the direction you're going to use in your
> > requests. Still, it is what I consistently see happening in
> > the tcrypt logging. Weird!
> 
> There's a known bug when the extra self-tests are enabled, where the first
> allocation of an algorithm actually returns the generic implementation, not the
> highest priority implementation.  See:
> https://lkml.kernel.org/linux-crypto/20190409181608.GA122471@gmail.com/
> Does that explain what you saw?
> 
Ah! That must indeed be the same problem. Encrypt is first here, so
that apparently gets generic and then decrypt gets the hw version.
So I guess that bug does not just apply to the self tests then ...(!)

> >
> > > - xts-aes-aesni (and the xts.c wrapper?) appear(s) broken in
> > >   some way not detected by testmgr but affecting tcrypt use,
> > >   while the inside-secure driver's local xts works just fine
> > >
> 
> Is this reproducible without any local patches?  If so, can you provide clear
> reproduction steps?
> 
I'm not aware of any local patches. I tried it after backing out the 
xts.c stuff Ard and I have been working on regarding CTS and that still 
failed.

Just try:
modprobe tcrypt mode=600 sec=1 num_mb=100

On a system that has aesni has the highest priority implementation.

> - Eric



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