On Wed, Apr 12, 2023 at 05:33:10PM +0200, Claudio Jeker wrote:
> This is the first big amount of flowspec specific code.
> It adds a new file (flowspec.c) which exposes basic API functions to work
> with flowspec. Right now apart from the regress test nothing uses these
> functions (but don't worry I have other diffs which make use of them).
>
> Flowspec encoding is super annoying. It is extremly flexible, the length
> of individual components is encoded into the component, there are different
> encodings for IPv4 and IPv6, some encodings make absolutly no sense, and the
> total length can be up to 4095 bytes (yikes).
This is complete madness.
> Because of this the idea is to keep flowspec as NLRI blob, use this API to
> validate the blob and provide a RFC compliant compare function.
> Additionally provide functions to extract components and helpers to print
> components.
Yes, that makes sense.
> I know some not so important functions are still missing but this is a
> decent start. Especially formating a flowspec into a string is not fully
> there yet.
Here's a first round of feedback. I need to ponder this more since it
involves a lot of scary pointer manipulations. I don't think my head
will cooperate without doing something else for a while.
Unless it blocks you from further progress, I would prefer to wait with
landing this for a day or two more.
> --
> :wq Claudio
>
> Index: usr.sbin/bgpd/Makefile
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/Makefile,v
> retrieving revision 1.38
> diff -u -p -r1.38 Makefile
> --- usr.sbin/bgpd/Makefile 11 Jan 2023 13:53:17 -0000 1.38
> +++ usr.sbin/bgpd/Makefile 12 Apr 2023 15:14:49 -0000
> @@ -5,7 +5,7 @@ SRCS= bgpd.c session.c log.c logmsg.c pa
> rde.c rde_rib.c rde_decide.c rde_prefix.c mrt.c kroute.c control.c \
> pfkey.c rde_update.c rde_attr.c rde_community.c printconf.c \
> rde_filter.c rde_sets.c rde_aspa.c rde_trie.c pftable.c name2id.c \
> - util.c carp.c timer.c rde_peer.c rtr.c rtr_proto.c
> + util.c carp.c timer.c rde_peer.c rtr.c rtr_proto.c flowspec.c
> CFLAGS+= -Wall -I${.CURDIR}
> CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
> CFLAGS+= -Wmissing-declarations
> Index: usr.sbin/bgpd/bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.470
> diff -u -p -r1.470 bgpd.h
> --- usr.sbin/bgpd/bgpd.h 3 Apr 2023 10:48:00 -0000 1.470
> +++ usr.sbin/bgpd/bgpd.h 12 Apr 2023 15:14:49 -0000
> @@ -1087,18 +1087,22 @@ struct ext_comm_pairs {
> extern const struct ext_comm_pairs iana_ext_comms[];
>
> /* BGP flowspec defines RFC 8955 and 8956 */
> -#define FLOWSPEC_LEN_LIMIT 0xf0
> -#define FLOWSPEC_OP_EOL 0x80
> -#define FLOWSPEC_OP_AND 0x40
> -#define FLOWSPEC_OP_LEN_MASK 0x30
> -#define FLOWSPEC_OP_LEN_SHIFT 4
> +#define FLOWSPEC_LEN_LIMIT 0xf0
> +#define FLOWSPEC_OP_EOL 0x80
> +#define FLOWSPEC_OP_AND 0x40
> +#define FLOWSPEC_OP_LEN_MASK 0x30
> +#define FLOWSPEC_OP_LEN_SHIFT 4
> #define FLOWSPEC_OP_LEN(op) \
> (1 << (((op) & FLOWSPEC_OP_LEN_MASK) >> FLOWSPEC_OP_LEN_SHIFT))
> -#define FLOWSPEC_OP_NUM_LT 0x04
> -#define FLOWSPEC_OP_NUM_GT 0x02
> -#define FLOWSPEC_OP_NUM_EQ 0x01
> -#define FLOWSPEC_OP_BIT_NOT 0x02
> -#define FLOWSPEC_OP_BIT_MATCH 0x01
> +#define FLOWSPEC_OP_NUM_LT 0x04
> +#define FLOWSPEC_OP_NUM_GT 0x02
> +#define FLOWSPEC_OP_NUM_EQ 0x01
> +#define FLOWSPEC_OP_NUM_LE (FLOWSPEC_OP_NUM_LT | FLOWSPEC_OP_NUM_EQ)
> +#define FLOWSPEC_OP_NUM_GE (FLOWSPEC_OP_NUM_GT | FLOWSPEC_OP_NUM_EQ)
> +#define FLOWSPEC_OP_NUM_NOT (FLOWSPEC_OP_NUM_GT | FLOWSPEC_OP_NUM_LT)
Maybe add
#define FLOWSPEC_OP_NUM_FALSE 0x00
#define FLOWSPEC_OP_NUM_TRUE 0x07
and use them in the switch in flowspec_fmt_num_op().
> +#define FLOWSPEC_OP_NUM_MASK 0x07
> +#define FLOWSPEC_OP_BIT_NOT 0x02
> +#define FLOWSPEC_OP_BIT_MATCH 0x01
>
> #define FLOWSPEC_TYPE_MIN 1
> #define FLOWSPEC_TYPE_DEST 1
> @@ -1510,6 +1514,7 @@ int aspath_verify(void *, uint16_t, in
> #define AS_ERR_BAD -3
> #define AS_ERR_SOFT -4
> u_char *aspath_inflate(void *, uint16_t, uint16_t *);
> +int extract_prefix(const u_char *, int, void *, uint8_t, uint8_t);
> int nlri_get_prefix(u_char *, uint16_t, struct bgpd_addr *,
> uint8_t *);
> int nlri_get_prefix6(u_char *, uint16_t, struct bgpd_addr *,
> @@ -1532,6 +1537,15 @@ int af2aid(sa_family_t, uint8_t, uint8
> struct sockaddr *addr2sa(const struct bgpd_addr *, uint16_t, socklen_t
> *);
> void sa2addr(struct sockaddr *, struct bgpd_addr *, uint16_t *);
> const char * get_baudrate(unsigned long long, char *);
> +
> +/* flowspec.c */
> +int flowspec_valid(const uint8_t *, int, int);
> +int flowspec_cmp(const uint8_t *, int, const uint8_t *, int, int);
> +int flowspec_get_component(const uint8_t *, int, int, int,
> + const uint8_t **, int *);
> +int flowspec_get_addr(const uint8_t *, int, int, int, struct bgpd_addr *,
> + uint8_t *, uint8_t *);
> +const char *flowspec_fmt_num_op(const uint8_t *, int, int *);
>
> static const char * const log_procnames[] = {
> "parent",
> Index: usr.sbin/bgpd/flowspec.c
> ===================================================================
> RCS file: usr.sbin/bgpd/flowspec.c
> diff -N usr.sbin/bgpd/flowspec.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ usr.sbin/bgpd/flowspec.c 12 Apr 2023 15:14:49 -0000
> @@ -0,0 +1,475 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2023 Claudio Jeker <clau...@openbsd.org>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <string.h>
> +#include <stdio.h>
> +
> +#include "bgpd.h"
> +#include "rde.h"
> +
> +/*
> + * Extract the next component from a flowspec NLRI buffer.
> + * Returns the length of the component inclusive type field or -1 on failure.
I'd do s/inclusive/including
> + * Also checks that the prefix encoding is valid.
> + */
> +static int
> +flowspec_next_component(const uint8_t *buf, int len, int is_v6, int *type)
> +{
> + int vlen = 0;
> + uint8_t plen, off, op;
> +
> + *type = 0;
> + if (len < 1)
> + return -1;
> + *type = buf[vlen];
> + vlen++;
> + if (*type < FLOWSPEC_TYPE_MIN || *type > FLOWSPEC_TYPE_MAX)
> + return -1;
> +
> + switch (*type) {
> + case FLOWSPEC_TYPE_DEST:
> + case FLOWSPEC_TYPE_SOURCE:
> + if (!is_v6) {
Maybe add a reference to RFC 4271, section 4.3 here. The generic references in
RFC 8955 aren't helpful.
> + if (len < vlen + 1)
> + return -1;
> + plen = buf[vlen];
> + vlen += PREFIX_SIZE(plen);
> + if (plen > 32 || len < vlen)
> + return -1;
> + } else {
> + if (len < vlen + 2)
> + return -1;
> + plen = buf[vlen];
> + off = buf[vlen + 1];
> + if (plen > 128 || off >= plen)
> + return -1;
> + vlen += PREFIX_SIZE(plen - off) + 1; /* off is extra */
> + if (len < vlen)
> + return -1;
It took me way too long to figure out I have to go look at RFC 8956,
section 3.1. Starting from RFC 8955 I tried to find something about
this mysterious off in RFC 4271 and was utterly confused for the better
part of an hour...
In short: please add a comment for the stupid. Future me will be grateful.
> + }
> + break;
> + case FLOWSPEC_TYPE_FLOW:
> + if (!is_v6)
> + return -1;
> + /* FALLTHROUGH */
> + default:
> + do {
> + if (len < vlen + 1)
> + return -1;
> + op = buf[vlen];
> + /* first component cannot have and flag set */
> + if (vlen == 1 && op & FLOWSPEC_OP_AND)
> + return -1;
> + vlen += FLOWSPEC_OP_LEN(op) + 1;
> +
> + if (len < vlen)
> + return -1;
> + } while ((op & FLOWSPEC_OP_EOL) == 0);
> + break;
> + }
> + return vlen;
> +}
> +
> +#define MINIMUM(a, b) ((a) < (b) ? (a) : (b))
> +
> +/*
> + * Compare two IPv4 flowspec prefix components.
> + * Returns 1 if first prefix is preferred, -1 if second, 0 if equal.
> + */
> +static int
> +flowspec_cmp_prefix4(const uint8_t *abuf, int ablen, const uint8_t *bbuf,
> + int bblen)
> +{
> + uint8_t a[4] = { 0 }, b[4] = { 0 };
> + int alen, blen, clen, cmp;
> +
> + alen = abuf[1];
> + blen = bbuf[1];
> + clen = MINIMUM(alen, blen);
> +
> + /* only extract the common prefix */
> + if (extract_prefix(abuf + 2, ablen - 2, &a, clen, sizeof(a)) == -1)
> + fatalx("bad flowspec prefix encoding");
> + if (extract_prefix(bbuf + 2, bblen - 2, &b, clen, sizeof(b)) == -1)
> + fatalx("bad flowspec prefix encoding");
> +
> + /* lowest IP value has precedence */
> + cmp = memcmp(&a, &b, sizeof(a));
This is fine as it is, but I'd prefer to omit the &. This always confuses me.
> + if (cmp < 0)
> + return 1;
> + if (cmp > 0)
> + return -1;
> +
> + /* if common prefix, more specific route has precedence */
> + if (alen > blen)
> + return 1;
> + if (alen < blen)
> + return -1;
> + return 0;
> +}
> +
> +/*
> + * Compare two IPv6 flowspec prefix components.
> + * Returns 1 if first prefix is preferred, -1 if second, 0 if equal.
> + * As usual the encoding of IPv6 addresses is extra complex.
> + */
> +static int
> +flowspec_cmp_prefix6(const uint8_t *abuf, int ablen, const uint8_t *bbuf,
> + int bblen)
> +{
> + uint8_t a[16] = { 0 }, b[16] = { 0 };
> + int alen, blen, clen, cmp;
> +
> + /* lowest offset has precedence */
> + if (abuf[2] < bbuf[2])
> + return 1;
> + if (abuf[2] > bbuf[2])
> + return -1;
> +
> + /* calculate actual pattern size (len - offset) */
> + alen = abuf[1] - abuf[2];
> + blen = bbuf[1] - bbuf[2];
> + clen = MINIMUM(alen, blen);
> +
> + /* only extract the common prefix */
> + if (extract_prefix(abuf + 3, ablen - 3, &a, clen, sizeof(a)) == -1)
> + fatalx("bad flowspec prefix encoding");
> + if (extract_prefix(bbuf + 3, bblen - 3, &b, clen, sizeof(b)) == -1)
> + fatalx("bad flowspec prefix encoding");
> +
> + /* lowest IP value has precedence */
> + cmp = memcmp(&a, &b, sizeof(a));
ditto
> + if (cmp < 0)
> + return 1;
> + if (cmp > 0)
> + return -1;
> +
> + /* if common prefix, more specific route has precedence */
> + if (alen > blen)
> + return 1;
> + if (alen < blen)
> + return -1;
> + return 0;
> +}
> +
> +/*
> + * Check if the flowspec NLRI is syntactically valid.
> + */
> +int
> +flowspec_valid(const uint8_t *buf, int len, int is_v6)
> +{
> + int l, type, prev = 0;
> +
> + /* empty NLRI is invalid */
> + if (len == 0)
> + return -1;
> +
> + while (len > 0) {
> + l = flowspec_next_component(buf, len, is_v6, &type);
> + if (l == -1)
> + return -1;
> + /* ensure that types are ordered */
> + if (prev >= type)
> + return -1;
> + prev = type;
> + buf += l;
> + len -= l;
> + }
> + if (len < 0)
> + fatalx("flowspec overflow");
> + return 0;
> +}
> +
> +/*
> + * Compare two valid flowspec NLRI objects according to RFC 8955 & 8956.
> + * Returns 1 if the first object has preference, -1 if not, and 0 if the
> + * two objects are equal.
> + */
> +int
> +flowspec_cmp(const uint8_t *a, int alen, const uint8_t *b, int blen, int
> is_v6)
> +{
> + int atype, btype;
> + int acomplen, bcomplen;
> + int cmp;
> +
> + while (alen > 0 && blen > 0) {
> + acomplen = flowspec_next_component(a, alen, is_v6, &atype);
> + if (acomplen == -1)
> + fatalx("bad flowspec component");
> + bcomplen = flowspec_next_component(b, blen, is_v6, &btype);
> + if (acomplen == -1)
> + fatalx("bad flowspec component");
> +
> + /* If types differ, lowest type wins. */
> + if (atype < btype)
> + return 1;
> + if (atype > btype)
> + return -1;
> +
> + switch (atype) {
> + case FLOWSPEC_TYPE_DEST:
> + case FLOWSPEC_TYPE_SOURCE:
> + if (!is_v6) {
> + cmp = flowspec_cmp_prefix4(a, acomplen,
> + b, bcomplen);
> + } else {
> + cmp = flowspec_cmp_prefix6(a, acomplen,
> + b, bcomplen);
> + }
> + if (cmp != 0)
> + return cmp;
> + break;
> + default:
> + cmp = memcmp(a, b, MINIMUM(acomplen, bcomplen));
> + /*
> + * Lowest common component prefix wins also
> + * if both commponents are same lenght also lowest
s/lenght/length
> + * string has precedence.
> + */
> + if (cmp < 0)
> + return 1;
> + if (cmp > 0)
> + return -1;
> + /*
> + * Longest component wins when common prefix is equal.
> + * This is not really possible because EOL encoding will
> + * always tie break on the memcmp but the RFC mandates
> + * it (and it is cheap).
> + */
> + if (acomplen > bcomplen)
> + return 1;
> + if (acomplen < bcomplen)
> + return -1;
> + break;
> + }
> + a += acomplen;
> + alen -= acomplen;
> + b += bcomplen;
> + blen -= bcomplen;
> +
> + /* Rule with more components wins */
> + if (alen > 0 && blen <= 0)
> + return 1;
> + if (alen <= 0 && blen > 0)
> + return -1;
> + }
> + return 0;
> +}
> +
> +static void
> +shift_right(uint8_t *dst, const uint8_t *src, int off, int len)
> +{
> + uint8_t carry = 0;
> + int i;
> +
> + dst += off / 8; /* go to inital start point */
> + off %= 8;
> + len = (len + 7) / 8; /* how much to copy in bytes */
> +
> + for (i = 0; i < len; i++) {
> + dst[i] = carry | src[i] >> off;
> + if (off != 0)
> + carry = src[i] << (8 - off);
> + }
> + dst[i] = carry;
> +}
> +
> +/*
> + * Extract a flowspec component and return its buffer and size.
> + * Returns 1 on success, 0 if component is not present and -1 on error.
> + */
> +int
> +flowspec_get_component(const uint8_t *flow, int flowlen, int type, int is_v6,
> + const uint8_t **buf, int *len)
> +{
> + int complen, t;
> +
> + *buf = NULL;
> + *len = 0;
> +
> + do {
> + complen = flowspec_next_component(flow, flowlen, is_v6, &t);
> + if (complen == -1)
> + return -1;
> + if (type == t)
> + break;
> + if (type < t)
> + return 0;
> +
> + flow += complen;
> + flowlen -= complen;
> + } while (1);
> +
> + *buf = flow + 1;
> + *len = complen - 1;
> +
> + return 1;
> +}
> +
> +/*
> + * Extract source or destination address into provided bgpd_addr.
> + * Returns 1 on success, 0 if no address was present, -1 on error.
> + * Sets plen to the prefix len and olen to the offset for IPv6 case.
> + * If olen is set to NULL when an IPv6 prefix with offset is fetched
> + * the function will return -1.
> + */
> +int
> +flowspec_get_addr(const uint8_t *flow, int flowlen, int type, int is_v6,
> + struct bgpd_addr *addr, uint8_t *plen, uint8_t *olen)
> +{
> + const uint8_t *comp;
> + int complen, rv;
> +
> + memset(addr, 0, sizeof(*addr));
> + *plen = 0;
> + if (olen != NULL)
> + *olen = 0;
> +
> + rv = flowspec_get_component(flow, flowlen, type, is_v6,
> + &comp, &complen);
> + if (rv != 1)
> + return rv;
> +
> + /* flowspec_get_component only returns valid encodings */
> + if (!is_v6) {
> + addr->aid = AID_INET;
> + if (extract_prefix(comp + 1, complen - 1, &addr->v4, comp[0],
> + sizeof(addr->v4)) == -1)
> + return -1;
> + *plen = comp[0];
> + } else {
> + uint8_t buf[16] = { 0 };
> + int xlen, xoff = 0;
> +
> + addr->aid = AID_INET6;
> + xlen = comp[0];
> + if (comp[1] != 0) {
> + if (olen == NULL)
> + return -1;
> + xoff = comp[1];
> + xlen -= xoff;
> + }
> + if (extract_prefix(comp + 2, complen - 2, buf, xlen,
> + sizeof(buf)) == -1)
> + return -1;
> + shift_right(addr->v6.s6_addr, buf, *olen, xlen);
> + *plen = comp[0];
> + if (olen != NULL)
> + *olen = comp[1];
> + }
> +
> + return 1;
> +}
> +
> +static long long
> +extract_val(const uint8_t *comp, int len)
> +{
> + long long val = 0;
I'm surprised val isn't a uint64_t.
> +
> + while (len-- > 0) {
> + val <<= 8;
If the function is called with a len of 8 and the top bit of comp[0] is set,
this will "shift into the sign bit" of val, which is undefined behavior.
> + val |= *comp++;
> + }
> + return val;
> +}
> +
> +const char *
> +flowspec_fmt_num_op(const uint8_t *comp, int complen, int *off)
> +{
> + static char buf[32];
> + long long val, val2 = 0;
> + uint8_t op, op2 = 0;
> + int len, len2 = 0;
> +
> + if (*off == -1)
> + return "";
> + if (complen < *off + 1)
> + return "bad encoding";
> +
> + op = comp[*off];
> + len = FLOWSPEC_OP_LEN(op) + 1;
> + if (complen < *off + len)
> + return "bad encoding";
> + val = extract_val(comp + *off + 1, FLOWSPEC_OP_LEN(op));
> +
> + if ((op & FLOWSPEC_OP_EOL) == 0) {
> + if (complen < *off + len + 1)
> + return "bad encoding";
> + op2 = comp[*off + len];
> + /*
> + * Check if this is a range specification else fall back
> + * to basic rules.
> + */
> + if (op2 & FLOWSPEC_OP_AND &&
> + (op & FLOWSPEC_OP_NUM_MASK) == FLOWSPEC_OP_NUM_GE &&
> + (op2 & FLOWSPEC_OP_NUM_MASK) == FLOWSPEC_OP_NUM_LE) {
> + len2 = FLOWSPEC_OP_LEN(op2) + 1;
> + val2 = extract_val(comp + *off + len + 1,
> + FLOWSPEC_OP_LEN(op2));
> + } else
> + op2 = 0;
> + }
> +
> + if (op2 & FLOWSPEC_OP_AND) {
> + /* binary range operation */
> + snprintf(buf, sizeof(buf), "%lld - %lld", val, val2);
> + } else {
> + /* unary operation */
> + switch (op & FLOWSPEC_OP_NUM_MASK) {
> + case 0:
> + snprintf(buf, sizeof(buf), "%sfalse",
> + op & FLOWSPEC_OP_AND ? "&& " : "");
> + break;
> + case FLOWSPEC_OP_NUM_EQ:
> + snprintf(buf, sizeof(buf), "%s%lld",
> + op & FLOWSPEC_OP_AND ? "&& " : "", val);
> + break;
> + case FLOWSPEC_OP_NUM_GT:
> + snprintf(buf, sizeof(buf), "%s> %lld",
> + op & FLOWSPEC_OP_AND ? "&& " : "", val);
> + break;
> + case FLOWSPEC_OP_NUM_GE:
> + snprintf(buf, sizeof(buf), "%s>= %lld",
> + op & FLOWSPEC_OP_AND ? "&& " : "", val);
> + break;
> + case FLOWSPEC_OP_NUM_LT:
> + snprintf(buf, sizeof(buf), "%s< %lld",
> + op & FLOWSPEC_OP_AND ? "&& " : "", val);
> + break;
> + case FLOWSPEC_OP_NUM_LE:
> + snprintf(buf, sizeof(buf), "%s<= %lld",
> + op & FLOWSPEC_OP_AND ? "&& " : "", val);
> + break;
> + case FLOWSPEC_OP_NUM_NOT:
> + snprintf(buf, sizeof(buf), "%s!= %lld",
> + op & FLOWSPEC_OP_AND ? "&& " : "", val);
> + break;
> + case 0x7:
> + snprintf(buf, sizeof(buf), "%sfalse",
That should be true, not false
> + op & FLOWSPEC_OP_AND ? "&& " : "");
> + break;
> + }
> + }
> +
> + if (op2 & FLOWSPEC_OP_EOL || op & FLOWSPEC_OP_EOL)
> + *off = -1;
> + else
> + *off += len + len2;
> +
> + return buf;
> +}
> Index: usr.sbin/bgpd/util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 util.c
> --- usr.sbin/bgpd/util.c 3 Apr 2023 10:48:00 -0000 1.76
> +++ usr.sbin/bgpd/util.c 12 Apr 2023 15:14:49 -0000
> @@ -494,8 +494,8 @@ aspath_inflate(void *data, uint16_t len,
> }
>
> /* NLRI functions to extract prefixes from the NLRI blobs */
> -static int
> -extract_prefix(u_char *p, uint16_t len, void *va, uint8_t pfxlen, uint8_t
> max)
> +int
> +extract_prefix(const u_char *p, int len, void *va, uint8_t pfxlen, uint8_t
> max)
> {
> static u_char addrmask[] = {
> 0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff };
> Index: regress/usr.sbin/bgpd/unittests/Makefile
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/bgpd/unittests/Makefile,v
> retrieving revision 1.10
> diff -u -p -r1.10 Makefile
> --- regress/usr.sbin/bgpd/unittests/Makefile 11 Jan 2023 13:55:08 -0000
> 1.10
> +++ regress/usr.sbin/bgpd/unittests/Makefile 12 Apr 2023 15:15:50 -0000
> @@ -7,6 +7,7 @@ PROGS += rde_trie_test
> PROGS += rde_community_test
> PROGS += rde_decide_test
> PROGS += rde_aspa_test
> +PROGS += rde_flowspec_test
>
> .for p in ${PROGS}
> REGRESS_TARGETS += run-regress-$p
> @@ -38,5 +39,7 @@ run-regress-rde_trie_test-${n}: rde_trie
> SRCS_rde_community_test= rde_community_test.c rde_community.c
>
> SRCS_rde_decide_test= rde_decide_test.c rde_decide.c rde_attr.c util.c
> +
> +SRCS_rde_flowspec_test= rde_flowspec_test.c flowspec.c util.c
>
> .include <bsd.regress.mk>
> Index: regress/usr.sbin/bgpd/unittests/rde_flowspec_test.c
> ===================================================================
> RCS file: regress/usr.sbin/bgpd/unittests/rde_flowspec_test.c
> diff -N regress/usr.sbin/bgpd/unittests/rde_flowspec_test.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ regress/usr.sbin/bgpd/unittests/rde_flowspec_test.c 12 Apr 2023
> 15:15:50 -0000
> @@ -0,0 +1,198 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2023 Claudio Jeker <clau...@openbsd.org>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <err.h>
> +#include <stdio.h>
> +
> +#include "bgpd.h"
> +#include "rde.h"
> +
> +const uint8_t ordered0[] = { 0x01, 0x00, 0x02, 0x00, 0x03, 0x80, 0x00 };
> +const uint8_t ordered1[] = { 0x02, 0x00, 0x01, 0x00, 0x03, 0x80, 0x00 };
> +const uint8_t ordered2[] = { 0x02, 0x00, 0x03, 0x80, 0x00, 0x01, 0x00 };
> +const uint8_t ordered3[] = { 0x01, 0x00, 0x01, 0x00, 0x03, 0x80, 0x00 };
> +
> +const uint8_t minmax0[] = { 0x00, 0x00 };
> +const uint8_t minmax1[] = { 0x0e, 0x00 };
> +const uint8_t minmax2[] = { 0xfe, 0x00 };
> +
> +const uint8_t flow[] = { 0x0d, 0x80, 0x00 };
> +
> +const uint8_t badand[] = { 0x04, 0xc0, 0x00 };
> +const uint8_t goodand[] = { 0x04, 0x00, 0x00, 0xc0, 0x00 };
> +
> +const uint8_t overflow0[] = { 0x04 };
> +const uint8_t overflow1[] = { 0x04, 0x80 };
> +const uint8_t overflow2[] = { 0x04, 0x90, 0x00 };
> +const uint8_t overflow3[] = { 0x04, 0xc0, 0x00, 0x00, 0x00 };
> +const uint8_t overflow4[] = { 0x04, 0x00, 0x00 };
> +const uint8_t overflow5[] = { 0x04, 0x00, 0x00, 0x80 };
> +const uint8_t overflow6[] = { 0x04, 0x10, 0x00, 0x80 };
> +const uint8_t prefix0[] = { 0x01 };
> +const uint8_t prefix1[] = { 0x01, 0x07 };
> +const uint8_t prefix2[] = { 0x01, 0x0a, 0xef };
> +const uint8_t prefix3[] = { 0x01, 0x11, 0xef, 0x00 };
> +const uint8_t prefix4[] = { 0x01, 0x21, 0xef, 0x00, 0x00, 0x01, 0x00 };
> +const uint8_t prefix60[] = { 0x01 };
> +const uint8_t prefix61[] = { 0x01, 0x07 };
> +const uint8_t prefix62[] = { 0x01, 0x10, 0x1 };
> +const uint8_t prefix63[] = { 0x01, 0x10, 0x01, 0x20 };
> +const uint8_t prefix64[] = { 0x01, 0x81, 0x73, 0x20, 0x01 };
> +const uint8_t prefix65[] = { 0x01, 0x80, 0x83, 0x20, 0x01 };
> +const uint8_t prefix66[] = { 0x01, 0x40, 0x40, 0x20, 0x01 };
> +
> +static void
> +test_flowspec_valid(void)
> +{
> + /* empty NLRI is invalid */
> + if (flowspec_valid(NULL, 0, 0) != -1)
> + errx(1, "empty NLRI is not invalid");
> +
> + /* ensure that type range is checked */
> + if (flowspec_valid(minmax0, sizeof(minmax0), 0) != -1 ||
> + flowspec_valid(minmax1, sizeof(minmax1), 0) != -1 ||
> + flowspec_valid(minmax2, sizeof(minmax2), 0) != -1)
> + errx(1, "out of range type is not invalid");
> +
> + /* ensure that types are ordered */
> + if (flowspec_valid(ordered0, sizeof(ordered0), 0) != 0)
> + errx(1, "in order NLRI is not valid");
> + if (flowspec_valid(ordered1, sizeof(ordered1), 0) != -1 ||
> + flowspec_valid(ordered2, sizeof(ordered2), 0) != -1 ||
> + flowspec_valid(ordered3, sizeof(ordered3), 0) != -1)
> + errx(1, "out of order types are not invalid");
> +
> + /* flow is only valid in the IPv6 case */
> + if (flowspec_valid(flow, sizeof(flow), 0) != -1)
> + errx(1, "FLOW type for IPv4 is not invalid");
> + if (flowspec_valid(flow, sizeof(flow), 1) != 0)
> + errx(1, "FLOW type for IPv4 is not valid");
> +
> + /* first component cannot have and flag set */
> + if (flowspec_valid(badand, sizeof(badand), 0) != -1)
> + errx(1, "AND in first element is not invalid");
> + if (flowspec_valid(goodand, sizeof(goodand), 0) != 0)
> + errx(1, "AND in other element is not valid");
> +
> + /* various overflows */
> + if (flowspec_valid(overflow0, sizeof(overflow0), 0) != -1 ||
> + flowspec_valid(overflow1, sizeof(overflow1), 0) != -1 ||
> + flowspec_valid(overflow2, sizeof(overflow2), 0) != -1 ||
> + flowspec_valid(overflow3, sizeof(overflow3), 0) != -1 ||
> + flowspec_valid(overflow4, sizeof(overflow4), 0) != -1 ||
> + flowspec_valid(overflow5, sizeof(overflow5), 0) != -1 ||
> + flowspec_valid(overflow6, sizeof(overflow6), 0) != -1)
> + errx(1, "overflow not detected");
> +
> + if (flowspec_valid(prefix0, sizeof(prefix0), 0) != -1 ||
> + flowspec_valid(prefix1, sizeof(prefix1), 0) != -1 ||
> + flowspec_valid(prefix2, sizeof(prefix2), 0) != -1 ||
> + flowspec_valid(prefix3, sizeof(prefix3), 0) != -1 ||
> + flowspec_valid(prefix4, sizeof(prefix4), 0) != -1)
> + errx(1, "bad prefix encoding is not invalid");
> +
> + if (flowspec_valid(prefix60, sizeof(prefix60), 1) != -1 ||
> + flowspec_valid(prefix61, sizeof(prefix61), 1) != -1 ||
> + flowspec_valid(prefix62, sizeof(prefix62), 1) != -1 ||
> + flowspec_valid(prefix63, sizeof(prefix63), 1) != -1 ||
> + flowspec_valid(prefix64, sizeof(prefix64), 1) != -1 ||
> + flowspec_valid(prefix65, sizeof(prefix65), 1) != -1 ||
> + flowspec_valid(prefix66, sizeof(prefix66), 1) != -1)
> + errx(1, "bad IPv6 prefix encoding is not invalid");
> +}
> +
> +static int
> +do_cmp(const uint8_t *a, int alen, const uint8_t *b, int blen, int is_v6)
> +{
> + if (flowspec_cmp(a, alen, b, blen, is_v6) != 1 ||
> + flowspec_cmp(b, blen, a, alen, is_v6) != -1)
> + return -1;
> + return 0;
> +}
> +
> +const uint8_t cmp1[] = { 0x01, 0x00 };
> +const uint8_t cmp2[] = { 0x02, 0x00 };
> +const uint8_t cmp3[] = { 0x01, 0x00, 0x2, 0x00 };
> +const uint8_t cmp4[] = { 0x04, 0x80, 0x2 };
> +const uint8_t cmp5[] = { 0x04, 0x80, 0x3 };
> +const uint8_t cmp6[] = { 0x04, 0x00, 0x3, 0x80, 0x02 };
> +
> +const uint8_t cmp41[] = { 0x01, 24, 192, 168, 16 };
> +const uint8_t cmp42[] = { 0x01, 24, 192, 168, 32 };
> +const uint8_t cmp43[] = { 0x01, 24, 192, 168, 42 };
> +const uint8_t cmp44[] = { 0x01, 20, 192, 168, 32 };
> +
> +const uint8_t cmp61[] = { 0x01, 48, 0, 0x20, 0x01, 0x0d, 0xb8, 0xc0, 0xfe };
> +const uint8_t cmp62[] = { 0x01, 48, 8, 0x01, 0x0d, 0xb8, 0xc0, 0xfe };
> +const uint8_t cmp63[] = { 0x01, 40, 0, 0x20, 0x01, 0x0d, 0xb8, 0xc0 };
> +const uint8_t cmp64[] = { 0x01, 40, 0, 0x20, 0x01, 0x0d, 0xb8, 0xd0 };
> +
> +static void
> +test_flowspec_cmp(void)
> +{
> + if (do_cmp(cmp1, sizeof(cmp1), cmp2, sizeof(cmp2), 0) != 0)
> + errx(1, "cmp on type failed");
> + if (do_cmp(cmp3, sizeof(cmp3), cmp1, sizeof(cmp1), 0) != 0)
> + errx(1, "cmp on more components failed");
> + if (do_cmp(cmp4, sizeof(cmp4), cmp5, sizeof(cmp5), 0) != 0)
> + errx(1, "cmp on lowest common component failed");
> + if (do_cmp(cmp6, sizeof(cmp6), cmp5, sizeof(cmp5), 0) != 0)
> + errx(1, "cmp on lowest common component failed");
> + if (do_cmp(cmp6, sizeof(cmp6), cmp4, sizeof(cmp4), 0) != 0)
> + errx(1, "cmp on lowest common component failed");
> +
> + if (do_cmp(cmp41, sizeof(cmp41), cmp42, sizeof(cmp42), 0) != 0)
> + errx(1, "cmp 1 on prefix component failed");
> + if (do_cmp(cmp41, sizeof(cmp41), cmp43, sizeof(cmp43), 0) != 0)
> + errx(1, "cmp 2 on prefix component failed");
> + if (do_cmp(cmp41, sizeof(cmp41), cmp44, sizeof(cmp44), 0) != 0)
> + errx(1, "cmp 3 on prefix component failed");
> + if (do_cmp(cmp42, sizeof(cmp42), cmp43, sizeof(cmp43), 0) != 0)
> + errx(1, "cmp 4 on prefix component failed");
> + if (do_cmp(cmp42, sizeof(cmp42), cmp44, sizeof(cmp44), 0) != 0)
> + errx(1, "cmp 5 on prefix component failed");
> + if (do_cmp(cmp43, sizeof(cmp43), cmp44, sizeof(cmp44), 0) != 0)
> + errx(1, "cmp 6 on prefix component failed");
> +
> + if (do_cmp(cmp61, sizeof(cmp61), cmp62, sizeof(cmp62), 1) != 0)
> + errx(1, "cmp 1 on inet6 prefix component failed");
> + if (do_cmp(cmp61, sizeof(cmp61), cmp63, sizeof(cmp63), 1) != 0)
> + errx(1, "cmp 1 on inet6 prefix component failed");
> + if (do_cmp(cmp61, sizeof(cmp61), cmp64, sizeof(cmp64), 1) != 0)
> + errx(1, "cmp 1 on inet6 prefix component failed");
> + if (do_cmp(cmp63, sizeof(cmp63), cmp64, sizeof(cmp64), 1) != 0)
> + errx(1, "cmp 1 on inet6 prefix component failed");
> +}
> +
> +int
> +main(int argc, char **argv)
> +{
> + test_flowspec_valid();
> + test_flowspec_cmp();
> + printf("OK\n");
> + return 0;
> +}
> +
> +__dead void
> +fatalx(const char *emsg, ...)
> +{
> + va_list ap;
> + va_start(ap, emsg);
> + verrx(2, emsg, ap);
> +}
> +
>
