Hi Akhil,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Akhil Goyal <akhil.go...@nxp.com>
> Sent: Sunday, April 5, 2020 8:54 PM
> To: Anoob Joseph <ano...@marvell.com>; Radu Nicolau
> <radu.nico...@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathr...@marvell.com>; Tejasree Kondoj
> <ktejas...@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 <ano...@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejas...@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
