From 3fb06fea1d14ef147a567052391a5b359704e971 Mon Sep 17 00:00:00 2001
From: Ondrej Zajicek <santiago@crfreenet.org>
Date: Thu, 23 Nov 2023 20:54:22 +0100
Subject: [PATCH] BGP: Add options to require BGP capabilities

Some BGP capabilities change the BGP behavior in a significant way, so if
the configuration depends on it, it is better to not establish BGP
session when the capability is not available.

Add several BGP option to require individual BGP capabilities during
session negotiation.
---
 doc/bird.sgml       | 73 +++++++++++++++++++++++++++++++++++++--------
 proto/bgp/bgp.c     | 53 +++++++++++++++++++++++++-------
 proto/bgp/bgp.h     | 10 +++++++
 proto/bgp/config.Y  |  9 ++++++
 proto/bgp/packets.c | 38 +++++++++++++++++++++--
 5 files changed, 157 insertions(+), 26 deletions(-)

diff --git a/doc/bird.sgml b/doc/bird.sgml
index d2b6459b..10c6f121 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -2996,14 +2996,22 @@ using the following configuration parameters:
 	refresh requests. Disabling Route Refresh also disables Enhanced Route Refresh.
 	Default: on.
 
+	<tag><label id="bgp-require-route-refresh">require route refresh <m/switch/</tag>
+	If enabled, the BGP Route Refresh capability (<rfc id="2918">) must be
+	announced by the BGP neighbor, otherwise the BGP session will not be
+	established. Default: off.
+
 	<tag><label id="bgp-enable-enhanced-route-refresh">enable enhanced route refresh <m/switch/</tag>
-	BGP protocol extension Enhanced Route Refresh (<rfc id="7313">) specifies explicit
-	begin and end for Route Refresh (see previous option),
-	therefore the receiver can remove
-	stale routes that were not advertised during the exchange. This option
-	specifies whether BIRD advertises this capability and supports
-	related procedures.
-	Default: on.
+	BGP protocol extension Enhanced Route Refresh (<rfc id="7313">)
+	specifies explicit begin and end for Route Refresh (see previous
+	option), therefore the receiver can remove stale routes that were not
+	advertised during the exchange. This option specifies whether BIRD
+	advertises this capability and supports related procedures. Default: on.
+
+	<tag><label id="bgp-require-enhanced-route-refresh">require enhanced route refresh <m/switch/</tag>
+	If enabled, the BGP Enhanced Route Refresh capability (<rfc id="7313">)
+	must be announced by the BGP neighbor, otherwise the BGP session
+	will not be established. Default: off.
 
 	<tag><label id="bgp-graceful-restart">graceful restart <m/switch/|aware</tag>
 	When a BGP speaker restarts or crashes, neighbors will discard all
@@ -3020,11 +3028,16 @@ using the following configuration parameters:
 	restart requires also configuration of other protocols. Default: aware.
 
 	<tag><label id="bgp-graceful-restart-time">graceful restart time <m/number/</tag>
-	The restart time is announced in the BGP graceful restart capability
+	The restart time is announced in the BGP Graceful Restart capability
 	and specifies how long the neighbor would wait for the BGP session to
 	re-establish after a restart before deleting stale routes. Default:
 	120 seconds.
 
+	<tag><label id="bgp-require-graceful-restart">require graceful restart <m/switch/</tag>
+	If enabled, the BGP Graceful Restart capability (<rfc id="4724">)
+	must be announced by the BGP neighbor, otherwise the BGP session
+	will not be established. Default: off.
+
 	<tag><label id="bgp-long-lived-graceful-restart">long lived graceful restart <m/switch/|aware</tag>
 	The long-lived graceful restart is an extension of the traditional
 	<ref id="bgp-graceful-restart" name="BGP graceful restart">, where stale
@@ -3038,12 +3051,17 @@ using the following configuration parameters:
 	graceful restart is disabled.
 
 	<tag><label id="bgp-long-lived-stale-time">long lived stale time <m/number/</tag>
