File Quagga-CVE-2023-38802-bgpd-withdraw.bsc1213284.patch of Package quagga.36746
From ab91e2aa78ce8464d01a57aebbc2dc5afc8cf119 Mon Sep 17 00:00:00 2001
From: Marius Tomaschewski <mt@suse.com>
Date: Wed, 13 Sep 2023 12:30:52 +0200
Subject: [PATCH 1/4] bgpd: treat-as-withdraw for malformed attributes
Upsteam: no
References: CVE-2023-38802,bsc#1213284
Backported frr commit 4ba5a9c55f761e0c32d45050014b68e431d8b254
from https://github.com/FRRouting/frr/pull/5744:
```
Author: Donatas Abraitis <donatas.abraitis@gmail.com>
Date: Wed Jan 29 15:09:37 2020 +0200
bgpd: Update some attributes how they are handled if malformed
According to https://tools.ietf.org/html/rfc7606 some of the attributes
MUST be handled as "treat-as-withdraw" approach.
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
```
Signed-off-by: Marius Tomaschewski <mt@suse.com>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 43955125..1eb5bc5d 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -918,7 +918,7 @@ bgp_attr_malformed (struct bgp_attr_parser_args *args, u_char subcode,
return BGP_ATTR_PARSE_PROCEED;
/* Core attributes, particularly ones which may influence route
- * selection, should always cause session resets
+ * selection, should be treat-as-withdraw.
*/
case BGP_ATTR_ORIGIN:
case BGP_ATTR_AS_PATH:
@@ -926,11 +926,12 @@ bgp_attr_malformed (struct bgp_attr_parser_args *args, u_char subcode,
case BGP_ATTR_MULTI_EXIT_DISC:
case BGP_ATTR_LOCAL_PREF:
case BGP_ATTR_COMMUNITIES:
+ case BGP_ATTR_EXT_COMMUNITIES:
case BGP_ATTR_ORIGINATOR_ID:
case BGP_ATTR_CLUSTER_LIST:
+ return BGP_ATTR_PARSE_WITHDRAW;
case BGP_ATTR_MP_REACH_NLRI:
case BGP_ATTR_MP_UNREACH_NLRI:
- case BGP_ATTR_EXT_COMMUNITIES:
bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
notify_datap, length);
return BGP_ATTR_PARSE_ERROR;
@@ -1169,23 +1170,19 @@ bgp_attr_aspath_check (struct peer *const peer, struct attr *const attr)
(peer->sort == BGP_PEER_EBGP && aspath_confed_check (attr->aspath)))
{
zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host);
- bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_WITHDRAW;
}
/* First AS check for EBGP. */
if (bgp != NULL && bgp_flag_check (bgp, BGP_FLAG_ENFORCE_FIRST_AS))
{
if (peer->sort == BGP_PEER_EBGP
- && ! aspath_firstas_check (attr->aspath, peer->as))
- {
- zlog (peer->log, LOG_ERR,
- "%s incorrect first AS (must be %u)", peer->host, peer->as);
- bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH);
- return BGP_ATTR_PARSE_ERROR;
- }
+ && ! aspath_firstas_check (attr->aspath, peer->as))
+ {
+ zlog (peer->log, LOG_ERR,
+ "%s incorrect first AS (must be %u)", peer->host, peer->as);
+ return BGP_ATTR_PARSE_WITHDRAW;
+ }
}
/* local-as prepend */
@@ -1309,8 +1306,12 @@ bgp_attr_local_pref (struct bgp_attr_parser_args *args)
struct attr *const attr = args->attr;
const bgp_size_t length = args->length;
- /* Length check. */
- if (length != 4)
+ /* if received from an internal neighbor, it SHALL be considered
+ * malformed if its length is not equal to 4. If malformed, the
+ * UPDATE message SHALL be handled using the approach of "treat-as-
+ * withdraw".
+ */
+ if (peer->sort == BGP_PEER_IBGP && length != 4)
{
zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute length isn't 4 [%u]",
length);
@@ -1372,7 +1373,8 @@ bgp_attr_aggregator (struct bgp_attr_parser_args *args)
struct attr_extra *attre = bgp_attr_extra_get (attr);
/* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */
- if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
+ if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)
+ && CHECK_FLAG(peer->cap, PEER_CAP_AS4_ADV))
wantedlen = 8;
if (length != wantedlen)
@@ -1551,6 +1553,9 @@ bgp_attr_community (struct bgp_attr_parser_args *args)
/* XXX: fix community_parse to use stream API and remove this */
stream_forward_getp (peer->ibuf, length);
+ /* The Community attribute SHALL be considered malformed if its
+ * length is not a non-zero multiple of 4.
+ */
if (!attr->community)
return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
@@ -1569,7 +1574,11 @@ bgp_attr_originator_id (struct bgp_attr_parser_args *args)
struct attr *const attr = args->attr;
const bgp_size_t length = args->length;
- /* Length check. */
+ /* if received from an internal neighbor, it SHALL be considered
+ * malformed if its length is not equal to 4. If malformed, the
+ * UPDATE message SHALL be handled using the approach of "treat-as-
+ * withdraw".
+ */
if (length != 4)
{
zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length);
@@ -1595,7 +1604,11 @@ bgp_attr_cluster_list (struct bgp_attr_parser_args *args)
struct attr *const attr = args->attr;
const bgp_size_t length = args->length;
- /* Check length. */
+ /* if received from an internal neighbor, it SHALL be considered
+ * malformed if its length is not a non-zero multiple of 4. If
+ * malformed, the UPDATE message SHALL be handled using the approach
+ * of "treat-as-withdraw".
+ */
if (length % 4)
{
zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
@@ -1816,7 +1829,10 @@ bgp_attr_ext_communities (struct bgp_attr_parser_args *args)
ecommunity_parse ((u_int8_t *)stream_pnt (peer->ibuf), length);
/* XXX: fix ecommunity_parse to use stream API */
stream_forward_getp (peer->ibuf, length);
-
+
+ /* The Extended Community attribute SHALL be considered malformed if
+ * its length is not a non-zero multiple of 8.
+ */
if (!attr->extra->ecommunity)
return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
@@ -2047,16 +2063,16 @@ 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;
+ /* If any of the well-known mandatory attributes are not present
+ * in an UPDATE message, then "treat-as-withdraw" MUST be used.
+ */
if (type)
{
zlog (peer->log, LOG_WARNING,
"%s Missing well-known attribute %d / %s",
peer->host, type, LOOKUP (attr_str, type));
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MISS_ATTR,
- &type, 1);
- return BGP_ATTR_PARSE_ERROR;
+
+ return BGP_ATTR_PARSE_WITHDRAW;
}
return BGP_ATTR_PARSE_PROCEED;
}
diff --git a/tests/aspath_test.c b/tests/aspath_test.c
index 5a0899ec..4f752af1 100644
--- a/tests/aspath_test.c
+++ b/tests/aspath_test.c
@@ -637,7 +637,7 @@ static struct aspath_tests {
"4b AS4_PATH w/o AS_PATH",
&test_segments[6],
NULL,
- AS4_DATA, -1,
+ AS4_DATA, -2,
PEER_CAP_AS4_ADV,
{ COMMON_ATTRS,
BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
--
2.35.3
From d286bd5bef266b0b1cde75359d96a447eab1cf0f Mon Sep 17 00:00:00 2001
From: Marius Tomaschewski <mt@suse.com>
Date: Wed, 13 Sep 2023 12:30:52 +0200
Subject: [PATCH 2/4] bgpd: treat 0-length cluster_list as withdraw
Upsteam: no
References: CVE-2023-38802,bsc#1213284
Backported frr commit 33ba22c24874b12916c4ea0fc57e9e188ce7c18d
from https://github.com/FRRouting/frr/pull/6167:
```
Author: Quentin Young <qlyoung@cumulusnetworks.com>
Date: Mon Apr 6 12:30:35 2020 -0400
bgpd: treat 0-length cluster_list as withdraw
See source comment...
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
```
Signed-off-by: Marius Tomaschewski <mt@suse.com>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 1eb5bc5d..e3a7546a 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1609,7 +1609,7 @@ bgp_attr_cluster_list (struct bgp_attr_parser_args *args)
* malformed, the UPDATE message SHALL be handled using the approach
* of "treat-as-withdraw".
*/
- if (length % 4)
+ if (length == 0 || length % 4)
{
zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
--
2.35.3
From 8a6dc173b2afa1b8904787b02635a724f098da6b Mon Sep 17 00:00:00 2001
From: Marius Tomaschewski <mt@suse.com>
Date: Wed, 13 Sep 2023 12:30:52 +0200
Subject: [PATCH 3/4] bgpd: Treat-as-withdraw if [e]community length is zero
Upsteam: no
References: CVE-2023-38802,bsc#1213284
Backported frr commit 6680b5508c8d37b61910ee141d9696bd3d10315e
from https://github.com/FRRouting/frr/pull/6173:
```
Author: Donatas Abraitis <donatas.abraitis@gmail.com>
Date: Tue Apr 7 10:08:16 2020 +0300
bgpd: Treat-as-withdraw if [el]community length is zero
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
```
Signed-off-by: Marius Tomaschewski <mt@suse.com>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index e3a7546a..017ac2b6 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1544,7 +1544,8 @@ bgp_attr_community (struct bgp_attr_parser_args *args)
if (length == 0)
{
attr->community = NULL;
- return BGP_ATTR_PARSE_PROCEED;
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
}
attr->community =
@@ -1822,7 +1823,8 @@ bgp_attr_ext_communities (struct bgp_attr_parser_args *args)
if (attr->extra)
attr->extra->ecommunity = NULL;
/* Empty extcomm doesn't seem to be invalid per se */
- return BGP_ATTR_PARSE_PROCEED;
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
}
(bgp_attr_extra_get (attr))->ecommunity =
--
2.35.3
From e35b24f747ffb3491041e31c5a06c14fe21b751e Mon Sep 17 00:00:00 2001
From: Marius Tomaschewski <mt@suse.com>
Date: Fri, 15 Sep 2023 10:34:20 +0200
Subject: [PATCH 4/4] bgpd: treat-as-withdraw for tunnel encapsulation
Upsteam: no
References: CVE-2023-38802,bsc#1213284
Backported frr commit bcb6b58d9530173df41d3a3cbc4c600ee0b4b186
from https://github.com/FRRouting/frr/pull/14295 (CVE-2023-38802):
```
bgpd: Use treat-as-withdraw for tunnel encapsulation attribute
Before this path we used session reset method, which is discouraged by rfc7606.
Handle this as rfc requires.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
```
Signed-off-by: Marius Tomaschewski <mt@suse.com>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 017ac2b6..fce4b542 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -929,6 +929,7 @@ bgp_attr_malformed (struct bgp_attr_parser_args *args, u_char subcode,
case BGP_ATTR_EXT_COMMUNITIES:
case BGP_ATTR_ORIGINATOR_ID:
case BGP_ATTR_CLUSTER_LIST:
+ case BGP_ATTR_ENCAP:
return BGP_ATTR_PARSE_WITHDRAW;
case BGP_ATTR_MP_REACH_NLRI:
case BGP_ATTR_MP_UNREACH_NLRI:
@@ -1847,31 +1848,24 @@ bgp_attr_ext_communities (struct bgp_attr_parser_args *args)
/* Parse Tunnel Encap attribute in an UPDATE */
static int
-bgp_attr_encap(
- uint8_t type,
- struct peer *peer, /* IN */
- bgp_size_t length, /* IN: attr's length field */
- struct attr *attr, /* IN: caller already allocated */
- u_char flag, /* IN: attr's flags field */
- u_char *startp)
-{
- bgp_size_t total;
+bgp_attr_encap(struct bgp_attr_parser_args *args)
+{
struct attr_extra *attre = NULL;
struct bgp_attr_encap_subtlv *stlv_last = NULL;
uint16_t tunneltype;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ bgp_size_t length = args->length;
+ uint8_t type = args->type;
+ uint8_t flag = args->flags;
if (!CHECK_FLAG(flag, BGP_ATTR_FLAG_TRANS)
|| !CHECK_FLAG(flag, BGP_ATTR_FLAG_OPTIONAL))
{
zlog (peer->log, LOG_ERR,
"Tunnel Encap attribute flag isn't optional and transitive %d", flag);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
}
if (BGP_ATTR_ENCAP == type) {
@@ -1881,11 +1875,8 @@ bgp_attr_encap(
if (length < 4) {
zlog (peer->log, LOG_ERR,
"Tunnel Encap attribute not long enough to contain outer T,L");
- bgp_notify_send_with_data(peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
}
tunneltype = stream_getw (BGP_INPUT (peer));
tlv_length = stream_getw (BGP_INPUT (peer));
@@ -1912,11 +1903,8 @@ bgp_attr_encap(
zlog (peer->log, LOG_ERR,
"Tunnel Encap attribute sub-tlv length %d exceeds remaining length %d",
sublength, length);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
}
/* alloc and copy sub-tlv */
@@ -1953,11 +1941,8 @@ bgp_attr_encap(
/* spurious leftover data */
zlog (peer->log, LOG_ERR,
"Tunnel Encap attribute length is bad: %d leftover octets", length);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
- startp, total);
- return -1;
+ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
}
return 0;
@@ -2266,7 +2251,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
ret = bgp_attr_ext_communities (&attr_args);
break;
case BGP_ATTR_ENCAP:
- ret = bgp_attr_encap (type, peer, length, attr, flag, startp);
+ ret = bgp_attr_encap (&attr_args);
break;
default:
ret = bgp_attr_unknown (&attr_args);
--
2.35.3
