[19/22] crypto: inside-secure - add check for xts input length equal to zero

From: Andrei Botila <andrei.botila@nxp.com>

Standardize the way input lengths equal to 0 are handled in all skcipher
algorithms. All the algorithms return 0 for input lengths equal to zero.

Cc: Antoine Tenart <antoine.tenart@bootlin.com>
Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
---
 drivers/crypto/inside-secure/safexcel_cipher.c | 6 ++++++
 1 file changed, 6 insertions(+)

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of Andrei Botila
> Sent: Friday, August 7, 2020 6:20 PM
> To: Herbert Xu <herbert@gondor.apana.org.au>; David S. Miller <davem@davemloft.net>
> Cc: linux-crypto@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> linux-s390@vger.kernel.org; x86@kernel.org; linux-arm-kernel@axis.com; Andrei Botila <andrei.botila@nxp.com>; Antoine Tenart
> <antoine.tenart@bootlin.com>
> Subject: [PATCH 19/22] crypto: inside-secure - add check for xts input length equal to zero
>
> <<< External Email >>>
> From: Andrei Botila <andrei.botila@nxp.com>
>
> Standardize the way input lengths equal to 0 are handled in all skcipher
> algorithms. All the algorithms return 0 for input lengths equal to zero.
>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>
> Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
> ---
>  drivers/crypto/inside-secure/safexcel_cipher.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> index 1ac3253b7903..03d06556ea98 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -2533,6 +2533,9 @@ static int safexcel_skcipher_aes_xts_cra_init(struct crypto_tfm *tfm)
>
>  static int safexcel_encrypt_xts(struct skcipher_request *req)
>  {
> +if (!req->cryptlen)
> +return 0;
> +
>  if (req->cryptlen < XTS_BLOCK_SIZE)
>  return -EINVAL;
>  return safexcel_queue_req(&req->base, skcipher_request_ctx(req),
> @@ -2541,6 +2544,9 @@ static int safexcel_encrypt_xts(struct skcipher_request *req)
>
>  static int safexcel_decrypt_xts(struct skcipher_request *req)
>  {
> +if (!req->cryptlen)
> +return 0;
> +
>  if (req->cryptlen < XTS_BLOCK_SIZE)
>  return -EINVAL;
>  return safexcel_queue_req(&req->base, skcipher_request_ctx(req),
> --
> 2.17.1

With all due respect, but this makes no sense.

For XTS, any length below 16 is illegal, as applying CTS in order to handle non-cipher
block multiples (16 bytes in case of AES) requires _more_ data than 1 cipher block.

There is no benefit to explicitly check for zero length if there is already a check for
less-than-16. That's just wasting CPU cycles and  a branch predictor entry, for no
benefit whatsoever. (except for academic "alignment with other ciphers").

XTS has very specific use cases. No one in their right mind would call it for a
situation where it can't be applied in the first place, e.g. anything < 16 bytes.

Regards,
Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>

On Mon, Aug 10, 2020 at 10:20:20AM +0000, Van Leeuwen, Pascal wrote:
>
> With all due respect, but this makes no sense.

I agree.  This is a lot of churn for no gain.

Thanks,

On 8/10/2020 4:45 PM, Herbert Xu wrote:
> On Mon, Aug 10, 2020 at 10:20:20AM +0000, Van Leeuwen, Pascal wrote:
>>
>> With all due respect, but this makes no sense.
> 
> I agree.  This is a lot of churn for no gain.
> 
I would say the gain is that all skcipher algorithms would behave the same
when input length equals zero - i.e. treat the request as a no-op.

We can't say "no input" has any meaning to the other skcipher algorithms,
but the convention is to accept this case and just return 0.
I don't see why XTS has to be handled differently.

Thanks,
Horia

On Mon, Aug 10, 2020 at 05:33:39PM +0300, Horia Geantă wrote:
> On 8/10/2020 4:45 PM, Herbert Xu wrote:
> > On Mon, Aug 10, 2020 at 10:20:20AM +0000, Van Leeuwen, Pascal wrote:
> >>
> >> With all due respect, but this makes no sense.
> > 
> > I agree.  This is a lot of churn for no gain.
> > 
> I would say the gain is that all skcipher algorithms would behave the same
> when input length equals zero - i.e. treat the request as a no-op.
> 
> We can't say "no input" has any meaning to the other skcipher algorithms,
> but the convention is to accept this case and just return 0.
> I don't see why XTS has to be handled differently.
> 

CTS also rejects empty inputs.

The rule it follows is just that all input lengths >= blocksize are allowed.
Input lengths < blocksize aren't allowed.

- Eric

> -----Original Message-----
> From: Horia Geantă <horia.geanta@nxp.com>
> Sent: Monday, August 10, 2020 4:34 PM
> To: Herbert Xu <herbert@gondor.apana.org.au>; Van Leeuwen, Pascal <pvanleeuwen@rambus.com>
> Cc: Andrei Botila (OSS) <andrei.botila@oss.nxp.com>; David S. Miller <davem@davemloft.net>; linux-crypto@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-s390@vger.kernel.org;
> x86@kernel.org; linux-arm-kernel@axis.com; Andrei Botila <andrei.botila@nxp.com>; Antoine Tenart <antoine.tenart@bootlin.com>
> Subject: Re: [PATCH 19/22] crypto: inside-secure - add check for xts input length equal to zero
>
> <<< External Email >>>
> On 8/10/2020 4:45 PM, Herbert Xu wrote:
> > On Mon, Aug 10, 2020 at 10:20:20AM +0000, Van Leeuwen, Pascal wrote:
> >>
> >> With all due respect, but this makes no sense.
> >
> > I agree.  This is a lot of churn for no gain.
> >
> I would say the gain is that all skcipher algorithms would behave the same
> when input length equals zero - i.e. treat the request as a no-op.
>
XTS already behaves differently because it can accept any byte amount as long
as it is not in the range 0 -16. So far, you got an EINVAL error for lengths < 16.
The special exception on top of that for length 0 does not improve anything.

Treating a request of length 0 as a no-op is not a useful feature here, as there
is no use case where that would make sense. XTS encrypts blocks (usually disk
sectors), and cannot be chained. So an attempt to encrypt a zero length block
is most certainly some kind of error (e.g. trying to use XTS for something it
was not designed to do - big security mistake!).

> We can't say "no input" has any meaning to the other skcipher algorithms,
> but the convention is to accept this case and just return 0.
> I don't see why XTS has to be handled differently.
>
I don't see why you would blindly follow some historical convention ...
unless maybe there was some existing real use case that would benefit?

BTW: for generic ciphers I could think of some use cases where the zero
length request being a no-op makes sense if the application does not
bother to check how much data it has gathered to process (which may be
nothing), but I can't see how this could apply to XTS, being block-based.

> Thanks,
> Horia

Regards,
Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>

On 8/10/2020 8:03 PM, Eric Biggers wrote:
> On Mon, Aug 10, 2020 at 05:33:39PM +0300, Horia Geantă wrote:
>> On 8/10/2020 4:45 PM, Herbert Xu wrote:
>>> On Mon, Aug 10, 2020 at 10:20:20AM +0000, Van Leeuwen, Pascal wrote:
>>>>
>>>> With all due respect, but this makes no sense.
>>>
>>> I agree.  This is a lot of churn for no gain.
>>>
>> I would say the gain is that all skcipher algorithms would behave the same
>> when input length equals zero - i.e. treat the request as a no-op.
>>
>> We can't say "no input" has any meaning to the other skcipher algorithms,
>> but the convention is to accept this case and just return 0.
>> I don't see why XTS has to be handled differently.
>>
> 
> CTS also rejects empty inputs.
> 
> The rule it follows is just that all input lengths >= blocksize are allowed.
> Input lengths < blocksize aren't allowed.
> 
Indeed, thanks.

What about, for example, CBC?
AFAICT cbc(aes) with input length = 0 is valid.

Same for CTR (with the note that blocksize = 1) and several other algorithms
mentioned in the cover letter.

What's the rule in these cases?

Thanks,
Horia

On Tue, Aug 11, 2020 at 06:28:39PM +0300, Horia Geantă wrote:
>
> What about, for example, CBC?
> AFAICT cbc(aes) with input length = 0 is valid.

That's just because CBC accepts any input which is a multiple
of blocksize.

> Same for CTR (with the note that blocksize = 1) and several other algorithms
> mentioned in the cover letter.

CTR accepts any input size.

> What's the rule in these cases?

What input size is accepted depends on the algorithm.

Cheers,