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

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

[Uri Shir]

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;
 		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,
 		.template_skcipher = {
 			.setkey = cc_cipher_sethkey,
 			.encrypt = cc_cipher_encrypt,
@@ -963,7 +959,7 @@ static const struct cc_alg_template skcipher_algs[] = {
 	{
 		.name = "xts512(paes)",
 		.driver_name = "xts-paes-du512-ccree",
-		.blocksize = AES_BLOCK_SIZE,
+		.blocksize = 1,
 		.template_skcipher = {
 			.setkey = cc_cipher_sethkey,
 			.encrypt = cc_cipher_encrypt,
@@ -982,7 +978,7 @@ static const struct cc_alg_template skcipher_algs[] = {
 	{
 		.name = "xts4096(paes)",
 		.driver_name = "xts-paes-du4096-ccree",
-		.blocksize = AES_BLOCK_SIZE,
+		.blocksize = 1,
 		.template_skcipher = {
 			.setkey = cc_cipher_sethkey,
 			.encrypt = cc_cipher_encrypt,
@@ -1203,7 +1199,7 @@ static const struct cc_alg_template skcipher_algs[] = {
 	{
 		.name = "xts(aes)",
 		.driver_name = "xts-aes-ccree",
-		.blocksize = AES_BLOCK_SIZE,
+		.blocksize = 1,
 		.template_skcipher = {
 			.setkey = cc_cipher_setkey,
 			.encrypt = cc_cipher_encrypt,
@@ -1220,7 +1216,7 @@ static const struct cc_alg_template skcipher_algs[] = {
 	{
 		.name = "xts512(aes)",
 		.driver_name = "xts-aes-du512-ccree",
-		.blocksize = AES_BLOCK_SIZE,
+		.blocksize = 1,
 		.template_skcipher = {
 			.setkey = cc_cipher_setkey,
 			.encrypt = cc_cipher_encrypt,
@@ -1238,7 +1234,7 @@ static const struct cc_alg_template skcipher_algs[] = {
 	{
 		.name = "xts4096(aes)",
 		.driver_name = "xts-aes-du4096-ccree",
-		.blocksize = AES_BLOCK_SIZE,
+		.blocksize = 1,
 		.template_skcipher = {
 			.setkey = cc_cipher_setkey,
 			.encrypt = cc_cipher_encrypt,
-- 
2.7.4

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

[Gilad Ben-Yossef]

On Sun, Sep 8, 2019 at 11:04 AM 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>


Acked-by: Gilad Ben-Yossef <gilad@benyossef.com>

Gilad

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

[Ard Biesheuvel]

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.

>                 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.

>                 .template_skcipher = {
>                         .setkey = cc_cipher_sethkey,
>                         .encrypt = cc_cipher_encrypt,
> @@ -963,7 +959,7 @@ static const struct cc_alg_template skcipher_algs[] = {
>         {
>                 .name = "xts512(paes)",
>                 .driver_name = "xts-paes-du512-ccree",
> -               .blocksize = AES_BLOCK_SIZE,
> +               .blocksize = 1,
>                 .template_skcipher = {
>                         .setkey = cc_cipher_sethkey,
>                         .encrypt = cc_cipher_encrypt,
> @@ -982,7 +978,7 @@ static const struct cc_alg_template skcipher_algs[] = {
>         {
>                 .name = "xts4096(paes)",
>                 .driver_name = "xts-paes-du4096-ccree",
> -               .blocksize = AES_BLOCK_SIZE,
> +               .blocksize = 1,
>                 .template_skcipher = {
>                         .setkey = cc_cipher_sethkey,
>                         .encrypt = cc_cipher_encrypt,
> @@ -1203,7 +1199,7 @@ static const struct cc_alg_template skcipher_algs[] = {
>         {
>                 .name = "xts(aes)",
>                 .driver_name = "xts-aes-ccree",
> -               .blocksize = AES_BLOCK_SIZE,
> +               .blocksize = 1,
>                 .template_skcipher = {
>                         .setkey = cc_cipher_setkey,
>                         .encrypt = cc_cipher_encrypt,
> @@ -1220,7 +1216,7 @@ static const struct cc_alg_template skcipher_algs[] = {
>         {
>                 .name = "xts512(aes)",
>                 .driver_name = "xts-aes-du512-ccree",
> -               .blocksize = AES_BLOCK_SIZE,
> +               .blocksize = 1,
>                 .template_skcipher = {
>                         .setkey = cc_cipher_setkey,
>                         .encrypt = cc_cipher_encrypt,
> @@ -1238,7 +1234,7 @@ static const struct cc_alg_template skcipher_algs[] = {
>         {
>                 .name = "xts4096(aes)",
>                 .driver_name = "xts-aes-du4096-ccree",
> -               .blocksize = AES_BLOCK_SIZE,
> +               .blocksize = 1,
>                 .template_skcipher = {
>                         .setkey = cc_cipher_setkey,
>                         .encrypt = cc_cipher_encrypt,
> --
> 2.7.4
>

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

[Gilad Ben-Yossef]

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.

>
> >                 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.

Thanks,
Gilad

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

[Ard Biesheuvel]

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.

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

[Gilad Ben-Yossef]

On Mon, Sep 9, 2019 at 5:38 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> 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.

I see. That makes sense. Thanks for the explanation.

Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

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

[Herbert Xu]

On Mon, Sep 09, 2019 at 03:38:02PM +0100, Ard Biesheuvel wrote:
>
> 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.

I think it should be changed because this is no different than
CTR where only the last block is allowed to be an arbitrary size.
Of course we should change everything in one go due to the testing
code.

This does raise the issue that we may be using blocksize in places
where we should be using chunksize instead, e.g., in algif_skcipher.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

On Sun, Sep 08, 2019 at 11:04:26AM +0300, Uri Shir 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(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt