Follow rfc8277 more closely and make make sure bgpd is encoding VPNv4

withdraws they way other systems are doing it. Interop problem discovered
by Andrew Thrift. Tested by Andrew and job@.

4 files changed, 51 insertions, 32 deletions

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 478bad48cba..b21becdde9f 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.378 2018/02/10 01:24:28 benno Exp $ */
+/*	$OpenBSD: rde.c,v 1.379 2018/02/10 05:54:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -68,7 +68,7 @@ int		 rde_update_get_prefix(u_char *, u_int16_t, struct bgpd_addr *,
 int		 rde_update_get_prefix6(u_char *, u_int16_t, struct bgpd_addr *,
 		     u_int8_t *);
 int		 rde_update_get_vpn4(u_char *, u_int16_t, struct bgpd_addr *,
-		     u_int8_t *);
+		     u_int8_t *, int);
 void		 rde_update_err(struct rde_peer *, u_int8_t , u_int8_t,
 		     void *, u_int16_t);
 void		 rde_update_log(const char *, u_int16_t,
@@ -1146,7 +1146,7 @@ rde_update_dispatch(struct imsg *imsg)
 		case AID_VPN_IPv4:
 			while (mplen > 0) {
 				if ((pos = rde_update_get_vpn4(mpp, mplen,
-				    &prefix, &prefixlen)) == -1) {
+				    &prefix, &prefixlen, 1)) == -1) {
 					log_peer_warnx(&peer->conf,
 					    "bad VPNv4 withdraw prefix");
 					rde_update_err(peer, ERR_UPDATE,
@@ -1318,7 +1318,7 @@ rde_update_dispatch(struct imsg *imsg)
 		case AID_VPN_IPv4:
 			while (mplen > 0) {
 				if ((pos = rde_update_get_vpn4(mpp, mplen,
-				    &prefix, &prefixlen)) == -1) {
+				    &prefix, &prefixlen, 0)) == -1) {
 					log_peer_warnx(&peer->conf,
 					    "bad VPNv4 nlri prefix");
 					rde_update_err(peer, ERR_UPDATE,
@@ -1998,7 +1998,7 @@ rde_update_get_prefix6(u_char *p, u_int16_t len, struct bgpd_addr *prefix,
 
 int
 rde_update_get_vpn4(u_char *p, u_int16_t len, struct bgpd_addr *prefix,
-    u_int8_t *prefixlen)
+    u_int8_t *prefixlen, int withdraw)
 {
 	int		 rv, done = 0;
 	u_int8_t	 pfxlen;
@@ -2020,6 +2020,12 @@ rde_update_get_vpn4(u_char *p, u_int16_t len, struct bgpd_addr *prefix,
 		if (prefix->vpn4.labellen + 3U >
 		    sizeof(prefix->vpn4.labelstack))
 			return (-1);
+		if (withdraw) {
+			/* on withdraw ignore the labelstack all together */
+			plen += 3;
+			pfxlen -= 3 * 8;
+			break;
+		}
 		prefix->vpn4.labelstack[prefix->vpn4.labellen++] = *p++;
 		prefix->vpn4.labelstack[prefix->vpn4.labellen++] = *p++;
 		prefix->vpn4.labelstack[prefix->vpn4.labellen] = *p++;
