[dpdk-dev] [PATCH] examples/ipsec-secgw: support 192/256 AES key sizes

[dpdk-dev] [PATCH] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Adding support for the following,
1. AES-192-GCM
2. AES-256-GCM
3. AES-192-CBC

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
---
 examples/ipsec-secgw/ipsec.h |  2 +-
 examples/ipsec-secgw/sa.c    | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index f8f29f9..46a974e 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -72,7 +72,7 @@ struct ip_addr {
 	} ip;
 };
 
-#define MAX_KEY_SIZE		32
+#define MAX_KEY_SIZE		36
 
 /*
  * application wide SA parameters
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 0eb52d1..0d7d460 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -77,6 +77,13 @@ const struct supported_cipher_algo cipher_algos[] = {
 		.key_len = 16
 	},
 	{
+		.keyword = "aes-192-cbc",
+		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
+		.iv_len = 16,
+		.block_size = 16,
+		.key_len = 24
+	},
+	{
 		.keyword = "aes-256-cbc",
 		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
 		.iv_len = 16,
@@ -130,6 +137,24 @@ const struct supported_aead_algo aead_algos[] = {
 		.key_len = 20,
 		.digest_len = 16,
 		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-192-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 28,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-256-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 36,
+		.digest_len = 16,
+		.aad_len = 8,
 	}
 };
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: support 192/256 AES key sizes

[Akhil Goyal]

> 
> Adding support for the following,
> 1. AES-192-GCM
> 2. AES-256-GCM
> 3. AES-192-CBC
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec.h |  2 +-
>  examples/ipsec-secgw/sa.c    | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 

This would also need update in "doc/guides/sample_app_ug/ipsec_secgw.rst"

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, March 26, 2020 12:07 AM
> To: Anoob Joseph <anoobj@marvell.com>; Radu Nicolau
> <radu.nicolau@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathreya@marvell.com>; Tejasree Kondoj
> <ktejasree@marvell.com>; dev@dpdk.org
> Subject: [EXT] RE: [PATCH] examples/ipsec-secgw: support 192/256 AES key
> sizes
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> >
> > Adding support for the following,
> > 1. AES-192-GCM
> > 2. AES-256-GCM
> > 3. AES-192-CBC
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > ---
> >  examples/ipsec-secgw/ipsec.h |  2 +-
> >  examples/ipsec-secgw/sa.c    | 25 +++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 1 deletion(-)
> >
> 
> This would also need update in "doc/guides/sample_app_ug/ipsec_secgw.rst"

Will update in v2. Do I need to update release notes as well?

Thanks,
Anoob

Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: support 192/256 AES key sizes

[Akhil Goyal]

> > >
> > > Adding support for the following,
> > > 1. AES-192-GCM
> > > 2. AES-256-GCM
> > > 3. AES-192-CBC
> > >
> > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > > ---
> > >  examples/ipsec-secgw/ipsec.h |  2 +-
> > >  examples/ipsec-secgw/sa.c    | 25 +++++++++++++++++++++++++
> > >  2 files changed, 26 insertions(+), 1 deletion(-)
> > >
> >
> > This would also need update in "doc/guides/sample_app_ug/ipsec_secgw.rst"
> 
> Will update in v2. Do I need to update release notes as well?
> 
Yes you can add it.

[dpdk-dev] [PATCH v2] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Adding support for the following,
1. AES-192-GCM
2. AES-256-GCM
3. AES-192-CBC

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
---
v2:
* Updated doc and release notes

 doc/guides/rel_notes/release_20_05.rst   |  7 +++++++
 doc/guides/sample_app_ug/ipsec_secgw.rst |  3 +++
 examples/ipsec-secgw/ipsec.h             |  2 +-
 examples/ipsec-secgw/sa.c                | 25 +++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
index 1dfcfcc..c0b0625 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -70,6 +70,13 @@ New Features
   by making use of the event device capabilities. The event mode currently supports
   only inline IPsec protocol offload.
 
+* **Added 192/256 AES key sizes in ipsec-secgw application.**
+
+  Updated ipsec-secgw application to support the following key sizes,
+    - AES-192-CBC
+    - AES-192-GCM
+    - AES-256-GCM
+
 
 Removed Items
 -------------
diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
index 038f593..f5e94bf 100644
--- a/doc/guides/sample_app_ug/ipsec_secgw.rst
+++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
@@ -538,6 +538,7 @@ where each options means:
 
    * *null*: NULL algorithm
    * *aes-128-cbc*: AES-CBC 128-bit algorithm
+   * *aes-192-cbc*: AES-CBC 192-bit algorithm
    * *aes-256-cbc*: AES-CBC 256-bit algorithm
    * *aes-128-ctr*: AES-CTR 128-bit algorithm
    * *3des-cbc*: 3DES-CBC 192-bit algorithm
@@ -593,6 +594,8 @@ where each options means:
  * Available options:
 
    * *aes-128-gcm*: AES-GCM 128-bit algorithm
+   * *aes-192-gcm*: AES-GCM 192-bit algorithm
+   * *aes-256-gcm*: AES-GCM 256-bit algorithm
 
  * Syntax: *cipher_algo <your algorithm>*
 
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index f8f29f9..46a974e 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -72,7 +72,7 @@ struct ip_addr {
 	} ip;
 };
 
-#define MAX_KEY_SIZE		32
+#define MAX_KEY_SIZE		36
 
 /*
  * application wide SA parameters
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 0eb52d1..0d7d460 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -77,6 +77,13 @@ const struct supported_cipher_algo cipher_algos[] = {
 		.key_len = 16
 	},
 	{
+		.keyword = "aes-192-cbc",
+		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
+		.iv_len = 16,
+		.block_size = 16,
+		.key_len = 24
+	},
+	{
 		.keyword = "aes-256-cbc",
 		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
 		.iv_len = 16,
@@ -130,6 +137,24 @@ const struct supported_aead_algo aead_algos[] = {
 		.key_len = 20,
 		.digest_len = 16,
 		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-192-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 28,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-256-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 36,
+		.digest_len = 16,
+		.aad_len = 8,
 	}
 };
 
-- 
2.7.4

[dpdk-dev] [PATCH v3] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Adding support for the following,
1. AES-192-GCM
2. AES-256-GCM
3. AES-192-CBC

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
---
v3:
* Fixed incorrect AES-GCM key length being printed during app startup
* Introduced new macro 'SALT_SIZE' to make the usage more obvious (AES-GCM
  key has key following 4 byte salt)
* Minor cleanup for the existing code.

v2:
* Updated doc and release notes

 doc/guides/rel_notes/release_20_05.rst   |  7 ++++++
 doc/guides/sample_app_ug/ipsec_secgw.rst |  3 +++
 examples/ipsec-secgw/ipsec.h             |  3 ++-
 examples/ipsec-secgw/sa.c                | 38 ++++++++++++++++++++++++++------
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
index 1dfcfcc..c0b0625 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -70,6 +70,13 @@ New Features
   by making use of the event device capabilities. The event mode currently supports
   only inline IPsec protocol offload.
 
+* **Added 192/256 AES key sizes in ipsec-secgw application.**
+
+  Updated ipsec-secgw application to support the following key sizes,
+    - AES-192-CBC
+    - AES-192-GCM
+    - AES-256-GCM
+
 
 Removed Items
 -------------
diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
index 038f593..f5e94bf 100644
--- a/doc/guides/sample_app_ug/ipsec_secgw.rst
+++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
@@ -538,6 +538,7 @@ where each options means:
 
    * *null*: NULL algorithm
    * *aes-128-cbc*: AES-CBC 128-bit algorithm
+   * *aes-192-cbc*: AES-CBC 192-bit algorithm
    * *aes-256-cbc*: AES-CBC 256-bit algorithm
    * *aes-128-ctr*: AES-CTR 128-bit algorithm
    * *3des-cbc*: 3DES-CBC 192-bit algorithm
@@ -593,6 +594,8 @@ where each options means:
  * Available options:
 
    * *aes-128-gcm*: AES-GCM 128-bit algorithm
+   * *aes-192-gcm*: AES-GCM 192-bit algorithm
+   * *aes-256-gcm*: AES-GCM 256-bit algorithm
 
  * Syntax: *cipher_algo <your algorithm>*
 
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index f8f29f9..476a6d5 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -73,6 +73,7 @@ struct ip_addr {
 };
 
 #define MAX_KEY_SIZE		32
+#define SALT_SIZE		4
 
 /*
  * application wide SA parameters
@@ -133,7 +134,7 @@ struct ipsec_sa {
 #define IP6_TRANSPORT (1 << 4)
 	struct ip_addr src;
 	struct ip_addr dst;
-	uint8_t cipher_key[MAX_KEY_SIZE];
+	uint8_t cipher_key[MAX_KEY_SIZE + SALT_SIZE];
 	uint16_t cipher_key_len;
 	uint8_t auth_key[MAX_KEY_SIZE];
 	uint16_t auth_key_len;
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 0eb52d1..fc6bc97 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -77,6 +77,13 @@ const struct supported_cipher_algo cipher_algos[] = {
 		.key_len = 16
 	},
 	{
+		.keyword = "aes-192-cbc",
+		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
+		.iv_len = 16,
+		.block_size = 16,
+		.key_len = 24
+	},
+	{
 		.keyword = "aes-256-cbc",
 		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
 		.iv_len = 16,
@@ -127,7 +134,25 @@ const struct supported_aead_algo aead_algos[] = {
 		.algo = RTE_CRYPTO_AEAD_AES_GCM,
 		.iv_len = 8,
 		.block_size = 4,
-		.key_len = 20,
+		.key_len = 16,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-192-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 24,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-256-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 32,
 		.digest_len = 16,
 		.aad_len = 8,
 	}
@@ -495,16 +520,14 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
 				return;
 
 			key_len = parse_key_string(tokens[ti],
-				rule->cipher_key);
+				rule->cipher_key) - SALT_SIZE;
 			APP_CHECK(key_len == rule->cipher_key_len, status,
 				"unrecognized input \"%s\"", tokens[ti]);
 			if (status->status < 0)
 				return;
 
-			key_len -= 4;
-			rule->cipher_key_len = key_len;
-			memcpy(&rule->salt,
-				&rule->cipher_key[key_len], 4);
+			memcpy(&rule->salt, &rule->cipher_key[key_len],
+				SALT_SIZE);
 
 			aead_algo_p = 1;
 			continue;
@@ -751,7 +774,8 @@ print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
 	}
 
 	for (i = 0; i < RTE_DIM(aead_algos); i++) {
-		if (aead_algos[i].algo == sa->aead_algo) {
+		if (aead_algos[i].algo == sa->aead_algo &&
+				aead_algos[i].key_len == sa->cipher_key_len) {
 			printf("%s ", aead_algos[i].keyword);
 			break;
 		}
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: support 192/256 AES key sizes

[Akhil Goyal]

Hi Anoob,

> 
> Adding support for the following,
> 1. AES-192-GCM
> 2. AES-256-GCM
> 3. AES-192-CBC
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> ---
> v3:
> * Fixed incorrect AES-GCM key length being printed during app startup
> * Introduced new macro 'SALT_SIZE' to make the usage more obvious (AES-GCM
>   key has key following 4 byte salt)
> * Minor cleanup for the existing code.

I believe GCM keys are extended by 4 bytes to include the SALT value in many apps.
We may add a comment that it is including the SALT value, but it makes more confusing
now.

The length which is being printed is 16Bytes but we expect the user to have 20Bytes
In the ep0.cfg file. This will be confusing also to configure the packet capturing APPs
Like wireshark which accepts 20Byte keys in case of GCM.

> 
> v2:
> * Updated doc and release notes
> 
>  doc/guides/rel_notes/release_20_05.rst   |  7 ++++++
>  doc/guides/sample_app_ug/ipsec_secgw.rst |  3 +++
>  examples/ipsec-secgw/ipsec.h             |  3 ++-
>  examples/ipsec-secgw/sa.c                | 38 ++++++++++++++++++++++++++------
>  4 files changed, 43 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_20_05.rst
> b/doc/guides/rel_notes/release_20_05.rst
> index 1dfcfcc..c0b0625 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -70,6 +70,13 @@ New Features
>    by making use of the event device capabilities. The event mode currently
> supports
>    only inline IPsec protocol offload.
> 
> +* **Added 192/256 AES key sizes in ipsec-secgw application.**
> +
> +  Updated ipsec-secgw application to support the following key sizes,
> +    - AES-192-CBC
> +    - AES-192-GCM
> +    - AES-256-GCM
> +
> 
>  Removed Items
>  -------------
> diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst
> b/doc/guides/sample_app_ug/ipsec_secgw.rst
> index 038f593..f5e94bf 100644
> --- a/doc/guides/sample_app_ug/ipsec_secgw.rst
> +++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
> @@ -538,6 +538,7 @@ where each options means:
> 
>     * *null*: NULL algorithm
>     * *aes-128-cbc*: AES-CBC 128-bit algorithm
> +   * *aes-192-cbc*: AES-CBC 192-bit algorithm
>     * *aes-256-cbc*: AES-CBC 256-bit algorithm
>     * *aes-128-ctr*: AES-CTR 128-bit algorithm
>     * *3des-cbc*: 3DES-CBC 192-bit algorithm
> @@ -593,6 +594,8 @@ where each options means:
>   * Available options:
> 
>     * *aes-128-gcm*: AES-GCM 128-bit algorithm
> +   * *aes-192-gcm*: AES-GCM 192-bit algorithm
> +   * *aes-256-gcm*: AES-GCM 256-bit algorithm
> 
>   * Syntax: *cipher_algo <your algorithm>*
> 
> diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> index f8f29f9..476a6d5 100644
> --- a/examples/ipsec-secgw/ipsec.h
> +++ b/examples/ipsec-secgw/ipsec.h
> @@ -73,6 +73,7 @@ struct ip_addr {
>  };
> 
>  #define MAX_KEY_SIZE		32
> +#define SALT_SIZE		4
> 
>  /*
>   * application wide SA parameters
> @@ -133,7 +134,7 @@ struct ipsec_sa {
>  #define IP6_TRANSPORT (1 << 4)
>  	struct ip_addr src;
>  	struct ip_addr dst;
> -	uint8_t cipher_key[MAX_KEY_SIZE];
> +	uint8_t cipher_key[MAX_KEY_SIZE + SALT_SIZE];
>  	uint16_t cipher_key_len;
>  	uint8_t auth_key[MAX_KEY_SIZE];
>  	uint16_t auth_key_len;
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 0eb52d1..fc6bc97 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -77,6 +77,13 @@ const struct supported_cipher_algo cipher_algos[] = {
>  		.key_len = 16
>  	},
>  	{
> +		.keyword = "aes-192-cbc",
> +		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
> +		.iv_len = 16,
> +		.block_size = 16,
> +		.key_len = 24
> +	},
> +	{
>  		.keyword = "aes-256-cbc",
>  		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
>  		.iv_len = 16,
> @@ -127,7 +134,25 @@ const struct supported_aead_algo aead_algos[] = {
>  		.algo = RTE_CRYPTO_AEAD_AES_GCM,
>  		.iv_len = 8,
>  		.block_size = 4,
> -		.key_len = 20,
> +		.key_len = 16,
> +		.digest_len = 16,
> +		.aad_len = 8,
> +	},
> +	{
> +		.keyword = "aes-192-gcm",
> +		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> +		.iv_len = 8,
> +		.block_size = 4,
> +		.key_len = 24,
> +		.digest_len = 16,
> +		.aad_len = 8,
> +	},
> +	{
> +		.keyword = "aes-256-gcm",
> +		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> +		.iv_len = 8,
> +		.block_size = 4,
> +		.key_len = 32,
>  		.digest_len = 16,
>  		.aad_len = 8,
>  	}
> @@ -495,16 +520,14 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  				return;
> 
>  			key_len = parse_key_string(tokens[ti],
> -				rule->cipher_key);
> +				rule->cipher_key) - SALT_SIZE;
>  			APP_CHECK(key_len == rule->cipher_key_len, status,
>  				"unrecognized input \"%s\"", tokens[ti]);
>  			if (status->status < 0)
>  				return;
> 
> -			key_len -= 4;
> -			rule->cipher_key_len = key_len;
> -			memcpy(&rule->salt,
> -				&rule->cipher_key[key_len], 4);
> +			memcpy(&rule->salt, &rule->cipher_key[key_len],
> +				SALT_SIZE);
> 
>  			aead_algo_p = 1;
>  			continue;
> @@ -751,7 +774,8 @@ print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
>  	}
> 
>  	for (i = 0; i < RTE_DIM(aead_algos); i++) {
> -		if (aead_algos[i].algo == sa->aead_algo) {
> +		if (aead_algos[i].algo == sa->aead_algo &&
> +				aead_algos[i].key_len == sa->cipher_key_len) {
>  			printf("%s ", aead_algos[i].keyword);
>  			break;
>  		}
> --
> 2.7.4

Re: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Hi Akhil,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Sunday, April 5, 2020 8:54 PM
> To: Anoob Joseph <anoobj@marvell.com>; Radu Nicolau
> <radu.nicolau@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathreya@marvell.com>; Tejasree Kondoj
> <ktejasree@marvell.com>; dev@dpdk.org
> Subject: [EXT] RE: [PATCH v3] examples/ipsec-secgw: support 192/256 AES key
> sizes
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Anoob,
> 
> >
> > Adding support for the following,
> > 1. AES-192-GCM
> > 2. AES-256-GCM
> > 3. AES-192-CBC
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > ---
> > v3:
> > * Fixed incorrect AES-GCM key length being printed during app startup
> > * Introduced new macro 'SALT_SIZE' to make the usage more obvious (AES-
> GCM
> >   key has key following 4 byte salt)
> > * Minor cleanup for the existing code.
> 
> I believe GCM keys are extended by 4 bytes to include the SALT value in many
> apps.
> We may add a comment that it is including the SALT value, but it makes more
> confusing now.
> 
> The length which is being printed is 16Bytes but we expect the user to have
> 20Bytes In the ep0.cfg file. This will be confusing also to configure the packet
> capturing APPs Like wireshark which accepts 20Byte keys in case of GCM.

[Anoob] The ones I've edited is just internal data structures. These are not exposed and not directly printed anywhere.

spi_in( 51):aes-128-gcm mode:IP4Tunnel 10.0.10.1 10.0.10.2 type:inline-protocol-offload 
spi_in( 52):aes-192-gcm mode:IP4Tunnel 10.0.20.1 10.0.20.2 type:inline-protocol-offload 
spi_in( 53):aes-256-gcm mode:IP4Tunnel 10.0.30.1 10.0.30.2 type:inline-protocol-offload

Also, my initial patch didn't try to address this aspect. In that patch, I had the following addition, in which key length was clearly not matching the string.

	{
		.keyword = "aes-192-gcm",
		.algo = RTE_CRYPTO_AEAD_AES_GCM,
		.iv_len = 8,
		.block_size = 4,
		.key_len = 28,
		.digest_len = 16,
		.aad_len = 8,
	},

In either case, the "misleading" part in config file would stay as the string would be "aes-128-gcm"/"aes-192-gcm"/"aes-256-gcm", and the key specified will have additional 4 bytes. Please do comment inline on what you think is the right approach. You can check if you are fine with v2 approach. I can resend that with a minor change required in the print. 

One more thing. I was just checking the ipsec-secgw documentation of AEAD keys. I think we need to update that as well.

Syntax: Hexadecimal bytes (0x0-0xFF) concatenate by colon symbol ':'. The number of bytes should be as same as the specified AEAD algorithm key size.

For example: aead_key A1:B2:C3:D4:A1:B2:C3:D4:A1:B2:C3:D4: A1:B2:C3:D4

Can you advice on what should be the approach here?

> 
> >
> > v2:
> > * Updated doc and release notes
> >
> >  doc/guides/rel_notes/release_20_05.rst   |  7 ++++++
> >  doc/guides/sample_app_ug/ipsec_secgw.rst |  3 +++
> >  examples/ipsec-secgw/ipsec.h             |  3 ++-
> >  examples/ipsec-secgw/sa.c                | 38 ++++++++++++++++++++++++++------
> >  4 files changed, 43 insertions(+), 8 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_05.rst
> > b/doc/guides/rel_notes/release_20_05.rst
> > index 1dfcfcc..c0b0625 100644
> > --- a/doc/guides/rel_notes/release_20_05.rst
> > +++ b/doc/guides/rel_notes/release_20_05.rst
> > @@ -70,6 +70,13 @@ New Features
> >    by making use of the event device capabilities. The event mode
> > currently supports
> >    only inline IPsec protocol offload.
> >
> > +* **Added 192/256 AES key sizes in ipsec-secgw application.**
> > +
> > +  Updated ipsec-secgw application to support the following key sizes,
> > +    - AES-192-CBC
> > +    - AES-192-GCM
> > +    - AES-256-GCM
> > +
> >
> >  Removed Items
> >  -------------
> > diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst
> > b/doc/guides/sample_app_ug/ipsec_secgw.rst
> > index 038f593..f5e94bf 100644
> > --- a/doc/guides/sample_app_ug/ipsec_secgw.rst
> > +++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
> > @@ -538,6 +538,7 @@ where each options means:
> >
> >     * *null*: NULL algorithm
> >     * *aes-128-cbc*: AES-CBC 128-bit algorithm
> > +   * *aes-192-cbc*: AES-CBC 192-bit algorithm
> >     * *aes-256-cbc*: AES-CBC 256-bit algorithm
> >     * *aes-128-ctr*: AES-CTR 128-bit algorithm
> >     * *3des-cbc*: 3DES-CBC 192-bit algorithm @@ -593,6 +594,8 @@ where
> > each options means:
> >   * Available options:
> >
> >     * *aes-128-gcm*: AES-GCM 128-bit algorithm
> > +   * *aes-192-gcm*: AES-GCM 192-bit algorithm
> > +   * *aes-256-gcm*: AES-GCM 256-bit algorithm
> >
> >   * Syntax: *cipher_algo <your algorithm>*
> >
> > diff --git a/examples/ipsec-secgw/ipsec.h
> > b/examples/ipsec-secgw/ipsec.h index f8f29f9..476a6d5 100644
> > --- a/examples/ipsec-secgw/ipsec.h
> > +++ b/examples/ipsec-secgw/ipsec.h
> > @@ -73,6 +73,7 @@ struct ip_addr {
> >  };
> >
> >  #define MAX_KEY_SIZE		32
> > +#define SALT_SIZE		4
> >
> >  /*
> >   * application wide SA parameters
> > @@ -133,7 +134,7 @@ struct ipsec_sa {
> >  #define IP6_TRANSPORT (1 << 4)
> >  	struct ip_addr src;
> >  	struct ip_addr dst;
> > -	uint8_t cipher_key[MAX_KEY_SIZE];
> > +	uint8_t cipher_key[MAX_KEY_SIZE + SALT_SIZE];
> >  	uint16_t cipher_key_len;
> >  	uint8_t auth_key[MAX_KEY_SIZE];
> >  	uint16_t auth_key_len;
> > diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> > index 0eb52d1..fc6bc97 100644
> > --- a/examples/ipsec-secgw/sa.c
> > +++ b/examples/ipsec-secgw/sa.c
> > @@ -77,6 +77,13 @@ const struct supported_cipher_algo cipher_algos[] = {
> >  		.key_len = 16
> >  	},
> >  	{
> > +		.keyword = "aes-192-cbc",
> > +		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
> > +		.iv_len = 16,
> > +		.block_size = 16,
> > +		.key_len = 24
> > +	},
> > +	{
> >  		.keyword = "aes-256-cbc",
> >  		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
> >  		.iv_len = 16,
> > @@ -127,7 +134,25 @@ const struct supported_aead_algo aead_algos[] = {
> >  		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> >  		.iv_len = 8,
> >  		.block_size = 4,
> > -		.key_len = 20,
> > +		.key_len = 16,
> > +		.digest_len = 16,
> > +		.aad_len = 8,
> > +	},
> > +	{
> > +		.keyword = "aes-192-gcm",
> > +		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> > +		.iv_len = 8,
> > +		.block_size = 4,
> > +		.key_len = 24,
> > +		.digest_len = 16,
> > +		.aad_len = 8,
> > +	},
> > +	{
> > +		.keyword = "aes-256-gcm",
> > +		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> > +		.iv_len = 8,
> > +		.block_size = 4,
> > +		.key_len = 32,
> >  		.digest_len = 16,
> >  		.aad_len = 8,
> >  	}
> > @@ -495,16 +520,14 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
> >  				return;
> >
> >  			key_len = parse_key_string(tokens[ti],
> > -				rule->cipher_key);
> > +				rule->cipher_key) - SALT_SIZE;
> >  			APP_CHECK(key_len == rule->cipher_key_len, status,
> >  				"unrecognized input \"%s\"", tokens[ti]);
> >  			if (status->status < 0)
> >  				return;
> >
> > -			key_len -= 4;
> > -			rule->cipher_key_len = key_len;
> > -			memcpy(&rule->salt,
> > -				&rule->cipher_key[key_len], 4);
> > +			memcpy(&rule->salt, &rule->cipher_key[key_len],
> > +				SALT_SIZE);
> >
> >  			aead_algo_p = 1;
> >  			continue;
> > @@ -751,7 +774,8 @@ print_one_sa_rule(const struct ipsec_sa *sa, int
> inbound)
> >  	}
> >
> >  	for (i = 0; i < RTE_DIM(aead_algos); i++) {
> > -		if (aead_algos[i].algo == sa->aead_algo) {
> > +		if (aead_algos[i].algo == sa->aead_algo &&
> > +				aead_algos[i].key_len == sa->cipher_key_len) {
> >  			printf("%s ", aead_algos[i].keyword);
> >  			break;
> >  		}
> > --
> > 2.7.4

Re: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: support 192/256 AES key sizes

[Akhil Goyal]

> > Hi Anoob,
> >
> > >
> > > Adding support for the following,
> > > 1. AES-192-GCM
> > > 2. AES-256-GCM
> > > 3. AES-192-CBC
> > >
> > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > > ---
> > > v3:
> > > * Fixed incorrect AES-GCM key length being printed during app startup
> > > * Introduced new macro 'SALT_SIZE' to make the usage more obvious (AES-
> > GCM
> > >   key has key following 4 byte salt)
> > > * Minor cleanup for the existing code.
> >
> > I believe GCM keys are extended by 4 bytes to include the SALT value in many
> > apps.
> > We may add a comment that it is including the SALT value, but it makes more
> > confusing now.
> >
> > The length which is being printed is 16Bytes but we expect the user to have
> > 20Bytes In the ep0.cfg file. This will be confusing also to configure the packet
> > capturing APPs Like wireshark which accepts 20Byte keys in case of GCM.
> 
> [Anoob] The ones I've edited is just internal data structures. These are not
> exposed and not directly printed anywhere.
> 
> spi_in( 51):aes-128-gcm mode:IP4Tunnel 10.0.10.1 10.0.10.2 type:inline-
> protocol-offload
> spi_in( 52):aes-192-gcm mode:IP4Tunnel 10.0.20.1 10.0.20.2 type:inline-
> protocol-offload
> spi_in( 53):aes-256-gcm mode:IP4Tunnel 10.0.30.1 10.0.30.2 type:inline-
> protocol-offload
> 
> Also, my initial patch didn't try to address this aspect. In that patch, I had the
> following addition, in which key length was clearly not matching the string.
> 
> 	{
> 		.keyword = "aes-192-gcm",
> 		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> 		.iv_len = 8,
> 		.block_size = 4,
> 		.key_len = 28,
> 		.digest_len = 16,
> 		.aad_len = 8,
> 	},
> 
> In either case, the "misleading" part in config file would stay as the string would
> be "aes-128-gcm"/"aes-192-gcm"/"aes-256-gcm", and the key specified will
> have additional 4 bytes. Please do comment inline on what you think is the right
> approach. You can check if you are fine with v2 approach. I can resend that with
> a minor change required in the print.
> 
> One more thing. I was just checking the ipsec-secgw documentation of AEAD
> keys. I think we need to update that as well.
> 
> Syntax: Hexadecimal bytes (0x0-0xFF) concatenate by colon symbol ':'. The
> number of bytes should be as same as the specified AEAD algorithm key size.
> 
> For example: aead_key A1:B2:C3:D4:A1:B2:C3:D4:A1:B2:C3:D4: A1:B2:C3:D4
> 
> Can you advice on what should be the approach here?
> 
I think it is better to have the key len include the 4 bytes of SALT and cfg file has those 4 bytes
Inline with the key. We can add a print to specify that last 4 bytes are salt.
And Yes for AEAD doc, we can add a statement that keylen should include the the 4bytes of SALT.
And user should specify the extra 4 bytes.

So I believe your v2 was good enough with some additional documentations.

Re: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Hi Akhil,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Monday, April 6, 2020 12:12 PM
> To: Anoob Joseph <anoobj@marvell.com>; Radu Nicolau
> <radu.nicolau@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathreya@marvell.com>; Tejasree Kondoj
> <ktejasree@marvell.com>; dev@dpdk.org
> Subject: [EXT] RE: [PATCH v3] examples/ipsec-secgw: support 192/256 AES key
> sizes
> 
> External Email
> 
> ----------------------------------------------------------------------
> > > Hi Anoob,
> > >
> > > >
> > > > Adding support for the following,
> > > > 1. AES-192-GCM
> > > > 2. AES-256-GCM
> > > > 3. AES-192-CBC
> > > >
> > > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > > > ---
> > > > v3:
> > > > * Fixed incorrect AES-GCM key length being printed during app
> > > > startup
> > > > * Introduced new macro 'SALT_SIZE' to make the usage more obvious
> > > > (AES-
> > > GCM
> > > >   key has key following 4 byte salt)
> > > > * Minor cleanup for the existing code.
> > >
> > > I believe GCM keys are extended by 4 bytes to include the SALT value
> > > in many apps.
> > > We may add a comment that it is including the SALT value, but it
> > > makes more confusing now.
> > >
> > > The length which is being printed is 16Bytes but we expect the user
> > > to have 20Bytes In the ep0.cfg file. This will be confusing also to
> > > configure the packet capturing APPs Like wireshark which accepts 20Byte
> keys in case of GCM.
> >
> > [Anoob] The ones I've edited is just internal data structures. These
> > are not exposed and not directly printed anywhere.
> >
> > spi_in( 51):aes-128-gcm mode:IP4Tunnel 10.0.10.1 10.0.10.2
> > type:inline- protocol-offload spi_in( 52):aes-192-gcm mode:IP4Tunnel
> > 10.0.20.1 10.0.20.2 type:inline- protocol-offload spi_in(
> > 53):aes-256-gcm mode:IP4Tunnel 10.0.30.1 10.0.30.2 type:inline-
> > protocol-offload
> >
> > Also, my initial patch didn't try to address this aspect. In that
> > patch, I had the following addition, in which key length was clearly not
> matching the string.
> >
> > 	{
> > 		.keyword = "aes-192-gcm",
> > 		.algo = RTE_CRYPTO_AEAD_AES_GCM,
> > 		.iv_len = 8,
> > 		.block_size = 4,
> > 		.key_len = 28,
> > 		.digest_len = 16,
> > 		.aad_len = 8,
> > 	},
> >
> > In either case, the "misleading" part in config file would stay as the
> > string would be "aes-128-gcm"/"aes-192-gcm"/"aes-256-gcm", and the key
> > specified will have additional 4 bytes. Please do comment inline on
> > what you think is the right approach. You can check if you are fine
> > with v2 approach. I can resend that with a minor change required in the print.
> >
> > One more thing. I was just checking the ipsec-secgw documentation of
> > AEAD keys. I think we need to update that as well.
> >
> > Syntax: Hexadecimal bytes (0x0-0xFF) concatenate by colon symbol ':'.
> > The number of bytes should be as same as the specified AEAD algorithm key
> size.
> >
> > For example: aead_key A1:B2:C3:D4:A1:B2:C3:D4:A1:B2:C3:D4: A1:B2:C3:D4
> >
> > Can you advice on what should be the approach here?
> >
> I think it is better to have the key len include the 4 bytes of SALT and cfg file has
> those 4 bytes Inline with the key. We can add a print to specify that last 4 bytes
> are salt.
> And Yes for AEAD doc, we can add a statement that keylen should include the
> the 4bytes of SALT.
> And user should specify the extra 4 bytes.
> 
> So I believe your v2 was good enough with some additional documentations.

[Anoob] Will submit v2 with the changes discussed.

[dpdk-dev] [PATCH v4] examples/ipsec-secgw: support 192/256 AES key sizes

[Anoob Joseph]

Adding support for the following,
1. AES-192-GCM
2. AES-256-GCM
3. AES-192-CBC

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
---

v4:
* Reverted to v2 as suggested by Akhil.
* Added additional check in print routines to make sure correct key size is
  printed.
* Updated incorrect documentation of 'aead_key'

v3:
* Fixed incorrect AES-GCM key length being printed during app startup
* Introduced new macro 'SALT_SIZE' to make the usage more obvious (AES-GCM
  key has key following 4 byte salt)
* Minor cleanup for the existing code.

v2:
* Updated doc and release notes

 doc/guides/rel_notes/release_20_05.rst   |  7 +++++++
 doc/guides/sample_app_ug/ipsec_secgw.rst | 10 +++++++---
 examples/ipsec-secgw/ipsec.h             |  2 +-
 examples/ipsec-secgw/sa.c                | 28 +++++++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
index 6b1a7c5..8cfcef2 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -81,6 +81,13 @@ New Features
   by making use of the event device capabilities. The event mode currently supports
   only inline IPsec protocol offload.
 
+* **Added 192/256 AES key sizes in ipsec-secgw application.**
+
+  Updated ipsec-secgw application to support the following key sizes,
+    - AES-192-CBC
+    - AES-192-GCM
+    - AES-256-GCM
+
 
 Removed Items
 -------------
diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
index 038f593..c02d16a 100644
--- a/doc/guides/sample_app_ug/ipsec_secgw.rst
+++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
@@ -538,6 +538,7 @@ where each options means:
 
    * *null*: NULL algorithm
    * *aes-128-cbc*: AES-CBC 128-bit algorithm
+   * *aes-192-cbc*: AES-CBC 192-bit algorithm
    * *aes-256-cbc*: AES-CBC 256-bit algorithm
    * *aes-128-ctr*: AES-CTR 128-bit algorithm
    * *3des-cbc*: 3DES-CBC 192-bit algorithm
@@ -593,6 +594,8 @@ where each options means:
  * Available options:
 
    * *aes-128-gcm*: AES-GCM 128-bit algorithm
+   * *aes-192-gcm*: AES-GCM 192-bit algorithm
+   * *aes-256-gcm*: AES-GCM 256-bit algorithm
 
  * Syntax: *cipher_algo <your algorithm>*
 
@@ -604,11 +607,12 @@ where each options means:
    Must be followed by <aead_algo> option
 
  * Syntax: Hexadecimal bytes (0x0-0xFF) concatenate by colon symbol ':'.
-   The number of bytes should be as same as the specified AEAD algorithm
-   key size.
+   Last 4 bytes of the provided key will be used as 'salt' and so, the
+   number of bytes should be same as the sum of specified AEAD algorithm
+   key size and salt size (4 bytes).
 
    For example: *aead_key A1:B2:C3:D4:A1:B2:C3:D4:A1:B2:C3:D4:
-   A1:B2:C3:D4*
+   A1:B2:C3:D4:A1:B2:C3:D4*
 
 ``<mode>``
 
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index f8f29f9..46a974e 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -72,7 +72,7 @@ struct ip_addr {
 	} ip;
 };
 
-#define MAX_KEY_SIZE		32
+#define MAX_KEY_SIZE		36
 
 /*
  * application wide SA parameters
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index a6bf5e8..5e3a7aa 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -77,6 +77,13 @@ const struct supported_cipher_algo cipher_algos[] = {
 		.key_len = 16
 	},
 	{
+		.keyword = "aes-192-cbc",
+		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
+		.iv_len = 16,
+		.block_size = 16,
+		.key_len = 24
+	},
+	{
 		.keyword = "aes-256-cbc",
 		.algo = RTE_CRYPTO_CIPHER_AES_CBC,
 		.iv_len = 16,
@@ -130,6 +137,24 @@ const struct supported_aead_algo aead_algos[] = {
 		.key_len = 20,
 		.digest_len = 16,
 		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-192-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 28,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-256-gcm",
+		.algo = RTE_CRYPTO_AEAD_AES_GCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 36,
+		.digest_len = 16,
+		.aad_len = 8,
 	}
 };
 
@@ -753,7 +778,8 @@ print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
 	}
 
 	for (i = 0; i < RTE_DIM(aead_algos); i++) {
-		if (aead_algos[i].algo == sa->aead_algo) {
+		if (aead_algos[i].algo == sa->aead_algo &&
+				aead_algos[i].key_len-4 == sa->cipher_key_len) {
 			printf("%s ", aead_algos[i].keyword);
 			break;
 		}
-- 
2.7.4

Re: [dpdk-dev] [PATCH v4] examples/ipsec-secgw: support 192/256 AES key sizes

[Akhil Goyal]

> 
> Adding support for the following,
> 1. AES-192-GCM
> 2. AES-256-GCM
> 3. AES-192-CBC
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> ---
> 
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Re: [dpdk-dev] [PATCH v4] examples/ipsec-secgw: support 192/256 AES key sizes

[Akhil Goyal]

> 
> 
> >
> > Adding support for the following,
> > 1. AES-192-GCM
> > 2. AES-256-GCM
> > 3. AES-192-CBC
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> > ---
> >
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Applied to dpdk-next-crypto

Thanks.