-	The long-lived stale time is announced in the BGP long-lived graceful
-	restart capability and specifies how long the neighbor would keep stale
+	The long-lived stale time is announced in the BGP Long-lived Graceful
+	Restart capability and specifies how long the neighbor would keep stale
 	routes depreferenced during long-lived graceful restart until either the
 	session is re-stablished and synchronized or the stale time expires and
 	routes are removed. Default: 3600 seconds.
 
+	<tag><label id="bgp-require-long-lived-graceful-restart">require long lived graceful restart <m/switch/</tag>
+	If enabled, the BGP Long-lived Graceful Restart capability (draft)
+	must be announced by the BGP neighbor, otherwise the BGP session
+	will not be established. Default: off.
+
 	<tag><label id="bgp-interpret-communities">interpret communities <m/switch/</tag>
 	<rfc id="1997"> demands that BGP speaker should process well-known
 	communities like no-export (65535, 65281) or no-advertise (65535,
@@ -3063,11 +3081,21 @@ using the following configuration parameters:
 	in neighbor's implementation of 4B AS extension. Even when disabled
 	(off), BIRD behaves internally as AS4-aware BGP router. Default: on.
 
+	<tag><label id="bgp-require-as4">require as4 <m/switch/</tag>
+	If enabled, the BGP 4B AS number capability (<rfc id="6793">) must be
+	announced by the BGP neighbor, otherwise the BGP session will not be
+	established. Default: off.
+
 	<tag><label id="bgp-enable-extended-messages">enable extended messages <m/switch/</tag>
 	The BGP protocol uses maximum message length of 4096 bytes. This option
 	provides an extension (<rfc id="8654">) to allow extended messages with
 	length up to 65535 bytes. Default: off.
 
+	<tag><label id="bgp-require-extended-messages">require extended messages <m/switch/</tag>
+	If enabled, the BGP Extended Message capability (<rfc id="8654">) must
+	be announced by the BGP neighbor, otherwise the BGP session will not be
+	established. Default: off.
+
 	<tag><label id="bgp-capabilities">capabilities <m/switch/</tag>
 	Use capability advertisement to advertise optional capabilities. This is
 	standard behavior for newer BGP implementations, but there might be some
@@ -3077,7 +3105,11 @@ using the following configuration parameters:
 	capability-related error.
 
 	<tag><label id="bgp-advertise-hostname">advertise hostname <m/switch/</tag>
-	Advertise hostname capability along with the hostname. Default: off.
+	Advertise the hostname capability along with the hostname. Default: off.
+
+	<tag><label id="bgp-require-hostname">require hostname <m/switch/</tag>
+	If enabled, the hostname capability must be announced by the BGP
+	neighbor, otherwise the BGP session negotiation fails. Default: off.
 
 	<tag><label id="bgp-disable-after-error">disable after error <m/switch/</tag>
 	When an error is encountered (either locally or by the other side),
@@ -3403,15 +3435,30 @@ be used in explicit configuration.
 	just IPv4-mapped IPv6 addresses are used, as described in
 	<rfc id="4798"> and <rfc id="4659">. Default: off.
 
+	<tag><label id="bgp-require-extended-next-hop">require extended next hop <m/switch/</tag>
+	If enabled, the BGP Extended Next Hop Encoding capability (<rfc id="8950">)
+	must be announced by the BGP neighbor, otherwise the BGP session will
+	not be established. Note that this option is relevant just for IPv4 /
+	VPNv4 channels, as IPv6 / VPNv6 channels use a different mechanism not
+	signalled by a capability. Default: off.
+
 	<tag><label id="bgp-add-paths">add paths <m/switch/|rx|tx</tag>
 	Standard BGP can propagate only one path (route) per destination network
-	(usually the selected one). This option controls the add-path protocol
+	(usually the selected one). This option controls the ADD-PATH protocol
 	extension, which allows to advertise any number of paths to a
-	destination. Note that to be active, add-path has to be enabled on both
+	destination. Note that to be active, ADD-PATH has to be enabled on both
 	sides of the BGP session, but it could be enabled separately for RX and
 	TX direction. When active, all available routes accepted by the export
 	filter are advertised to the neighbor. Default: off.
 
+	<tag><label id="bgp-require-add-paths">require add paths <m/switch/</tag>
+	If enabled, the BGP ADD-PATH capability (<rfc id="7911">) must be
+	announced by the BGP neighbor, otherwise the BGP session will not be
+	established. Announced directions in the capability must be compatible
+	with locally configured directions. E.g., If <cf/add path tx/ is
+	configured locally, then the neighbor capability must announce RX.
+	Default: off.
+
 	<tag><label id="bgp-aigp">aigp <m/switch/|originate</tag>
 	The BGP protocol does not use a common metric like other routing
 	protocols, instead it uses a set of criteria for route selection
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 914935b9..b14df932 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -2000,6 +2000,21 @@ bgp_postconfig(struct proto_config *CF)
   if (interior && (cf->local_role != BGP_ROLE_UNDEFINED))
     log(L_WARN "BGP roles are not recommended to be used within AS confederations");
 
+  if (cf->require_enhanced_refresh && !(cf->enable_refresh && cf->enable_enhanced_refresh))
+    cf_warn("Enhanced refresh required but disabled");
+
+  if (cf->require_as4 && !cf->enable_as4)
+    cf_warn("AS4 support required but disabled");
+
+  if (cf->require_extended_messages && !cf->enable_extended_messages)
+    cf_warn("Extended messages required but not enabled");
+
+  if (cf->require_gr && !cf->gr_mode)
+    cf_warn("Graceful restart required but not enabled");
+
+  if (cf->require_llgr && !cf->llgr_mode)
+    cf_warn("Long-lived graceful restart required but not enabled");
+
   if (cf->require_roles && (cf->local_role == BGP_ROLE_UNDEFINED))
     cf_error("Local role must be set if roles are required");
 
@@ -2123,6 +2138,12 @@ bgp_postconfig(struct proto_config *CF)
 
     if (cc->secondary && !cc->c.table->sorted)
       cf_error("BGP with secondary option requires sorted table");
+
+    if (cc->require_ext_next_hop && !cc->ext_next_hop)
+      cf_warn("Extended next hop required but not enabled");
+
+    if (cc->require_add_path && !cc->add_path)
+      cf_warn("ADD-PATH required but not enabled");
   }
 }
 
