From 3b381c32fc2c325cc4ffb9f9f30a7e96e9bd87c6 Mon Sep 17 00:00:00 2001 From: Avneesh Sachdev Date: Sun, 19 Feb 2012 10:19:52 -0800 Subject: 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. --- bgpd/bgp_open.c | 31 ++++++++++++++++++++++--------- bgpd/bgp_packet.c | 15 ++++++++++----- 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]; -- cgit v1.2.1