odp icon indicating copy to clipboard operation
odp copied to clipboard

[PATCH v5] api: ipsec: require more packet metadata in IPsec operations

Open JannePeltonen opened this issue 3 years ago • 13 comments

Require that input packets to IPsec operations have valid L3 type and L4 offset (except in outbound tunnel mode without L4 checksum offload) metadata in addition to L3 offset.

This can help implementations to be more efficient as they no longer need to parse the IP header.

This also resolves the ambiguity on whether possible destination options extension headers are to be included or excluded from IPsec encapsulation (the RFCs allow both) in transport mode. Now the provided L4 offset explicitly indicates the part of the packet to follow the IPsec header.

Signed-off-by: Janne Peltonen [email protected]

JannePeltonen avatar Jul 19 '21 13:07 JannePeltonen

Looks good to me. Reviewed-by: Nithin Dabilpuram [email protected]

@vvelumuri @asasidharan @anoobj

nithind1988 avatar Aug 02 '21 14:08 nithind1988

v2: Clarified when L4 offset does not need to be set, as proposed by @psavol .

JannePeltonen avatar Aug 24 '21 09:08 JannePeltonen

It seems to me that L4 type should be required too for outbound transport mode, at least for IPv6. Having just L4 offset readily available does not help the implementation avoid parsing the whole set of IPv6 headers since the payload protocol needs to be known for transport mode (so it can be written in the next header field of the ESP trailer). Or would this API change be useful to you with just L4 offset without L4 type?

If L4 type is required, there is the problem that there is no generic L4 type metadata but a bunch of L4 flags for the most common types. It would not be possible to apply IPsec to other payload protocols. An alternative would be pass the L4 type (i.e. the IPsec payload protocol) through IPsec output operation parameters. It would probably have to be in odp_ipsec_out_opt_t. It looks a bit messy. Yet another alternative would be to scratch this API change and keep the current API that requires only L3 offset and makes the implementation parse the headers.

JannePeltonen avatar Sep 02 '21 15:09 JannePeltonen

It seems to me that L4 type should be required too for outbound transport mode, at least for IPv6. Having just L4 offset readily available does not help the implementation avoid parsing the whole set of IPv6 headers since the payload protocol needs to be known for transport mode (so it can be written in the next header field of the ESP trailer). Or would this API change be useful to you with just L4 offset without L4 type?

If L4 type is required, there is the problem that there is no generic L4 type metadata but a bunch of L4 flags for the most common types. It would not be possible to apply IPsec to other payload protocols. An alternative would be pass the L4 type (i.e. the IPsec payload protocol) through IPsec output operation parameters. It would probably have to be in odp_ipsec_out_opt_t. It looks a bit messy. Yet another alternative would be to scratch this API change and keep the current API that requires only L3 offset and makes the implementation parse the headers.

For our platform l4 type is not needed and l4 offset is sufficient because our HW does read packet content from L3 header. By this spec change, we can avoid reading/parsing the packet in SW before submitting to HW. And the requirement for knowing l4 offset comes from the fact that we need to know in SW what is the final packet size post encryption to be able to get it ready for transmission on NIX. I agree that it probably doesn't make any difference in linux-generic if l4 type is not given but even part of metadata might be useful for HW platforms. In future if some platform l4 type as well then we can think of expanding the spec ?

Also regarding

Now the provided L4 offset explicitly indicates the part of the packet to follow the IPsec header.

Can we have a capability field as well to indicate if a platform supports it ? Today, our platform always inserts ipsec header in transport mode at a predefined offset based on RFC 4303 and we cannot really provide offset per pkt.

Also, ideally if we are talking about L4 offset, then pointing it to a IPv6 extension header doesn't align with the spec ? Maybe it should be some other offset in this case and L4 should always point to protocol after IPv6 such as TCP/UDP etc.

nithind1988 avatar Sep 07 '21 09:09 nithind1988

I still wonder where you would like the L4 offset to point to in IPv6 transport mode when the packet has a destination options extension header. To the destination option extension header or to the actual L4 protocol header?

The RFC says that destination options extension header can appear in a transport mode packet before or after the IPsec header. Which way is it in your implementation currently? The IPsec header cannot be at a predefined offset from the start of the IP packets because of the varying number of bytes in possible extension headers that are to be placed before the IPsec header.

You mentioned that L4 offset would help your implementation to figure out the final length of the ESP packet. If the destination options extension header is put after the ESP header in your implementation, then I would think that you would need L4 offset to point to the extension header and not the final L4 payload or otherwise your length calculation based on the L4 offset would be off.

JannePeltonen avatar Sep 08 '21 12:09 JannePeltonen

In our HW, we have the following modes choosable able per SA Mode #1

  • Mobility header is not supported.
  • If packet has a destination header as last extension header, then ESP will be inserted before that destination options header. Otherwise, ESP header will be inserted just before L4 header.

Mode #2

  • Mobility header is supported
  • Without mobility header, ESP will be always inserted before L4 header.
  • If packet has mobility header, then ESP will be inserted before mobility header. Destination options header will never be part of ESP payload with or without mobility header present.

Based on RFC https://datatracker.ietf.org/doc/html/rfc3776, Mobility header should be part of ESP payload and Destination options outside ESP payload which is mode #2 Our platform can default to mode #2 given today there is no requirement or SA params to choose such a mode. And L4 offset can point to either L4 header or mobility header in our case.

nithind1988 avatar Sep 21 '21 07:09 nithind1988

Based on RFC https://datatracker.ietf.org/doc/html/rfc3776, Mobility header should be part of ESP payload and Destination options outside ESP payload which is mode #2

The destination options header that carries the home address option should indeed come before ESP, but that does not mean that other destination options in other contexts than MIPv6 would not be protected by ESP.

Anyway, I think we could leave the L4 offset requirement pretty much as currently written in this PR, even if it can then express things that are not supported by all implementations. And I think from input side we can remove all additional requirements beside the L3 offset, as discussed.

JannePeltonen avatar Sep 21 '21 09:09 JannePeltonen

Anyway, I think we could leave the L4 offset requirement pretty much as currently written in this PR, even if it can then express things that are not supported by all implementations. And I think from input side we can remove all additional requirements beside the L3 offset, as discussed.

Ack.

nithind1988 avatar Sep 21 '21 09:09 nithind1988

v3:

  • Small API tweaks (L4 offset not required for non-UDP-encapsulated inbound packets)
  • Included validation and perf test program changes to comply with the changed API

JannePeltonen avatar Sep 28 '21 10:09 JannePeltonen

v4: rebased

JannePeltonen avatar Oct 01 '21 08:10 JannePeltonen

Reviewed-by: Nithin Dabilpuram [email protected]

nithind1988 avatar Oct 26 '21 21:10 nithind1988

v5: dropped L3 type requirement and the corresponding changes.

JannePeltonen avatar Nov 09 '21 10:11 JannePeltonen

Reviewed-by: Nithin Dabilpuram [email protected]

nithind1988 avatar Nov 10 '21 05:11 nithind1988