Convert IMSG_CTL_SHOW_RIB_ATTR over to the new ibuf API.
authorclaudio <claudio@openbsd.org>
Wed, 31 Jan 2024 11:23:19 +0000 (11:23 +0000)
committerclaudio <claudio@openbsd.org>
Wed, 31 Jan 2024 11:23:19 +0000 (11:23 +0000)
This converts show_attr() and json_attr() and with that also the
community specific functions. This removes some hacks inside of
show_attr() that where added before.

OK tb@

usr.sbin/bgpctl/bgpctl.c
usr.sbin/bgpctl/bgpctl.h
usr.sbin/bgpctl/output.c
usr.sbin/bgpctl/output_json.c

index 22032bf..35ec608 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bgpctl.c,v 1.303 2024/01/30 13:51:13 claudio Exp $ */
+/*     $OpenBSD: bgpctl.c,v 1.304 2024/01/31 11:23:19 claudio Exp $ */
 
 /*
  * Copyright (c) 2003 Henning Brauer <henning@openbsd.org>
@@ -471,7 +471,7 @@ show(struct imsg *imsg, struct parse_result *res)
        struct ctl_show_rib      rib;
        struct rde_memstats      stats;
        struct ibuf              ibuf;
-       u_int                    rescode, ilen;
+       u_int                    rescode;
 
        switch (imsg->hdr.type) {
        case IMSG_CTL_SHOW_NEIGHBOR:
@@ -542,14 +542,11 @@ show(struct imsg *imsg, struct parse_result *res)
                output->communities(&ibuf, res);
                break;
        case IMSG_CTL_SHOW_RIB_ATTR:
-               ilen = imsg->hdr.len - IMSG_HEADER_SIZE;
-               if (ilen < 3) {
-                       warnx("bad IMSG_CTL_SHOW_RIB_ATTR received");
-                       break;
-               }
                if (output->attr == NULL)
                        break;
-               output->attr(imsg->data, ilen, res->flags, 0);
+               if (imsg_get_ibuf(imsg, &ibuf) == -1)
+                       err(1, "imsg_get_ibuf");
+               output->attr(&ibuf, res->flags, 0);
                break;
        case IMSG_CTL_SHOW_RIB_MEM:
                if (output->rib_mem == NULL)
@@ -1295,9 +1292,11 @@ show_mrt_dump(struct mrt_rib *mr, struct mrt_peer *mp, void *arg)
                ibuf_from_buffer(&ibuf, mre->aspath, mre->aspath_len);
                output->rib(&ctl, &ibuf, &res);
                if (req->flags & F_CTL_DETAIL) {
-                       for (j = 0; j < mre->nattrs; j++)
-                               output->attr(mre->attrs[j].attr,
-                                   mre->attrs[j].attr_len, req->flags, 0);
+                       for (j = 0; j < mre->nattrs; j++) {
+                               ibuf_from_buffer(&ibuf, mre->attrs[j].attr,
+                                   mre->attrs[j].attr_len);
+                               output->attr(&ibuf, req->flags, 0);
+                       }
                }
        }
 }
@@ -1752,8 +1751,7 @@ show_mrt_update(u_char *p, uint16_t len, int reqflags, int addpath)
                if (ibuf_skip(&abuf, ibuf_size(&attrbuf)) == -1)
                        goto trunc;
 
-               output->attr(ibuf_data(&attrbuf), ibuf_size(&attrbuf),
-                   reqflags, addpath);
+               output->attr(&attrbuf, reqflags, addpath);
        }
 
        if (ibuf_size(b) > 0) {
index 4ec1086..b707017 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bgpctl.h,v 1.23 2024/01/30 13:51:13 claudio Exp $ */
+/*     $OpenBSD: bgpctl.h,v 1.24 2024/01/31 11:23:20 claudio Exp $ */
 
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
@@ -27,7 +27,7 @@ struct output {
        void    (*flowspec)(struct flowspec *);
        void    (*nexthop)(struct ctl_show_nexthop *);
        void    (*interface)(struct ctl_show_interface *);
-       void    (*attr)(u_char *, size_t, int, int);
+       void    (*attr)(struct ibuf *, int, int);
        void    (*communities)(struct ibuf *, struct parse_result *);
        void    (*rib)(struct ctl_show_rib *, struct ibuf *,
                    struct parse_result *);
index 05f151a..d4a5fb9 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: output.c,v 1.49 2024/01/30 13:51:13 claudio Exp $ */
+/*     $OpenBSD: output.c,v 1.50 2024/01/31 11:23:20 claudio Exp $ */
 
 /*
  * Copyright (c) 2003 Henning Brauer <henning@openbsd.org>
@@ -698,128 +698,103 @@ show_communities(struct ibuf *data, struct parse_result *res)
 }
 
 static void
-show_community(u_char *data, uint16_t len)
+show_community(struct ibuf *buf)
 {
        uint16_t        a, v;
-       uint16_t        i;
 
-       if (len & 0x3) {
-               printf("bad length");
-               return;
-       }
-
-       for (i = 0; i < len; i += 4) {
-               memcpy(&a, data + i, sizeof(a));
-               memcpy(&v, data + i + 2, sizeof(v));
-               a = ntohs(a);
-               v = ntohs(v);
+       while (ibuf_size(buf) > 0) {
+               if (ibuf_get_n16(buf, &a) == -1 ||
+                   ibuf_get_n16(buf, &v) == -1) {
+                       printf("bad length");
+                       return;
+               }
                printf("%s", fmt_community(a, v));
 
-               if (i + 4 < len)
+               if (ibuf_size(buf) > 0)
                        printf(" ");
        }
 }
 
 static void
-show_large_community(u_char *data, uint16_t len)
+show_large_community(struct ibuf *buf)
 {
        uint32_t        a, l1, l2;
-       uint16_t        i;
 
-       if (len % 12) {
-               printf("bad length");
-               return;
-       }
-
-       for (i = 0; i < len; i += 12) {
-               memcpy(&a, data + i, sizeof(a));
-               memcpy(&l1, data + i + 4, sizeof(l1));
-               memcpy(&l2, data + i + 8, sizeof(l2));
-               a = ntohl(a);
-               l1 = ntohl(l1);
-               l2 = ntohl(l2);
+       while (ibuf_size(buf) > 0) {
+               if (ibuf_get_n32(buf, &a) == -1 ||
+                   ibuf_get_n32(buf, &l1) == -1 ||
+                   ibuf_get_n32(buf, &l2) == -1) {
+                       printf("bad length");
+                       return;
+               }
                printf("%s", fmt_large_community(a, l1, l2));
 
-               if (i + 12 < len)
+               if (ibuf_size(buf) > 0)
                        printf(" ");
        }
 }
 
 static void
-show_ext_community(u_char *data, uint16_t len)
+show_ext_community(struct ibuf *buf)
 {
        uint64_t        ext;
-       uint16_t        i;
-
-       if (len & 0x7) {
-               printf("bad length");
-               return;
-       }
 
-       for (i = 0; i < len; i += 8) {
-               memcpy(&ext, data + i, sizeof(ext));
-               ext = be64toh(ext);
+       while (ibuf_size(buf) > 0) {
+               if (ibuf_get_n64(buf, &ext) == -1) {
+                       printf("bad length");
+                       return;
+               }
                printf("%s", fmt_ext_community(ext));
 
-               if (i + 8 < len)
+               if (ibuf_size(buf) > 0)
                        printf(" ");
        }
 }
 
 static void
-show_attr(u_char *data, size_t len, int reqflags, int addpath)
+show_attr(struct ibuf *buf, int reqflags, int addpath)
 {
        struct in_addr   id;
        struct bgpd_addr prefix;
-       struct ibuf      ibuf, *buf = &ibuf, asbuf, *path = NULL;
+       struct ibuf      asbuf, *path = NULL;
        char            *aspath;
-       uint32_t         as, pathid;
-       uint16_t         alen, ioff, short_as, afi;
-       uint8_t          flags, type, safi, aid, prefixlen;
-       int              i, e2, e4;
+       size_t           i, alen;
+       uint32_t         as, pathid, val;
+       uint16_t         short_as, afi;
+       uint8_t          flags, type, safi, aid, prefixlen, origin, b;
+       int              e2, e4;
 
-       if (len < 3) {
-               warnx("Too short BGP attribute");
-               return;
-       }
-
-       flags = data[0];
-       type = data[1];
+       if (ibuf_get_n8(buf, &flags) == -1 ||
+           ibuf_get_n8(buf, &type) == -1)
+               goto bad_len;
 
        /* get the attribute length */
        if (flags & ATTR_EXTLEN) {
-               if (len < 4) {
-                       warnx("Too short BGP attribute");
-                       return;
-               }
-               memcpy(&alen, data+2, sizeof(uint16_t));
-               alen = ntohs(alen);
-               data += 4;
-               len -= 4;
+               uint16_t attr_len;
+               if (ibuf_get_n16(buf, &attr_len) == -1)
+                       goto bad_len;
+               alen = attr_len;
        } else {
-               alen = data[2];
-               data += 3;
-               len -= 3;
+               uint8_t attr_len;
+               if (ibuf_get_n8(buf, &attr_len) == -1)
+                       goto bad_len;
+               alen = attr_len;
        }
 
        /* bad imsg len how can that happen!? */
-       if (alen > len) {
-               warnx("Bad BGP attribute length");
-               return;
-       }
+       if (alen > ibuf_size(buf))
+               goto bad_len;
 
        printf("    %s: ", fmt_attr(type, flags));
 
        switch (type) {
        case ATTR_ORIGIN:
-               if (alen == 1)
-                       printf("%s", fmt_origin(*data, 0));
-               else
-                       printf("bad length");
+               if (alen != 1 || ibuf_get_n8(buf, &origin) == -1)
+                       goto bad_len;
+               printf("%s", fmt_origin(origin, 0));
                break;
        case ATTR_ASPATH:
        case ATTR_AS4_PATH:
-               ibuf_from_buffer(buf, data, alen);
                /* prefer 4-byte AS here */
                e4 = aspath_verify(buf, 1, 0);
                e2 = aspath_verify(buf, 0, 0);
@@ -842,68 +817,48 @@ show_attr(u_char *data, size_t len, int reqflags, int addpath)
                ibuf_free(path);
                break;
        case ATTR_NEXTHOP:
-               if (alen == 4) {
-                       memcpy(&id, data, sizeof(id));
-                       printf("%s", inet_ntoa(id));
-               } else
-                       printf("bad length");
+       case ATTR_ORIGINATOR_ID:
+               if (alen != 4 || ibuf_get(buf, &id, sizeof(id)) == -1)
+                       goto bad_len;
+               printf("%s", inet_ntoa(id));
                break;
        case ATTR_MED:
        case ATTR_LOCALPREF:
-               if (alen == 4) {
-                       uint32_t val;
-                       memcpy(&val, data, sizeof(val));
-                       val = ntohl(val);
-                       printf("%u", val);
-               } else
-                       printf("bad length");
+               if (alen != 4 || ibuf_get_n32(buf, &val) == -1)
+                       goto bad_len;
+               printf("%u", val);
                break;
        case ATTR_AGGREGATOR:
        case ATTR_AS4_AGGREGATOR:
                if (alen == 8) {
-                       memcpy(&as, data, sizeof(as));
-                       memcpy(&id, data + sizeof(as), sizeof(id));
-                       as = ntohl(as);
+                       if (ibuf_get_n32(buf, &as) == -1 ||
+                           ibuf_get(buf, &id, sizeof(id)) == -1)
+                               goto bad_len;
                } else if (alen == 6) {
-                       memcpy(&short_as, data, sizeof(short_as));
-                       memcpy(&id, data + sizeof(short_as), sizeof(id));
-                       as = ntohs(short_as);
+                       if (ibuf_get_n16(buf, &short_as) == -1 ||
+                           ibuf_get(buf, &id, sizeof(id)) == -1)
+                               goto bad_len;
+                       as = short_as;
                } else {
-                       printf("bad length");
-                       break;
+                       goto bad_len;
                }
                printf("%s [%s]", log_as(as), inet_ntoa(id));
                break;
        case ATTR_COMMUNITIES:
-               show_community(data, alen);
-               break;
-       case ATTR_ORIGINATOR_ID:
-               if (alen == 4) {
-                       memcpy(&id, data, sizeof(id));
-                       printf("%s", inet_ntoa(id));
-               } else
-                       printf("bad length");
+               show_community(buf);
                break;
        case ATTR_CLUSTER_LIST:
-               for (ioff = 0; ioff + sizeof(id) <= alen;
-                   ioff += sizeof(id)) {
-                       memcpy(&id, data + ioff, sizeof(id));
+               while (ibuf_size(buf) > 0) {
+                       if (ibuf_get(buf, &id, sizeof(id)) == -1)
+                               goto bad_len;
                        printf(" %s", inet_ntoa(id));
                }
                break;
        case ATTR_MP_REACH_NLRI:
        case ATTR_MP_UNREACH_NLRI:
-               if (alen < 3) {
- bad_len:
-                       printf("bad length");
-                       break;
-               }
-               memcpy(&afi, data, 2);
-               data += 2;
-               alen -= 2;
-               afi = ntohs(afi);
-               safi = *data++;
-               alen--;
+               if (ibuf_get_n16(buf, &afi) == -1 ||
+                   ibuf_get_n8(buf, &safi) == -1)
+                       goto bad_len;
 
                if (afi2aid(afi, safi, &aid) == -1) {
                        printf("bad AFI/SAFI pair");
@@ -914,11 +869,7 @@ show_attr(u_char *data, size_t len, int reqflags, int addpath)
                if (type == ATTR_MP_REACH_NLRI) {
                        struct bgpd_addr nexthop;
                        uint8_t nhlen;
-                       if (len == 0)
-                               goto bad_len;
-                       nhlen = *data++;
-                       alen--;
-                       if (nhlen > len)
+                       if (ibuf_get_n8(buf, &nhlen) == -1)
                                goto bad_len;
                        memset(&nexthop, 0, sizeof(nexthop));
                        switch (aid) {
@@ -926,35 +877,39 @@ show_attr(u_char *data, size_t len, int reqflags, int addpath)
                                nexthop.aid = aid;
                                if (nhlen != 16 && nhlen != 32)
                                        goto bad_len;
-                               memcpy(&nexthop.v6.s6_addr, data, 16);
+                               if (ibuf_get(buf, &nexthop.v6,
+                                   sizeof(nexthop.v6)) == -1)
+                                       goto bad_len;
                                break;
                        case AID_VPN_IPv4:
                                if (nhlen != 12)
                                        goto bad_len;
                                nexthop.aid = AID_INET;
-                               memcpy(&nexthop.v4, data + sizeof(uint64_t),
-                                   sizeof(nexthop.v4));
+                               if (ibuf_skip(buf, sizeof(uint64_t)) == -1 ||
+                                   ibuf_get(buf, &nexthop.v4,
+                                   sizeof(nexthop.v4)) == -1)
+                                       goto bad_len;
                                break;
                        case AID_VPN_IPv6:
                                if (nhlen != 24)
                                        goto bad_len;
                                nexthop.aid = AID_INET6;
-                               memcpy(&nexthop.v6, data + sizeof(uint64_t),
-                                   sizeof(nexthop.v6));
+                               if (ibuf_skip(buf, sizeof(uint64_t)) == -1 ||
+                                   ibuf_get(buf, &nexthop.v6,
+                                   sizeof(nexthop.v6)) == -1)
+                                       goto bad_len;
                                break;
                        default:
                                printf("unhandled AID #%u", aid);
                                goto done;
                        }
                        /* ignore reserved (old SNPA) field as per RFC4760 */
-                       data += nhlen + 1;
-                       alen -= nhlen + 1;
+                       if (ibuf_skip(buf, 1) == -1)
+                               goto bad_len;
 
                        printf(" nexthop: %s", log_addr(&nexthop));
                }
 
-               ibuf_from_buffer(buf, data, alen);
-
                while (ibuf_size(buf) > 0) {
                        if (addpath)
                                if (ibuf_get_n32(buf, &pathid) == -1)
@@ -985,32 +940,36 @@ show_attr(u_char *data, size_t len, int reqflags, int addpath)
                }
                break;
        case ATTR_EXT_COMMUNITIES:
-               show_ext_community(data, alen);
+               show_ext_community(buf);
                break;
        case ATTR_LARGE_COMMUNITIES:
-               show_large_community(data, alen);
+               show_large_community(buf);
                break;
        case ATTR_OTC:
-               if (alen == 4) {
-                       memcpy(&as, data, sizeof(as));
-                       as = ntohl(as);
-                       printf("%s", log_as(as));
-               } else {
-                       printf("bad length");
-               }
+               if (alen != 4 || ibuf_get_n32(buf, &as) == -1)
+                       goto bad_len;
+               printf("%s", log_as(as));
                break;
        case ATTR_ATOMIC_AGGREGATE:
        default:
-               printf(" len %u", alen);
+               printf(" len %zu", alen);
                if (alen) {
                        printf(":");
-                       for (i=0; i < alen; i++)
-                               printf(" %02x", *(data+i));
+                       for (i = 0; i < alen; i++) {
+                               if (ibuf_get_n8(buf, &b) == -1)
+                                       goto bad_len;
+                               printf(" %02x", b);
+                       }
                }
                break;
        }
+
  done:
        printf("%c", EOL0(reqflags));
+       return;
+
+ bad_len:
+       printf("bad length%c", EOL0(reqflags));
 }
 
 static void
index d186723..d108a64 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: output_json.c,v 1.41 2024/01/30 13:51:13 claudio Exp $ */
+/*     $OpenBSD: output_json.c,v 1.42 2024/01/31 11:23:20 claudio Exp $ */
 
 /*
  * Copyright (c) 2020 Claudio Jeker <claudio@openbsd.org>
@@ -512,22 +512,18 @@ json_communities(struct ibuf *data, struct parse_result *res)
 }
 
 static void
-json_do_community(u_char *data, uint16_t len)
+json_do_community(struct ibuf *buf)
 {
-       uint16_t a, v, i;
-
-       if (len & 0x3) {
-               json_do_string("error", "bad length");
-               return;
-       }
+       uint16_t a, v;
 
        json_do_array("communities");
 
-       for (i = 0; i < len; i += 4) {
-               memcpy(&a, data + i, sizeof(a));
-               memcpy(&v, data + i + 2, sizeof(v));
-               a = ntohs(a);
-               v = ntohs(v);
+       while (ibuf_size(buf) > 0) {
+               if (ibuf_get_n16(buf, &a) == -1 ||
+                   ibuf_get_n16(buf, &v) == -1) {
+                       json_do_string("error", "bad length");
+                       return;
+               }
                json_do_string("community", fmt_community(a, v));
        }
 
@@ -535,49 +531,36 @@ json_do_community(u_char *data, uint16_t len)
 }
 
 static void
-json_do_large_community(u_char *data, uint16_t len)
+json_do_large_community(struct ibuf *buf)
 {
        uint32_t a, l1, l2;
-       uint16_t i;
-
-       if (len % 12) {
-               json_do_string("error", "bad length");
-               return;
-       }
 
        json_do_array("large_communities");
 
-       for (i = 0; i < len; i += 12) {
-               memcpy(&a, data + i, sizeof(a));
-               memcpy(&l1, data + i + 4, sizeof(l1));
-               memcpy(&l2, data + i + 8, sizeof(l2));
-               a = ntohl(a);
-               l1 = ntohl(l1);
-               l2 = ntohl(l2);
-
-               json_do_string("community",
-                   fmt_large_community(a, l1, l2));
+       while (ibuf_size(buf) > 0) {
+               if (ibuf_get_n32(buf, &a) == -1 ||
+                   ibuf_get_n32(buf, &l1) == -1 ||
+                   ibuf_get_n32(buf, &l2) == -1) {
+                       json_do_string("error", "bad length");
+                       return;
+               }
+               json_do_string("community", fmt_large_community(a, l1, l2));
        }
 
        json_do_end();
 }
 
 static void
-json_do_ext_community(u_char *data, uint16_t len)
+json_do_ext_community(struct ibuf *buf)
 {
        uint64_t ext;
-       uint16_t i;
-
-       if (len & 0x7) {
-               json_do_string("error", "bad length");
-               return;
-       }
-
        json_do_array("extended_communities");
 
-       for (i = 0; i < len; i += 8) {
-               memcpy(&ext, data + i, sizeof(ext));
-               ext = be64toh(ext);
+       while (ibuf_size(buf) > 0) {
+               if (ibuf_get_n64(buf, &ext) == -1) {
+                       json_do_string("error", "bad length");
+                       return;
+               }
                json_do_string("community", fmt_ext_community(ext));
        }
 
@@ -585,66 +568,57 @@ json_do_ext_community(u_char *data, uint16_t len)
 }
 
 static void
-json_attr(u_char *data, size_t len, int reqflags, int addpath)
+json_attr(struct ibuf *buf, int reqflags, int addpath)
 {
        struct bgpd_addr prefix;
        struct in_addr id;
-       struct ibuf ibuf, *buf = &ibuf, asbuf, *path = NULL;
+       struct ibuf asbuf, *path = NULL;
        char *aspath;
-       uint32_t as, pathid;
-       uint16_t alen, afi, off, short_as;
-       uint8_t flags, type, safi, aid, prefixlen;
+       uint32_t as, pathid, val;
+       uint16_t alen, afi, short_as;
+       uint8_t flags, type, safi, aid, prefixlen, origin;
        int e4, e2;
 
-       if (len < 3) {
-               warnx("Too short BGP attribute");
-               return;
-       }
-
-       flags = data[0];
-       type = data[1];
-       if (flags & ATTR_EXTLEN) {
-               if (len < 4) {
-                       warnx("Too short BGP attribute");
-                       return;
-               }
-               memcpy(&alen, data+2, sizeof(uint16_t));
-               alen = ntohs(alen);
-               data += 4;
-               len -= 4;
-       } else {
-               alen = data[2];
-               data += 3;
-               len -= 3;
-       }
-
-       /* bad imsg len how can that happen!? */
-       if (alen > len) {
-               warnx("Bad BGP attribute length");
-               return;
-       }
-
        json_do_array("attributes");
-
        json_do_object("attribute", 0);
+
+       if (ibuf_get_n8(buf, &flags) == -1 ||
+           ibuf_get_n8(buf, &type) == -1)
+               goto bad_len;
+
        json_do_string("type", fmt_attr(type, -1));
-       json_do_uint("length", alen);
        json_do_object("flags", 1);
        json_do_bool("partial", flags & ATTR_PARTIAL);
        json_do_bool("transitive", flags & ATTR_TRANSITIVE);
        json_do_bool("optional", flags & ATTR_OPTIONAL);
        json_do_end();
 
+       if (flags & ATTR_EXTLEN) {
+               uint16_t attr_len;
+               if (ibuf_get_n16(buf, &attr_len) == -1)
+                       goto bad_len;
+               alen = attr_len;
+       } else {
+               uint8_t attr_len;
+               if (ibuf_get_n8(buf, &attr_len) == -1)
+                       goto bad_len;
+               alen = attr_len;
+       }
+
+       json_do_uint("length", alen);
+
+       /* bad imsg len how can that happen!? */
+       if (alen > ibuf_size(buf))
+               goto bad_len;
+
        switch (type) {
        case ATTR_ORIGIN:
-               if (alen == 1)
-                       json_do_string("origin", fmt_origin(*data, 0));
-               else
-                       json_do_string("error", "bad length");
+               if (alen != 1 || ibuf_get_n8(buf, &origin) == -1)
+                       goto bad_len;
+               json_do_string("origin", fmt_origin(origin, 0));
                break;
        case ATTR_ASPATH:
        case ATTR_AS4_PATH:
-               ibuf_from_buffer(buf, data, alen);
                /* prefer 4-byte AS here */
                e4 = aspath_verify(buf, 1, 0);
                e2 = aspath_verify(buf, 0, 0);
@@ -668,70 +642,55 @@ json_attr(u_char *data, size_t len, int reqflags, int addpath)
                ibuf_free(path);
                break;
        case ATTR_NEXTHOP:
-               if (alen == 4) {
-                       memcpy(&id, data, sizeof(id));
-                       json_do_string("nexthop", inet_ntoa(id));
-               } else
-                       json_do_string("error", "bad length");
+               if (alen != 4 || ibuf_get(buf, &id, sizeof(id)) == -1)
+                       goto bad_len;
+               json_do_string("nexthop", inet_ntoa(id));
                break;
        case ATTR_MED:
        case ATTR_LOCALPREF:
-               if (alen == 4) {
-                       uint32_t val;
-                       memcpy(&val, data, sizeof(val));
-                       json_do_uint("metric", ntohl(val));
-               } else
-                       json_do_string("error", "bad length");
+               if (alen != 4 || ibuf_get_n32(buf, &val) == -1)
+                       goto bad_len;
+               json_do_uint("metric", val);
                break;
        case ATTR_AGGREGATOR:
        case ATTR_AS4_AGGREGATOR:
                if (alen == 8) {
-                       memcpy(&as, data, sizeof(as));
-                       memcpy(&id, data + sizeof(as), sizeof(id));
-                       as = ntohl(as);
+                       if (ibuf_get_n32(buf, &as) == -1 ||
+                           ibuf_get(buf, &id, sizeof(id)) == -1)
+                               goto bad_len;
                } else if (alen == 6) {
-                       memcpy(&short_as, data, sizeof(short_as));
-                       memcpy(&id, data + sizeof(short_as), sizeof(id));
-                       as = ntohs(short_as);
+                       if (ibuf_get_n16(buf, &short_as) == -1 ||
+                           ibuf_get(buf, &id, sizeof(id)) == -1)
+                               goto bad_len;
+                       as = short_as;
                } else {
-                       json_do_string("error", "bad AS-Path");
-                       break;
+                       goto bad_len;
                }
                json_do_uint("AS", as);
                json_do_string("router_id", inet_ntoa(id));
                break;
        case ATTR_COMMUNITIES:
-               json_do_community(data, alen);
+               json_do_community(buf);
                break;
        case ATTR_ORIGINATOR_ID:
-               if (alen == 4) {
-                       memcpy(&id, data, sizeof(id));
-                       json_do_string("originator", inet_ntoa(id));
-               } else
-                       json_do_string("error", "bad length");
+               if (alen != 4 || ibuf_get(buf, &id, sizeof(id)) == -1)
+                       goto bad_len;
+               json_do_string("originator", inet_ntoa(id));
                break;
        case ATTR_CLUSTER_LIST:
                json_do_array("cluster_list");
-               for (off = 0; off + sizeof(id) <= alen;
-                   off += sizeof(id)) {
-                       memcpy(&id, data + off, sizeof(id));
+               while (ibuf_size(buf) > 0) {
+                       if (ibuf_get(buf, &id, sizeof(id)) == -1)
+                               goto bad_len;
                        json_do_string("cluster_id", inet_ntoa(id));
                }
                json_do_end();
                break;
        case ATTR_MP_REACH_NLRI:
        case ATTR_MP_UNREACH_NLRI:
-               if (alen < 3) {
-bad_len:
-                       json_do_string("error", "bad length");
-                       break;
-               }
-               memcpy(&afi, data, 2);
-               data += 2;
-               alen -= 2;
-               afi = ntohs(afi);
-               safi = *data++;
-               alen--;
+               if (ibuf_get_n16(buf, &afi) == -1 ||
+                   ibuf_get_n8(buf, &safi) == -1)
+                       goto bad_len;
 
                if (afi2aid(afi, safi, &aid) == -1) {
                        json_do_printf("error", "bad AFI/SAFI pair: %d/%d",
@@ -743,11 +702,7 @@ bad_len:
                if (type == ATTR_MP_REACH_NLRI) {
                        struct bgpd_addr nexthop;
                        uint8_t nhlen;
-                       if (len == 0)
-                               goto bad_len;
-                       nhlen = *data++;
-                       alen--;
-                       if (nhlen > len)
+                       if (ibuf_get_n8(buf, &nhlen) == -1)
                                goto bad_len;
                        memset(&nexthop, 0, sizeof(nexthop));
                        switch (aid) {
@@ -755,21 +710,27 @@ bad_len:
                                nexthop.aid = aid;
                                if (nhlen != 16 && nhlen != 32)
                                        goto bad_len;
-                               memcpy(&nexthop.v6.s6_addr, data, 16);
+                               if (ibuf_get(buf, &nexthop.v6,
+                                   sizeof(nexthop.v6)) == -1)
+                                       goto bad_len;
                                break;
                        case AID_VPN_IPv4:
                                if (nhlen != 12)
                                        goto bad_len;
                                nexthop.aid = AID_INET;
-                               memcpy(&nexthop.v4, data + sizeof(uint64_t),
-                                   sizeof(nexthop.v4));
+                               if (ibuf_skip(buf, sizeof(uint64_t)) == -1 ||
+                                   ibuf_get(buf, &nexthop.v4,
+                                   sizeof(nexthop.v4)) == -1)
+                                       goto bad_len;
                                break;
                        case AID_VPN_IPv6:
                                if (nhlen != 24)
                                        goto bad_len;
                                nexthop.aid = AID_INET6;
-                               memcpy(&nexthop.v6, data + sizeof(uint64_t),
-                                   sizeof(nexthop.v6));
+                               if (ibuf_skip(buf, sizeof(uint64_t)) == -1 ||
+                                   ibuf_get(buf, &nexthop.v6,
+                                   sizeof(nexthop.v6)) == -1)
+                                       goto bad_len;
                                break;
                        default:
                                json_do_printf("error", "unhandled AID: %d",
@@ -777,14 +738,12 @@ bad_len:
                                return;
                        }
                        /* ignore reserved (old SNPA) field as per RFC4760 */
-                       data += nhlen + 1;
-                       alen -= nhlen + 1;
+                       if (ibuf_skip(buf, 1) == -1)
+                               goto bad_len;
 
                        json_do_string("nexthop", log_addr(&nexthop));
                }
 
-               ibuf_from_buffer(buf, data, alen);
-
                json_do_array("NLRI");
                while (ibuf_size(buf) > 0) {
                        json_do_object("prefix", 1);
@@ -821,25 +780,26 @@ bad_len:
                json_do_end();
                break;
        case ATTR_EXT_COMMUNITIES:
-               json_do_ext_community(data, alen);
+               json_do_ext_community(buf);
                break;
        case ATTR_LARGE_COMMUNITIES:
-               json_do_large_community(data, alen);
+               json_do_large_community(buf);
                break;
        case ATTR_OTC:
-               if (alen == 4) {
-                       memcpy(&as, data, sizeof(as));
-                       as = ntohl(as);
-                       json_do_uint("as", as);
-               } else
-                       json_do_string("error", "bad length");
+               if (alen != 4 || ibuf_get_n32(buf, &as) == -1)
+                       goto bad_len;
+               json_do_uint("as", as);
                break;
        case ATTR_ATOMIC_AGGREGATE:
        default:
                if (alen)
-                       json_do_hexdump("data", data, alen);
+                       json_do_hexdump("data", ibuf_data(buf), ibuf_size(buf));
                break;
        }
+       return;
+
+ bad_len:
+       json_do_string("error", "bad length");
 }
 
 static void