@@ -2167,20 +2188,30 @@ bgp_reconfigure(struct proto *P, struct proto_config *CF)
     if (C->stale)
       same = proto_configure_channel(P, &C, NULL) && same;
 
-  if (same)
-    proto_setup_mpls_map(P, RTS_BGP, 1);
-
-  if (same && (p->start_state > BSS_PREPARE))
-    bgp_update_bfd(p, new->bfd);
-
-  /* We should update our copy of configuration ptr as old configuration will be freed */
-  if (same)
-    p->cf = new;
-
   /* Reset name counter */
   p->dynamic_name_counter = 0;
 
-  return same;
+  if (!same)
+    return 0;
+
+  /* We should update our copy of configuration ptr as old configuration will be freed */
+  p->cf = new;
+
+  /* Check whether existing connections are compatible with required capabilities */
+  struct bgp_conn *ci = &p->incoming_conn;
+  if (((ci->state == BS_OPENCONFIRM) || (ci->state == BS_ESTABLISHED)) && !bgp_check_capabilities(ci))
+    return 0;
+
+  struct bgp_conn *co = &p->outgoing_conn;
+  if (((co->state == BS_OPENCONFIRM) || (co->state == BS_ESTABLISHED)) && !bgp_check_capabilities(co))
+    return 0;
+
+  proto_setup_mpls_map(P, RTS_BGP, 1);
+
+  if (p->start_state > BSS_PREPARE)
+    bgp_update_bfd(p, new->bfd);
+
+  return 1;
 }
 
 #define TABLE(cf, NAME) ((cf)->NAME ? (cf)->NAME->table : NULL )
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 2e95037c..61127562 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -138,6 +138,13 @@ struct bgp_config {
   const char *dynamic_name;		/* Name pattern for dynamic BGP */
   int dynamic_name_digits;		/* Minimum number of digits for dynamic names */
   int check_link;			/* Use iface link state for liveness detection */
+  int require_refresh;			/* Require remote support for route refresh [RFC 2918] */
+  int require_enhanced_refresh;		/* Require remote support for enhanced route refresh [RFC 7313] */
+  int require_as4;			/* Require remote support for 4B AS numbers [RFC 6793] */
+  int require_extended_messages;	/* Require remote support for extended messages [RFC 8654] */
+  int require_hostname;			/* Require remote support for hostname [draft] */
+  int require_gr;			/* Require remote support for graceful restart [RFC 4724] */
+  int require_llgr;			/* Require remote support for long-lived graceful restart [draft] */
   struct bfd_options *bfd;		/* Use BFD for liveness detection */
 };
 
