From: claudio <claudio@openbsd.org>
Date: Tue, 23 Jan 2024 16:16:15 +0000 (+0000)
Subject: Adopt bgpctl code to the ibuf changes done in bgpd.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=5ad4fcbad159abc601679860953f9a7f07c2729f;p=openbsd

Adopt bgpctl code to the ibuf changes done in bgpd.

Adjust code that calls nlri_get_prefix() and friends to work with
an ibuf. show_mrt_update() is mostly converted because of this.
The output functions and the rest of the mrtparser are just
minimally touched to keep the diff reasonable.
OK tb@
---

diff --git a/usr.sbin/bgpctl/bgpctl.c b/usr.sbin/bgpctl/bgpctl.c
index 4e5ba2b7c5a..90e262cbd75 100644
--- a/usr.sbin/bgpctl/bgpctl.c
+++ b/usr.sbin/bgpctl/bgpctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bgpctl.c,v 1.300 2024/01/18 14:46:21 claudio Exp $ */
+/*	$OpenBSD: bgpctl.c,v 1.301 2024/01/23 16:16:15 claudio Exp $ */
 
 /*
  * Copyright (c) 2003 Henning Brauer <henning@openbsd.org>
@@ -1709,118 +1709,85 @@ static void
 show_mrt_update(u_char *p, uint16_t len, int reqflags, int addpath)
 {
 	struct bgpd_addr prefix;
-	int pos;
+	struct ibuf *b, buf, wbuf, abuf;
 	uint32_t pathid;
 	uint16_t wlen, alen;
 	uint8_t prefixlen;
 
-	if (len < sizeof(wlen)) {
-		printf("bad length");
-		return;
-	}
-	memcpy(&wlen, p, sizeof(wlen));
-	wlen = ntohs(wlen);
-	p += sizeof(wlen);
-	len -= sizeof(wlen);
-
-	if (len < wlen) {
-		printf("bad withdraw length");
-		return;
-	}
+	ibuf_from_buffer(&buf, p, len);
+	b = &buf;
+	if (ibuf_get_n16(b, &wlen) == -1 ||
+	    ibuf_get_ibuf(b, wlen, &wbuf) == -1)
+		goto trunc;
 	if (wlen > 0) {
 		printf("\n     Withdrawn prefixes:");
-		while (wlen > 0) {
-			if (addpath) {
-				if (wlen <= sizeof(pathid)) {
-					printf("bad withdraw prefix");
-					return;
-				}
-				memcpy(&pathid, p, sizeof(pathid));
-				pathid = ntohl(pathid);
-				p += sizeof(pathid);
-				len -= sizeof(pathid);
-				wlen -= sizeof(pathid);
-			}
-			if ((pos = nlri_get_prefix(p, wlen, &prefix,
-			    &prefixlen)) == -1) {
-				printf("bad withdraw prefix");
-				return;
-			}
+		while (ibuf_size(&wbuf) > 0) {
+			if (addpath)
+				if (ibuf_get_n32(&wbuf, &pathid) == -1)
+					goto trunc;
+			if (nlri_get_prefix(&wbuf, &prefix, &prefixlen) == -1)
+				goto trunc;
+
 			printf(" %s/%u", log_addr(&prefix), prefixlen);
 			if (addpath)
 				printf(" path-id %u", pathid);
-			p += pos;
-			len -= pos;
-			wlen -= pos;
 		}
 	}
 
-	if (len < sizeof(alen)) {
-		printf("bad length");
-		return;
-	}
-	memcpy(&alen, p, sizeof(alen));
-	alen = ntohs(alen);
-	p += sizeof(alen);
-	len -= sizeof(alen);
+	if (ibuf_get_n16(b, &alen) == -1 ||
+	    ibuf_get_ibuf(b, alen, &abuf) == -1)
+		goto trunc;
 
-	if (len < alen) {
-		printf("bad attribute length");
-		return;
-	}
 	printf("\n");
 	/* alen attributes here */
