A crafted BGP UPDATE with a malformed path attribute length field causes
bgpd to dump up to 65535 bytes of application memory and send it as the
data field in a BGP NOTIFY message, which is truncated to 4075 bytes
after accounting for protocol headers. After reading a malformed length
field, a NOTIFY is generated that is supposed to contain the problematic
data, but the malformed length field is inadvertently used to compute
how much data we send.
CVE-2017-15865
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
If the user has configured the ability to override
the capabilities or if the afi/safi passed as part
of the _MP capability is not understood, then we
can enter into an infinite loop as part of the
capability parsing.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When we are displaying a extended community ECOMMUNITY_SITE_ORIGIN
the display sprintf is this:
len = sprintf(
str_buf + str_pnt,
"EVPN:%02x:%02x:%02x:%02x:%02x:%02x",
macaddr[0], macaddr[1], macaddr[2],
macaddr[3], macaddr[4], macaddr[5]);
The problem with this is that macaddr[0] is passed in as a integer
so the sprintf function thinks that the value to display is much
larger than it actually is. The ECOMMUNITY_STR_DEFAULT_LEN is 27
So the resulting string no-longer fits in memory and we write
off the end of the buffer and can crash. If we force the
passed in value to be a uint8_t then we get the expected output
since a single byte is displayed as 2 hex characters and the
resulting string fits in str_buf.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Problem reported that a receiver of a default route issued across bgp
unnumbered peering using default originate would have the route stay
as inactive. Discovered we were messing up the nexthop value sent to
the peer in this one particular case. Manual testing good, fix supplied
to the submitter and verified to resolve the problem. bgp-smoke
completed successfully.
Ticket: CM-18634
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
When we fail to bind to port 179 we are left in a situation
where we have not saved the bgp pointer created and when
the bgp cli mode is exited we leak the memory.
Additionally there is no recovery situation here that
could be easily programmed without fundamentally changing
the code.
So let's exit and output to the log file some useful
information to hopefully clue the user in on what is
going wrong.
Fixes: #1130
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Problem reported that we weren't adjusting the keepalive timer
correctly when we negotiated a lower hold time learned from a
peer. While working on this, found we didn't do inheritance
correctly at all. This fix solves the first problem and also
ensures that the timers are configured correctly based on this
priority order - peer defined > peer-group defined > global config.
This fix also displays the timers as "configured" regardless of
which of the three locations above is used.
Ticket: CM-18408
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: CCR-6807
Testing-performed: Manual testing successful, fix tested by
submitter, bgp-smoke completed successfully
This issue was discovered on a live session with an extremely
old cisco 7206VXR router running 12.2(33)SRE4. The sending router
is sending us an empty NLRI that is MP_REACH. From RFC
exploration(thanks Russ!) it appears that this was
considered a 'valid' way to send EOR.
Following discussion decided that we should treat
this situation as a EOR marker instead of bringing
down the session.
Applying this fix on the FRR router seeing this issue
allows it to continue it's peering relationship with
the ASR. Since this is a point fix I do not see
a high likelihood of further fallout.
Fixes: #1258
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When bgp is coming up and is reading a non-integrated config.
The bgp connection to zebra has not fully had a chance to start.
As such when a redistribute line is parsed the attempt is
made to install it but it was erroring out with a warning.
This caused the `redistribute XXX` line to create a error
message to the end user.
Since bgp calls zclient_send_reg_requests which re-registers
the redistribute call once the actual zebra connection is up
and once bgp comes alive this is ok.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There are multiple places that we use route-maps in bgp
There is no need to limit the route-map 'match peer ...' command
to just import and export route-map types. I see need for
using this in table-maps as well.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ignore the return value of some functions in the places we know they
can't fail, and other small fixes.
Regarding the change in bgpd/rfapi/rfapi_rib.c, asserting that
rfapiRaddr2Qprefix() didn't fail is the common idiom inside the rfapi
code.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
These are mostly trivial fixes for leaks in the error path of some functions.
The changes in bgpd/bgp_mpath.c deserves a bit of explanation though. In
the bgp_info_mpath_aggregate_update() function, we were allocating memory
for the lcomm variable but doing nothing with it. Since the code for
communities, extended communities and large communities is pretty much
the same in this function, it's clear that this was a copy and paste
error where most of the ext. community code was copied but not all of
it as it should have been.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
In some situations we already know the ifp and by extension
the ifindex there is no need to look it up for every
route we send to zebra.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Handle better stress situations when multiple peers are trying to
connect at the same time by bumping the TCP connection backlog limit.
This reduces the convergence time of BGPerf stress test.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
If upon bgp startup we have this config:
router bgp 64540
neighbor 192.168.201.134 remote-as external
!
address-family ipv4 unicast
no neighbor 192.168.201.134 activate
neighbor 192.168.201.134 route-map NEXTHOP in
exit-address-family
The route-map map pointer for the incoming(or outgoing)
filter was not being saved due to a pre-mature optimization
of not handling the routemap callback if the peer is not
activated. The function that handles the peers route-maps
is making sure that the peer is in established state
before attempting to actually apply anything so just
call it to set the map pointer.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Commit 8c9cc7bbf6 changed the size
of the `struct bgp_attr_encap_subtlv` type to be a zero length
array at the end instead of having a 1 byte. All memory allocations
for this subsuquently were off by 1 byte since those were not
adjusted either.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When retrieving information about neighbors or summary in json
add some information about what bestPath particulars we have for
this bgp session.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This improves code readability and also future-proofs our codebase
against new changes in the data structure used to store interfaces.
The FOR_ALL_INTERFACES_ADDRESSES macro was also moved to lib/ but
for now only babeld is using it.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Performance tests showed that, when running on a system with a large
number of interfaces, some daemons would spend a considerable amount
of time in the if_lookup_by_index() function. Introduce a new rb-tree
to solve this problem.
With this change, we need to use the if_set_index() function whenever
we want to change the ifindex of an interface. This is necessary to
ensure that the 'ifaces_by_index' rb-tree is updated accordingly. The
return value of all insert/remove operations in the interface rb-trees
is checked to ensure that an error is logged if a corruption is
detected.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
IFINDEX_DELETED is not necessary anymore as we moved from a global
list of interfaces to a list of interfaces per VRF.
This reverts commit 84361d615.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
