| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
| |
The addition of a MIN(X,Y) with a stream_getc in the Y
causes a double read of the stream due to the way that
MIN is defined.
This fix removes a crash in all protocols.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
| |
This reverts commit f89b09be92bed03b1e5add55dc14ef92e94c52e1.
Martin Winter has reported reliability issues in testing on some platforms.
We need a more comprehensive way to deal with defaults and updating them, e.g.
profiles of some kind.
Defer this change till after next release.
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch is part of the previously submitted
patch set on VPN and Encap SAFIs. It fixes
an issue identified by NetDEF CI.
Ensure temp stack structures are initialized
Add protection against double frees / post
free access to bgp_attr_flush
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* zclient.c: prefix length on router-id and interface address add
messages not sanity checked. fix.
* */*_zebra.c: Prefix length on zebra route read was not checked, and
clients use it to write to storage. An evil zebra could overflow
client structures by sending overly long prefixlen.
Prompted by discussions with:
Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* bgpd parses NLRIs twice, a first pass "sanity check" and then a second pass
that changes actual state. For most AFI/SAFIs this is done by
bgp_nlri_sanity_check and bgp_nlri_parse, which are almost identical.
As the required action on a syntactic error in an NLRI is to NOTIFY and
shut down the session, it should be acceptable to just do a one pass
parse. There is no need to atomically handle the NLRIs.
* bgp_route.h: (bgp_nlri_sanity_check) Delete
* bgp_route.c: (bgp_nlri_parse) Make the prefixlen size check more general
and don't hard-code AFI/SAFI details, e.g. use prefix_blen library function.
Add error logs consistent with bgp_nlri_sanity_check as much as possible.
Add a "defense in depth" type check of the prefixlen against the sizeof
the (struct prefix) storage - ala bgp_nlri_parse_vpn.
Update standards text from draft RFC4271 to the actual RFC4271 text.
Extend the semantic consistency test of IPv6. E.g. it should skip mcast
NLRIs for unicast safi as v4 does.
* bgp_mplsvpn.{c,h}: Delete bgp_nlri_sanity_check_vpn and make
bgp_nlri_parse_vpn_body the bgp_nlri_parse_vpn function again.
(bgp_nlri_parse_vpn) Remove the notifies. The sanity checks were
responsible for this, but bgp_update_receive handles sending NOTIFY
generically for bgp_nlri_parse.
* bgp_attr.c: (bgp_mp_reach_parse,bgp_mp_unreach_parse) Delete sanity check.
NLRI parsing done after attr parsing by bgp_update_receive.
Arising out of discussions on the need for two-pass NLRI parse with:
Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* bgp_packet.c: (bgp_update_receive) Lots of repeated code, doing same
thing for each AFI/SAFI. Except when it doesn't, e.g. the IPv4/VPN
case was missing the EoR bgp_clear_stale_route call - the only action
really needed for EoR.
Make this function a lot more regular, using common, AFI/SAFI
independent blocks so far as possible.
Replace the 4 separate bgp_nlris with an array, indexed by an enum.
The distinct blocks that handle calling bgp_nlri_parse for each
different AFI/SAFI can now be replaced with a loop.
Transmogrify the nlri SAFI from the SAFI_MPLS_LABELED_VPN code-point
used on the wire, to the SAFI_MPLS_VPN safi_t enum we use internally
as early as possible.
The existing code was not necessarily sending a NOTIFY for NLRI
parsing errors, if they arose via bgp_nlri_sanity_check. Send the
correct NOTIFY - INVAL_NETWORK for the classic NLRIs and OPT_ATTR_ERR
for the MP ones.
EoR can now be handled in one block. The existing code seemed broken
for EoR recognition in a number of ways:
1. A v4/unicast EoR should be an empty UPDATE. However, it seemed
to be treating an UPDATE with attributes, inc. MP REACH/UNREACH,
but no classic NLRIs, as a v4/uni EoR.
2. For other AFI/SAFIs, it was treating UPDATEs with no classic
withraw and with a zero-length MP withdraw as EoRs. However, that
would mean an UPDATE packet _with_ update NLRIs and a 0-len MP
withdraw could be classed as an EoR.
This seems to be loose coding leading to ambiguous protocol
situations and likely incorrect behaviour, rather than simply being
liberal. Be more strict about checking that an UPDATE really is an
EoR and definitely is not trying to update any NLRIs.
This same loose EoR parsing was noted by Chris Hall previously on
list.
(bgp_nlri_parse) Front end NLRI parse function, to fan-out to the correct
parser for the AFI/SAFI.
* bgp_route.c: (bgp_nlri_sanity_check) We try convert NLRI safi to
internal code-point ASAP, adjust switch for that. Leave the wire
code point in for defensive coding.
(bgp_nlri_parse) rename to bgp_nlri_parse_ip.
* tests/bgp_mp_attr_test.c: Can just use bgp_nlri_parse frontend.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* bgp_route.h: (bgp_nlri_sanity_check) The bulk of the args are equivalent
to a (struct bgp_nlri), consolidate.
* bgp_route.c: (bgp_nlri_sanity_check) Make this a frontend for all afi/safis.
Including SAFI_MPLS_LABELED_VPN.
(bgp_nlri_sanity_check_ip) Regular IP NLRI sanity check based on the
existing code, and adjusted for (struct bgp_nlri *) arg.
* bgp_attr.c: (bgp_mp_reach_parse) Adjust for passing (struct bgp_nlri *)
to bgp_nlri_sanity_check.
Get rid of special-casing to not sanity check VPN.
(bgp_mp_unreach_parse) Ditto.
* bgp_mplsvpn.c: Use the same VPN parsing code for both the sanity
check and the actual parse.
(bgp_nlri_parse_vpn) renamed to bgp_nlri_parse_vpn_body and made
internal.
(bgp_nlri_parse_vpn_body) Added (bool) argument to control whether it
is sanity checking or whether it should update routing state for each
NLRI. Send a NOTIFY and reset the session, if there's a parsing
error, as bgp_nlri_sanity_check_ip does, and as is required by the
RFC.
(bgp_nlri_parse_vpn) now a wrapper to call _body with update.
(bgp_nlri_sanity_check_vpn) wrapper to call parser without
updating.
* bgp_mplsvpn.h: (bgp_nlri_sanity_check_vpn) export for
bgp_nlri_sanity_check.
* bgp_packet.c: (bgp_update_receive) Adjust for bgp_nlri_sanity_check
argument changes.
* test/bgp_mp_attr_test.c: Extend to also test the NLRI parsing functions,
if the initial MP-attr parsing has succeeded. Fix the NLRI in the
VPN cases. Add further VPN tests.
* tests/bgpd.tests/testbgpmpattr.exp: Add the new test cases.
This commit a joint effort of:
Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
Paul Jakma <paul.jakma@hpe.com> / <paul@jakma.org>
|
|
|
|
|
|
|
|
|
| |
* bgp_encap.{c,h} (bgp_nlri_parse_encap) afi is already in the NLRI argument.
update or withdraw is signalled by attr being non-NULL or NULL.
* bgp_packet.c: (update_receive) fixup to match, and also make the attr
argument conform with NLRI_ATTR_ARG for correct error handling on
optional, transitive, partial, attributes.
|
|
|
|
|
|
|
| |
bgp_info_mpath_update is called with new_best == NULL, this
causes the dereference of new_best in order to get at the mpath_cfg.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
| |
Modify the various maxpath commands to use MULTIPATH_NUM
as the upper limit of allowed max paths in BGP. There
is no point in allowing a number of maximum paths greater
than what Quagga is compiled for.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Tested-by: NetDEF CI System <cisystem@netdef.org>
|
|
|
|
|
|
|
|
| |
The code has spots where MULTIPATH_NUM set to 0 is equal to 64.
Now that MULTIPATH_NUM is set from the makefile to never be 0,
remove the code that depends on this.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Changes made here:
1) MULTIPATH_NUM will never be 0. If user specifies --enable-multipath=0
then this translates to MULTIPATH_NUM being set to 64 inside of the build
system.
2) Move MULTIPATH_NUM from a Makefile construct to a config.h construct.
3) Allowed MULTIPATH_NUM to be a number > 99 but < 1000
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
| |
|
|
|
|
|
|
|
| |
The 'struct fifo' and it's accompanying #defines do not
belong in lib/zebra.h. Move them into their own header.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Tested-by: NetDEF CI System <cisystem@netdef.org>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
|
|
| |
Adds RFC5512 and Encapsulation Attribute.
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Reviewed-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
| |
Add code to print ENCAP communities.
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
|
| |
This fixes some minor mixups particularly in MPLS-related SAFIs, as well
as doing some stylistic changes & adding comments.
Signed-off-by: Lou Berger <lberger@labn.net>
Reviewed-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
There wasn't much missing for VPNv6 to begin with; just a few bits of
de- & encoding and a few lists to be updated.
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
[Editorial note: Signed-off-by may imply an authorship claim, but need not]
Edited-by: Paul Jakma <paul.jakma@hpe.com> / <paul@jakma.org>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
| |
This changes the existing _vpnv4 functions for MPLS-VPN into
SAFI-agnostic functions, renaming them from *_vpnv4 to *_safi.
Also adds route-map support while at it.
Signed-off-by: Lou Berger <lberger@labn.net>
Reviewed-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
| |
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
|
|
|
|
|
| |
lib/zebra.h has FILTER_X #define's. These do not belong there.
Put them in lib/filter.h where they belong.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
| |
* bgp_open.c: cap_minsize should be CAPABILITY_CODE_RESTART_LEN for RESTART
not 6.
|
|
|
|
|
|
|
|
|
|
| |
This code change does two things:
1) Removes ZEBRA_AFI_XXX #defines since they were redundant information
2) Switches afi_t to an enumerated type so that the compiler
can do a bit more compile time checking.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* bgp_route.c: (bgp_info_cmp) This function is supposed to return a
preference between the given paths, and does so as binary either or. When
mpath was added, the binary return value was left as is and instead an out
parameter 'paths_eq' was added to indicate the mpath-equality case. It's
a bit odd, as is the resulting logic in the caller.
Regularise things again by making the function return a strcmp like
trinary return value of -1,0,1. Get rid of the mpath specific arguments,
but pass in afi/safi as part of the general context - that plus the
(struct bgp *) is enough to access configuration.
Update the return values.
The mpath check was testing the IGP metric for equality, even though
previous to the mpath changes (and consistent with the behaviour of all
the other tests bar the end), equality results in continuing through to
the next comparison. Just go back to the previous way - each test finds a
preference to return, or continues on to let further tests have a go.
(bgp_best_selection) Get rid of the (struct bgp_maxpaths_cfg *) arg, we
can't add state for every optional feature to the argument list - they
have to look it up as needed. Do pass through the very general afi/safi
context though (saves several lookups through the route_node).
Adjust for the new trinary bgp_info_cmp return value and updated args.
Do the mpath clearing/accumulation in one place, in each loop.
Call to bgp_info_mpath_update similarly gets updated, as there's no cfg to
pass.
(bgp_process_{rsclient,main}) match bgp_best_selection changes.
* bgp_mpath.c: (bgp_mpath_is_configured_sort) Helper for whether mpath is
enabled by peer sort.
(bgp_mpath_is_configured) ditto, generally.
(bgp_info_mpath_update) caller no longer has the cfg to pass in, look it
up.
* bgp_mpath.h: Export the above and Match .c changes.
Requires commit:
"bgpd: bgp_scan shouldn't queue up route_nodes with no routes for processing"
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
| |
* bgp_nexthop.c: (bgp_scan) There is little point queueing an rn with no routing
information for processing.
* bgp_route.c: (bgp_process) Do nothing on rn's with no routes. Add an assert
for now, to try catch any other cases, but prob should be removed.
(bgp_best_selection) rn with no routes == finish early.
|
|
|
|
|
|
|
|
|
|
|
| |
* bgp_open.c: (cap_modsizes) Table of multiple a capability's data size
should fall on, if applicable.
(bgp_capability_parse) Check the header lengthcap_modsizes should fall on.
Inspiration from Cumulus bgpd-capability-cleanup.patch patch, with a
slightly different approach.
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
CEASE NOTIFICATION for OPEN parsing errors seems, to my reading of RFC4271
§6.2 to be incorrect.
* bgp_packet.c: (bgp_open_receive) OPEN/UNACEP_HOLDTIME is not an
appropriate error subcode if bgp_open_option_parse returns an error. Set
it to "Unspecific". Where a more specific subcode is appropriate, then lower
level should send that.
* bgp_open.c: (bgp_open_option_parse) Malformed OPENs should result in
NOTIFICATION with OPEN error, and OPEN/UNSPECIFIC sub-code - not CEASE.
(bgp_capability_{parse,orf_entry}) ditto.
* bgpd.h: Add BGP_NOTIFY_OPEN_UNSPECIFIC for 0. Note that IANA lists 0 as
reserved in the OPEN error sub-code registry, but RFC4271 page 32 says 0
is the "Unspecific" OPEN error subcode.
Have emailed IANA, they says it's a known errate to 4271 under review.
Some inspiration from Cumulus' bgpd-capability-cleanup.patch, though
v different result.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Address CERT vulnerability report VU#270232, memcpy to stack data structure
based on length field from packet data whose length field upper-bound was
not properly checked.
This likely allows BGP peers that are enabled to send Labeled-VPN SAFI
routes to Quagga bgpd to remotely exploit Quagga bgpd.
Mitigation: Do not enable Labeled-VPN SAFI with untrusted neighbours.
Impact: Labeled-VPN SAFI is not enabled by default.
* bgp_mplsvpn.c: (bgp_nlri_parse_vpnv4) The prefixlen is checked for
lower-bound, but not for upper-bound against received data length.
The packet data is then memcpy'd to the stack based on the prefixlen.
Extend the prefixlen check to ensure it is within the bound of the NLRI
packet data AND the on-stack prefix structure AND the maximum size for the
address family.
Reported-by: Kostya Kortchinsky <kostyak@google.com>
This commit a joint effort between:
Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
Paul Jakma <paul.jakma@hpe.com> / <paul@jakma.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* J Yu <jackiesyu@hotmail.com> noted a problem with bgpd of routes not
having their nexthop updated correctly.
Martin Winter <mwinter@opensourcerouting.org> pinned this down to the
case where a BGP route is updated from one with a valid nexthop to an
invalid next-hop, using a test tool. Once the problem occurs, the incorrect
route may remain, even after further UPDATEs, so long as the nexthop in the
zebra RIB does not match the BGP route's nexthop.
Jacqueline Yu then pinned the issue down further to being due to bgpd
sending the DELETE for the route to zebra with the new nexthop after a
BGP UPDATE updates an existing route, but then is found to be invalid,
and zebra not finding the route as it requires a match on all attributes.
* bgp_zebra.c: (bgp_zebra_withdraw) When deleting a prefix, we want it gone.
Do not send additional matching attributes like the nexthop, which can
only cause incorrect non-matches.
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
|
|
|
| |
Allow the bgp dump functionality to handle the Extended Time format
as specified in RFC 6396.
Fixes a segmentation fault with multiple dump rules as well.
Signed-off-by: Alexis Fasquel <alexis@pch.net>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
| |
The peer-groups parser is missing advertisement-interval and 'timers connect'
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Timo Teräs <timo.teras@iki.fi>
|
|
|
|
|
|
|
|
| |
rtt is calculated dynamically by the kernel. Refresh it on
soft clear.
Fixes: ef757700d0 "bgpd: allow using rtt in route-map's set metric"
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
|
|
|
|
|
|
|
|
|
|
|
| |
OPEN message handler moves the connection from the temporary
"struct peer" (used to accept it) to the real "struct peer" based
on the configuration. RTT needs to be updated only to the real
struct peer, and this patch moves the RTT query to point where
realpeer is known.
Fixes: ef757700d0 "bgpd: allow using rtt in route-map's set metric"
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
|
|
|
|
|
|
|
|
|
| |
bgp_btoa was abandoned at some point in time in the past.
This commit gets it to compile and to be added to /usr/bin.
At this point in time no work has done for 'correctness' of execution
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
|
|
|
|
|
|
| |
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
There was a crash from not NULLing out peer->hostname but I cleaned
up a bunch of other suspect ones as well.
|
|
|
|
|
| |
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
|