-	while (alen > 3) {
-		uint8_t flags;
+	while (ibuf_size(&abuf) > 0) {
+		struct ibuf attrbuf;
 		uint16_t attrlen;
+		uint8_t flags;
 
-		flags = p[0];
-		/* type = p[1]; */
+		ibuf_from_ibuf(&abuf, &attrbuf);
+		if (ibuf_get_n8(&attrbuf, &flags) == -1 ||
+		    ibuf_skip(&attrbuf, 1) == -1)
+			goto trunc;
 
 		/* get the attribute length */
 		if (flags & ATTR_EXTLEN) {
-			if (len < sizeof(attrlen) + 2)
-				printf("bad attribute length");
-			memcpy(&attrlen, &p[2], sizeof(attrlen));
-			attrlen = ntohs(attrlen);
-			attrlen += sizeof(attrlen) + 2;
+			if (ibuf_get_n16(&attrbuf, &attrlen) == -1)
+				goto trunc;
 		} else {
-			attrlen = p[2];
-			attrlen += 1 + 2;
+			uint8_t tmp;
+			if (ibuf_get_n8(&attrbuf, &tmp) == -1)
+				goto trunc;
+			attrlen = tmp;
 		}
-
-		output->attr(p, attrlen, reqflags, addpath);
-		p += attrlen;
-		alen -= attrlen;
-		len -= attrlen;
+		if (ibuf_truncate(&attrbuf, attrlen) == -1)
+			goto trunc;
+		ibuf_rewind(&attrbuf);
+		if (ibuf_skip(&abuf, ibuf_size(&attrbuf)) == -1)
+			goto trunc;
+
+		output->attr(ibuf_data(&attrbuf), ibuf_size(&attrbuf),
+		    reqflags, addpath);
 	}
 
-	if (len > 0) {
+	if (ibuf_size(b) > 0) {
 		printf("    NLRI prefixes:");
-		while (len > 0) {
-			if (addpath) {
-				if (len <= sizeof(pathid)) {
-					printf(" bad nlri prefix: pathid, "
-					    "len %d", len);
-					return;
-				}
-				memcpy(&pathid, p, sizeof(pathid));
-				pathid = ntohl(pathid);
-				p += sizeof(pathid);
-				len -= sizeof(pathid);
-			}
-			if ((pos = nlri_get_prefix(p, len, &prefix,
-			    &prefixlen)) == -1) {
-				printf(" bad nlri prefix");
-				return;
-			}
+		while (ibuf_size(b) > 0) {
+			if (addpath)
+				if (ibuf_get_n32(b, &pathid) == -1)
+					goto trunc;
+			if (nlri_get_prefix(b, &prefix, &prefixlen) == -1)
+				goto trunc;
+
 			printf(" %s/%u", log_addr(&prefix), prefixlen);
 			if (addpath)
 				printf(" path-id %u", pathid);
-			p += pos;
-			len -= pos;
 		}
 	}
+	return;
+
+ trunc:
+	printf("truncated message");
 }
 
 void
diff --git a/usr.sbin/bgpctl/mrtparser.c b/usr.sbin/bgpctl/mrtparser.c
index 39b101c141d..35c1b60d61c 100644
--- a/usr.sbin/bgpctl/mrtparser.c
+++ b/usr.sbin/bgpctl/mrtparser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mrtparser.c,v 1.20 2023/11/20 14:18:21 claudio Exp $ */
+/*	$OpenBSD: mrtparser.c,v 1.21 2024/01/23 16:16:15 claudio Exp $ */
 /*
  * Copyright (c) 2011 Claudio Jeker <claudio@openbsd.org>
  *
@@ -398,7 +398,7 @@ mrt_parse_v2_rib(struct mrt_hdr *hdr, void *msg, int verbose)
 		/* prefix */
 		ret = mrt_extract_prefix(b, len, AID_INET, &r->prefix,
 		    &r->prefixlen, verbose);
-		if (ret == 1)
+		if (ret == -1)
 			goto fail;
 		break;
 	case MRT_DUMP_V2_RIB_IPV6_UNICAST_ADDPATH:
@@ -410,7 +410,7 @@ mrt_parse_v2_rib(struct mrt_hdr *hdr, void *msg, int verbose)
 		/* prefix */
 		ret = mrt_extract_prefix(b, len, AID_INET6, &r->prefix,
 		    &r->prefixlen, verbose);
-		if (ret == 1)
+		if (ret == -1)
 			goto fail;
 		break;
 	case MRT_DUMP_V2_RIB_GENERIC_ADDPATH:
@@ -439,7 +439,7 @@ mrt_parse_v2_rib(struct mrt_hdr *hdr, void *msg, int verbose)
 		/* prefix */
 		ret = mrt_extract_prefix(b, len, aid, &r->prefix,
 		    &r->prefixlen, verbose);
-		if (ret == 1)
+		if (ret == -1)
 			goto fail;
 		break;
 	default:
@@ -744,7 +744,7 @@ mrt_parse_dump_mp(struct mrt_hdr *hdr, void *msg, struct mrt_peer **pp,
 	/* prefix */
 	ret = mrt_extract_prefix(b, len, aid, &r->prefix, &r->prefixlen,
 	    verbose);
-	if (ret == 1)
+	if (ret == -1)
 		goto fail;
 	b += ret;
 	len -= ret;
@@ -1032,23 +1032,25 @@ mrt_extract_addr(void *msg, u_int len, struct bgpd_addr *addr, uint8_t aid)
 }
 
 int
-mrt_extract_prefix(void *msg, u_int len, uint8_t aid,
+mrt_extract_prefix(void *m, u_int len, uint8_t aid,
     struct bgpd_addr *prefix, uint8_t *prefixlen, int verbose)
 {
+	struct ibuf buf, *msg = &buf;
 	int r;
 
+	ibuf_from_buffer(msg, m, len); /* XXX */
 	switch (aid) {
 	case AID_INET:
-		r = nlri_get_prefix(msg, len, prefix, prefixlen);
+		r = nlri_get_prefix(msg, prefix, prefixlen);
 		break;
 	case AID_INET6:
-		r = nlri_get_prefix6(msg, len, prefix, prefixlen);
+		r = nlri_get_prefix6(msg, prefix, prefixlen);
 		break;
 	case AID_VPN_IPv4:
-		r = nlri_get_vpn4(msg, len, prefix, prefixlen, 0);
+		r = nlri_get_vpn4(msg, prefix, prefixlen, 0);
 		break;
 	case AID_VPN_IPv6:
-		r = nlri_get_vpn6(msg, len, prefix, prefixlen, 0);
+		r = nlri_get_vpn6(msg, prefix, prefixlen, 0);
 		break;
 	default:
 		if (verbose)
@@ -1057,6 +1059,8 @@ mrt_extract_prefix(void *msg, u_int len, uint8_t aid,
 	}
 	if (r == -1 && verbose)
 		printf("failed to parse prefix of AID %d\n", aid);
+	if (r != -1)
+		r = len - ibuf_size(msg); /* XXX */
 	return r;
 }
 
diff --git a/usr.sbin/bgpctl/output.c b/usr.sbin/bgpctl/output.c
index 816e0b3d7bd..3eddeaff834 100644
--- a/usr.sbin/bgpctl/output.c
+++ b/usr.sbin/bgpctl/output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: output.c,v 1.46 2024/01/11 14:34:49 claudio Exp $ */
+/*	$OpenBSD: output.c,v 1.47 2024/01/23 16:16:15 claudio Exp $ */
 
 /*
  * Copyright (c) 2003 Henning Brauer <henning@openbsd.org>
@@ -772,11 +772,12 @@ show_attr(u_char *data, size_t len, int reqflags, int addpath)
 	u_char		*path;
 	struct in_addr	 id;
 	struct bgpd_addr prefix;
+	struct ibuf	 ibuf, *buf = &ibuf;
 	char		*aspath;
 	uint32_t	 as, pathid;
 	uint16_t	 alen, ioff, short_as, afi;
 	uint8_t		 flags, type, safi, aid, prefixlen;
-	int		 i, pos, e2, e4;
+	int		 i, e2, e4;
 
 	if (len < 3) {
 		warnx("Too short BGP attribute");
@@ -951,43 +952,35 @@ show_attr(u_char *data, size_t len, int reqflags, int addpath)
 			printf(" nexthop: %s", log_addr(&nexthop));
 		}
 
-		while (alen > 0) {
-			if (addpath) {
-				if (alen <= sizeof(pathid)) {
-					printf("bad nlri prefix");
-					return;
-				}
-				memcpy(&pathid, data, sizeof(pathid));
-				pathid = ntohl(pathid);
-				data += sizeof(pathid);
-				alen -= sizeof(pathid);
-			}
+		ibuf_from_buffer(buf, data, alen);
+
+		while (ibuf_size(buf) > 0) {
+			if (addpath)
+				if (ibuf_get_n32(buf, &pathid) == -1)
+					goto bad_len;
 			switch (aid) {
 			case AID_INET6:
-				pos = nlri_get_prefix6(data, alen, &prefix,
-				    &prefixlen);
+				if (nlri_get_prefix6(buf, &prefix,
+				    &prefixlen) == -1)
+					goto bad_len;
 				break;
 			case AID_VPN_IPv4:
-				pos = nlri_get_vpn4(data, alen, &prefix,
-				    &prefixlen, 1);
+				if (nlri_get_vpn4(buf, &prefix,
+				    &prefixlen, 1) == -1)
+					goto bad_len;
 				break;
 			case AID_VPN_IPv6:
-				pos = nlri_get_vpn6(data, alen, &prefix,
-				    &prefixlen, 1);
+				if (nlri_get_vpn6(buf, &prefix,
+				    &prefixlen, 1) == -1)
+					goto bad_len;
 				break;
 			default:
 				printf("unhandled AID #%u", aid);
 				goto done;
 			}
-			if (pos == -1) {
-				printf("bad %s prefix", aid2str(aid));
-				break;
-			}
 			printf(" %s/%u", log_addr(&prefix), prefixlen);
 			if (addpath)
 				printf(" path-id %u", pathid);
-			data += pos;
-			alen -= pos;
 		}
 		break;
 	case ATTR_EXT_COMMUNITIES:
diff --git a/usr.sbin/bgpctl/output_json.c b/usr.sbin/bgpctl/output_json.c
index 25c3950f9da..f04a41be86f 100644
--- a/usr.sbin/bgpctl/output_json.c
+++ b/usr.sbin/bgpctl/output_json.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: output_json.c,v 1.38 2024/01/11 13:09:41 claudio Exp $ */
+/*	$OpenBSD: output_json.c,v 1.39 2024/01/23 16:16:15 claudio Exp $ */
 
 /*
  * Copyright (c) 2020 Claudio Jeker <claudio@openbsd.org>
@@ -589,12 +589,13 @@ json_attr(u_char *data, size_t len, int reqflags, int addpath)
 {
 	struct bgpd_addr prefix;
 	struct in_addr id;
+	struct ibuf ibuf, *buf = &ibuf;
 	char *aspath;
 	u_char *path;
 	uint32_t as, pathid;
 	uint16_t alen, afi, off, short_as;
 	uint8_t flags, type, safi, aid, prefixlen;
-	int e4, e2, pos;
+	int e4, e2;
 
 	if (len < 3) {
 		warnx("Too short BGP attribute");
@@ -780,48 +781,39 @@ bad_len:
 			json_do_string("nexthop", log_addr(&nexthop));
 		}
 
+		ibuf_from_buffer(buf, data, alen);
+
 		json_do_array("NLRI");
-		while (alen > 0) {
+		while (ibuf_size(buf) > 0) {
 			json_do_object("prefix", 1);
-			if (addpath) {
-				if (alen <= sizeof(pathid)) {
-					json_do_string("error", "bad path-id");
-					break;
-				}
-				memcpy(&pathid, data, sizeof(pathid));
-				pathid = ntohl(pathid);
-				data += sizeof(pathid);
-				alen -= sizeof(pathid);
-			}
+			if (addpath)
+				if (ibuf_get_n32(buf, &pathid) == -1)
+					goto bad_len;
 			switch (aid) {
 			case AID_INET6:
-				pos = nlri_get_prefix6(data, alen, &prefix,
-				    &prefixlen);
+				if (nlri_get_prefix6(buf, &prefix,
+				    &prefixlen) == -1)
+					goto bad_len;
 				break;
 			case AID_VPN_IPv4:
-				 pos = nlri_get_vpn4(data, alen, &prefix,
-				     &prefixlen, 1);
-				 break;
+				if (nlri_get_vpn4(buf, &prefix,
+				    &prefixlen, 1) == -1)
+					goto bad_len;
+				break;
 			case AID_VPN_IPv6:
-				 pos = nlri_get_vpn6(data, alen, &prefix,
-				     &prefixlen, 1);
-				 break;
+				if (nlri_get_vpn6(buf, &prefix,
+				    &prefixlen, 1) == -1)
+					goto bad_len;
+				break;
 			default:
 				json_do_printf("error", "unhandled AID: %d",
 				    aid);
 				return;
 			}
-			if (pos == -1) {
-				json_do_printf("error", "bad %s prefix",
-				    aid2str(aid));
-				break;
-			}
 			json_do_printf("prefix", "%s/%u", log_addr(&prefix),
 			    prefixlen);
 			if (addpath)
 				 json_do_uint("path_id", pathid);
-			data += pos;
-			alen -= pos;
 			json_do_end();
 		}
 		json_do_end();