@@ -159,7 +166,9 @@ struct bgp_channel_config {
   u8 llgr_able;				/* Allow full long-lived GR for the channel */
   uint llgr_time;			/* Long-lived graceful restart stale time */
   u8 ext_next_hop;			/* Allow both IPv4 and IPv6 next hops */
+  u8 require_ext_next_hop;		/* Require remote support of IPv4 NLRI with IPv6 next hops [RFC 8950] */
   u8 add_path;				/* Use ADD-PATH extension [RFC 7911] */
+  u8 require_add_path;			/* Require remote support of ADD-PATH extension [RFC 7911] */
   u8 aigp;				/* AIGP is allowed on this session */
   u8 aigp_originate;			/* AIGP is originated automatically */
   u32 cost;				/* IGP cost for direct next hops */
@@ -657,6 +666,7 @@ bgp_total_aigp_metric(rte *r)
 
 void bgp_dump_state_change(struct bgp_conn *conn, uint old, uint new);
 void bgp_prepare_capabilities(struct bgp_conn *conn);
+int bgp_check_capabilities(struct bgp_conn *conn);
 const struct bgp_af_desc *bgp_get_af_desc(u32 afi);
 const struct bgp_af_caps *bgp_find_af_caps(struct bgp_caps *caps, u32 afi);
 void bgp_schedule_packet(struct bgp_conn *conn, struct bgp_channel *c, int type);
diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y
index 6981a117..0fcbb276 100644
--- a/proto/bgp/config.Y
+++ b/proto/bgp/config.Y
@@ -191,6 +191,13 @@ bgp_proto:
  | bgp_proto ENABLE AS4 bool ';' { BGP_CFG->enable_as4 = $4; }
  | bgp_proto ENABLE EXTENDED MESSAGES bool ';' { BGP_CFG->enable_extended_messages = $5; }
  | bgp_proto ADVERTISE HOSTNAME bool ';' { BGP_CFG->enable_hostname = $4; }
+ | bgp_proto REQUIRE ROUTE REFRESH bool ';' { BGP_CFG->require_refresh = $5; }
+ | bgp_proto REQUIRE ENHANCED ROUTE REFRESH bool ';' { BGP_CFG->require_enhanced_refresh = $6; }
+ | bgp_proto REQUIRE AS4 bool ';' { BGP_CFG->require_as4 = $4; }
+ | bgp_proto REQUIRE EXTENDED MESSAGES bool ';' { BGP_CFG->require_extended_messages = $5; }
+ | bgp_proto REQUIRE HOSTNAME bool ';' { BGP_CFG->require_hostname = $4; }
+ | bgp_proto REQUIRE GRACEFUL RESTART bool ';' { BGP_CFG->require_gr = $5; }
+ | bgp_proto REQUIRE LONG LIVED GRACEFUL RESTART bool ';' { BGP_CFG->require_llgr = $7; }
  | bgp_proto CAPABILITIES bool ';' { BGP_CFG->capabilities = $3; }
  | bgp_proto PASSWORD text ';' { BGP_CFG->password = $3; }
  | bgp_proto SETKEY bool ';' { BGP_CFG->setkey = $3; }
@@ -284,9 +291,11 @@ bgp_channel_item:
  | LONG LIVED GRACEFUL RESTART bool { BGP_CC->llgr_able = $5; }
  | LONG LIVED STALE TIME expr { BGP_CC->llgr_time = $5; }
  | EXTENDED NEXT HOP bool { BGP_CC->ext_next_hop = $4; }
+ | REQUIRE EXTENDED NEXT HOP bool { BGP_CC->require_ext_next_hop = $5;  if (BGP_AFI(BGP_CC->afi) != BGP_AFI_IPV4) cf_warn("Require extended next hop option ignored for non-IPv4 channels"); }
  | ADD PATHS RX { BGP_CC->add_path = BGP_ADD_PATH_RX; }
  | ADD PATHS TX { BGP_CC->add_path = BGP_ADD_PATH_TX; }
  | ADD PATHS bool { BGP_CC->add_path = $3 ? BGP_ADD_PATH_FULL : 0; }
+ | REQUIRE ADD PATHS bool { BGP_CC->require_add_path = $4; }
  | IMPORT TABLE bool { BGP_CC->import_table = $3; }
  | EXPORT TABLE bool { BGP_CC->export_table = $3; }
  | AIGP bool { BGP_CC->aigp = $2; BGP_CC->aigp_originate = 0; }
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index 395169a4..1e5a226f 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -694,7 +694,7 @@ err:
   return -1;
 }
 
