BGP: Fix Notification for errors in OPEN message

Ensure that when the received OPEN message has errors, NOTIFICATION is
generated with the proper error code.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Daniel Walton <dwalton@cumulusnetworks.com>
Ticket:  CM-5974
Reviewed By: CCR-3945
Testing Done: Manual

Note: This fix should be sent upstream.

diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index d7a828b0237cb0e5772f243b39d3f869e13b717c..ed99a8eda43c79b39739c1108903d0b3a5847a0b 100644 (file)
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -292,7 +292,7 @@ bgp_capability_orf_entry (struct peer *peer, struct capability_header *hdr)
       zlog_info ("%s ORF Capability entry length error,"
                  " Cap length %u, num %u",
                  peer->host, hdr->length, entry.num);
-      bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+      bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
       return -1;
     }
 
@@ -732,7 +732,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int *mp_capability,
       if (stream_get_getp(s) + 2 > end)
        {
          zlog_info ("%s Capability length error (< header)", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
          return -1;
        }
       
@@ -744,7 +744,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int *mp_capability,
       if (start + caphdr.length > end)
        {
          zlog_info ("%s Capability length error (< length)", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
          return -1;
        }
       
@@ -778,7 +778,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int *mp_capability,
                              LOOKUP (capcode_str, caphdr.code),
                              caphdr.length, 
                             (unsigned) cap_minsizes[caphdr.code]);
-                  bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+                  bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
                   return -1;
                 }
           /* we deliberately ignore unknown codes, see below */
@@ -866,7 +866,7 @@ bgp_capability_parse (struct peer *peer, size_t length, int *mp_capability,
 
       if (ret < 0)
        {
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
          return -1;
        }
       if (stream_get_getp(s) != (start + caphdr.length))
@@ -913,10 +913,6 @@ peek_for_as4_capability (struct peer *peer, u_char length)
   size_t end = orig_getp + length;
   as_t as4 = 0;
   
-  /* The full capability parser will better flag the error.. */
-  if (STREAM_READABLE(s) < length)
-    return 0;
-
   if (BGP_DEBUG (as4, AS4))
     zlog_info ("%s [AS4] rcv OPEN w/ OPTION parameter len: %u,"
                 " peeking for as4",
@@ -1009,7 +1005,7 @@ bgp_open_option_parse (struct peer *peer, u_char length, int *mp_capability)
       if (STREAM_READABLE(s) < 2)
        {
          zlog_info ("%s Option length error", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
          return -1;
        }
 
@@ -1021,7 +1017,7 @@ bgp_open_option_parse (struct peer *peer, u_char length, int *mp_capability)
       if (STREAM_READABLE (s) < opt_length)
        {
          zlog_info ("%s Option length error", peer->host);
-         bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+         bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_MALFORMED_ATTR);
          return -1;
        }
 

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 7f4611c2d0eca9a4bbab40dcc71d42cf0c77ae01..be3a52864ad0759c3be05a51ed36fe15851784ad 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -918,6 +918,14 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
   
   if (optlen != 0)
     {
+      /* If not enough bytes, it is an error. */
+      if (STREAM_READABLE(peer->ibuf) < optlen)
+        {
+          bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR,
+                           BGP_NOTIFY_OPEN_MALFORMED_ATTR);
+          return -1;
+        }
+
       /* We need the as4 capability value *right now* because
        * if it is there, we have not got the remote_as yet, and without
        * that we do not know which peer is connecting to us now.