Hi BIRD maintainers, I was messing with BGP flowspec when I encountered BIRD's inconsistencies with RFC. Details and patch is attached below. Thanks! Cheers, Eric From 3008eeb2d4df45b2a4ac78b038f7c830b7495a88 Mon Sep 17 00:00:00 2001 From: Eric Long <i at hack3r dot moe> Date: Mon, 14 Oct 2024 00:30:13 +0800 Subject: [PATCH] Fix BGP IPv6 flowspec prefix when offset is not multiple of 8 Current implementation handles flowspec prefix length and offset only in bytes, but RFC 8956 (Dissemination of Flow Specification Rules for IPv6) Section 3.1 [1] and example in Section 3.8.2 [2] states the pattern should begin right after offset *bits*. For example, pattern "::1:1234:5678:9800:0/60-104" is currently serialized as "02 68 3c 01 12 34 56 78 98", but it should shift its pattern 4 more bits to the left: "02 68 3c 11 23 45 67 89 80". This patch implements shifting left/right for IPv6 type and use it to correct the behaviour. Test data are replaced with the correct ones. [1]: https://www.rfc-editor.org/rfc/rfc8956.html#section-3.1 [2]: https://www.rfc-editor.org/rfc/rfc8956.html#section-3.8.2 --- lib/flowspec.c | 12 +++++------ lib/flowspec_test.c | 4 ++-- lib/ip.h | 50 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/lib/flowspec.c b/lib/flowspec.c index df5c8da3..6e693def 100644 --- a/lib/flowspec.c +++ b/lib/flowspec.c @@ -293,13 +293,11 @@ flow_read_ip4_part(const byte *part) 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); + memcpy(((byte *) &ip), px, BYTES(pxlen - pxoffset)); - return ip6_ntoh(ip); + return ip6_shift_right(ip6_ntoh(ip), pxoffset); } ip6_addr @@ -476,7 +474,7 @@ flow_validate(const byte *nlri, uint len, int ipv6) uint pxoffset = *pos++; if (pxoffset > IP6_MAX_PREFIX_LENGTH || pxoffset > pxlen) return FLOW_ST_EXCEED_MAX_PREFIX_OFFSET; - bytes -= pxoffset / 8; + bytes = BYTES(pxlen - pxoffset); } pos += bytes; @@ -749,12 +747,12 @@ flow_builder6_add_pfx(struct flow_builder *fb, const net_addr_ip6 *n6, u32 pxoff if (!builder_add_prepare(fb)) return 0; - ip6_addr ip6 = ip6_hton(n6->prefix); + ip6_addr ip6 = ip6_hton(ip6_shift_left(n6->prefix, pxoffset)); BUFFER_PUSH(fb->data) = fb->this_type; BUFFER_PUSH(fb->data) = n6->pxlen; BUFFER_PUSH(fb->data) = pxoffset; - push_pfx_to_buffer(fb, BYTES(n6->pxlen) - (pxoffset / 8), ((byte *) &ip6) + (pxoffset / 8)); + push_pfx_to_buffer(fb, BYTES(n6->pxlen - pxoffset), ((byte *) &ip6)); builder_add_finish(fb); return 1; diff --git a/lib/flowspec_test.c b/lib/flowspec_test.c index 03649b99..ae7444b2 100644 --- a/lib/flowspec_test.c +++ b/lib/flowspec_test.c @@ -533,7 +533,7 @@ t_builder6(void) byte nlri[] = { 27, - FLOW_TYPE_DST_PREFIX, 103, 61, 0x01, 0x12, 0x34, 0x56, 0x78, 0x98, + FLOW_TYPE_DST_PREFIX, 103, 61, 0x22, 0x46, 0x8a, 0xcf, 0x13, 0x00, FLOW_TYPE_SRC_PREFIX, 8, 0, 0xc0, FLOW_TYPE_NEXT_HEADER, 0x80, 0x06, FLOW_TYPE_PORT, 0x03, 0x89, 0x45, 0x8b, 0x91, 0x1f, 0x90, @@ -641,7 +641,7 @@ t_formatting6(void) byte nlri[] = { 0, - FLOW_TYPE_DST_PREFIX, 103, 61, 0x01, 0x12, 0x34, 0x56, 0x78, 0x98, + FLOW_TYPE_DST_PREFIX, 103, 61, 0x22, 0x46, 0x8a, 0xcf, 0x13, 0x00, FLOW_TYPE_SRC_PREFIX, 8, 0, 0xc0, FLOW_TYPE_NEXT_HEADER, 0x81, 0x06, FLOW_TYPE_PORT, 0x03, 20, 0x45, 40, 0x91, 0x01, 0x11, diff --git a/lib/ip.h b/lib/ip.h index f9aa7f66..6282c111 100644 --- a/lib/ip.h +++ b/lib/ip.h @@ -467,6 +467,56 @@ int ip4_pton(const char *a, ip4_addr *o); int ip6_pton(const char *a, ip6_addr *o); +/* + * IPv6 bit shifting + */ + +static inline ip6_addr +ip6_shift_left(ip6_addr a, uint bits) { + int bytes, rem, i; + + if (bits == 0) + return a; + + if (bits > 127) + return IP6_NONE; + + bytes = bits / 32; + rem = bits % 32; + + for (i = 0; i < 3 - bytes; i++) + a.addr[i] = (a.addr[i + bytes] << rem) | + (rem ? (a.addr[i + bytes + 1] >> (32 - rem)) : 0); + + a.addr[3 - bytes] = a.addr[3] << rem; + memset(&a.addr[4 - bytes], 0, bytes * 4); + + return a; +} + +static inline ip6_addr +ip6_shift_right(ip6_addr a, uint bits) { + int bytes, rem, i; + + if (bits == 0) + return a; + + if (bits > 127) + return IP6_NONE; + + bytes = bits / 32; + rem = bits % 32; + for (i = 3; i > bytes; i--) + a.addr[i] = (a.addr[i - bytes] >> rem) | + (rem ? (a.addr[i - bytes - 1] << (32 - rem)) : 0); + + a.addr[bytes] = a.addr[0] >> rem; + memset(&a.addr[0], 0, bytes * 4); + + return a; +} + + /* * Miscellaneous */ -- 2.47.0