@@ -2746,7 +2752,7 @@ rde_update_queue_runner(void)
 			/* first withdraws */
 			wpos = 2; /* reserve space for the length field */
 			r = up_dump_prefix(queue_buf + wpos, len - wpos - 2,
-			    &peer->withdraws[AID_INET], peer);
+			    &peer->withdraws[AID_INET], peer, 1);
 			wd_len = r;
 			/* write withdraws length filed */
 			wd_len = htons(wd_len);
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index 8b0dcb389af..caf682a55e1 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.167 2018/02/10 01:24:28 benno Exp $ */
+/*	$OpenBSD: rde.h,v 1.168 2018/02/10 05:54:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -484,7 +484,7 @@ void		 path_put(struct rde_aspath *);
 #define	PREFIX_SIZE(x)	(((x) + 7) / 8 + 1)
 int		 prefix_remove(struct rib *, struct rde_peer *,
 		    struct bgpd_addr *, int, u_int32_t);
-int		 prefix_write(u_char *, int, struct bgpd_addr *, u_int8_t);
+int		 prefix_write(u_char *, int, struct bgpd_addr *, u_int8_t, int);
 int		 prefix_writebuf(struct ibuf *, struct bgpd_addr *, u_int8_t);
 struct prefix	*prefix_bypeer(struct rib_entry *, struct rde_peer *,
 		     u_int32_t);
@@ -529,7 +529,7 @@ void		 up_generate_default(struct filter_head *, struct rde_peer *,
 		     u_int8_t);
 int		 up_generate_marker(struct rde_peer *, u_int8_t);
 int		 up_dump_prefix(u_char *, int, struct uplist_prefix *,
-		     struct rde_peer *);
+		     struct rde_peer *, int);
 int		 up_dump_attrnlri(u_char *, int, struct rde_peer *);
 u_char		*up_dump_mp_unreach(u_char *, u_int16_t *, struct rde_peer *,
 		     u_int8_t);
diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c
index cddb8d3e23d..fa84dadc53b 100644
--- a/usr.sbin/bgpd/rde_rib.c
+++ b/usr.sbin/bgpd/rde_rib.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_rib.c,v 1.158 2018/02/07 00:02:02 claudio Exp $ */
+/*	$OpenBSD: rde_rib.c,v 1.159 2018/02/10 05:54:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org>
@@ -863,7 +863,8 @@ prefix_remove(struct rib *rib, struct rde_peer *peer, struct bgpd_addr *prefix,
 
 /* dump a prefix into specified buffer */
 int
-prefix_write(u_char *buf, int len, struct bgpd_addr *prefix, u_int8_t plen)
+prefix_write(u_char *buf, int len, struct bgpd_addr *prefix, u_int8_t plen,
+    int withdraw)
 {
 	int	totlen;
 
@@ -878,15 +879,30 @@ prefix_write(u_char *buf, int len, struct bgpd_addr *prefix, u_int8_t plen)
 		memcpy(buf, &prefix->ba, totlen - 1);
 		return (totlen);
 	case AID_VPN_IPv4:
-		totlen = PREFIX_SIZE(plen) + sizeof(prefix->vpn4.rd) +
-		    prefix->vpn4.labellen;
-		plen += (sizeof(prefix->vpn4.rd) + prefix->vpn4.labellen) * 8;
+		totlen = PREFIX_SIZE(plen) + sizeof(prefix->vpn4.rd);
+		plen += sizeof(prefix->vpn4.rd) * 8;
+		if (withdraw) {
+			/* withdraw have one compat label as placeholder */
+			totlen += 3;
+			plen += 3 * 8;
+		} else {
+			totlen += prefix->vpn4.labellen;
+			plen += prefix->vpn4.labellen * 8;
+		}
 
 		if (totlen > len)
 			return (-1);
 		*buf++ = plen;
-		memcpy(buf, &prefix->vpn4.labelstack, prefix->vpn4.labellen);
-		buf += prefix->vpn4.labellen;
+		if (withdraw) {
+			/* magic compatibility label as per rfc8277 */
+			*buf++ = 0x80;
+			*buf++ = 0x0;
+			*buf++ = 0x0;
+		} else  {
+			memcpy(buf, &prefix->vpn4.labelstack,
+			    prefix->vpn4.labellen);
+			buf += prefix->vpn4.labellen;
+		}
 		memcpy(buf, &prefix->vpn4.rd, sizeof(prefix->vpn4.rd));
 		buf += sizeof(prefix->vpn4.rd);
 		memcpy(buf, &prefix->vpn4.addr, PREFIX_SIZE(plen) - 1);
@@ -917,7 +933,7 @@ prefix_writebuf(struct ibuf *buf, struct bgpd_addr *prefix, u_int8_t plen)
 
 	if ((bptr = ibuf_reserve(buf, totlen)) == NULL)
 		return (-1);
-	if (prefix_write(bptr, totlen, prefix, plen) == -1)
+	if (prefix_write(bptr, totlen, prefix, plen, 0) == -1)
 		return (-1);
 	return (0);
 }
