bgpd: Treat empty reachable NLRI as a EOR

This issue was discovered on a live session with an extremely
old cisco 7206VXR router running 12.2(33)SRE4.  The sending router
is sending us an empty NLRI that is MP_REACH.  From RFC
exploration(thanks Russ!) it appears that this was
considered a 'valid' way to send EOR.

Following discussion decided that we should treat
this situation as a EOR marker instead of bringing
down the session.

Applying this fix on the FRR router seeing this issue
allows it to continue it's peering relationship with
the ASR.  Since this is a point fix I do not see
a high likelihood of further fallout.

Fixes: #1258
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 2d3158d8471de4d36ae2a4000e61a578a1483e2b..97cfeee4e4ec445cae9699829926a755eaa4f8ac 100644 (file)
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1709,11 +1709,20 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
 
        /* must have nrli_len, what is left of the attribute */
        nlri_len = LEN_LEFT;
-       if ((!nlri_len) || (nlri_len > STREAM_READABLE(s))) {
+       if (nlri_len > STREAM_READABLE(s)) {
                zlog_info("%s: (%s) Failed to read NLRI", __func__, peer->host);
                return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
        }
 
+       if (!nlri_len) {
+               zlog_info("%s: (%s) No Reachability, Treating as a EOR marker",
+                         __func__, peer->host);
+
+               mp_update->afi = afi;
+               mp_update->safi = safi;
+               return BGP_ATTR_PARSE_EOR;
+       }
+
        mp_update->afi = afi;
        mp_update->safi = safi;
        mp_update->nlri = stream_pnt(s);
@@ -2378,6 +2387,12 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
                        ret = BGP_ATTR_PARSE_ERROR;
                }
 
+               if (ret == BGP_ATTR_PARSE_EOR) {
+                       if (as4_path)
+                               aspath_unintern(&as4_path);
+                       return ret;
+               }
+
                /* If hard error occured immediately return to the caller. */
                if (ret == BGP_ATTR_PARSE_ERROR) {
                        zlog_warn("%s: Attribute %s, parse error", peer->host,

diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index 6edbf439025ad9cce9b15509a51aec32c8688edd..80ff36b59fdd106164008b9e1bed15bd4c77065f 100644 (file)
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -227,6 +227,7 @@ typedef enum {
        /* only used internally, send notify + convert to BGP_ATTR_PARSE_ERROR
           */
        BGP_ATTR_PARSE_ERROR_NOTIFYPLS = -3,
+       BGP_ATTR_PARSE_EOR = -4,
 } bgp_attr_parse_ret_t;
 
 struct bpacket_attr_vec_arr;

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 003fbaff632913bdc0d5851da1503277a03e11c6..a66d0590c951a8c02211e2bc477158fdb054192e 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1553,7 +1553,9 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
         * Non-MP IPv4/Unicast EoR is a completely empty UPDATE
         * and MP EoR should have only an empty MP_UNREACH
         */
-       if (!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0) {
+       if ((!update_len && !withdraw_len &&
+            nlris[NLRI_MP_UPDATE].length == 0) ||
+           (attr_parse_ret == BGP_ATTR_PARSE_EOR)) {
                afi_t afi = 0;
                safi_t safi;
 
@@ -1568,6 +1570,9 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
                           && nlris[NLRI_MP_WITHDRAW].length == 0) {
                        afi = nlris[NLRI_MP_WITHDRAW].afi;
                        safi = nlris[NLRI_MP_WITHDRAW].safi;
+               } else if (attr_parse_ret == BGP_ATTR_PARSE_EOR) {
+                       afi = nlris[NLRI_MP_UPDATE].afi;
+                       safi = nlris[NLRI_MP_UPDATE].safi;
                }
 
                if (afi && peer->afc[afi][safi]) {