From 0f88200247cc61175c7a1d98a3e935aedce93f3b Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 18 Nov 2019 17:44:34 +0100 Subject: [PATCH] BGP: Fix processing of IPv6 Flowspec During NLRI parsing of IPv6 Flowspec, dst prefix was not properly extracted from NLRI, therefore a received flow was stored in a different position in flowspec routing table, and was not reachable by command 'show route '. Add proper prefix part accessors to flowspec code and use them from BGP NLRI parsing code. Thanks to Alex D. for the bugreport. --- lib/flowspec.c | 83 +++++++++++++++++++++++++++------------------ lib/flowspec.h | 8 +++++ lib/flowspec_test.c | 54 ++++++++++++++++++++++++++++- proto/bgp/packets.c | 12 ++----- 4 files changed, 113 insertions(+), 44 deletions(-) diff --git a/lib/flowspec.c b/lib/flowspec.c index 87ce0206..e9290b88 100644 --- a/lib/flowspec.c +++ b/lib/flowspec.c @@ -112,7 +112,6 @@ get_value_length(const byte *op) } - /* * Flowspec iterators */ @@ -245,6 +244,45 @@ flow6_next_part(const byte *pos, const byte *end) } +/* + * Flowspec accessors + */ + + +static inline ip4_addr +flow_read_ip4(const byte *px, uint pxlen) +{ + ip4_addr ip = IP4_NONE; + memcpy(&ip, px, BYTES(pxlen)); + return ip4_ntoh(ip); +} + +ip4_addr +flow_read_ip4_part(const byte *part) +{ + return flow_read_ip4(part + 2, part[1]); +} + +static inline ip6_addr +flow_read_ip6(const byte *px, uint pxlen, uint pxoffset) +{ + uint floor_offset = BYTES(pxoffset - (pxoffset % 8)); + uint ceil_len = BYTES(pxlen); + ip6_addr ip = IP6_NONE; + + memcpy(((byte *) &ip) + floor_offset, px, ceil_len - floor_offset); + + return ip6_ntoh(ip); +} + +ip6_addr +flow_read_ip6_part(const byte *part) +{ + return flow_read_ip6(part + 3, part[1], part[2]); +} + + + /* * Flowspec validation */ @@ -779,26 +817,6 @@ flow_builder_set_type(struct flow_builder *fb, enum flow_type type) fb->this_type = type; } -static ip4_addr -flow_read_ip4(const byte *px, uint pxlen) -{ - ip4_addr ip = IP4_NONE; - memcpy(&ip, px, BYTES(pxlen)); - return ip4_ntoh(ip); -} - -static ip6_addr -flow_read_ip6(const byte *px, uint pxlen, uint pxoffset) -{ - uint floor_offset = BYTES(pxoffset - (pxoffset % 8)); - uint ceil_len = BYTES(pxlen); - ip6_addr ip = IP6_NONE; - - memcpy(((byte *) &ip) + floor_offset, px, ceil_len - floor_offset); - - return ip6_ntoh(ip); -} - static void builder_write_parts(struct flow_builder *fb, byte *buf) { @@ -831,9 +849,9 @@ flow_builder4_finalize(struct flow_builder *fb, linpool *lpool) if (fb->parts[FLOW_TYPE_DST_PREFIX].length) { - byte *p = fb->data.data + fb->parts[FLOW_TYPE_DST_PREFIX].offset + 1; - pxlen = *p++; - prefix = flow_read_ip4(p, pxlen); + byte *part = fb->data.data + fb->parts[FLOW_TYPE_DST_PREFIX].offset; + prefix = flow_read_ip4_part(part); + pxlen = part[1]; } *f = NET_ADDR_FLOW4(prefix, pxlen, data_len); @@ -861,10 +879,9 @@ flow_builder6_finalize(struct flow_builder *fb, linpool *lpool) if (fb->parts[FLOW_TYPE_DST_PREFIX].length) { - byte *p = fb->data.data + fb->parts[FLOW_TYPE_DST_PREFIX].offset + 1; - pxlen = *p++; - uint pxoffset = *p++; - prefix = flow_read_ip6(p, pxlen, pxoffset); + byte *part = fb->data.data + fb->parts[FLOW_TYPE_DST_PREFIX].offset; + prefix = flow_read_ip6_part(part); + pxlen = part[1]; } *n = NET_ADDR_FLOW6(prefix, pxlen, data_len); @@ -947,18 +964,18 @@ fragment_val_str(u8 val) static void net_format_flow_ip(buffer *b, const byte *part, int ipv6) { - uint pxlen = *(part+1); + uint pxlen = part[1]; if (ipv6) { - uint pxoffset = *(part+2); + uint pxoffset = part[2]; if (pxoffset) - buffer_print(b, "%I6/%u offset %u; ", flow_read_ip6(part+3,pxlen,pxoffset), pxlen, pxoffset); + buffer_print(b, "%I6/%u offset %u; ", flow_read_ip6_part(part), pxlen, pxoffset); else - buffer_print(b, "%I6/%u; ", flow_read_ip6(part+3,pxlen,0), pxlen); + buffer_print(b, "%I6/%u; ", flow_read_ip6_part(part), pxlen); } else { - buffer_print(b, "%I4/%u; ", flow_read_ip4(part+2,pxlen), pxlen); + buffer_print(b, "%I4/%u; ", flow_read_ip4_part(part), pxlen); } } diff --git a/lib/flowspec.h b/lib/flowspec.h index fa90c70d..d486cda0 100644 --- a/lib/flowspec.h +++ b/lib/flowspec.h @@ -85,6 +85,14 @@ const byte *flow4_next_part(const byte *pos, const byte *end); const byte *flow6_next_part(const byte *pos, const byte *end); +/* + * Flowspec accessors + */ + +ip4_addr flow_read_ip4_part(const byte *part); +ip6_addr flow_read_ip6_part(const byte *part); + + /* * Flowspec Builder */ diff --git a/lib/flowspec_test.c b/lib/flowspec_test.c index dd71dc7b..bff33e7e 100644 --- a/lib/flowspec_test.c +++ b/lib/flowspec_test.c @@ -137,7 +137,7 @@ static int t_iterators6(void) { net_addr_flow6 *f; - NET_ADDR_FLOW6_(f, ip6_build(0,0,0x12345678,0x9a000000), 64, ((byte[]) { + NET_ADDR_FLOW6_(f, ip6_build(0,0,0x12345678,0x9a000000), 0x68, ((byte[]) { 26, /* Length */ FLOW_TYPE_DST_PREFIX, 0x68, 0x40, 0x12, 0x34, 0x56, 0x78, 0x9a, FLOW_TYPE_SRC_PREFIX, 0x08, 0x0, 0xc0, @@ -166,6 +166,56 @@ t_iterators6(void) return 1; } +static int +t_accessors4(void) +{ + net_addr_flow4 *f; + NET_ADDR_FLOW4_(f, ip4_build(5,6,7,0), 24, ((byte[]) { + 25, /* Length */ + FLOW_TYPE_DST_PREFIX, 24, 5, 6, 7, + FLOW_TYPE_SRC_PREFIX, 32, 10, 11, 12, 13, + FLOW_TYPE_IP_PROTOCOL, 0x81, 0x06, + FLOW_TYPE_PORT, 0x03, 0x89, 0x45, 0x8b, 0x91, 0x1f, 0x90, + FLOW_TYPE_TCP_FLAGS, 0x80, 0x55, + })); + + const byte *p1_dst_px = &f->data[1]; + const ip4_addr p1_dst_addr = ip4_build(5,6,7,0); + + const byte *p2_src_px = &f->data[6]; + const ip4_addr p2_src_addr = ip4_build(10,11,12,13); + + bt_assert(ip4_equal(flow_read_ip4_part(p1_dst_px), p1_dst_addr)); + bt_assert(ip4_equal(flow_read_ip4_part(p2_src_px), p2_src_addr)); + + return 1; +} + +static int +t_accessors6(void) +{ + net_addr_flow6 *f; + NET_ADDR_FLOW6_(f, ip6_build(0,0,0x12345678,0x9a000000), 0x68, ((byte[]) { + 26, /* Length */ + FLOW_TYPE_DST_PREFIX, 0x68, 0x40, 0x12, 0x34, 0x56, 0x78, 0x9a, + FLOW_TYPE_SRC_PREFIX, 0x08, 0x0, 0xc0, + FLOW_TYPE_NEXT_HEADER, 0x81, 0x06, + FLOW_TYPE_PORT, 0x03, 0x89, 0x45, 0x8b, 0x91, 0x1f, 0x90, + FLOW_TYPE_LABEL, 0x80, 0x55, + })); + + const byte *p1_dst_px = &f->data[1]; + const ip6_addr p1_dst_addr = ip6_build(0,0,0x12345678,0x9a000000); + + const byte *p2_src_px = &f->data[9]; + const ip6_addr p2_src_addr = ip6_build(0xc0000000, 0, 0, 0); + + bt_assert(ip6_equal(flow_read_ip6_part(p1_dst_px), p1_dst_addr)); + bt_assert(ip6_equal(flow_read_ip6_part(p2_src_px), p2_src_addr)); + + return 1; +} + static int t_validation4(void) { @@ -628,6 +678,8 @@ main(int argc, char *argv[]) bt_test_suite(t_first_part, "Searching first part in net_addr_flow"); bt_test_suite(t_iterators4, "Testing iterators (IPv4)"); bt_test_suite(t_iterators6, "Testing iterators (IPv6)"); + bt_test_suite(t_accessors4, "Testing accessors (IPv4)"); + bt_test_suite(t_accessors6, "Testing accessors (IPv6)"); bt_test_suite(t_validation4, "Testing validation (IPv4)"); bt_test_suite(t_validation6, "Testing validation (IPv6)"); bt_test_suite(t_builder4, "Inserting components into existing Flow Specification (IPv4)"); diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c index c3bd600a..0bc63c55 100644 --- a/proto/bgp/packets.c +++ b/proto/bgp/packets.c @@ -1831,13 +1831,9 @@ bgp_decode_nlri_flow4(struct bgp_parse_state *s, byte *pos, uint len, rta *a) } /* Decode dst prefix */ - ip4_addr px = IP4_NONE; + ip4_addr px = flow_read_ip4_part(data); uint pxlen = data[1]; - // FIXME: Use some generic function - memcpy(&px, data+2, BYTES(pxlen)); - px = ip4_and(ip4_ntoh(px), ip4_mkmask(pxlen)); - /* Prepare the flow */ net_addr *n = alloca(sizeof(struct net_addr_flow4) + flen); net_fill_flow4(n, px, pxlen, pos, flen); @@ -1923,13 +1919,9 @@ bgp_decode_nlri_flow6(struct bgp_parse_state *s, byte *pos, uint len, rta *a) } /* Decode dst prefix */ - ip6_addr px = IP6_NONE; + ip6_addr px = flow_read_ip6_part(data); uint pxlen = data[1]; - // FIXME: Use some generic function - memcpy(&px, data+2, BYTES(pxlen)); - px = ip6_and(ip6_ntoh(px), ip6_mkmask(pxlen)); - /* Prepare the flow */ net_addr *n = alloca(sizeof(struct net_addr_flow6) + flen); net_fill_flow6(n, px, pxlen, pos, flen);