diff mbox series

[v3,8/8] examples/ipsec-secgw: add support for additional algorithms

Message ID 20211001095202.3343782-9-radu.nicolau@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series IPsec Sec GW new features |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build fail github build: failed
ci/iol-spell-check-testing warning Testing issues
ci/iol-aarch64-compile-testing fail Testing issues
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-x86_64-compile-testing fail Testing issues
ci/Intel-compilation fail Compilation issues

Commit Message

Radu Nicolau Oct. 1, 2021, 9:52 a.m. UTC 
  Add support for AES-GMAC, AES_CTR, AES_XCBC_MAC,
AES_CCM, CHACHA20_POLY1305

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 examples/ipsec-secgw/ipsec.h |   3 +-
 examples/ipsec-secgw/sa.c    | 133 ++++++++++++++++++++++++++++++++---
 2 files changed, 126 insertions(+), 10 deletions(-)

Comments

Akhil Goyal Oct. 8, 2021, 7:07 p.m. UTC | #1 
> Add support for AES-GMAC, AES_CTR, AES_XCBC_MAC,
> AES_CCM, CHACHA20_POLY1305
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>  examples/ipsec-secgw/ipsec.h |   3 +-
>  examples/ipsec-secgw/sa.c    | 133 ++++++++++++++++++++++++++++++++---
>  2 files changed, 126 insertions(+), 10 deletions(-)
> 

Documentation?
Release notes?


> diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> index 8a76405ad9..5fa4e62f37 100644
> --- a/examples/ipsec-secgw/ipsec.h
> +++ b/examples/ipsec-secgw/ipsec.h
> @@ -65,8 +65,7 @@ struct ip_addr {
>  	} ip;
>  };
> 
> -#define MAX_KEY_SIZE		36
> -
> +#define MAX_KEY_SIZE		96

Max key length defined below is 64, then why 96?

