'conn_key_extract()' in userspace conntrack is including L2
(Ethernet) pad bytes for both L3 and L4 sizes. One problem is
any packet with non-zero L2 padding can incorrectly fail L4
checksum validation.

This patch fixes conn_key_extract() by ignoring L2 pad bytes.

Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
CC: Daniele Di Proietto <diproiet...@ovn.org>
Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajm...@ericsson.com>
Co-authored-by: Venkatesan Pradeep <venkatesan.prad...@ericsson.com>
Co-authored-by: Nitin Katiyar <nitin.kati...@ericsson.com>
Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajm...@ericsson.com>
Signed-off-by: Venkatesan Pradeep <venkatesan.prad...@ericsson.com>
Signed-off-by: Nitin Katiyar <nitin.kati...@ericsson.com>
Signed-off-by: Darrell Ball <dlu...@gmail.com>
---
 lib/conntrack.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 4284770..1917fd8 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -897,6 +897,9 @@ extract_l4_icmp6(struct conn_key *key, const void *data, 
size_t size,
  * processed, the function will extract the key from the packet nested
  * in the ICMP paylod and set '*related' to true.
  *
+ * 'size' here is the layer 4 size, which can be a nested size if parsing
+ * an ICMP or ICMP6 header. *
+ *
  * If 'related' is NULL, it means that we're already parsing a header nested
  * in an ICMP error.  In this case, we skip checksum and length validation. */
 static inline bool
@@ -929,7 +932,6 @@ conn_key_extract(struct conntrack *ct, struct dp_packet 
*pkt, ovs_be16 dl_type,
     const struct eth_header *l2 = dp_packet_l2(pkt);
     const struct ip_header *l3 = dp_packet_l3(pkt);
     const char *l4 = dp_packet_l4(pkt);
-    const char *tail = dp_packet_tail(pkt);
     bool ok;
 
     memset(ctx, 0, sizeof *ctx);
@@ -974,15 +976,17 @@ conn_key_extract(struct conntrack *ct, struct dp_packet 
*pkt, ovs_be16 dl_type,
      */
     ctx->key.dl_type = dl_type;
     if (ctx->key.dl_type == htons(ETH_TYPE_IP)) {
-        ok = extract_l3_ipv4(&ctx->key, l3, tail - (char *) l3, NULL, true);
+        ok = extract_l3_ipv4(&ctx->key, l3, dp_packet_l3_size(pkt), NULL,
+                             true);
     } else if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
-        ok = extract_l3_ipv6(&ctx->key, l3, tail - (char *) l3, NULL);
+        ok = extract_l3_ipv6(&ctx->key, l3,  dp_packet_l3_size(pkt), NULL);
     } else {
         ok = false;
     }
 
     if (ok) {
-        if (extract_l4(&ctx->key, l4, tail - l4, &ctx->related, l3)) {
+        if (extract_l4(&ctx->key, l4,  dp_packet_l4_size(pkt),
+                       &ctx->related, l3)) {
             ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis);
             return true;
         }
-- 
1.9.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to