2019-07-30 16:45:27

by Gary R Hook

[permalink] [raw]
Subject: [PATCH 0/3] AES GCM fixes for the CCP crypto driver

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


2019-07-30 16:45:38

by Gary R Hook

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

From: Gary R Hook <[email protected]>

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 <[email protected]>
---
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

2019-08-02 08:59:25

by Herbert Xu

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

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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt