In some cases (large scale) it's desired to avoid changing configurations, but
let the BGP to automatically handle ASN changes.
`auto` means the peering can be iBGP or eBGP. It will be automatically detected
and adjusted from the OPEN message.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Introduce BGP-wide flags to denote if BGP has started gracefully
and GR is in progress or not. Use this for setting of the R-bit in
the GR capability, and not a timer which is set for any new
instance creation. Mark graceful restart is complete when the
deferred path selection has been done and route sync with zebra as
well as deferred EOR advertisement has been initiated.
Introduce a function to check on F-bit setting rather than just
base it on configuration.
Subsequent commits will extend these functionalities.
Signed-off-by: Vivek Venkatraman <vivek@nvidia.com>
We advance data pointer (data++), but we do memcpy() with the length that is 1-byte
over, which is technically heap overflow.
```
==411461==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50600011da1a at pc 0xc4f45a9786f0 bp 0xffffed1e2740 sp 0xffffed1e1f30
READ of size 4 at 0x50600011da1a thread T0
0 0xc4f45a9786ec in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x3586ec) (BuildId: e794c5f796eee20c8973d7efb9bf5735e54d44cd)
1 0xc4f45abf15f8 in bgp_dynamic_capability_fqdn /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3457:4
2 0xc4f45abdd408 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3911:4
3 0xc4f45abdbeb4 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9
4 0xc4f45abde2cc in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11
5 0xc4f45a9b6110 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3
```
Found by fuzzing.
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
If we receive CAPABILITY message (software-version), we SHOULD check if we really
have enough data before doing memcpy(), that could also lead to buffer overflow.
(data + len > end) is not enough, because after this check we do data++ and later
memcpy(..., data, len). That means we have one more byte.
Hit this through fuzzing by
```
0 0xaaaaaadf872c in __asan_memcpy (/home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/.libs/bgpd+0x35872c) (BuildId: 9c6e455d0d9a20f5a4d2f035b443f50add9564d7)
1 0xaaaaab06bfbc in bgp_dynamic_capability_software_version /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3713:3
2 0xaaaaab05ccb4 in bgp_capability_msg_parse /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3839:4
3 0xaaaaab05c074 in bgp_capability_receive /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:3980:9
4 0xaaaaab05e48c in bgp_process_packet /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_packet.c:4109:11
5 0xaaaaaae36150 in LLVMFuzzerTestOneInput /home/ubuntu/frr-public/frr_public_private-libfuzzer/bgpd/bgp_main.c:582:3
```
Hit this again by Iggy \m/
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When we receive a hard-reset notification, we always show it if it was a hard,
or not.
For sending side, we missed that. Let's display it too.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Before we checked for received Graceful Restart capability, but that was also
incorrect, because we SHOULD HAVE checked it per AFI/SAFI instead.
https://datatracker.ietf.org/doc/html/rfc4724 says:
Although the End-of-RIB marker is specified for the purpose of BGP
graceful restart, it is noted that the generation of such a marker
upon completion of the initial update would be useful for routing
convergence in general, and thus the practice is recommended.
Thus, it might be reasonable to send EoR regardless of whether the Graceful Restart
capability is received or not from the peer.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
If FQDN capability comes as dynamic capability we should check if the encoding
is proper.
Before this patch we returned an error if the hostname/domainname length check
was > end. But technically, if the length is also == end, this is
a malformed capability, because we use the data incorrectly after we check the
length.
This causes heap overflow (when compiled with address-sanitizer).
Signed-off-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When sending the packets over the network (dynamic capability) it reports 6 bytes
instead of 5 bytes, and causes some issues between little/big endian machines.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When using dynamic capabilities, do not forget to unset advertised capabilities.
Otherwise, it's kept as advertised.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
This is required by the current (latest/-02 draft).
IANA has registered code 8 for "Send Hold Timer Expired" in the "BGP
Error (Notification) Codes" sub-registry under the "Border Gateway
Protocol (BGP) Parameters" registry.
https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-sendholdtimer
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Since we have a knob to disable sending FQDN capability, it MUST be checked
before sending it using dynamic capabilities.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
I've kept the assignment in a comment because I am concerned
about new code being added later that the data pointer would
not be set correctly. Next coder can see the commented
out line and uncomment it.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When filtering with `debug bgp updates in x.x.x.x prefix-list plist`, we want
to filter out unnecessary messages like:
```
127.0.0.1(Unknown) rcvd UPDATE wlen 0 attrlen 20 alen 5
```
Such a line as above will be repeated for all the paths received and it's useless
without knowing the prefix (because NLRIs are not parsed yet).
But want to see only relevant ones:
```
127.0.0.1(Unknown) rcvd UPDATE w/ attr: nexthop 127.0.0.1, origin i, path 65002
127.0.0.1(Unknown) rcvd 10.255.255.1/32 IPv4 unicast
```
With `debug bgp updates detail` we can combine this to something like:
```
127.0.0.1(Unknown) rcvd UPDATE w/ attr: nexthop 127.0.0.1, origin i, path 65002
127.0.0.1(Unknown) rcvd UPDATE wlen 0 attrlen 20 alen 5
127.0.0.1(Unknown) rcvd 10.255.255.1/32 IPv4 unicast
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Send/Receive:
This field indicates whether the sender is (a) able to receive
multiple paths from its peer (value 1), (b) able to send
multiple paths to its peer (value 2), or (c) both (value 3) for
the <AFI, SAFI>.
If any other value is received, then the capability SHOULD be
treated as not understood and ignored [RFC5492].
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
This is already handled above, no need to do here, because we could have an
overrun situation where len > 64 and we do out-of-bound actions.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
If we receive MP_UNREACH_NLRI, we should stop handling remaining NLRIs if
no mandatory path attributes received.
In other words, if MP_UNREACH_NLRI received, the remaining NLRIs should be handled
as a new data, but without mandatory attributes, it's a malformed packet.
In normal case, this MUST not happen at all, but to avoid crashing bgpd, we MUST
handle that.
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Add an ability to enable/disable ORF capability dynamically without tearing
down the session.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Changing Addpath type, and or disabling RX (receiving) flag, we can do this
without tearing down the session, and using dynamic capabilities.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We should not allow exceeding the stream's length, and also software version
can't be larger than 64 bytes.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Add the ability to store link-state prefixes in the BGP table.
Store a raw copy of the BGP link state NLRI TLVs as received in the
packet in 'p.u.prefix_linkstate.ptr'.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Accept the BGP Link-State AFI/SAFI capability when received from a peer
OPEN message.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
LLGR stale time is exchanged using OPEN messages. In order to
reduce stal time before doing an actual graceful restart + LLGR, it might be useful
to increase the time, but this is not possible without resetting the session.
With this change, it's possible to send dynamic capability with a new value, and
GR will respect a new reset time value when LLGR kicks in.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Pass through a bunch of BGP_EVENT_ADD's and make
the code use a proper connection instead of a
peer->connection. There still are a bunch
of places where peer->connection is used and
later commits will probably go through and
clean these up more.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Modify all the receive functions to pass around the actual
connection being acted upon. Modify the collision detection
function to look at the possible two connections.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Modify bgp_fsm_change_status to be connection oriented and
also make the BGP_TIMER_ON and BGP_EVENT_ADD macros connection
oriented as well. Attempt to make peer_xfer_conn a bit more
understandable because, frankly it was/is confusing.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
