[PATCH 0/3] AES GCM fixes for the CCP crypto driver

[PATCH 0/3] AES GCM fixes for the CCP crypto driver

[Hook, Gary]

Additional testing features added to the crypto framework (including fuzzy
probing and variations of the lengths of input parameters such as AAD and
authsize) expose some gaps in robustness and function in the CCP driver.
Address these gaps:

Input text is allowed to be zero bytes in length. In this case no
encryption/decryption occurs, and certain data structures are not
allocated. Don't clean up what doesn't exist.

Valid auth tag sizes are 4, 8, 12, 13, 14, 15 or 16 bytes.
Note: since the CCP driver has been designed to be used directly, add
      validation of the authsize parameter at this layer.

AES GCM defines the input text for decryption as the concatenation of
the AAD, the ciphertext, and the tag. Only the cipher text needs to
be decrypted; the tag is simple used for comparison.

Gary R Hook (3):
  crypto: ccp - Fix oops by properly managing allocated structures
  crypto: ccp - Add support for valid authsize values less than 16
  crypto: ccp - Ignore tag length when decrypting GCM ciphertext

 drivers/crypto/ccp/ccp-crypto-aes-galois.c | 14 +++++++++
 drivers/crypto/ccp/ccp-ops.c               | 33 ++++++++++++++++------
 include/linux/ccp.h                        |  2 ++
 3 files changed, 40 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH 1/3] crypto: ccp - Fix oops by properly managing allocated structures

[Hook, Gary]

From: Gary R Hook <gary.hook@amd.com>

A plaintext or ciphertext length of 0 is allowed in AES, in which case
no encryption occurs. Ensure that we don't clean up data structures
that were never allocated.

Fixes: 36cf515b9bbe2 ("crypto: ccp - Enable support for AES GCM on v5 CCPs")

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/crypto/ccp/ccp-ops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index 42d167574131..35b6b3397d49 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -858,11 +858,11 @@ ccp_run_aes_gcm_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
 	ccp_dm_free(&final_wa);
 
 e_dst:
-	if (aes->src_len && !in_place)
+	if (ilen > 0 && !in_place)
 		ccp_free_data(&dst, cmd_q);
 
 e_src:
-	if (aes->src_len)
+	if (ilen > 0)
 		ccp_free_data(&src, cmd_q);
 
 e_aad:
-- 
2.17.1

[PATCH 2/3] crypto: ccp - Add support for valid authsize values less than 16

[Hook, Gary]

From: Gary R Hook <gary.hook@amd.com>

AES GCM encryption allows for authsize values of 4, 8, and 12-16 bytes.
Validate the requested authsize, and retain it to save in the request
context.

Fixes: 36cf515b9bbe2 ("crypto: ccp - Enable support for AES GCM on v5 CCPs")

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/crypto/ccp/ccp-crypto-aes-galois.c | 14 ++++++++++++
 drivers/crypto/ccp/ccp-ops.c               | 26 +++++++++++++++++-----
 include/linux/ccp.h                        |  2 ++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-aes-galois.c b/drivers/crypto/ccp/ccp-crypto-aes-galois.c
