[PATCH] Fix BGP IPv6 flowspec prefix when offset is not multiple of 8

[Eric Long]

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