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@.

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 478bad4..b21becd 100644 (file)
--- 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 8b0dcb3..caf682a 100644 (file)
--- 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 cddb8d3..fa84dad 100644 (file)
--- 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 80c1319..9028f57 100644 (file)
--- 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);