File 0017-bgpd-fix-error-handling-when-receiving-BGP... of Package frr

Overview Repositories Revisions Requests Users Attributes Meta

File 0017-bgpd-fix-error-handling-when-receiving-BGP-prefix-SID.patch of Package frr (Revision 80bb841138c12123bbb7c1fb60b34e8a)

Currently displaying revision 80bb841138c12123bbb7c1fb60b34e8a , Show latest

From ba6a8f1a31e1a88df2de69ea46068e8bd9b97138 Mon Sep 17 00:00:00 2001
From: Donatas Abraitis <donatas@opensourcerouting.org>
Date: Wed, 27 Mar 2024 18:42:56 +0200
Subject: [PATCH 1/2] bgpd: Fix error handling when receiving BGP Prefix SID
 attribute
References: bsc#1222518 CVE-2024-31948 https://github.com/FRRouting/frr/commit/ba6a8f1a31e1a88df2de69ea46068e8bd9b97138

Without this patch, we always set the BGP Prefix SID attribute flag without
checking if it's malformed or not. RFC8669 says that this attribute MUST be discarded.

Also, this fixes the bgpd crash when a malformed Prefix SID attribute is received,
with malformed transitive flags and/or TLVs.

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit ba6a8f1a31e1a88df2de69ea46068e8bd9b97138)
---
 bgpd/bgp_attr.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: frr-frr-7.4/bgpd/bgp_attr.c
===================================================================
--- frr-frr-7.4.orig/bgpd/bgp_attr.c
+++ frr-frr-7.4/bgpd/bgp_attr.c
@@ -1178,6 +1178,7 @@ bgp_attr_malformed(struct bgp_attr_parse
 	case BGP_ATTR_AS4_AGGREGATOR:
 	case BGP_ATTR_AGGREGATOR:
 	case BGP_ATTR_ATOMIC_AGGREGATE:
+	case BGP_ATTR_PREFIX_SID:
 		return BGP_ATTR_PARSE_PROCEED;
 
 	/* Core attributes, particularly ones which may influence route
@@ -2632,8 +2633,6 @@ bgp_attr_parse_ret_t bgp_attr_prefix_sid
 	struct attr *const attr = args->attr;
 	bgp_attr_parse_ret_t ret;
 
-	attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID);
-
 	uint8_t type;
 	uint16_t length;
 	size_t headersz = sizeof(type) + sizeof(length);
@@ -2685,6 +2684,8 @@ bgp_attr_parse_ret_t bgp_attr_prefix_sid
 		}
 	}
 
+	attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID);
+
 	return BGP_ATTR_PARSE_PROCEED;
 }

From babb23b74855e23c987a63f8256d24e28c044d07 Mon Sep 17 00:00:00 2001
From: Donatas Abraitis <donatas@opensourcerouting.org>
Date: Wed, 27 Mar 2024 19:08:38 +0200
Subject: [PATCH 2/2] bgpd: Prevent from one more CVE triggering this place
References: bsc#1222518 CVE-2024-31948 https://github.com/FRRouting/frr/pull/15628/commits/babb23b74855e23c987a63f8256d24e28c044d07

If we receive an attribute that is handled by bgp_attr_malformed(), use
treat-as-withdraw behavior for unknown (or missing to add - if new) attributes.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit babb23b74855e23c987a63f8256d24e28c044d07)
---
 bgpd/bgp_attr.c |   33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

Index: frr-frr-7.4/bgpd/bgp_attr.c
===================================================================
--- frr-frr-7.4.orig/bgpd/bgp_attr.c
+++ frr-frr-7.4/bgpd/bgp_attr.c
@@ -1169,6 +1169,15 @@ bgp_attr_malformed(struct bgp_attr_parse
 			(args->startp - STREAM_DATA(BGP_INPUT(peer)))
 				+ args->total);
 
+	/* Partial optional attributes that are malformed should not cause
+	 * the whole session to be reset. Instead treat it as a withdrawal
+	 * of the routes, if possible.
+	 */
+	if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS)
+	    && CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL)
+	    && CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL))
+		return BGP_ATTR_PARSE_WITHDRAW;
+
 	switch (args->type) {
 	/* where an attribute is relatively inconsequential, e.g. it does not
 	 * affect route selection, and can be safely ignored, then any such
@@ -1201,19 +1210,21 @@ bgp_attr_malformed(struct bgp_attr_parse
 		bgp_notify_send_with_data(peer, BGP_NOTIFY_UPDATE_ERR, subcode,
 					  notify_datap, length);
 		return BGP_ATTR_PARSE_ERROR;
+	default:
+		/* Unknown attributes, that are handled by this function
+		 * should be treated as withdraw, to prevent one more CVE
+		 * from being introduced.
+		 * RFC 7606 says:
+		 * The "treat-as-withdraw" approach is generally preferred
+		 * and the "session reset" approach is discouraged.
+		 */
+		flog_err(EC_BGP_ATTR_FLAG,
+				"%s(%u) attribute received, while it is not known how to handle it, treating as withdraw",
+				lookup_msg(attr_str, args->type, NULL), args->type);
+		break;
 	}
 
-	/* Partial optional attributes that are malformed should not cause
-	 * the whole session to be reset. Instead treat it as a withdrawal
-	 * of the routes, if possible.
-	 */
-	if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS)
-	    && CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL)
-	    && CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL))
-		return BGP_ATTR_PARSE_WITHDRAW;
-
-	/* default to reset */
-	return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
+	return BGP_ATTR_PARSE_WITHDRAW;
 }
 
 /* Find out what is wrong with the path attribute flag bits and log the error.

Places

File 0017-bgpd-fix-error-handling-when-receiving-BGP-prefix-SID.patch of Package frr (Revision 80bb841138c12123bbb7c1fb60b34e8a)

Places