-static int
+int
 bgp_check_capabilities(struct bgp_conn *conn)
 {
   struct bgp_proto *p = conn->bgp;
@@ -706,6 +706,29 @@ bgp_check_capabilities(struct bgp_conn *conn)
   /* This is partially overlapping with bgp_conn_enter_established_state(),
      but we need to run this just after we receive OPEN message */
 
+  if (p->cf->require_refresh && !remote->route_refresh)
+    return 0;
+
+  if (p->cf->require_enhanced_refresh && !remote->enhanced_refresh)
+    return 0;
+
+  if (p->cf->require_as4 && !remote->as4_support)
+    return 0;
+
+  if (p->cf->require_extended_messages && !remote->ext_messages)
+    return 0;
+
+  if (p->cf->require_hostname && !remote->hostname)
+    return 0;
+
+  if (p->cf->require_gr && !remote->gr_aware)
+    return 0;
+
+  if (p->cf->require_llgr && !remote->llgr_aware)
+    return 0;
+
+  /* No check for require_roles, as it uses error code 2.11 instead of 2.7 */
+
   BGP_WALK_CHANNELS(p, c)
   {
     const struct bgp_af_caps *loc = bgp_find_af_caps(local,  c->afi);
@@ -719,7 +742,18 @@ bgp_check_capabilities(struct bgp_conn *conn)
       return 0;
 
     if (active)
+    {
+      if (c->cf->require_ext_next_hop && !rem->ext_next_hop)
+	return 0;
+
+      if (c->cf->require_add_path && (loc->add_path & BGP_ADD_PATH_RX) && !(rem->add_path & BGP_ADD_PATH_TX))
+	return 0;
+
+      if (c->cf->require_add_path && (loc->add_path & BGP_ADD_PATH_TX) && !(rem->add_path & BGP_ADD_PATH_RX))
+	return 0;
+
       count++;
+    }
   }
 
   /* We need at least one channel active */
@@ -905,7 +939,7 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
   if (!id || (p->is_internal && id == p->local_id))
   { bgp_error(conn, 2, 3, pkt+24, -4); return; }
 
-  /* RFC 5492 4 - check for required capabilities */
+  /* RFC 5492 5 - check for required capabilities */
   if (p->cf->capabilities && !bgp_check_capabilities(conn))
   { bgp_error(conn, 2, 7, NULL, 0); return; }