bgpd: fix issue in capability negotiation (BZ#700)

Address problem where bgpd would reject a session if a peer sent some
capabilities in its Open message, but did not include a Multiprotocol
extensions capability. Note that the session would come up if there
were no capabilities at all in the Open message.

  * Add the 'mp_capability' out parameter to
    bgp_capability_parse(). Set it to '1' if a Multiprotocol
    extensions capability is encountered.

  * Switch on 'mp_capability' instead of 'capability' in the calling
    functions to determine if the peer indicated the set of AFI/SAFIs
    it supports.

    The net result is that when a peer does not send an MP capability,
    it is assumed to support the AFI/SAFIs configured for it locally.

2 files changed, 32 insertions, 14 deletions

diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index b5b50bb5..0326d01b 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -464,11 +464,16 @@ static const size_t cap_minsizes[] =
   [CAPABILITY_CODE_ORF_OLD]	= sizeof (struct capability_orf_entry),
 };
 
-/* Parse given capability.
+/**
+ * Parse given capability.
  * XXX: This is reading into a stream, but not using stream API
+ *
+ * @param[out] mp_capability Set to 1 on return iff one or more Multiprotocol
+ *                           capabilities were encountered.
  */
 static int
-bgp_capability_parse (struct peer *peer, size_t length, u_char **error)
+bgp_capability_parse (struct peer *peer, size_t length, int *mp_capability,
+		      u_char **error)
 {
   int ret;
   struct stream *s = BGP_INPUT (peer);
@@ -540,6 +545,8 @@ bgp_capability_parse (struct peer *peer, size_t length, u_char **error)
         {
           case CAPABILITY_CODE_MP:
             {
+	      *mp_capability = 1;
+
               /* Ignore capability when override-capability is set. */
               if (! CHECK_FLAG (peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
                 {
@@ -712,9 +719,13 @@ end:
   return as4;
 }
 
-/* Parse open option */
+/**
+ * Parse open option.
+ *
+ * @param[out] mp_capability @see bgp_capability_parse() for semantics.
+ */
 int
-bgp_open_option_parse (struct peer *peer, u_char length, int *capability)
+bgp_open_option_parse (struct peer *peer, u_char length, int *mp_capability)
 {
   int ret;
   u_char *error;
@@ -767,8 +778,7 @@ bgp_open_option_parse (struct peer *peer, u_char length, int *capability)
 	  ret = bgp_auth_parse (peer, opt_length);
 	  break;
 	case BGP_OPEN_OPT_CAP:
-	  ret = bgp_capability_parse (peer, opt_length, &error);
-	  *capability = 1;
+	  ret = bgp_capability_parse (peer, opt_length, mp_capability, &error);
 	  break;
 	default:
 	  bgp_notify_send (peer, 
@@ -811,9 +821,10 @@ bgp_open_option_parse (struct peer *peer, u_char length, int *capability)
 	}
     }
 
-  /* Check there is no common capability send Unsupported Capability
+  /* Check there are no common AFI/SAFIs and send Unsupported Capability
      error. */
-  if (*capability && ! CHECK_FLAG (peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
+  if (*mp_capability &&
+      ! CHECK_FLAG (peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
     {
       if (! peer->afc_nego[AFI_IP][SAFI_UNICAST] 
 	  && ! peer->afc_nego[AFI_IP][SAFI_MULTICAST]
@@ -821,7 +832,9 @@ bgp_open_option_parse (struct peer *peer, u_char length, int *capability)
 	  && ! peer->afc_nego[AFI_IP6][SAFI_UNICAST]
 	  && ! peer->afc_nego[AFI_IP6][SAFI_MULTICAST])
 	{
-	  plog_err (peer->log, "%s [Error] No common capability", peer->host);
+	  plog_err (peer->log, "%s [Error] Configured AFI/SAFIs do not "
+		    "overlap with received MP capabilities",
+		    peer->host);
 
 	  if (error != error_data)
 
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 5d8087a8..390b5563 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1152,7 +1152,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
   as_t as4 = 0;
   struct peer *realpeer;
   struct in_addr remote_id;
-  int capability;
+  int mp_capability;
   u_int8_t notify_data_remote_as[2];
   u_int8_t notify_data_remote_id[4];
 
@@ -1174,7 +1174,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
 	        inet_ntoa (remote_id));
   
   /* BEGIN to read the capability here, but dont do it yet */
-  capability = 0;
+  mp_capability = 0;
   optlen = stream_getc (peer->ibuf);
   
   if (optlen != 0)
@@ -1459,7 +1459,7 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
   /* Open option part parse. */
   if (optlen != 0) 
     {
-      if ((ret = bgp_open_option_parse (peer, optlen, &capability)) < 0)
+      if ((ret = bgp_open_option_parse (peer, optlen, &mp_capability)) < 0)
         {
           bgp_notify_send (peer,
                  BGP_NOTIFY_OPEN_ERR,
@@ -1474,8 +1474,13 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
 		   peer->host);
     }
 
-  /* Override capability. */
-  if (! capability || CHECK_FLAG (peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
+  /* 
+   * Assume that the peer supports the locally configured set of
+   * AFI/SAFIs if the peer did not send us any Mulitiprotocol
+   * capabilities, or if 'override-capability' is configured.
+   */
+  if (! mp_capability ||
+      CHECK_FLAG (peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
     {
       peer->afc_nego[AFI_IP][SAFI_UNICAST] = peer->afc[AFI_IP][SAFI_UNICAST];
       peer->afc_nego[AFI_IP][SAFI_MULTICAST] = peer->afc[AFI_IP][SAFI_MULTICAST];