[dpdk-dev] [PATCH 0/3] Fix compability issues between crypto drivers for GCM test cases

[dpdk-dev] [PATCH 0/3] Fix compability issues between crypto drivers for GCM test cases

[Arek Kusztal]

This patchset fix pre-counter block issues between crypto divers for AES-GCM tests.
Pre-counter block 96b computation from test file is moved into the AES-GCM PMD.
This patch set fixes too problems with verification of digest for AES-GCM.

Arek Kusztal (3):
  crypto/aesni_gcm: move pre-counter block to GCM driver
  app/test: move pre-counter block computation from test files
  app/test: fix verification of digest in GCM crypto test

 app/test/test_cryptodev.c                | 20 ++++++++------------
 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c |  7 ++++++-
 2 files changed, 14 insertions(+), 13 deletions(-)

-- 
2.1.0

[dpdk-dev] [PATCH 1/3] crypto/aesni_gcm: move pre-counter block to GCM driver

[Arek Kusztal]

This patch moves computing of pre-counter block into the AESNI-GCM
driver so it can be moved from test files.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index dc0b033..d8b6287 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -230,11 +230,16 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_sym_op *op,
 					op->cipher.data.offset);
 
 	/* sanity checks */
-	if (op->cipher.iv.length != 16 && op->cipher.iv.length != 0) {
+	if (op->cipher.iv.length != 16 && op->cipher.iv.length != 12 &&
+			op->cipher.iv.length != 0) {
 		GCM_LOG_ERR("iv");
 		return -1;
 	}
 
+	if (op->cipher.iv.length == 12) {
+		op->cipher.iv.data[15] = 1;
+	}
+
 	if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
 			op->auth.aad.length != 0) {
 		GCM_LOG_ERR("iv");
-- 
2.1.0

[dpdk-dev] [PATCH 2/3] app/test: move pre-counter block computation from test files

[Arek Kusztal]

This patch removes pre-counter block computation from
test_cryptodev.c file for AES GCM

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 app/test/test_cryptodev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 647787d..227bd7d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -3083,14 +3083,10 @@ create_gcm_operation(enum rte_crypto_cipher_operation op,
 
 	memset(sym_op->cipher.iv.data, 0, iv_pad_len);
 	sym_op->cipher.iv.phys_addr = rte_pktmbuf_mtophys(ut_params->ibuf);
-	sym_op->cipher.iv.length = iv_pad_len;
+	sym_op->cipher.iv.length = iv_len;
 
 	rte_memcpy(sym_op->cipher.iv.data, iv, iv_len);
 
-	/* CalcY0 */
-	if (iv_len != 16)
-		sym_op->cipher.iv.data[15] = 1;
-
 	/*
 	 * Always allocate the aad up to the block size.
 	 * The cryptodev API calls out -
-- 
2.1.0

[dpdk-dev] [PATCH 3/3] app/test: fix verification of digest in GCM crypto test

[Arek Kusztal]

This patch fixes verification of digest in test_cryptodev.c file
for AES GCM test cases

Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto operations")

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 app/test/test_cryptodev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 227bd7d..215056f 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -2988,13 +2988,13 @@ test_snow3g_encrypted_authentication_test_case_1(void)
 static int
 create_gcm_session(uint8_t dev_id, enum rte_crypto_cipher_operation op,
 		const uint8_t *key, const uint8_t key_len,
-		const uint8_t aad_len, const uint8_t auth_len)
+		const uint8_t aad_len, const uint8_t auth_len,
+		enum rte_crypto_auth_operation auth_op)
 {
 	uint8_t cipher_key[key_len];
 
 	struct crypto_unittest_params *ut_params = &unittest_params;
 
-
 	memcpy(cipher_key, key, key_len);
 
 	/* Setup Cipher Parameters */
@@ -3002,7 +3002,7 @@ create_gcm_session(uint8_t dev_id, enum rte_crypto_cipher_operation op,
 	ut_params->cipher_xform.next = NULL;
 
 	ut_params->cipher_xform.cipher.algo = RTE_CRYPTO_CIPHER_AES_GCM;
-	ut_params->auth_xform.auth.op = RTE_CRYPTO_AUTH_OP_GENERATE;
+	ut_params->auth_xform.auth.op = auth_op;
 	ut_params->cipher_xform.cipher.op = op;
 	ut_params->cipher_xform.cipher.key.data = cipher_key;
 	ut_params->cipher_xform.cipher.key.length = key_len;
@@ -3057,8 +3057,6 @@ create_gcm_operation(enum rte_crypto_cipher_operation op,
 
 	struct rte_crypto_sym_op *sym_op = ut_params->op->sym;
 
-
-
 	sym_op->auth.digest.data = (uint8_t *)rte_pktmbuf_append(
 			ut_params->ibuf, auth_tag_len);
 	TEST_ASSERT_NOT_NULL(sym_op->auth.digest.data,
@@ -3135,7 +3133,8 @@ test_mb_AES_GCM_authenticated_encryption(const struct gcm_test_data *tdata)
 	retval = create_gcm_session(ts_params->valid_devs[0],
 			RTE_CRYPTO_CIPHER_OP_ENCRYPT,
 			tdata->key.data, tdata->key.len,
-			tdata->aad.len, tdata->auth_tag.len);
+			tdata->aad.len, tdata->auth_tag.len,
+			RTE_CRYPTO_AUTH_OP_GENERATE);
 	if (retval < 0)
 		return retval;
 
@@ -3265,7 +3264,8 @@ test_mb_AES_GCM_authenticated_decryption(const struct gcm_test_data *tdata)
 	retval = create_gcm_session(ts_params->valid_devs[0],
 			RTE_CRYPTO_CIPHER_OP_DECRYPT,
 			tdata->key.data, tdata->key.len,
-			tdata->aad.len, tdata->auth_tag.len);
+			tdata->aad.len, tdata->auth_tag.len,
+			RTE_CRYPTO_AUTH_OP_VERIFY);
 	if (retval < 0)
 		return retval;
 
-- 
2.1.0

Re: [dpdk-dev] [PATCH 0/3] Fix compability issues between crypto drivers for GCM test cases

[Jain, Deepak K]

> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Thursday, August 25, 2016 1:03 PM
> To: dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Griffin, John <john.griffin@intel.com>;
> Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH 0/3] Fix compability issues between crypto drivers for GCM
> test cases
> 
> This patchset fix pre-counter block issues between crypto divers for AES-
> GCM tests.
> Pre-counter block 96b computation from test file is moved into the AES-GCM
> PMD.
> This patch set fixes too problems with verification of digest for AES-GCM.
> 
> Arek Kusztal (3):
>   crypto/aesni_gcm: move pre-counter block to GCM driver
>   app/test: move pre-counter block computation from test files
>   app/test: fix verification of digest in GCM crypto test
> 
>  app/test/test_cryptodev.c                | 20 ++++++++------------
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c |  7 ++++++-
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> --
> 2.1.0
Acked-by: Deepak Kumar Jain <deepak.k.jain@intel.com>

Re: [dpdk-dev] [PATCH 1/3] crypto/aesni_gcm: move pre-counter block to GCM driver

[De Lara Guarch, Pablo]

Hi Arek,

> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Thursday, August 25, 2016 5:03 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; Jain, Deepak K; De Lara Guarch, Pablo; Griffin, John;
> Kusztal, ArkadiuszX
> Subject: [PATCH 1/3] crypto/aesni_gcm: move pre-counter block to GCM
> driver
> 
> This patch moves computing of pre-counter block into the AESNI-GCM
> driver so it can be moved from test files.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> index dc0b033..d8b6287 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> @@ -230,11 +230,16 @@ process_gcm_crypto_op(struct aesni_gcm_qp
> *qp, struct rte_crypto_sym_op *op,
>  					op->cipher.data.offset);
> 
>  	/* sanity checks */
> -	if (op->cipher.iv.length != 16 && op->cipher.iv.length != 0) {
> +	if (op->cipher.iv.length != 16 && op->cipher.iv.length != 12 &&
> +			op->cipher.iv.length != 0) {
>  		GCM_LOG_ERR("iv");
>  		return -1;
>  	}
> 
> +	if (op->cipher.iv.length == 12) {
> +		op->cipher.iv.data[15] = 1;
> +	}

Is this correct? In the second patch, you are removing code that set this byte when IV length was 16,
so I would expect this to be the same. Also, if length is 12, then data[15] looks like overflow.
Probably a comment would be useful here.

> +
>  	if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
>  			op->auth.aad.length != 0) {
>  		GCM_LOG_ERR("iv");
> --
> 2.1.0

Re: [dpdk-dev] [PATCH 1/3] crypto/aesni_gcm: move pre-counter block to GCM driver

[Kusztal, ArkadiuszX]

Hi Pablo,

GCM works with two IV length modes:
First case if - If (len(IV) == 12B) => J0 = iv || 0^31 || 1 => Len = 16B

And according to the API comments rte_crypto_sym.h: 435 (iv.data)
			 * - For GCM mode, this is either the IV (if the length
			 * is 96 bits) or J0 (for other sizes), where J0 is as
			 * defined by NIST SP800-38D. Regardless of the IV
			 * length, a full 16 bytes needs to be allocated.
So there is no worry of overflowing.

Thanks,
Arek


-----Original Message-----
From: De Lara Guarch, Pablo 
Sent: Tuesday, September 20, 2016 10:38 PM
To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
Cc: Trahe, Fiona <fiona.trahe@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>; Griffin, John <john.griffin@intel.com>
Subject: RE: [PATCH 1/3] crypto/aesni_gcm: move pre-counter block to GCM driver

Hi Arek,

> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Thursday, August 25, 2016 5:03 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; Jain, Deepak K; De Lara Guarch, Pablo; Griffin, 
> John; Kusztal, ArkadiuszX
> Subject: [PATCH 1/3] crypto/aesni_gcm: move pre-counter block to GCM 
> driver
> 
> This patch moves computing of pre-counter block into the AESNI-GCM 
> driver so it can be moved from test files.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> index dc0b033..d8b6287 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> @@ -230,11 +230,16 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, 
> struct rte_crypto_sym_op *op,
>  					op->cipher.data.offset);
> 
>  	/* sanity checks */
> -	if (op->cipher.iv.length != 16 && op->cipher.iv.length != 0) {
> +	if (op->cipher.iv.length != 16 && op->cipher.iv.length != 12 &&
> +			op->cipher.iv.length != 0) {
>  		GCM_LOG_ERR("iv");
>  		return -1;
>  	}
> 
> +	if (op->cipher.iv.length == 12) {
> +		op->cipher.iv.data[15] = 1;
> +	}

Is this correct? In the second patch, you are removing code that set this byte when IV length was 16, so I would expect this to be the same. Also, if length is 12, then data[15] looks like overflow.
Probably a comment would be useful here.

> +
>  	if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
>  			op->auth.aad.length != 0) {
>  		GCM_LOG_ERR("iv");
> --
> 2.1.0