diff --git a/usr.sbin/bgpd/rde_update.c b/usr.sbin/bgpd/rde_update.c
index 80c13193291..9028f5766a3 100644
--- a/usr.sbin/bgpd/rde_update.c
+++ b/usr.sbin/bgpd/rde_update.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_update.c,v 1.88 2018/02/05 03:55:54 claudio Exp $ */
+/*	$OpenBSD: rde_update.c,v 1.89 2018/02/10 05:54:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -956,29 +956,26 @@ up_generate_attr(struct rde_peer *peer, struct update_attr *upa,
 #define MIN_PREFIX_LEN	5	/* 1 byte prefix length + 4 bytes addr */
 int
 up_dump_prefix(u_char *buf, int len, struct uplist_prefix *prefix_head,
-    struct rde_peer *peer)
+    struct rde_peer *peer, int withdraw)
 {
 	struct update_prefix	*upp;
 	int			 r, wpos = 0;
-	u_int8_t		 i;
 
 	while ((upp = TAILQ_FIRST(prefix_head)) != NULL) {
 		if ((r = prefix_write(buf + wpos, len - wpos,
-		    &upp->prefix, upp->prefixlen)) == -1)
+		    &upp->prefix, upp->prefixlen, withdraw)) == -1)
 			break;
 		wpos += r;
 		if (RB_REMOVE(uptree_prefix, &peer->up_prefix, upp) == NULL)
 			log_warnx("dequeuing update failed.");
 		TAILQ_REMOVE(upp->prefix_h, upp, prefix_l);
 		peer->up_pcnt--;
-		for (i = 0; i < AID_MAX; i++) {
-			if (upp->prefix_h == &peer->withdraws[i]) {
-				peer->up_wcnt--;
-				peer->prefix_sent_withdraw++;
-			} else {
-				peer->up_nlricnt--;
-				peer->prefix_sent_update++;
-			}
+		if (withdraw) {
+			peer->up_wcnt--;
+			peer->prefix_sent_withdraw++;
+		} else {
+			peer->up_nlricnt--;
+			peer->prefix_sent_update++;
 		}
 		free(upp);
 	}
@@ -1034,7 +1031,7 @@ up_dump_attrnlri(u_char *buf, int len, struct rde_peer *peer)
 	wpos += upa->attr_len;
 
 	/* last but not least dump the nlri */
-	r = up_dump_prefix(buf + wpos, len - wpos, &upa->prefix_h, peer);
+	r = up_dump_prefix(buf + wpos, len - wpos, &upa->prefix_h, peer, 0);
 	wpos += r;
 
 	/* now check if all prefixes were written */
@@ -1070,7 +1067,7 @@ up_dump_mp_unreach(u_char *buf, u_int16_t *len, struct rde_peer *peer,
 		return (NULL);
 
 	datalen = up_dump_prefix(buf + wpos, *len - wpos,
-	    &peer->withdraws[aid], peer);
+	    &peer->withdraws[aid], peer, 1);
 	if (datalen == 0)
 		return (NULL);
 
@@ -1162,7 +1159,7 @@ up_dump_mp_reach(u_char *buf, u_int16_t *len, struct rde_peer *peer,
 		return (-2);
 
 	datalen = up_dump_prefix(buf + wpos, *len - wpos,
-	    &upa->prefix_h, peer);
+	    &upa->prefix_h, peer, 0);
 	if (datalen == 0)
 		return (-2);