Re: [PATCH 0/2] crypto: talitos: Add AES-XTS mode

["Horia Geantă" <horia.geanta@freescale.com>]

On 3/13/2015 4:08 PM, Martin Hicks wrote:
> Hi Horia,
> 
> On Wed, Mar 11, 2015 at 11:48 AM, Horia Geantă
> <horia.geanta@freescale.com> wrote:
>>
>> While here: note that xts-talitos supports only two key lengths - 256
>> and 512 bits. There are tcrypt speed tests that check also for 384-bit
>> keys (which is out-of-spec, but still...), leading to a "Key Size Error"
>> - see below (KSE bit in AESU Interrupt Status Register is set)
> 
> Ok.  I've limited the keysize to 32 or 64 bytes for AES-XTS in the
> talitos driver.
> 
> This was my first experiments with the tcrypt module.  It also brought
> up another issue related to the IV limitations of this hardware.  The
> latest patch that I have returns an error when there is a non-zero
> value in the second 8 bytes of the IV:
> 
> +       /*
> +        * AES-XTS uses the first two AES Context registers for:
> +        *
> +        *     Register 1:   Sector Number (Little Endian)
> +        *     Register 2:   Sector Size   (Big Endian)
> +        *
> +        * Whereas AES-CBC uses registers 1/2 as a 16-byte IV.
> +        */
> +       if ((ctx->desc_hdr_template &
> +            (DESC_HDR_SEL0_MASK | DESC_HDR_MODE0_MASK)) ==
> +            (DESC_HDR_SEL0_AESU | DESC_HDR_MODE0_AESU_XTS)) {
> +               u64 *aesctx2 = (u64 *)areq->info + 1;
> +
> +               if (*aesctx2 != 0) {
> +                       dev_err(ctx->dev,
> +                               "IV length limited to the first 8 bytes.");
> +                       return ERR_PTR(-EINVAL);
> +               }
> +
> +               /* Fixed sized sector */
> +               *aesctx2 = cpu_to_be64(1 << SECTOR_SHIFT);
> +       }
> 
> 
> This approach causes the tcrypt tests to fail because tcrypt sets all
> 16 bytes of the IV to 0xff.  I think returning an error is the right
> approach for the talitos module, but it would be nice if tcrypt still
> worked.  Should tcrypt just set the IV bytes to 0 instead of 0xff?
> Isn't one IV just as good as another?  I think adding exceptions to
> the tcrypt code would be ugly, but maybe one should be made for XTS
> since the standard dictates that the IV should be plain or plain64?

AFAICT xts-aes standard does not mandate for plain or plain64.
The requirements are the following (below IV = tweak value, sector =
data unit):
-IV size: 16 bytes
-IV format: little endian byte array
-IV values: non-negative; consecutive IV values for consecutive sectors

In practice, an 8-byte IV should be enough to represent the sector index
even for large capacity storage devices.
However, dm-crypt has support for a user-provided iv_offset that is
added to the sector index: IV = sector_index + iv_offset.
While in most of the cases user would choose iv_offset = 0, in theory
anything is possible.

IMHO the correct approach would be to use a fallback tfm that would
handle all the requests with IVs > 8 bytes.
We can take this off-list if you prefer.

Horia