bgpd: improve NEXT_HOP attribute checks (BZ#680)

* lib/prefix.h
  * IPV4_CLASS_DE(): new helper macro
* bgp_attr.c
  * bgp_attr_nexthop(): add check for "partial" bit, refresh flag error
    reporting, explain meaning of RFC4271 section 6.3 and implement it

2 files changed, 48 insertions, 8 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 989c22b1..1300ab84 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -898,17 +898,37 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
 		  struct attr *attr, u_char flag, u_char *startp)
 {
   bgp_size_t total;
+  in_addr_t nexthop_h, nexthop_n;
 
   total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
 
-  /* Flag check. */
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
-      || ! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
+  /* Flags check. */
+  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
     {
-      zlog (peer->log, LOG_ERR, 
-	    "Origin attribute flag isn't transitive %d", flag);
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
+      zlog (peer->log, LOG_ERR,
+	    "NEXT_HOP attribute must not be flagged as \"optional\" (%u)", flag);
+      bgp_notify_send_with_data (peer,
+				 BGP_NOTIFY_UPDATE_ERR,
+				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+				 startp, total);
+      return -1;
+    }
+  if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
+    {
+      zlog (peer->log, LOG_ERR,
+	    "NEXT_HOP attribute must be flagged as \"transitive\" (%u)", flag);
+      bgp_notify_send_with_data (peer,
+				 BGP_NOTIFY_UPDATE_ERR,
+				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+				 startp, total);
+      return -1;
+    }
+  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
+    {
+      zlog (peer->log, LOG_ERR,
+	    "NEXT_HOP attribute must not be flagged as \"partial\" (%u)", flag);
+      bgp_notify_send_with_data (peer,
+				 BGP_NOTIFY_UPDATE_ERR,
 				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
 				 startp, total);
       return -1;
@@ -927,7 +947,26 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
       return -1;
     }
 
-  attr->nexthop.s_addr = stream_get_ipv4 (peer->ibuf);
+  /* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP
+     attribute must result in a NOTIFICATION message (this is implemented below).
+     At the same time, semantically incorrect NEXT_HOP is more likely to be just
+     logged locally (this is implemented somewhere else). The UPDATE message
+     gets ignored in any of these cases. */
+  nexthop_n = stream_get_ipv4 (peer->ibuf);
+  nexthop_h = ntohl (nexthop_n);
+  if (IPV4_NET0 (nexthop_h) || IPV4_NET127 (nexthop_h) || IPV4_CLASS_DE (nexthop_h))
+    {
+      char buf[INET_ADDRSTRLEN];
+      inet_ntop (AF_INET, &nexthop_h, buf, INET_ADDRSTRLEN);
+      zlog (peer->log, LOG_ERR, "Martian nexthop %s", buf);
+      bgp_notify_send_with_data (peer,
+				 BGP_NOTIFY_UPDATE_ERR,
+				 BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP,
+				 startp, total);
+      return -1;
+    }
+
+  attr->nexthop.s_addr = nexthop_n;
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP);
 
   return 0;
diff --git a/lib/prefix.h b/lib/prefix.h
index 5f1ff05c..1cb91b2a 100644
--- a/lib/prefix.h
+++ b/lib/prefix.h
@@ -112,6 +112,7 @@ struct prefix_rd
 #define IPV4_NET0(a)    ((((u_int32_t) (a)) & 0xff000000) == 0x00000000)
 #define IPV4_NET127(a)  ((((u_int32_t) (a)) & 0xff000000) == 0x7f000000)
 #define IPV4_LINKLOCAL(a) ((((u_int32_t) (a)) & 0xffff0000) == 0xa9fe0000)
+#define IPV4_CLASS_DE(a) ((u_int32_t) (a) >= 0xe0000000)
 
 /* Max bit/byte length of IPv6 address. */
 #define IPV6_MAX_BYTELEN    16