Skip to content

Commit

Permalink
Fix tcp syn check during TPR validation
Browse files Browse the repository at this point in the history
Summary:
TPR validation after decap didn't parse the tcp flag from packet before checking it. The effect was that it flag wasn't set, effectively validating even SYN packets. Not a huge concern, but stil better to fix it.
Fixed the parsing and also added a test to verify.

Reviewed By: avasylev

Differential Revision: D50098977

fbshipit-source-id: 3dc3ade9a7691ef5f4f15ecea212aa4f7f9cf612
  • Loading branch information
Nikhil Dixit Limaye authored and facebook-github-bot committed Oct 11, 2023
1 parent 1dd11cf commit 8d34c0f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
12 changes: 10 additions & 2 deletions katran/decap/bpf/decap.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,16 @@ __attribute__((__always_inline__)) static inline void validate_tpr_server_id(
is_ipv6) >= 0) {
return;
}
// only check for TCP non SYN packets
if (inner_protocol == IPPROTO_TCP && !(inner_pckt.flags & F_SYN_SET)) {
// only check for TCP
if (inner_protocol != IPPROTO_TCP) {
return;
}
// parse tcp header for flags
if (!parse_tcp(data, data_end, is_ipv6, &inner_pckt)) {
return;
}
// check for TCP non SYN packets
if (!(inner_pckt.flags & F_SYN_SET)) {
// lookup server id from tpr header option and compare against server_id on
// this host (if available)
__u32 s_key = 0;
Expand Down
24 changes: 24 additions & 0 deletions katran/decap/testing/XdpDecapGueTestFixtures.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,30 @@ const std::vector<PacketAttributes> gueTestFixtures = {
.expectedReturnValue = "XDP_PASS",
.expectedOutputPacket = "AgAAAAAAAQAAAAAACABFAAA/AAEAAEAGrUbAqAEBCsgBAXppAFAAAAAAAAAAAHAQIACI1AAAtwbIAAAAAABrYXRyYW4gdGVzdCBwa3Q="
},
// 15
{ // data_value = int(200).to_bytes(4, byteorder='little')
// Ether(src="0x1", dst="0x2")/IPv6(src="100::1", dst="100::2")/UDP(sport=1337, dport=9886)/IP(src="192.168.1.1", dst="10.200.1.1")/TCP(sport=31337, dport=80,flags="S", options=[(0xB7, data_value)])/"katran test pkt"
.inputPacket = "AgAAAAAAAQAAAAAAht1gAAAAAEcRQAEAAAAAAAAAAAAAAAAAAAEBAAAAAAAAAAAAAAAAAAACBTkmngBHnypFAAA/AAEAAEAGrUbAqAEBCsgBAXppAFAAAAAAAAAAAHACIACI4gAAtwbIAAAAAABrYXRyYW4gdGVzdCBwa3Q=",
.description = "ipv6 gue ipv4 innner with TPR option set on SYN packet. We decap the packet but it should't be checked for TPR option",
.expectedReturnValue = "XDP_PASS",
.expectedOutputPacket = "AgAAAAAAAQAAAAAACABFAAA/AAEAAEAGrUbAqAEBCsgBAXppAFAAAAAAAAAAAHACIACI4gAAtwbIAAAAAABrYXRyYW4gdGVzdCBwa3Q="
},
// 16
{ // data_value = int(200).to_bytes(4, byteorder='little')
// Ether(src="0x1", dst="0x2")/IPv6(src="100::1", dst="100::2")/UDP(sport=1337, dport=9886)/IP(src="192.168.1.1", dst="10.200.1.1")/TCP(sport=31337, dport=80,flags="S", options=[(0xB7, data_value)])/"katran test pkt"
.inputPacket = "AgAAAAAAAQAAAAAAht1gAAAAAFsRQAEAAAAAAAAAAAAAAAAAAAEBAAAAAAAAAAAAAAAAAAACBTkmngBbayRgAAAAACsGQPwAAAIAAAAAAAAAAAAAAAH8AAABAAAAAAAAAAAAAAABemkAUAAAAAAAAAAAcAIgAF5OAAC3BsgAAAAAAGthdHJhbiB0ZXN0IHBrdA==",
.description = "ipv6 gue ipv6 innner with TPR option set on SYN packet. We decap the packet but it should't be checked for TPR option",
.expectedReturnValue = "XDP_PASS",
.expectedOutputPacket = "AgAAAAAAAQAAAAAAht1gAAAAACsGQPwAAAIAAAAAAAAAAAAAAAH8AAABAAAAAAAAAAAAAAABemkAUAAAAAAAAAAAcAIgAF5OAAC3BsgAAAAAAGthdHJhbiB0ZXN0IHBrdA=="
},
// 17
{ // data_value = int(100).to_bytes(4, byteorder='little')
// Ether(src="0x1", dst="0x2")/IPv6(src="100::1", dst="100::2")/UDP(sport=1337, dport=9886)/IP(src="192.168.1.1", dst="10.200.1.1")/TCP(sport=31337, dport=80,flags="S", options=[(0xB7, data_value)])/"katran test pkt"
.inputPacket = "AgAAAAAAAQAAAAAAht1gAAAAAFsRQAEAAAAAAAAAAAAAAAAAAAEBAAAAAAAAAAAAAAAAAAACBTkmngBbayRgAAAAACsGQPwAAAIAAAAAAAAAAAAAAAH8AAABAAAAAAAAAAAAAAABemkAUAAAAAAAAAAAcAggAF5IAAC3BsgAAAAAAGthdHJhbiB0ZXN0IHBrdA==",
.description = "ipv6 gue ipv6 innner with TPR option set on Push packet, TPR misrouted",
.expectedReturnValue = "XDP_PASS",
.expectedOutputPacket = "AgAAAAAAAQAAAAAAht1gAAAAACsGQPwAAAIAAAAAAAAAAAAAAAH8AAABAAAAAAAAAAAAAAABemkAUAAAAAAAAAAAcAggAF5IAAC3BsgAAAAAAGthdHJhbiB0ZXN0IHBrdA=="
}
};

} // namespace testing
Expand Down
8 changes: 4 additions & 4 deletions katran/decap/testing/xdpdecap_tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ void testXdpDecapCounters(katran::XdpDecap& decap) {
LOG(INFO) << "Testing counter's sanity";
auto stats = decap.getXdpDecapStats();
int expectedV4DecapPkts = 1;
int expectedV6DecapPkts = FLAGS_gue ? 6 : 2;
int expectedTotalPkts = FLAGS_gue ? 7 : 7;
int expectedTotalTPRPkts = 3;
int expectedMisroutedTPRPkts = 2;
int expectedV6DecapPkts = FLAGS_gue ? 9 : 2;
int expectedTotalPkts = FLAGS_gue ? 10 : 7;
int expectedTotalTPRPkts = 4;
int expectedMisroutedTPRPkts = 3;
if (stats.decap_v4 != expectedV4DecapPkts ||
stats.decap_v6 != expectedV6DecapPkts ||
stats.total != expectedTotalPkts ||
Expand Down

0 comments on commit 8d34c0f

Please sign in to comment.