[3/4] crypto: arm64/aes-ce-ccm - fix decrypt path with new skcipher interface

Message ID 1480424733-10797-3-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Nov. 29, 2016, 1:05 p.m.
The new skcipher walk interface does not take into account whether we
are encrypting or decrypting. In the latter case, the walk should
disregard the MAC. Fix this in the arm64 CE driver.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 arch/arm64/crypto/aes-ce-ccm-glue.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ard Biesheuvel Nov. 30, 2016, 1:24 p.m. | #1
On 30 November 2016 at 13:14, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Nov 29, 2016 at 01:05:32PM +0000, Ard Biesheuvel wrote:

>> The new skcipher walk interface does not take into account whether we

>> are encrypting or decrypting. In the latter case, the walk should

>> disregard the MAC. Fix this in the arm64 CE driver.

>>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>

> Thanks for the patch.  I'm going to build this into the AEAD walker

> instead, by providing separate entry points for encryption and

> decryption.  Like this,

>


Yes, that's better, thanks

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> ---8<---

> Subject: crypto: skcipher - Add separate walker for AEAD decryption

>

> The AEAD decrypt interface includes the authentication tag in

> req->cryptlen.  Therefore we need to exlucde that when doing

> a walk over it.

>

> This patch adds separate walker functions for AEAD encryption

> and decryption.

>

> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

>

> diff --git a/crypto/skcipher.c b/crypto/skcipher.c

> index 5367f81..aca07c6 100644

> --- a/crypto/skcipher.c

> +++ b/crypto/skcipher.c

> @@ -500,8 +500,8 @@ int skcipher_walk_async(struct skcipher_walk *walk,

>  }

>  EXPORT_SYMBOL_GPL(skcipher_walk_async);

>

> -int skcipher_walk_aead(struct skcipher_walk *walk, struct aead_request *req,

> -                      bool atomic)

> +static int skcipher_walk_aead_common(struct skcipher_walk *walk,

> +                                    struct aead_request *req, bool atomic)

>  {

>         struct crypto_aead *tfm = crypto_aead_reqtfm(req);

>         int err;

> @@ -514,7 +514,6 @@ int skcipher_walk_aead(struct skcipher_walk *walk, struct aead_request *req,

>         scatterwalk_copychunks(NULL, &walk->in, req->assoclen, 2);

>         scatterwalk_copychunks(NULL, &walk->out, req->assoclen, 2);

>

> -       walk->total = req->cryptlen;

>         walk->iv = req->iv;

>         walk->oiv = req->iv;

>

> @@ -535,8 +534,36 @@ int skcipher_walk_aead(struct skcipher_walk *walk, struct aead_request *req,

>

>         return err;

>  }

> +

> +int skcipher_walk_aead(struct skcipher_walk *walk, struct aead_request *req,

> +                      bool atomic)

> +{

> +       walk->total = req->cryptlen;

> +

> +       return skcipher_walk_aead_common(walk, req, atomic);

> +}

>  EXPORT_SYMBOL_GPL(skcipher_walk_aead);

>

> +int skcipher_walk_aead_encrypt(struct skcipher_walk *walk,

> +                              struct aead_request *req, bool atomic)

> +{

> +       walk->total = req->cryptlen;

> +

> +       return skcipher_walk_aead_common(walk, req, atomic);

> +}

> +EXPORT_SYMBOL_GPL(skcipher_walk_aead_encrypt);

> +

> +int skcipher_walk_aead_decrypt(struct skcipher_walk *walk,

> +                              struct aead_request *req, bool atomic)

> +{

> +       struct crypto_aead *tfm = crypto_aead_reqtfm(req);

> +

> +       walk->total = req->cryptlen - crypto_aead_authsize(tfm);

> +

> +       return skcipher_walk_aead_common(walk, req, atomic);

> +}

> +EXPORT_SYMBOL_GPL(skcipher_walk_aead_decrypt);

> +

>  static unsigned int crypto_skcipher_extsize(struct crypto_alg *alg)

>  {

>         if (alg->cra_type == &crypto_blkcipher_type)

> diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h

> index d55041f..8735979 100644

> --- a/include/crypto/internal/skcipher.h

> +++ b/include/crypto/internal/skcipher.h

> @@ -149,6 +149,10 @@ int skcipher_walk_async(struct skcipher_walk *walk,

>                         struct skcipher_request *req);

>  int skcipher_walk_aead(struct skcipher_walk *walk, struct aead_request *req,

>                        bool atomic);

> +int skcipher_walk_aead_encrypt(struct skcipher_walk *walk,

> +                              struct aead_request *req, bool atomic);

> +int skcipher_walk_aead_decrypt(struct skcipher_walk *walk,

> +                              struct aead_request *req, bool atomic);

>  void skcipher_walk_complete(struct skcipher_walk *walk, int err);

>

>  static inline void ablkcipher_request_complete(struct ablkcipher_request *req,

> --

> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
index d4f35685363b..1a011d658387 100644
--- a/arch/arm64/crypto/aes-ce-ccm-glue.c
+++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
@@ -204,10 +204,10 @@  static int ccm_decrypt(struct aead_request *req)
 	struct skcipher_walk walk;
 	u8 __aligned(8) mac[AES_BLOCK_SIZE];
 	u8 buf[AES_BLOCK_SIZE];
-	u32 len = req->cryptlen - authsize;
 	int err;
 
-	err = ccm_init_mac(req, mac, len);
+	req->cryptlen -= authsize;
+	err = ccm_init_mac(req, mac, req->cryptlen);
 	if (err)
 		return err;
 
@@ -242,8 +242,7 @@  static int ccm_decrypt(struct aead_request *req)
 		return err;
 
 	/* compare calculated auth tag with the stored one */
-	scatterwalk_map_and_copy(buf, req->src,
-				 req->assoclen + req->cryptlen - authsize,
+	scatterwalk_map_and_copy(buf, req->src, req->assoclen + req->cryptlen,
 				 authsize, 0);
 
 	if (crypto_memneq(mac, buf, authsize))