From: Claudio Jeker Subject: bgpd: rde ibuf parser part 2 To: tech@openbsd.org Date: Wed, 24 Jan 2024 15:05:27 +0100 Next step of going all ibuf in the RDE update parser. This switches communities over (community_add(), community_large_add() and community_ext_add()). This requires some adjustments in regress which are included. -- :wq Claudio Index: usr.sbin/bgpd/rde.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v diff -u -p -r1.616 rde.c --- usr.sbin/bgpd/rde.c 23 Jan 2024 16:13:35 -0000 1.616 +++ usr.sbin/bgpd/rde.c 23 Jan 2024 16:51:57 -0000 @@ -2139,8 +2139,8 @@ rde_attr_parse(struct ibuf *buf, struct if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; - if (community_add(&state->communities, flags, p, - attr_len) == -1) { + if (community_add(&state->communities, flags, + &attrbuf) == -1) { /* * mark update as bad and withdraw all routes as per * RFC 7606 @@ -2149,14 +2149,13 @@ rde_attr_parse(struct ibuf *buf, struct log_peer_warnx(&peer->conf, "bad COMMUNITIES, " "path invalidated and prefix withdrawn"); } - plen += attr_len; break; case ATTR_LARGE_COMMUNITIES: if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; - if (community_large_add(&state->communities, flags, p, - attr_len) == -1) { + if (community_large_add(&state->communities, flags, + &attrbuf) == -1) { /* * mark update as bad and withdraw all routes as per * RFC 7606 @@ -2165,14 +2164,13 @@ rde_attr_parse(struct ibuf *buf, struct log_peer_warnx(&peer->conf, "bad LARGE COMMUNITIES, " "path invalidated and prefix withdrawn"); } - plen += attr_len; break; case ATTR_EXT_COMMUNITIES: if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; if (community_ext_add(&state->communities, flags, - peer->conf.ebgp, p, attr_len) == -1) { + peer->conf.ebgp, &attrbuf) == -1) { /* * mark update as bad and withdraw all routes as per * RFC 7606 @@ -2181,7 +2179,6 @@ rde_attr_parse(struct ibuf *buf, struct log_peer_warnx(&peer->conf, "bad EXT_COMMUNITIES, " "path invalidated and prefix withdrawn"); } - plen += attr_len; break; case ATTR_ORIGINATOR_ID: if (attr_len != 4) @@ -2338,14 +2335,11 @@ rde_attr_add(struct filterstate *state, switch (type) { case ATTR_COMMUNITIES: - return community_add(&state->communities, flags, - ibuf_data(buf), attr_len); + return community_add(&state->communities, flags, buf); case ATTR_LARGE_COMMUNITIES: - return community_large_add(&state->communities, flags, - ibuf_data(buf), attr_len); + return community_large_add(&state->communities, flags, buf); case ATTR_EXT_COMMUNITIES: - return community_ext_add(&state->communities, flags, 0, - ibuf_data(buf), attr_len); + return community_ext_add(&state->communities, flags, 0, buf); } if (attr_optadd(&state->aspath, flags, type, ibuf_data(buf), Index: usr.sbin/bgpd/rde.h =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v diff -u -p -r1.298 rde.h --- usr.sbin/bgpd/rde.h 23 Jan 2024 16:13:35 -0000 1.298 +++ usr.sbin/bgpd/rde.h 23 Jan 2024 16:45:08 -0000 @@ -435,10 +435,9 @@ int community_set(struct rde_community * void community_delete(struct rde_community *, struct community *, struct rde_peer *); -int community_add(struct rde_community *, int, void *, size_t); -int community_large_add(struct rde_community *, int, void *, size_t); -int community_ext_add(struct rde_community *, int, int, void *, size_t); - +int community_add(struct rde_community *, int, struct ibuf *); +int community_large_add(struct rde_community *, int, struct ibuf *); +int community_ext_add(struct rde_community *, int, int, struct ibuf *); int community_writebuf(struct rde_community *, uint8_t, int, struct ibuf *); void communities_shutdown(void); Index: usr.sbin/bgpd/rde_community.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_community.c,v diff -u -p -r1.14 rde_community.c --- usr.sbin/bgpd/rde_community.c 10 Oct 2023 14:36:28 -0000 1.14 +++ usr.sbin/bgpd/rde_community.c 23 Jan 2024 16:45:08 -0000 @@ -418,24 +418,23 @@ struct rde_peer *peer) * - community_ext_add for ATTR_EXT_COMMUNITIES */ int -community_add(struct rde_community *comm, int flags, void *buf, size_t len) +community_add(struct rde_community *comm, int flags, struct ibuf *buf) { struct community set = { .flags = COMMUNITY_TYPE_BASIC }; - uint8_t *b = buf; - uint16_t c; - size_t l; + uint16_t data1, data2; - if (len == 0 || len % 4 != 0) + if (ibuf_size(buf) == 0 || ibuf_size(buf) % 4 != 0) return -1; if (flags & ATTR_PARTIAL) comm->flags |= PARTIAL_COMMUNITIES; - for (l = 0; l < len; l += 4, b += 4) { - memcpy(&c, b, sizeof(c)); - set.data1 = ntohs(c); - memcpy(&c, b + 2, sizeof(c)); - set.data2 = ntohs(c); + while (ibuf_size(buf) > 0) { + if (ibuf_get_n16(buf, &data1) == -1 || + ibuf_get_n16(buf, &data2) == -1) + return -1; + set.data1 = data1; + set.data2 = data2; insert_community(comm, &set); } @@ -443,26 +442,21 @@ community_add(struct rde_community *comm } int -community_large_add(struct rde_community *comm, int flags, void *buf, - size_t len) +community_large_add(struct rde_community *comm, int flags, struct ibuf *buf) { struct community set = { .flags = COMMUNITY_TYPE_LARGE }; - uint8_t *b = buf; - size_t l; - if (len == 0 || len % 12 != 0) + if (ibuf_size(buf) == 0 || ibuf_size(buf) % 12 != 0) return -1; if (flags & ATTR_PARTIAL) comm->flags |= PARTIAL_LARGE_COMMUNITIES; - for (l = 0; l < len; l += 12, b += 12) { - memcpy(&set.data1, b, sizeof(set.data1)); - memcpy(&set.data2, b + 4, sizeof(set.data2)); - memcpy(&set.data3, b + 8, sizeof(set.data3)); - set.data1 = ntohl(set.data1); - set.data2 = ntohl(set.data2); - set.data3 = ntohl(set.data3); + while (ibuf_size(buf) > 0) { + if (ibuf_get_n32(buf, &set.data1) == -1 || + ibuf_get_n32(buf, &set.data2) == -1 || + ibuf_get_n32(buf, &set.data3) == -1) + return -1; insert_community(comm, &set); } @@ -471,23 +465,22 @@ community_large_add(struct rde_community int community_ext_add(struct rde_community *comm, int flags, int ebgp, - void *buf, size_t len) + struct ibuf *buf) { struct community set = { .flags = COMMUNITY_TYPE_EXT }; - uint8_t *b = buf, type; uint64_t c; - size_t l; + uint8_t type; - if (len == 0 || len % 8 != 0) + if (ibuf_size(buf) == 0 || ibuf_size(buf) % 8 != 0) return -1; if (flags & ATTR_PARTIAL) comm->flags |= PARTIAL_EXT_COMMUNITIES; - for (l = 0; l < len; l += 8, b += 8) { - memcpy(&c, b, 8); + while (ibuf_size(buf) > 0) { + if (ibuf_get_n64(buf, &c) == -1) + return (-1); - c = be64toh(c); type = c >> 56; /* filter out non-transitive ext communuties from ebgp peers */ if (ebgp && (type & EXT_COMMUNITY_NON_TRANSITIVE)) Index: regress/usr.sbin/bgpd/unittests/rde_community_test.c =================================================================== RCS file: /cvs/src/regress/usr.sbin/bgpd/unittests/rde_community_test.c,v diff -u -p -r1.9 rde_community_test.c --- regress/usr.sbin/bgpd/unittests/rde_community_test.c 11 Oct 2023 07:05:11 -0000 1.9 +++ regress/usr.sbin/bgpd/unittests/rde_community_test.c 24 Jan 2024 14:01:01 -0000 @@ -43,9 +43,9 @@ dump(uint8_t *b, size_t len) } static int -test_parsing(size_t num, uint8_t *in, size_t inlen, uint8_t *out, size_t outlen) +test_parsing(size_t num, struct ibuf *in, struct ibuf *out) { - struct ibuf *buf; + struct ibuf *buf, abuf; uint8_t flags, type, attr[256]; size_t skip; uint16_t attr_len; @@ -54,42 +54,40 @@ test_parsing(size_t num, uint8_t *in, si communities_clean(&comm); do { - flags = in[0]; - type = in[1]; - skip = 2; + if (ibuf_get_n8(in, &flags) == -1 || + ibuf_get_n8(in, &type) == -1) + goto bad; if (flags & ATTR_EXTLEN) { - memcpy(&attr_len, in + 2, sizeof(attr_len)); - attr_len = ntohs(attr_len); - skip += 2; + if (ibuf_get_n16(in, &attr_len) == -1) + goto bad; } else { - attr_len = in[2]; - skip += 1; + uint8_t tmp; + if (ibuf_get_n8(in, &tmp) == -1) + goto bad; + attr_len = tmp; } - if (skip + attr_len > inlen) { + if (ibuf_get_ibuf(in, attr_len, &abuf) == -1) { + bad: printf("Test %zu: attribute parse failure\n", num); return -1; } switch (type) { case ATTR_COMMUNITIES: - r = community_add(&comm, flags, in + skip, attr_len); + r = community_add(&comm, flags, &abuf); break; case ATTR_EXT_COMMUNITIES: - r = community_ext_add(&comm, flags, 0, in + skip, - attr_len); + r = community_ext_add(&comm, flags, 0, &abuf); break; case ATTR_LARGE_COMMUNITIES: - r = community_large_add(&comm, flags, in + skip, - attr_len); + r = community_large_add(&comm, flags, &abuf); break; } if (r == -1) { printf("Test %zu: community_add failed\n", num); return -1; } - in += skip + attr_len; - inlen -= (skip + attr_len); - } while (inlen > 0); + } while (ibuf_size(in) > 0); if ((buf = ibuf_dynamic(0, 4096)) == NULL) { printf("Test %zu: ibuf_dynamic failed\n", num); @@ -109,19 +107,19 @@ test_parsing(size_t num, uint8_t *in, si return -1; } - if (ibuf_size(buf) != outlen) { + if (ibuf_size(buf) != ibuf_size(out)) { printf("Test %zu: ibuf size value %zd != %zd:", - num, ibuf_size(buf), outlen); + num, ibuf_size(buf), ibuf_size(out)); dump(ibuf_data(buf), ibuf_size(buf)); printf("expected: "); - dump(out, outlen); + dump(ibuf_data(out), ibuf_size(out)); return -1; } - if (memcmp(ibuf_data(buf), out, outlen) != 0) { + if (memcmp(ibuf_data(buf), ibuf_data(out), ibuf_size(out)) != 0) { printf("Test %zu: unexpected encoding: ", num); dump(ibuf_data(buf), ibuf_size(buf)); printf("expected: "); - dump(out, outlen); + dump(ibuf_data(out), ibuf_size(out)); return -1; } @@ -206,16 +204,17 @@ main(int argc, char *argv[]) int error = 0; for (t = 0; t < sizeof(vectors) / sizeof(*vectors); t++) { - size_t outlen = vectors[t].expsize; - uint8_t *out = vectors[t].expected; + struct ibuf in, out; - if (vectors[t].expected == NULL) { - outlen = vectors[t].size; - out = vectors[t].data; - } + ibuf_from_buffer(&in, vectors[t].data, vectors[t].size); + if (vectors[t].expected == NULL) + ibuf_from_buffer(&out, + vectors[t].data, vectors[t].size); + else + ibuf_from_buffer(&out, + vectors[t].expected, vectors[t].expsize); - if (test_parsing(t, vectors[t].data, vectors[t].size, - out, outlen) == -1) + if (test_parsing(t, &in, &out) == -1) error = 1; }