From 28e8abbf19acaabd884735915e2d2d6156bcff94 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Wed, 18 Aug 2021 09:17:57 +0200 Subject: [PATCH] Fix option length computation in Geneve analyzer. We previously computed the length of the Geneve options field incorrectly which lead to us passing data at an incorrect offset to inner analyzers. With this patch we now interpret the length field correctly, according the the spec https://datatracker.ietf.org/doc/html/rfc8926#section-3.4. Closes #1726. --- src/analyzer/protocol/geneve/Geneve.cc | 2 +- testing/btest/Baseline/core.tunnels.geneve/conn.log | 6 +++--- testing/btest/Baseline/core.tunnels.geneve/out | 3 +++ testing/btest/Baseline/core.tunnels.geneve/tunnel.log | 6 ++++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/analyzer/protocol/geneve/Geneve.cc b/src/analyzer/protocol/geneve/Geneve.cc index c3db3109bb..17a058551a 100644 --- a/src/analyzer/protocol/geneve/Geneve.cc +++ b/src/analyzer/protocol/geneve/Geneve.cc @@ -47,7 +47,7 @@ void Geneve_Analyzer::DeliverPacket(int len, const u_char* data, bool orig, EncapsulatingConn inner(Conn(), BifEnum::Tunnel::GENEVE); outer->Add(inner); - auto tunnel_opt_len = data[0] << 1; + uint8_t tunnel_opt_len = (data[0] & 0x3F) * 4; auto vni = (data[4] << 16) + (data[5] << 8) + (data[6] << 0); if ( len < tunnel_header_len + tunnel_opt_len ) diff --git a/testing/btest/Baseline/core.tunnels.geneve/conn.log b/testing/btest/Baseline/core.tunnels.geneve/conn.log index 7afcacef10..a4fb26d63c 100644 --- a/testing/btest/Baseline/core.tunnels.geneve/conn.log +++ b/testing/btest/Baseline/core.tunnels.geneve/conn.log @@ -7,7 +7,7 @@ #open XXXX-XX-XX-XX-XX-XX #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents #types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] -XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 20.0.0.2 0 20.0.0.1 6081 udp geneve 1.999999 318 0 S0 - - 0 D 3 402 0 0 - -XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 20.0.0.1 50901 20.0.0.2 6081 udp - 1.999995 342 0 S0 - - 0 D 3 426 0 0 - -XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 30.0.0.2 0 30.0.0.1 8 icmp - 1.999999 168 0 OTH - - 0 - 3 252 0 0 ClEkJM2Vm5giqnMf4h +XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 20.0.0.2 0 20.0.0.1 6081 udp geneve 1.999999 318 0 S0 - - 0 D 3 402 0 0 - +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 20.0.0.1 50901 20.0.0.2 6081 udp geneve 1.999995 342 0 S0 - - 0 D 3 426 0 0 - +XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 30.0.0.1 8 30.0.0.2 0 icmp - 2.000182 168 168 OTH - - 0 - 3 252 3 252 CHhAvVGS1DHFjwGM9,C4J4Th3PJpwUYZZ6gc #close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/core.tunnels.geneve/out b/testing/btest/Baseline/core.tunnels.geneve/out index 9518466dc3..05bb572661 100644 --- a/testing/btest/Baseline/core.tunnels.geneve/out +++ b/testing/btest/Baseline/core.tunnels.geneve/out @@ -1,4 +1,7 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +geneve_packet, [orig_h=20.0.0.1, orig_p=50901/udp, resp_h=20.0.0.2, resp_p=6081/udp], [ip=[hl=20, tos=0, len=84, id=0, ttl=64, p=1, src=30.0.0.1, dst=30.0.0.2], ip6=, tcp=, udp=, icmp=[icmp_type=8]], 0 geneve_packet, [orig_h=20.0.0.2, orig_p=0/udp, resp_h=20.0.0.1, resp_p=6081/udp], [ip=[hl=20, tos=0, len=84, id=4503, ttl=64, p=1, src=30.0.0.2, dst=30.0.0.1], ip6=, tcp=, udp=, icmp=[icmp_type=0]], 0 +geneve_packet, [orig_h=20.0.0.1, orig_p=50901/udp, resp_h=20.0.0.2, resp_p=6081/udp], [ip=[hl=20, tos=0, len=84, id=0, ttl=64, p=1, src=30.0.0.1, dst=30.0.0.2], ip6=, tcp=, udp=, icmp=[icmp_type=8]], 0 geneve_packet, [orig_h=20.0.0.2, orig_p=0/udp, resp_h=20.0.0.1, resp_p=6081/udp], [ip=[hl=20, tos=0, len=84, id=4504, ttl=64, p=1, src=30.0.0.2, dst=30.0.0.1], ip6=, tcp=, udp=, icmp=[icmp_type=0]], 0 +geneve_packet, [orig_h=20.0.0.1, orig_p=50901/udp, resp_h=20.0.0.2, resp_p=6081/udp], [ip=[hl=20, tos=0, len=84, id=0, ttl=64, p=1, src=30.0.0.1, dst=30.0.0.2], ip6=, tcp=, udp=, icmp=[icmp_type=8]], 0 geneve_packet, [orig_h=20.0.0.2, orig_p=0/udp, resp_h=20.0.0.1, resp_p=6081/udp], [ip=[hl=20, tos=0, len=84, id=4505, ttl=64, p=1, src=30.0.0.2, dst=30.0.0.1], ip6=, tcp=, udp=, icmp=[icmp_type=0]], 0 diff --git a/testing/btest/Baseline/core.tunnels.geneve/tunnel.log b/testing/btest/Baseline/core.tunnels.geneve/tunnel.log index e838657093..79cf36fd42 100644 --- a/testing/btest/Baseline/core.tunnels.geneve/tunnel.log +++ b/testing/btest/Baseline/core.tunnels.geneve/tunnel.log @@ -7,6 +7,8 @@ #open XXXX-XX-XX-XX-XX-XX #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p tunnel_type action #types time string addr port addr port enum enum -XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 20.0.0.2 0 20.0.0.1 6081 Tunnel::GENEVE Tunnel::DISCOVER -XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 20.0.0.2 0 20.0.0.1 6081 Tunnel::GENEVE Tunnel::CLOSE +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 20.0.0.1 50901 20.0.0.2 6081 Tunnel::GENEVE Tunnel::DISCOVER +XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 20.0.0.2 0 20.0.0.1 6081 Tunnel::GENEVE Tunnel::DISCOVER +XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 20.0.0.2 0 20.0.0.1 6081 Tunnel::GENEVE Tunnel::CLOSE +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 20.0.0.1 50901 20.0.0.2 6081 Tunnel::GENEVE Tunnel::CLOSE #close XXXX-XX-XX-XX-XX-XX