>  /*
>   * application wide SA parameters
>   */
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index d52dd94056..08fd1efea8 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -46,6 +46,7 @@ struct supported_cipher_algo {
>  struct supported_auth_algo {
>  	const char *keyword;
>  	enum rte_crypto_auth_algorithm algo;
> +	uint16_t iv_len;
>  	uint16_t digest_len;
>  	uint16_t key_len;
>  	uint8_t key_not_req;
> @@ -98,6 +99,20 @@ const struct supported_cipher_algo cipher_algos[] = {
>  		.block_size = 4,
>  		.key_len = 20
>  	},
> +	{
> +		.keyword = "aes-192-ctr",
> +		.algo = RTE_CRYPTO_CIPHER_AES_CTR,
> +		.iv_len = 16,
> +		.block_size = 16,
> +		.key_len = 28
> +	},
> +	{
> +		.keyword = "aes-256-ctr",
> +		.algo = RTE_CRYPTO_CIPHER_AES_CTR,
> +		.iv_len = 16,
> +		.block_size = 16,
> +		.key_len = 36
> +	},
>  	{
>  		.keyword = "3des-cbc",
>  		.algo = RTE_CRYPTO_CIPHER_3DES_CBC,
> @@ -126,6 +141,31 @@ const struct supported_auth_algo auth_algos[] = {
>  		.algo = RTE_CRYPTO_AUTH_SHA256_HMAC,
>  		.digest_len = 16,
>  		.key_len = 32
> +	},
> +	{
> +		.keyword = "sha384-hmac",
> +		.algo = RTE_CRYPTO_AUTH_SHA384_HMAC,
> +		.digest_len = 24,
> +		.key_len = 48
> +	},
> +	{
> +		.keyword = "sha512-hmac",
> +		.algo = RTE_CRYPTO_AUTH_SHA512_HMAC,
> +		.digest_len = 32,
> +		.key_len = 64
> +	},
> +	{
> +		.keyword = "aes-gmac",
> +		.algo = RTE_CRYPTO_AUTH_AES_GMAC,
> +		.iv_len = 8,
> +		.digest_len = 16,
> +		.key_len = 20
> +	},
> +	{
> +		.keyword = "aes-xcbc-mac-96",
> +		.algo = RTE_CRYPTO_AUTH_AES_XCBC_MAC,
> +		.digest_len = 12,
> +		.key_len = 16
>  	}
>  };
> 
> @@ -156,6 +196,42 @@ const struct supported_aead_algo aead_algos[] = {
>  		.key_len = 36,
>  		.digest_len = 16,
>  		.aad_len = 8,
> +	},
> +	{
> +		.keyword = "aes-128-ccm",
> +		.algo = RTE_CRYPTO_AEAD_AES_CCM,
> +		.iv_len = 8,
> +		.block_size = 4,
> +		.key_len = 20,
> +		.digest_len = 16,
> +		.aad_len = 8,
> +	},
> +	{
> +		.keyword = "aes-192-ccm",
> +		.algo = RTE_CRYPTO_AEAD_AES_CCM,
> +		.iv_len = 8,
> +		.block_size = 4,
> +		.key_len = 28,
> +		.digest_len = 16,
> +		.aad_len = 8,
> +	},
> +	{
> +		.keyword = "aes-256-ccm",
> +		.algo = RTE_CRYPTO_AEAD_AES_CCM,
> +		.iv_len = 8,
> +		.block_size = 4,
> +		.key_len = 36,
> +		.digest_len = 16,
> +		.aad_len = 8,
> +	},
> +	{
> +		.keyword = "chacha20-poly1305",
> +		.algo = RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
> +		.iv_len = 12,
> +		.block_size = 64,
> +		.key_len = 36,
> +		.digest_len = 16,
> +		.aad_len = 8,
>  	}
>  };
> 
> @@ -484,6 +560,15 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>  			if (status->status < 0)
>  				return;
> 
> +			if (algo->algo == RTE_CRYPTO_AUTH_AES_GMAC) {
> +				key_len -= 4;
> +				rule->auth_key_len = key_len;
> +				rule->iv_len = algo->iv_len;
> +				memcpy(&rule->salt,
> +					&rule->auth_key[key_len], 4);
> +			}
> +
> +

Extra line

>  			auth_algo_p = 1;
>  			continue;
>  		}
> @@ -1181,8 +1266,20 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  			break;
>  		}
> 
> -		if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
> -			iv_length = 12;
> +
> +		if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM ||
> +			sa->aead_algo == RTE_CRYPTO_AEAD_AES_CCM ||
> +			sa->aead_algo ==
> RTE_CRYPTO_AEAD_CHACHA20_POLY1305) {
> +
> +			if (ips->type ==
> +
> 	RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
> +				iv_length = 8;

How is IV length dependent on the action type?
It should be same for all modes for a particular algorithm. Right?

> +			} else {
> +				if (sa->aead_algo ==
> RTE_CRYPTO_AEAD_AES_CCM)
> +					iv_length = 11;
> +				else
> +					iv_length = 12;
> +			}
> 
>  			sa_ctx->xf[idx].a.type =
> RTE_CRYPTO_SYM_XFORM_AEAD;
>  			sa_ctx->xf[idx].a.aead.algo = sa->aead_algo;
> @@ -1206,10 +1303,8 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  			case RTE_CRYPTO_CIPHER_NULL:
>  			case RTE_CRYPTO_CIPHER_3DES_CBC:
>  			case RTE_CRYPTO_CIPHER_AES_CBC:
> -				iv_length = sa->iv_len;
> -				break;
>  			case RTE_CRYPTO_CIPHER_AES_CTR:
> -				iv_length = 16;
> +				iv_length = sa->iv_len;
>  				break;
>  			default:
>  				RTE_LOG(ERR, IPSEC_ESP,
> @@ -1218,6 +1313,15 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  				return -EINVAL;
>  			}
> 
> +			if (sa->auth_algo == RTE_CRYPTO_AUTH_AES_GMAC)
> {
> +				if (ips->type ==
> +
> RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
> +					iv_length = 8;

Same comment here.

> +				} else {
> +					iv_length = 12;
> +				}
> +			}
> +
>  			if (inbound) {
>  				sa_ctx->xf[idx].b.type =
> RTE_CRYPTO_SYM_XFORM_CIPHER;
>  				sa_ctx->xf[idx].b.cipher.algo = sa-
> >cipher_algo;
> @@ -1239,6 +1343,9 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  					sa->digest_len;
>  				sa_ctx->xf[idx].a.auth.op =
>  					RTE_CRYPTO_AUTH_OP_VERIFY;
> +				sa_ctx->xf[idx].a.auth.iv.offset = IV_OFFSET;
> +				sa_ctx->xf[idx].a.auth.iv.length = iv_length;
> +
>  			} else { /* outbound */
>  				sa_ctx->xf[idx].a.type =
> RTE_CRYPTO_SYM_XFORM_CIPHER;
>  				sa_ctx->xf[idx].a.cipher.algo = sa-
> >cipher_algo;
> @@ -1260,11 +1367,21 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  					sa->digest_len;
>  				sa_ctx->xf[idx].b.auth.op =
>  					RTE_CRYPTO_AUTH_OP_GENERATE;
> +				sa_ctx->xf[idx].b.auth.iv.offset = IV_OFFSET;
> +				sa_ctx->xf[idx].b.auth.iv.length = iv_length;
> +
>  			}
> 
> -			sa_ctx->xf[idx].a.next = &sa_ctx->xf[idx].b;
> -			sa_ctx->xf[idx].b.next = NULL;
> -			sa->xforms = &sa_ctx->xf[idx].a;
> +			if (sa->auth_algo == RTE_CRYPTO_AUTH_AES_GMAC)
> {
> +				sa->xforms = inbound ?
> +					&sa_ctx->xf[idx].a : &sa_ctx-
> >xf[idx].b;
> +				sa->xforms->next = NULL;
> +
> +			} else {
> +				sa_ctx->xf[idx].a.next = &sa_ctx->xf[idx].b;
> +				sa_ctx->xf[idx].b.next = NULL;
> +				sa->xforms = &sa_ctx->xf[idx].a;
> +			}
>  		}
> 
>  		if (ips->type ==
> --
> 2.25.1
Radu Nicolau Oct. 11, 2021, 3:40 p.m. UTC | #2 
Hi Akhil, thanks for the feedback, some comments below.

On 10/8/2021 8:07 PM, Akhil Goyal wrote:
>> Add support for AES-GMAC, AES_CTR, AES_XCBC_MAC,
>> AES_CCM, CHACHA20_POLY1305
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>>   examples/ipsec-secgw/ipsec.h |   3 +-
>>   examples/ipsec-secgw/sa.c    | 133 ++++++++++++++++++++++++++++++++---
>>   2 files changed, 126 insertions(+), 10 deletions(-)
>>
> Documentation?
> Release notes?

I will follow up with an updated patchset.


>> -
>> +#define MAX_KEY_SIZE		96
> Max key length defined below is 64, then why 96?
I guess just to have some room, I will set it back to 64.
>
>>   /*
>>   		}
>>
>> -		if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
>> -			iv_length = 12;
>> +
>> +		if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM ||
>> +			sa->aead_algo == RTE_CRYPTO_AEAD_AES_CCM ||
>> +			sa->aead_algo ==
>> RTE_CRYPTO_AEAD_CHACHA20_POLY1305) {
>> +
>> +			if (ips->type ==
>> +
>> 	RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
>> +				iv_length = 8;
> How is IV length dependent on the action type?
> It should be same for all modes for a particular algorithm. Right?

The issue here with inline crypto and AEAD algorithms is that we have 
the IV and the salt used for building the nonce but only the IV is 
included in the ESP header. And technically and according to the RFCs 
the value in the inline branch is the correct one, the other is actually 
the nonce buffer length, not the IV length. I'm not sure if this is the 
proper fix, probably it's not, but it's probably a change that will not 
break it for other crypto devices.
diff mbox series

Patch

diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index 8a76405ad9..5fa4e62f37 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -65,8 +65,7 @@  struct ip_addr {
 	} ip;
 };
 
-#define MAX_KEY_SIZE		36
-
+#define MAX_KEY_SIZE		96
 /*
  * application wide SA parameters
  */
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index d52dd94056..08fd1efea8 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -46,6 +46,7 @@  struct supported_cipher_algo {
 struct supported_auth_algo {
 	const char *keyword;
 	enum rte_crypto_auth_algorithm algo;
+	uint16_t iv_len;
 	uint16_t digest_len;
 	uint16_t key_len;
 	uint8_t key_not_req;
@@ -98,6 +99,20 @@  const struct supported_cipher_algo cipher_algos[] = {
 		.block_size = 4,
 		.key_len = 20
 	},
+	{
+		.keyword = "aes-192-ctr",
+		.algo = RTE_CRYPTO_CIPHER_AES_CTR,
+		.iv_len = 16,
+		.block_size = 16,
+		.key_len = 28
+	},
+	{
+		.keyword = "aes-256-ctr",
+		.algo = RTE_CRYPTO_CIPHER_AES_CTR,
+		.iv_len = 16,
+		.block_size = 16,
+		.key_len = 36
+	},
 	{
 		.keyword = "3des-cbc",
 		.algo = RTE_CRYPTO_CIPHER_3DES_CBC,
@@ -126,6 +141,31 @@  const struct supported_auth_algo auth_algos[] = {
 		.algo = RTE_CRYPTO_AUTH_SHA256_HMAC,
 		.digest_len = 16,
 		.key_len = 32
+	},
+	{
+		.keyword = "sha384-hmac",
+		.algo = RTE_CRYPTO_AUTH_SHA384_HMAC,
+		.digest_len = 24,
+		.key_len = 48
+	},
+	{
+		.keyword = "sha512-hmac",
+		.algo = RTE_CRYPTO_AUTH_SHA512_HMAC,
+		.digest_len = 32,
+		.key_len = 64
+	},
+	{
+		.keyword = "aes-gmac",
+		.algo = RTE_CRYPTO_AUTH_AES_GMAC,
+		.iv_len = 8,
+		.digest_len = 16,
+		.key_len = 20
+	},
+	{
+		.keyword = "aes-xcbc-mac-96",
+		.algo = RTE_CRYPTO_AUTH_AES_XCBC_MAC,
+		.digest_len = 12,
+		.key_len = 16
 	}
 };
 
@@ -156,6 +196,42 @@  const struct supported_aead_algo aead_algos[] = {
 		.key_len = 36,
 		.digest_len = 16,
 		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-128-ccm",
+		.algo = RTE_CRYPTO_AEAD_AES_CCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 20,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-192-ccm",
+		.algo = RTE_CRYPTO_AEAD_AES_CCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 28,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "aes-256-ccm",
+		.algo = RTE_CRYPTO_AEAD_AES_CCM,
+		.iv_len = 8,
+		.block_size = 4,
+		.key_len = 36,
+		.digest_len = 16,
+		.aad_len = 8,
+	},
+	{
+		.keyword = "chacha20-poly1305",
+		.algo = RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
+		.iv_len = 12,
+		.block_size = 64,
+		.key_len = 36,
+		.digest_len = 16,
+		.aad_len = 8,
 	}
 };
 
@@ -484,6 +560,15 @@  parse_sa_tokens(char **tokens, uint32_t n_tokens,
 			if (status->status < 0)
 				return;
 
+			if (algo->algo == RTE_CRYPTO_AUTH_AES_GMAC) {
+				key_len -= 4;
+				rule->auth_key_len = key_len;
+				rule->iv_len = algo->iv_len;
+				memcpy(&rule->salt,
+					&rule->auth_key[key_len], 4);
+			}
+
+
 			auth_algo_p = 1;
 			continue;
 		}
@@ -1181,8 +1266,20 @@  sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 			break;
 		}
 
-		if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
-			iv_length = 12;
+
+		if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM ||
+			sa->aead_algo == RTE_CRYPTO_AEAD_AES_CCM ||
+			sa->aead_algo == RTE_CRYPTO_AEAD_CHACHA20_POLY1305) {
+
+			if (ips->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
+				iv_length = 8;
+			} else {
+				if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_CCM)
+					iv_length = 11;
+				else
+					iv_length = 12;
+			}
 
 			sa_ctx->xf[idx].a.type = RTE_CRYPTO_SYM_XFORM_AEAD;
 			sa_ctx->xf[idx].a.aead.algo = sa->aead_algo;
