Re: [PATCH] crypto: ccree - enable CTS support in AES-XTS

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Mon, 9 Sep 2019 at 13:34, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>
> On Mon, Sep 9, 2019 at 3:20 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Sun, 8 Sep 2019 at 09:04, Uri Shir <uri.shir@arm.com> wrote:
> > >
> > > In XTS encryption/decryption the plaintext byte size
> > > can be >= AES_BLOCK_SIZE. This patch enable the AES-XTS ciphertext
> > > stealing implementation in ccree driver.
> > >
> > > Signed-off-by: Uri Shir <uri.shir@arm.com>
> > > ---
> > >  drivers/crypto/ccree/cc_cipher.c | 16 ++++++----------
> > >  1 file changed, 6 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
> > > index 5b58226..a95d3bd 100644
> > > --- a/drivers/crypto/ccree/cc_cipher.c
> > > +++ b/drivers/crypto/ccree/cc_cipher.c
> > > @@ -116,10 +116,6 @@ static int validate_data_size(struct cc_cipher_ctx *ctx_p,
> > >         case S_DIN_to_AES:
> > >                 switch (ctx_p->cipher_mode) {
> > >                 case DRV_CIPHER_XTS:
> > > -                       if (size >= AES_BLOCK_SIZE &&
> > > -                           IS_ALIGNED(size, AES_BLOCK_SIZE))
> > > -                               return 0;
> > > -                       break;
> >
> > You should still check for size < block size.
> Look again - he does via the fall through aspect of the case.
>

Ah right - I missed that.

> >
> > >                 case DRV_CIPHER_CBC_CTS:
> > >                         if (size >= AES_BLOCK_SIZE)
> > >                                 return 0;
> > > @@ -945,7 +941,7 @@ static const struct cc_alg_template skcipher_algs[] = {
> > >         {
> > >                 .name = "xts(paes)",
> > >                 .driver_name = "xts-paes-ccree",
> > > -               .blocksize = AES_BLOCK_SIZE,
> > > +               .blocksize = 1,
> >
> > No need for these blocksize changes - just keep them as they are.
>
> hm... I'm a little confused about this.
> Why do we have, say CTR template, announce a block size of 1 (which
> makes sense since it effectively turns a block cipher to a stream
> cipher) but here stick to the underlying block size?
> I mean, you can request processing for any granularity (subject to the
> bigger than 1 block), just like CTR so I'm not sure what information
> is supposed to be conveyed here.
>

The blocksize is primarily used by the walking code to ensure that the
input is a round multiple. In the XTS case, we can't blindly use the
skcipher walk interface to go over the data anyway, since the last
full block needs special handling as well.

So the answer is really that we had no reason to change it for the
other drivers, and changing it here will trigger a failure in the
testing code that compares against the generic implementations.