[v4,02/10] security: add UDP params for IPsec NAT-T

Message ID 20210903112626.304692-3-radu.nicolau@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series new features for ipsec and security libraries |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Radu Nicolau Sept. 3, 2021, 11:26 a.m. UTC
  Add support for specifying UDP port params for UDP encapsulation option.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
---
 lib/security/rte_security.h | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Fan Zhang Sept. 3, 2021, 12:51 p.m. UTC | #1
> -----Original Message-----
> From: Nicolau, Radu <radu.nicolau@intel.com>
> Sent: Friday, September 3, 2021 12:26 PM
> To: Akhil Goyal <gakhil@marvell.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Cc: dev@dpdk.org; mdr@ashroe.eu; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Medvedkin, Vladimir
> <vladimir.medvedkin@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> hemant.agrawal@nxp.com; anoobj@marvell.com; Sinha, Abhijit
> <abhijit.sinha@intel.com>; Buckley, Daniel M <daniel.m.buckley@intel.com>;
> marchana@marvell.com; ktejasree@marvell.com; matan@nvidia.com;
> Nicolau, Radu <radu.nicolau@intel.com>
> Subject: [PATCH v4 02/10] security: add UDP params for IPsec NAT-T
> 
> Add support for specifying UDP port params for UDP encapsulation option.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> ---
>  lib/security/rte_security.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> index 45896a77d0..03572b10ab 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -112,6 +112,12 @@ struct rte_security_ipsec_tunnel_param {
>  	};
>  };
> 
> +struct rte_security_ipsec_udp_param {
> +
> +	uint16_t sport;
> +	uint16_t dport;
> +};
> +
>  /**
>   * IPsec Security Association option flags
>   */
> @@ -224,6 +230,8 @@ struct rte_security_ipsec_xform {
>  	/**< IPsec SA Mode - transport/tunnel */
>  	struct rte_security_ipsec_tunnel_param tunnel;
>  	/**< Tunnel parameters, NULL for transport mode */
> +	struct rte_security_ipsec_udp_param udp;
> +	/**< UDP parameters, ignored when udp_encap option not
> specified */
>  	uint64_t esn_soft_limit;
>  	/**< ESN for which the overflow event need to be raised */
>  	uint32_t replay_win_sz;
> --
> 2.25.1
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
  
Akhil Goyal Sept. 5, 2021, 2:19 p.m. UTC | #2
Hi Radu,

> Add support for specifying UDP port params for UDP encapsulation option.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>

Do we really need to specify the port numbers for NAT-T?
I suppose they are fixed as 4500.
Could you please specify what the user need to set here for session
creation?

> ---
>  lib/security/rte_security.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> index 45896a77d0..03572b10ab 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -112,6 +112,12 @@ struct rte_security_ipsec_tunnel_param {
>  	};
>  };
> 
> +struct rte_security_ipsec_udp_param {
> +
> +	uint16_t sport;
> +	uint16_t dport;
> +};
> +
>  /**
>   * IPsec Security Association option flags
>   */
> @@ -224,6 +230,8 @@ struct rte_security_ipsec_xform {
>  	/**< IPsec SA Mode - transport/tunnel */
>  	struct rte_security_ipsec_tunnel_param tunnel;
>  	/**< Tunnel parameters, NULL for transport mode */
> +	struct rte_security_ipsec_udp_param udp;
> +	/**< UDP parameters, ignored when udp_encap option not specified
> */
>  	uint64_t esn_soft_limit;
>  	/**< ESN for which the overflow event need to be raised */
>  	uint32_t replay_win_sz;
> --
> 2.25.1
  
Radu Nicolau Sept. 6, 2021, 11:09 a.m. UTC | #3
On 9/5/2021 3:19 PM, Akhil Goyal wrote:
> Hi Radu,
>
>> Add support for specifying UDP port params for UDP encapsulation option.
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> Do we really need to specify the port numbers for NAT-T?
> I suppose they are fixed as 4500.
> Could you please specify what the user need to set here for session
> creation?

 From what I'm seeing here 
https://datatracker.ietf.org/doc/html/rfc3948#section-2.1 there is no 
requirement in general for UDP encapsulation so I think it's better to 
make the API flexible as to allow any port to be used.


>
>> ---
>>   lib/security/rte_security.h | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
>> index 45896a77d0..03572b10ab 100644
>> --- a/lib/security/rte_security.h
>> +++ b/lib/security/rte_security.h
>> @@ -112,6 +112,12 @@ struct rte_security_ipsec_tunnel_param {
>>   	};
>>   };
>>
>> +struct rte_security_ipsec_udp_param {
>> +
>> +	uint16_t sport;
>> +	uint16_t dport;
>> +};
>> +
>>   /**
>>    * IPsec Security Association option flags
>>    */
>> @@ -224,6 +230,8 @@ struct rte_security_ipsec_xform {
>>   	/**< IPsec SA Mode - transport/tunnel */
>>   	struct rte_security_ipsec_tunnel_param tunnel;
>>   	/**< Tunnel parameters, NULL for transport mode */
>> +	struct rte_security_ipsec_udp_param udp;
>> +	/**< UDP parameters, ignored when udp_encap option not specified
>> */
>>   	uint64_t esn_soft_limit;
>>   	/**< ESN for which the overflow event need to be raised */
>>   	uint32_t replay_win_sz;
>> --
>> 2.25.1
  
Hemant Agrawal Sept. 24, 2021, 9:11 a.m. UTC | #4
On 9/6/2021 4:39 PM, Nicolau, Radu wrote:
>
> On 9/5/2021 3:19 PM, Akhil Goyal wrote:
>> Hi Radu,
>>
>>> Add support for specifying UDP port params for UDP encapsulation 
>>> option.
>>>
>>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
>>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
>> Do we really need to specify the port numbers for NAT-T?
>> I suppose they are fixed as 4500.
>> Could you please specify what the user need to set here for session
>> creation?
>
> From what I'm seeing here 
> https://datatracker.ietf.org/doc/html/rfc3948#section-2.1 there is no 
> requirement in general for UDP encapsulation so I think it's better to 
> make the API flexible as to allow any port to be used.


This section states that :

o  the Source Port and Destination Port MUST be the same as that used by IKE traffic,

IKE usages port 4500

am I missing something?

>
>
>>
>>> ---
>>>   lib/security/rte_security.h | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
>>> index 45896a77d0..03572b10ab 100644
>>> --- a/lib/security/rte_security.h
>>> +++ b/lib/security/rte_security.h
>>> @@ -112,6 +112,12 @@ struct rte_security_ipsec_tunnel_param {
>>>       };
>>>   };
>>>
>>> +struct rte_security_ipsec_udp_param {
>>> +
>>> +    uint16_t sport;
>>> +    uint16_t dport;
>>> +};
>>> +
>>>   /**
>>>    * IPsec Security Association option flags
>>>    */
>>> @@ -224,6 +230,8 @@ struct rte_security_ipsec_xform {
>>>       /**< IPsec SA Mode - transport/tunnel */
>>>       struct rte_security_ipsec_tunnel_param tunnel;
>>>       /**< Tunnel parameters, NULL for transport mode */
>>> +    struct rte_security_ipsec_udp_param udp;
>>> +    /**< UDP parameters, ignored when udp_encap option not specified
>>> */
>>>       uint64_t esn_soft_limit;
>>>       /**< ESN for which the overflow event need to be raised */
>>>       uint32_t replay_win_sz;
>>> -- 
>>> 2.25.1
  
Radu Nicolau Sept. 27, 2021, 9:16 a.m. UTC | #5
On 9/24/2021 10:11 AM, Hemant Agrawal wrote:
>
>
> On 9/6/2021 4:39 PM, Nicolau, Radu wrote:
>>
>> On 9/5/2021 3:19 PM, Akhil Goyal wrote:
>>> Hi Radu,
>>>
>>>> Add support for specifying UDP port params for UDP encapsulation 
>>>> option.
>>>>
>>>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
>>>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
>>> Do we really need to specify the port numbers for NAT-T?
>>> I suppose they are fixed as 4500.
>>> Could you please specify what the user need to set here for session
>>> creation?
>>
>> From what I'm seeing here 
>> https://datatracker.ietf.org/doc/html/rfc3948#section-2.1 there is no 
>> requirement in general for UDP encapsulation so I think it's better 
>> to make the API flexible as to allow any port to be used.
>
>
> This section states that :
>
> o  the Source Port and Destination Port MUST be the same as that used by IKE traffic,
>
> IKE usages port 4500
>
> am I missing something?


I think there's enough confusion in the RFCs so I think it's better to 
keep this option flexible:

For example https://datatracker.ietf.org/doc/html/rfc5996#section-2.23:

>     It is a common practice of NATs to translate TCP and UDP port numbers
>     as well as addresses and use the port numbers of inbound packets to
>     decide which internal node should get a given packet.  For this
>     reason, even though IKE packets MUST be sent to and from UDP port 500
>     or 4500, they MUST be accepted coming from any port and responses
>     MUST be sent to the port from whence they came.  This is because the
>     ports may be modified as the packets pass through NATs.  Similarly,
>     IP addresses of the IKE endpoints are generally not included in the
>     IKE payloads because the payloads are cryptographically protected and
>     could not be transparently modified by NATs.
  
Akhil Goyal Sept. 28, 2021, 7:07 a.m. UTC | #6
RFC states about NAT-T, that it should be 4500 but for UDP encapsulation it does not specify.
Hence it should be generic here.

From: Nicolau, Radu <radu.nicolau@intel.com>
Sent: Monday, September 27, 2021 2:47 PM
To: hemant.agrawal@nxp.com; Akhil Goyal <gakhil@marvell.com>; Declan Doherty <declan.doherty@intel.com>
Cc: dev@dpdk.org; mdr@ashroe.eu; konstantin.ananyev@intel.com; vladimir.medvedkin@intel.com; bruce.richardson@intel.com; roy.fan.zhang@intel.com; Anoob Joseph <anoobj@marvell.com>; abhijit.sinha@intel.com; daniel.m.buckley@intel.com; Archana Muniganti <marchana@marvell.com>; Tejasree Kondoj <ktejasree@marvell.com>; matan@nvidia.com
Subject: Re: [dpdk-dev] [EXT] [PATCH v4 02/10] security: add UDP params for IPsec NAT-T



On 9/24/2021 10:11 AM, Hemant Agrawal wrote:


On 9/6/2021 4:39 PM, Nicolau, Radu wrote:

On 9/5/2021 3:19 PM, Akhil Goyal wrote:

Hi Radu,


Add support for specifying UDP port params for UDP encapsulation option.

Signed-off-by: Declan Doherty <declan.doherty@intel.com><mailto:declan.doherty@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com><mailto:radu.nicolau@intel.com>
Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com><mailto:abhijit.sinha@intel.com>
Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com><mailto:daniel.m.buckley@intel.com>
Do we really need to specify the port numbers for NAT-T?
I suppose they are fixed as 4500.
Could you please specify what the user need to set here for session
creation?

From what I'm seeing here https://datatracker.ietf.org/doc/html/rfc3948#section-2.1<https://urldefense.proofpoint.com/v2/url?u=https-3A__datatracker.ietf.org_doc_html_rfc3948-23section-2D2.1&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=YEEGklabxsppAUjLVd0Lm_8ZiM_fgw7QUDfaRIcXoZA&s=_j1X7QKzxfp4fPOrPr8nYopLrLkcwYElWx0dbrq1fTI&e=> there is no requirement in general for UDP encapsulation so I think it's better to make the API flexible as to allow any port to be used.



This section states that :

o  the Source Port and Destination Port MUST be the same as that used by IKE traffic,



IKE usages port 4500



am I missing something?



I think there's enough confusion in the RFCs so I think it's better to keep this option flexible:

For example https://datatracker.ietf.org/doc/html/rfc5996#section-2.23<https://urldefense.proofpoint.com/v2/url?u=https-3A__datatracker.ietf.org_doc_html_rfc5996-23section-2D2.23&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=YEEGklabxsppAUjLVd0Lm_8ZiM_fgw7QUDfaRIcXoZA&s=t9fLK5bOmzKvHRH63Qdvoma3JtMHmOwjF5FnbvfCmvI&e=>:

   It is a common practice of NATs to translate TCP and UDP port numbers

   as well as addresses and use the port numbers of inbound packets to

   decide which internal node should get a given packet.  For this

   reason, even though IKE packets MUST be sent to and from UDP port 500

   or 4500, they MUST be accepted coming from any port and responses

   MUST be sent to the port from whence they came.  This is because the

   ports may be modified as the packets pass through NATs.  Similarly,

   IP addresses of the IKE endpoints are generally not included in the

   IKE payloads because the payloads are cryptographically protected and

   could not be transparently modified by NATs.
  

Patch

diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index 45896a77d0..03572b10ab 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -112,6 +112,12 @@  struct rte_security_ipsec_tunnel_param {
 	};
 };
 
+struct rte_security_ipsec_udp_param {
+
+	uint16_t sport;
+	uint16_t dport;
+};
+
 /**
  * IPsec Security Association option flags
  */
@@ -224,6 +230,8 @@  struct rte_security_ipsec_xform {
 	/**< IPsec SA Mode - transport/tunnel */
 	struct rte_security_ipsec_tunnel_param tunnel;
 	/**< Tunnel parameters, NULL for transport mode */
+	struct rte_security_ipsec_udp_param udp;
+	/**< UDP parameters, ignored when udp_encap option not specified */
 	uint64_t esn_soft_limit;
 	/**< ESN for which the overflow event need to be raised */
 	uint32_t replay_win_sz;