diff options
| -rw-r--r-- | bgpd/bgp_evpn.c | 18 | ||||
| -rw-r--r-- | bgpd/bgp_flowspec.c | 14 | ||||
| -rw-r--r-- | bgpd/bgp_label.c | 12 | ||||
| -rw-r--r-- | bgpd/bgp_mplsvpn.c | 12 | ||||
| -rw-r--r-- | bgpd/bgp_packet.c | 7 | ||||
| -rw-r--r-- | bgpd/bgp_route.c | 22 | ||||
| -rw-r--r-- | bgpd/bgp_route.h | 18 | ||||
| -rw-r--r-- | tests/topotests/bgp_maximum_prefix_invalid_update/__init__.py | 0 | ||||
| -rw-r--r-- | tests/topotests/bgp_maximum_prefix_invalid_update/r1/bgpd.conf | 4 | ||||
| -rw-r--r-- | tests/topotests/bgp_maximum_prefix_invalid_update/r1/zebra.conf | 9 | ||||
| -rw-r--r-- | tests/topotests/bgp_maximum_prefix_invalid_update/r2/bgpd.conf | 4 | ||||
| -rw-r--r-- | tests/topotests/bgp_maximum_prefix_invalid_update/r2/zebra.conf | 6 | ||||
| -rw-r--r-- | tests/topotests/bgp_maximum_prefix_invalid_update/test_bgp_maximum_prefix_invalid_update.py | 113 |
13 files changed, 199 insertions, 40 deletions
diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 7f6d34808f..ab3ea248c7 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -4828,7 +4828,7 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, if (addpath_encoded) { /* When packet overflow occurs return immediately. */ if (pnt + BGP_ADDPATH_ID_LEN > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; addpath_id = ntohl(*((uint32_t *)pnt)); pnt += BGP_ADDPATH_ID_LEN; @@ -4836,14 +4836,14 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, /* All EVPN NLRI types start with type and length. */ if (pnt + 2 > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_EVPN_MISSING_TYPE; rtype = *pnt++; psize = *pnt++; /* When packet overflow occur return immediately. */ if (pnt + psize > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; switch (rtype) { case BGP_EVPN_MAC_IP_ROUTE: @@ -4854,7 +4854,7 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, EC_BGP_EVPN_FAIL, "%u:%s - Error in processing EVPN type-2 NLRI size %d", peer->bgp->vrf_id, peer->host, psize); - return -1; + return BGP_NLRI_PARSE_ERROR_EVPN_TYPE2_SIZE; } break; @@ -4866,7 +4866,7 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, EC_BGP_PKT_PROCESS, "%u:%s - Error in processing EVPN type-3 NLRI size %d", peer->bgp->vrf_id, peer->host, psize); - return -1; + return BGP_NLRI_PARSE_ERROR_EVPN_TYPE3_SIZE; } break; @@ -4878,7 +4878,7 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, EC_BGP_PKT_PROCESS, "%u:%s - Error in processing EVPN type-4 NLRI size %d", peer->bgp->vrf_id, peer->host, psize); - return -1; + return BGP_NLRI_PARSE_ERROR_EVPN_TYPE4_SIZE; } break; @@ -4889,7 +4889,7 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, EC_BGP_PKT_PROCESS, "%u:%s - Error in processing EVPN type-5 NLRI size %d", peer->bgp->vrf_id, peer->host, psize); - return -1; + return BGP_NLRI_PARSE_ERROR_EVPN_TYPE5_SIZE; } break; @@ -4900,9 +4900,9 @@ int bgp_nlri_parse_evpn(struct peer *peer, struct attr *attr, /* Packet length consistency check. */ if (pnt != lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; - return 0; + return BGP_NLRI_PARSE_OK; } /* diff --git a/bgpd/bgp_flowspec.c b/bgpd/bgp_flowspec.c index ab8bfcb770..9554638735 100644 --- a/bgpd/bgp_flowspec.c +++ b/bgpd/bgp_flowspec.c @@ -105,14 +105,14 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr, if (afi == AFI_IP6) { flog_err(EC_LIB_DEVELOPMENT, "BGP flowspec IPv6 not supported"); - return -1; + return BGP_NLRI_PARSE_ERROR_FLOWSPEC_IPV6_NOT_SUPPORTED; } if (packet->length >= FLOWSPEC_NLRI_SIZELIMIT) { flog_err(EC_BGP_FLOWSPEC_PACKET, "BGP flowspec nlri length maximum reached (%u)", packet->length); - return -1; + return BGP_NLRI_PARSE_ERROR_FLOWSPEC_NLRI_SIZELIMIT; } for (; pnt < lim; pnt += psize) { @@ -121,7 +121,7 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr, /* All FlowSpec NLRI begin with length. */ if (pnt + 1 > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; psize = *pnt++; @@ -131,13 +131,13 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr, EC_BGP_FLOWSPEC_PACKET, "Flowspec NLRI length inconsistent ( size %u seen)", psize); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; } if (bgp_fs_nlri_validate(pnt, psize) < 0) { flog_err( EC_BGP_FLOWSPEC_PACKET, "Bad flowspec format or NLRI options not supported"); - return -1; + return BGP_NLRI_PARSE_ERROR_FLOWSPEC_BAD_FORMAT; } p.family = AF_FLOWSPEC; p.prefixlen = 0; @@ -192,8 +192,8 @@ int bgp_nlri_parse_flowspec(struct peer *peer, struct attr *attr, flog_err(EC_BGP_FLOWSPEC_INSTALLATION, "Flowspec NLRI failed to be %s.", attr ? "added" : "withdrawn"); - return -1; + return BGP_NLRI_PARSE_ERROR; } } - return 0; + return BGP_NLRI_PARSE_OK; } diff --git a/bgpd/bgp_label.c b/bgpd/bgp_label.c index a219c407da..9511650842 100644 --- a/bgpd/bgp_label.c +++ b/bgpd/bgp_label.c @@ -355,7 +355,7 @@ int bgp_nlri_parse_label(struct peer *peer, struct attr *attr, /* When packet overflow occurs return immediately. */ if (pnt + BGP_ADDPATH_ID_LEN > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; addpath_id = ntohl(*((uint32_t *)pnt)); pnt += BGP_ADDPATH_ID_LEN; @@ -372,7 +372,7 @@ int bgp_nlri_parse_label(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error / L-U (prefix length %d exceeds packet size %u)", peer->host, prefixlen, (uint)(lim - pnt)); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; } /* Fill in the labels */ @@ -387,12 +387,12 @@ int bgp_nlri_parse_label(struct peer *peer, struct attr *attr, peer->host, prefixlen); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_INVAL_NETWORK); - return -1; + return BGP_NLRI_PARSE_ERROR_LABEL_LENGTH; } if ((afi == AFI_IP && p.prefixlen > 32) || (afi == AFI_IP6 && p.prefixlen > 128)) - return -1; + return BGP_NLRI_PARSE_ERROR_PREFIX_LENGTH; /* Fetch prefix from NLRI packet */ memcpy(&p.u.prefix, pnt + llen, psize - llen); @@ -463,8 +463,8 @@ int bgp_nlri_parse_label(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error / L-U (%zu data remaining after parsing)", peer->host, lim - pnt); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; } - return 0; + return BGP_NLRI_PARSE_OK; } diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 8204116aff..4368bc8666 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -139,7 +139,7 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, /* When packet overflow occurs return immediately. */ if (pnt + BGP_ADDPATH_ID_LEN > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; addpath_id = ntohl(*((uint32_t *)pnt)); pnt += BGP_ADDPATH_ID_LEN; @@ -155,7 +155,7 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error / VPN (prefix length %d less than VPN min length)", peer->host, prefixlen); - return -1; + return BGP_NLRI_PARSE_ERROR_PREFIX_LENGTH; } /* sanity check against packet data */ @@ -164,7 +164,7 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error / VPN (prefix length %d exceeds packet size %u)", peer->host, prefixlen, (uint)(lim - pnt)); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; } /* sanity check against storage for the IP address portion */ @@ -175,7 +175,7 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, peer->host, prefixlen - VPN_PREFIXLEN_MIN_BYTES * 8, sizeof(p.u)); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; } /* Sanity check against max bitlen of the address family */ @@ -186,7 +186,7 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, peer->host, prefixlen - VPN_PREFIXLEN_MIN_BYTES * 8, p.family, prefix_blen(&p)); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; } /* Copy label to prefix. */ @@ -244,7 +244,7 @@ int bgp_nlri_parse_vpn(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error / VPN (%zu data remaining after parsing)", peer->host, lim - pnt); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; } return 0; diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 73a07c8232..fe8a1a256c 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -308,7 +308,7 @@ int bgp_nlri_parse(struct peer *peer, struct attr *attr, case SAFI_FLOWSPEC: return bgp_nlri_parse_flowspec(peer, attr, packet, mp_withdraw); } - return -1; + return BGP_NLRI_PARSE_ERROR; } /* @@ -1568,10 +1568,11 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) nlri_ret = bgp_nlri_parse(peer, &attr, &nlris[i], 1); break; default: - nlri_ret = -1; + nlri_ret = BGP_NLRI_PARSE_ERROR; } - if (nlri_ret < 0) { + if (nlri_ret < BGP_NLRI_PARSE_OK + && nlri_ret != BGP_NLRI_PARSE_ERROR_PREFIX_OVERFLOW) { flog_err(EC_BGP_UPDATE_RCV, "%s [Error] Error parsing NLRI", peer->host); if (peer->status == Established) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5fe8d42ff7..38f3cad5ad 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -4266,7 +4266,7 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr, /* When packet overflow occurs return immediately. */ if (pnt + BGP_ADDPATH_ID_LEN > lim) - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; addpath_id = ntohl(*((uint32_t *)pnt)); pnt += BGP_ADDPATH_ID_LEN; @@ -4284,7 +4284,7 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error (wrong prefix length %d for afi %u)", peer->host, p.prefixlen, packet->afi); - return -1; + return BGP_NLRI_PARSE_ERROR_PREFIX_LENGTH; } /* Packet size overflow check. */ @@ -4296,7 +4296,7 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error (prefix length %d overflows packet)", peer->host, p.prefixlen); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW; } /* Defensive coding, double-check the psize fits in a struct @@ -4306,7 +4306,7 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error (prefix length %d too large for prefix storage %zu)", peer->host, p.prefixlen, sizeof(p.u)); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; } /* Fetch prefix from NLRI packet. */ @@ -4371,10 +4371,14 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr, BGP_ROUTE_NORMAL, NULL, NULL, 0, NULL); - /* Address family configuration mismatch or maximum-prefix count - overflow. */ + /* Do not send BGP notification twice when maximum-prefix count + * overflow. */ + if (CHECK_FLAG(peer->sflags, PEER_STATUS_PREFIX_OVERFLOW)) + return BGP_NLRI_PARSE_ERROR_PREFIX_OVERFLOW; + + /* Address family configuration mismatch. */ if (ret < 0) - return -1; + return BGP_NLRI_PARSE_ERROR_ADDRESS_FAMILY; } /* Packet length consistency check. */ @@ -4383,10 +4387,10 @@ int bgp_nlri_parse_ip(struct peer *peer, struct attr *attr, EC_BGP_UPDATE_RCV, "%s [Error] Update packet error (prefix length mismatch with total length)", peer->host); - return -1; + return BGP_NLRI_PARSE_ERROR_PACKET_LENGTH; } - return 0; + return BGP_NLRI_PARSE_OK; } static struct bgp_static *bgp_static_new(void) diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 8da568a9f1..98f4140e15 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -73,6 +73,24 @@ enum bgp_show_adj_route_type { */ #define BGP_MAX_LABELS 2 +/* Error codes for handling NLRI */ +#define BGP_NLRI_PARSE_OK 0 +#define BGP_NLRI_PARSE_ERROR_PREFIX_OVERFLOW -1 +#define BGP_NLRI_PARSE_ERROR_PACKET_OVERFLOW -2 +#define BGP_NLRI_PARSE_ERROR_PREFIX_LENGTH -3 +#define BGP_NLRI_PARSE_ERROR_PACKET_LENGTH -4 +#define BGP_NLRI_PARSE_ERROR_LABEL_LENGTH -5 +#define BGP_NLRI_PARSE_ERROR_EVPN_MISSING_TYPE -6 +#define BGP_NLRI_PARSE_ERROR_EVPN_TYPE2_SIZE -7 +#define BGP_NLRI_PARSE_ERROR_EVPN_TYPE3_SIZE -8 +#define BGP_NLRI_PARSE_ERROR_EVPN_TYPE4_SIZE -9 +#define BGP_NLRI_PARSE_ERROR_EVPN_TYPE5_SIZE -10 +#define BGP_NLRI_PARSE_ERROR_FLOWSPEC_IPV6_NOT_SUPPORTED -11 +#define BGP_NLRI_PARSE_ERROR_FLOWSPEC_NLRI_SIZELIMIT -12 +#define BGP_NLRI_PARSE_ERROR_FLOWSPEC_BAD_FORMAT -13 +#define BGP_NLRI_PARSE_ERROR_ADDRESS_FAMILY -14 +#define BGP_NLRI_PARSE_ERROR -32 + /* Ancillary information to struct bgp_path_info, * used for uncommonly used data (aggregation, MPLS, etc.) * and lazily allocated to save memory. diff --git a/tests/topotests/bgp_maximum_prefix_invalid_update/__init__.py b/tests/topotests/bgp_maximum_prefix_invalid_update/__init__.py new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/topotests/bgp_maximum_prefix_invalid_update/__init__.py diff --git a/tests/topotests/bgp_maximum_prefix_invalid_update/r1/bgpd.conf b/tests/topotests/bgp_maximum_prefix_invalid_update/r1/bgpd.conf new file mode 100644 index 0000000000..235b42b3d5 --- /dev/null +++ b/tests/topotests/bgp_maximum_prefix_invalid_update/r1/bgpd.conf @@ -0,0 +1,4 @@ +router bgp 65000 + neighbor 192.168.255.2 remote-as 65001 + address-family ipv4 unicast + redistribute connected diff --git a/tests/topotests/bgp_maximum_prefix_invalid_update/r1/zebra.conf b/tests/topotests/bgp_maximum_prefix_invalid_update/r1/zebra.conf new file mode 100644 index 0000000000..0a283c06d5 --- /dev/null +++ b/tests/topotests/bgp_maximum_prefix_invalid_update/r1/zebra.conf @@ -0,0 +1,9 @@ +! +interface lo + ip address 172.16.255.254/32 +! +interface r1-eth0 + ip address 192.168.255.1/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_maximum_prefix_invalid_update/r2/bgpd.conf b/tests/topotests/bgp_maximum_prefix_invalid_update/r2/bgpd.conf new file mode 100644 index 0000000000..e016284159 --- /dev/null +++ b/tests/topotests/bgp_maximum_prefix_invalid_update/r2/bgpd.conf @@ -0,0 +1,4 @@ +router bgp 65001 + neighbor 192.168.255.1 remote-as 65000 + address-family ipv4 + neighbor 192.168.255.1 maximum-prefix 1 diff --git a/tests/topotests/bgp_maximum_prefix_invalid_update/r2/zebra.conf b/tests/topotests/bgp_maximum_prefix_invalid_update/r2/zebra.conf new file mode 100644 index 0000000000..606c17bec9 --- /dev/null +++ b/tests/topotests/bgp_maximum_prefix_invalid_update/r2/zebra.conf @@ -0,0 +1,6 @@ +! +interface r2-eth0 + ip address 192.168.255.2/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_maximum_prefix_invalid_update/test_bgp_maximum_prefix_invalid_update.py b/tests/topotests/bgp_maximum_prefix_invalid_update/test_bgp_maximum_prefix_invalid_update.py new file mode 100644 index 0000000000..69b8c7ca5e --- /dev/null +++ b/tests/topotests/bgp_maximum_prefix_invalid_update/test_bgp_maximum_prefix_invalid_update.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python + +# +# bgp_local_as_private_remove.py +# Part of NetDEF Topology Tests +# +# Copyright (c) 2019 by +# Network Device Education Foundation, Inc. ("NetDEF") +# +# Permission to use, copy, modify, and/or distribute this software +# for any purpose with or without fee is hereby granted, provided +# that the above copyright notice and this permission notice appear +# in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE +# OF THIS SOFTWARE. +# + +""" +bgp_maximum_prefix_invalid_update.py: +Test if unnecesarry UPDATE message like below: + +[Error] Error parsing NLRI +%NOTIFICATION: sent to neighbor X.X.X.X 3/10 (UPDATE Message Error/Invalid Network Field) 0 bytes + +is not sent if maximum-prefix count is overflow. +""" + +import os +import sys +import json +import time +import pytest + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, '../')) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topolog import logger +from mininet.topo import Topo + +class TemplateTopo(Topo): + def build(self, *_args, **_opts): + tgen = get_topogen(self) + + for routern in range(1, 3): + tgen.add_router('r{}'.format(routern)) + + switch = tgen.add_switch('s1') + switch.add_link(tgen.gears['r1']) + switch.add_link(tgen.gears['r2']) + +def setup_module(mod): + tgen = Topogen(TemplateTopo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for i, (rname, router) in enumerate(router_list.iteritems(), 1): + router.load_config( + TopoRouter.RD_ZEBRA, + os.path.join(CWD, '{}/zebra.conf'.format(rname)) + ) + router.load_config( + TopoRouter.RD_BGP, + os.path.join(CWD, '{}/bgpd.conf'.format(rname)) + ) + + tgen.start_router() + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + +def test_bgp_maximum_prefix_invalid(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + def _bgp_converge(router): + while True: + output = json.loads(tgen.gears[router].vtysh_cmd("show ip bgp neighbor 192.168.255.1 json")) + if output['192.168.255.1']['connectionsEstablished'] > 3: + return True + time.sleep(1) + + def _bgp_parsing_nlri(router): + cmd_max_exceeded = 'grep "%MAXPFXEXCEED: No. of IPv4 Unicast prefix received" bgpd.log' + cmdt_error_parsing_nlri = 'grep "Error parsing NLRI" bgpd.log' + output_max_exceeded = tgen.gears[router].run(cmd_max_exceeded) + output_error_parsing_nlri = tgen.gears[router].run(cmdt_error_parsing_nlri) + + if len(output_max_exceeded) > 0: + if len(output_error_parsing_nlri) > 0: + return False + return True + + + if _bgp_converge('r2'): + assert _bgp_parsing_nlri('r2') == True + +if __name__ == '__main__': + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) |