index f9fec2ddf56a..94c1ad7eeddf 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-galois.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-galois.c
@@ -58,6 +58,19 @@ static int ccp_aes_gcm_setkey(struct crypto_aead *tfm, const u8 *key,
 static int ccp_aes_gcm_setauthsize(struct crypto_aead *tfm,
 				   unsigned int authsize)
 {
+	switch (authsize) {
+	case 16:
+	case 15:
+	case 14:
+	case 13:
+	case 12:
+	case 8:
+	case 4:
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -104,6 +117,7 @@ static int ccp_aes_gcm_crypt(struct aead_request *req, bool encrypt)
 	memset(&rctx->cmd, 0, sizeof(rctx->cmd));
 	INIT_LIST_HEAD(&rctx->cmd.entry);
 	rctx->cmd.engine = CCP_ENGINE_AES;
+	rctx->cmd.u.aes.authsize = crypto_aead_authsize(tfm);
 	rctx->cmd.u.aes.type = ctx->u.aes.type;
 	rctx->cmd.u.aes.mode = ctx->u.aes.mode;
 	rctx->cmd.u.aes.action = encrypt;
diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index 35b6b3397d49..553d8aa4f18d 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -621,6 +621,7 @@ ccp_run_aes_gcm_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
 
 	unsigned long long *final;
 	unsigned int dm_offset;
+	unsigned int authsize;
 	unsigned int jobid;
 	unsigned int ilen;
 	bool in_place = true; /* Default value */
@@ -642,6 +643,21 @@ ccp_run_aes_gcm_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
 	if (!aes->key) /* Gotta have a key SGL */
 		return -EINVAL;
 
+	/* Zero defaults to 16 bytes, the maximum size */
+	authsize = aes->authsize ? aes->authsize : AES_BLOCK_SIZE;
+	switch (authsize) {
+	case 16:
+	case 15:
+	case 14:
+	case 13:
+	case 12:
+	case 8:
+	case 4:
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	/* First, decompose the source buffer into AAD & PT,
 	 * and the destination buffer into AAD, CT & tag, or
 	 * the input into CT & tag.
@@ -656,7 +672,7 @@ ccp_run_aes_gcm_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
 		p_tag = scatterwalk_ffwd(sg_tag, p_outp, ilen);
 	} else {
 		/* Input length for decryption includes tag */
-		ilen = aes->src_len - AES_BLOCK_SIZE;
+		ilen = aes->src_len - authsize;
 		p_tag = scatterwalk_ffwd(sg_tag, p_inp, ilen);
 	}
 
@@ -838,19 +854,19 @@ ccp_run_aes_gcm_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
 
 	if (aes->action == CCP_AES_ACTION_ENCRYPT) {
 		/* Put the ciphered tag after the ciphertext. */
-		ccp_get_dm_area(&final_wa, 0, p_tag, 0, AES_BLOCK_SIZE);
+		ccp_get_dm_area(&final_wa, 0, p_tag, 0, authsize);
 	} else {
 		/* Does this ciphered tag match the input? */
-		ret = ccp_init_dm_workarea(&tag, cmd_q, AES_BLOCK_SIZE,
+		ret = ccp_init_dm_workarea(&tag, cmd_q, authsize,
 					   DMA_BIDIRECTIONAL);
 		if (ret)
 			goto e_tag;
-		ret = ccp_set_dm_area(&tag, 0, p_tag, 0, AES_BLOCK_SIZE);
+		ret = ccp_set_dm_area(&tag, 0, p_tag, 0, authsize);
 		if (ret)
 			goto e_tag;
 
 		ret = crypto_memneq(tag.address, final_wa.address,
-				    AES_BLOCK_SIZE) ? -EBADMSG : 0;
+				    authsize) ? -EBADMSG : 0;
 		ccp_dm_free(&tag);
 	}
 
diff --git a/include/linux/ccp.h b/include/linux/ccp.h
index 55cb455cfcb0..a5dfbaf2470d 100644
--- a/include/linux/ccp.h
+++ b/include/linux/ccp.h
@@ -170,6 +170,8 @@ struct ccp_aes_engine {
 	enum ccp_aes_mode mode;
 	enum ccp_aes_action action;
 
+	u32 authsize;
+
 	struct scatterlist *key;
 	u32 key_len;		/* In bytes */
 
-- 
2.17.1

[PATCH 3/3] crypto: ccp - Ignore tag length when decrypting GCM ciphertext

[Hook, Gary]

From: Gary R Hook <gary.hook@amd.com>

AES GCM input buffers for decryption contain AAD+CTEXT+TAG. Only
decrypt the ciphertext, and use the tag for comparison.

Fixes: 36cf515b9bbe2 ("crypto: ccp - Enable support for AES GCM on v5 CCPs")

Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
 drivers/crypto/ccp/ccp-ops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index 553d8aa4f18d..4c1b1445af79 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -781,8 +781,7 @@ ccp_run_aes_gcm_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
 		while (src.sg_wa.bytes_left) {
 			ccp_prepare_data(&src, &dst, &op, AES_BLOCK_SIZE, true);
 			if (!src.sg_wa.bytes_left) {
-				unsigned int nbytes = aes->src_len
-						      % AES_BLOCK_SIZE;
+				unsigned int nbytes = ilen % AES_BLOCK_SIZE;
 
 				if (nbytes) {
 					op.eom = 1;
-- 
2.17.1

Re: [PATCH 0/3] AES GCM fixes for the CCP crypto driver

[Herbert Xu]

On Tue, Jul 30, 2019 at 04:05:07PM +0000, Hook, Gary wrote:
> Additional testing features added to the crypto framework (including fuzzy
> probing and variations of the lengths of input parameters such as AAD and
> authsize) expose some gaps in robustness and function in the CCP driver.
> Address these gaps:
> 
> Input text is allowed to be zero bytes in length. In this case no
> encryption/decryption occurs, and certain data structures are not
> allocated. Don't clean up what doesn't exist.
> 
> Valid auth tag sizes are 4, 8, 12, 13, 14, 15 or 16 bytes.
> Note: since the CCP driver has been designed to be used directly, add
>       validation of the authsize parameter at this layer.
> 
> AES GCM defines the input text for decryption as the concatenation of
> the AAD, the ciphertext, and the tag. Only the cipher text needs to
> be decrypted; the tag is simple used for comparison.
> 
> Gary R Hook (3):
>   crypto: ccp - Fix oops by properly managing allocated structures
>   crypto: ccp - Add support for valid authsize values less than 16
>   crypto: ccp - Ignore tag length when decrypting GCM ciphertext
> 
>  drivers/crypto/ccp/ccp-crypto-aes-galois.c | 14 +++++++++
>  drivers/crypto/ccp/ccp-ops.c               | 33 ++++++++++++++++------
>  include/linux/ccp.h                        |  2 ++
>  3 files changed, 40 insertions(+), 9 deletions(-)

All 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