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

Message ID 1585882384-28213-1-git-send-email-anoobj@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series [v3] examples/ipsec-secgw: support 192/256 AES key sizes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues
ci/Intel-compilation success Compilation OK

Commit Message

Anoob Joseph April 3, 2020, 2:53 a.m. UTC
  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(-)
  

Comments

Akhil Goyal April 5, 2020, 3:24 p.m. UTC | #1
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
  
Anoob Joseph April 5, 2020, 5:10 p.m. UTC | #2
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
  
Akhil Goyal April 6, 2020, 6:42 a.m. UTC | #3
> > 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 Joseph April 6, 2020, 6:46 a.m. UTC | #4
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.
  

Patch

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;
 		}