@@ -1206,10 +1303,8 @@  sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 			case RTE_CRYPTO_CIPHER_NULL:
 			case RTE_CRYPTO_CIPHER_3DES_CBC:
 			case RTE_CRYPTO_CIPHER_AES_CBC:
-				iv_length = sa->iv_len;
-				break;
 			case RTE_CRYPTO_CIPHER_AES_CTR:
-				iv_length = 16;
+				iv_length = sa->iv_len;
 				break;
 			default:
 				RTE_LOG(ERR, IPSEC_ESP,
@@ -1218,6 +1313,15 @@  sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 				return -EINVAL;
 			}
 
+			if (sa->auth_algo == RTE_CRYPTO_AUTH_AES_GMAC) {
+				if (ips->type ==
+				    RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
+					iv_length = 8;
+				} else {
+					iv_length = 12;
+				}
+			}
+
 			if (inbound) {
 				sa_ctx->xf[idx].b.type = RTE_CRYPTO_SYM_XFORM_CIPHER;
 				sa_ctx->xf[idx].b.cipher.algo = sa->cipher_algo;
@@ -1239,6 +1343,9 @@  sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 					sa->digest_len;
 				sa_ctx->xf[idx].a.auth.op =
 					RTE_CRYPTO_AUTH_OP_VERIFY;
+				sa_ctx->xf[idx].a.auth.iv.offset = IV_OFFSET;
+				sa_ctx->xf[idx].a.auth.iv.length = iv_length;
+
 			} else { /* outbound */
 				sa_ctx->xf[idx].a.type = RTE_CRYPTO_SYM_XFORM_CIPHER;
 				sa_ctx->xf[idx].a.cipher.algo = sa->cipher_algo;
@@ -1260,11 +1367,21 @@  sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 					sa->digest_len;
 				sa_ctx->xf[idx].b.auth.op =
 					RTE_CRYPTO_AUTH_OP_GENERATE;
+				sa_ctx->xf[idx].b.auth.iv.offset = IV_OFFSET;
+				sa_ctx->xf[idx].b.auth.iv.length = iv_length;
+
 			}
 
-			sa_ctx->xf[idx].a.next = &sa_ctx->xf[idx].b;
-			sa_ctx->xf[idx].b.next = NULL;
-			sa->xforms = &sa_ctx->xf[idx].a;
+			if (sa->auth_algo == RTE_CRYPTO_AUTH_AES_GMAC) {
+				sa->xforms = inbound ?
+					&sa_ctx->xf[idx].a : &sa_ctx->xf[idx].b;
+				sa->xforms->next = NULL;
+
+			} else {
+				sa_ctx->xf[idx].a.next = &sa_ctx->xf[idx].b;
+				sa_ctx->xf[idx].b.next = NULL;
+				sa->xforms = &sa_ctx->xf[idx].a;
+			}
 		}
 
 		if (ips->type ==