Merge pull request #14661 from opensourcerouting/feature/enable_enfor…

…ce_first_as_by_default bgpd: Enable `enforce-first-as` by default
FRRouting · Oct 31, 2023 · 95f841c · 95f841c
2 parents 43cac6f + 3224629
commit 95f841c
Show file tree

Hide file tree

Showing 11 changed files with 79 additions and 13 deletions.
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
@@ -122,6 +122,10 @@ FRR_CFG_DEFAULT_BOOL(BGP_SOFT_VERSION_CAPABILITY,
 	{ .val_bool = true, .match_profile = "datacenter", },
 	{ .val_bool = false },
 );
+FRR_CFG_DEFAULT_BOOL(BGP_ENFORCE_FIRST_AS,
+	{ .val_bool = false, .match_version = "< 9.1", },
+	{ .val_bool = true },
+);
 
 DEFINE_HOOK(bgp_inst_config_write,
 		(struct bgp *bgp, struct vty *vty),
@@ -615,6 +619,8 @@ int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name,
 		if (DFLT_BGP_SOFT_VERSION_CAPABILITY)
 			SET_FLAG((*bgp)->flags,
 				 BGP_FLAG_SOFT_VERSION_CAPABILITY);
+		if (DFLT_BGP_ENFORCE_FIRST_AS)
+			SET_FLAG((*bgp)->flags, BGP_FLAG_ENFORCE_FIRST_AS);
 
 		ret = BGP_SUCCESS;
 	}
@@ -2828,6 +2834,23 @@ DEFUN(no_bgp_ebgp_requires_policy, no_bgp_ebgp_requires_policy_cmd,
 	return CMD_SUCCESS;
 }
 
+DEFPY(bgp_enforce_first_as,
+      bgp_enforce_first_as_cmd,
+      "[no] bgp enforce-first-as",
+      NO_STR
+      BGP_STR
+      "Enforce the first AS for EBGP routes\n")
+{
+	VTY_DECLVAR_CONTEXT(bgp, bgp);
+
+	if (no)
+		UNSET_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS);
+	else
+		SET_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS);
+
+	return CMD_SUCCESS;
+}
+
 DEFPY(bgp_lu_uses_explicit_null, bgp_lu_uses_explicit_null_cmd,
       "[no] bgp labeled-unicast <explicit-null|ipv4-explicit-null|ipv6-explicit-null>$value",
       NO_STR BGP_STR
@@ -18000,8 +18023,13 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp,
 			addr);
 
 	/* enforce-first-as */
-	if (peergroup_flag_check(peer, PEER_FLAG_ENFORCE_FIRST_AS))
-		vty_out(vty, " neighbor %s enforce-first-as\n", addr);
+	if (CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS)) {
+		if (!peergroup_flag_check(peer, PEER_FLAG_ENFORCE_FIRST_AS))
+			vty_out(vty, " no neighbor %s enforce-first-as\n", addr);
+	} else {
+		if (peergroup_flag_check(peer, PEER_FLAG_ENFORCE_FIRST_AS))
+			vty_out(vty, " neighbor %s enforce-first-as\n", addr);
+	}
 
 	/* update-source */
 	if (peergroup_flag_check(peer, PEER_FLAG_UPDATE_SOURCE)) {
@@ -18626,6 +18654,15 @@ int bgp_config_write(struct vty *vty)
 					? ""
 					: "no ");
 
+		/* bgp enforce-first-as */
+		if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS) !=
+		    SAVE_BGP_ENFORCE_FIRST_AS)
+			vty_out(vty, " %sbgp enforce-first-as\n",
+				CHECK_FLAG(bgp->flags,
+					   BGP_FLAG_ENFORCE_FIRST_AS)
+					? ""
+					: "no ");
+
 		if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_LU_IPV4_EXPLICIT_NULL) &&
 		    !!CHECK_FLAG(bgp->flags, BGP_FLAG_LU_IPV6_EXPLICIT_NULL))
 			vty_out(vty, " bgp labeled-unicast explicit-null\n");
@@ -19621,6 +19658,9 @@ void bgp_vty_init(void)
 	install_element(BGP_NODE, &bgp_ebgp_requires_policy_cmd);
 	install_element(BGP_NODE, &no_bgp_ebgp_requires_policy_cmd);
 
+	/* bgp enforce-first-as */
+	install_element(BGP_NODE, &bgp_enforce_first_as_cmd);
+
 	/* bgp labeled-unicast explicit-null */
 	install_element(BGP_NODE, &bgp_lu_uses_explicit_null_cmd);
 

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
@@ -1919,6 +1919,9 @@ struct peer *peer_create(union sockunion *su, const char *conf_if,
 		}
 	}
 
+	if (CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS))
+		SET_FLAG(peer->flags, PEER_FLAG_ENFORCE_FIRST_AS);
+
 	/* auto shutdown if configured */
 	if (bgp->autoshutdown)
 		peer_flag_set(peer, PEER_FLAG_SHUTDOWN);

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
@@ -518,6 +518,7 @@ struct bgp {
 /* For BGP-LU, force IPv6 local prefixes to use ipv6-explicit-null label */
 #define BGP_FLAG_LU_IPV6_EXPLICIT_NULL (1ULL << 34)
 #define BGP_FLAG_SOFT_VERSION_CAPABILITY (1ULL << 35)
+#define BGP_FLAG_ENFORCE_FIRST_AS (1ULL << 36)
 
 	/* BGP default address-families.
 	 * New peers inherit enabled afi/safis from bgp instance.

diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst
@@ -527,6 +527,27 @@ Reject routes with AS_SET or AS_CONFED_SET types
 
    This command enables rejection of incoming and outgoing routes having AS_SET or AS_CONFED_SET type.
 
+Enforce first AS
+----------------
+
+.. clicmd:: bgp enforce-first-as
+
+   To configure a router to deny an update received from an external BGP (eBGP)
+   peer that does not list its autonomous system number at the beginning of
+   the `AS_PATH` in the incoming update, use the ``bgp enforce-first-as`` command
+   in router configuration mode.
+
+   In order to exclude an arbitrary neighbor from this enforcement, use the
+   command ``no neighbor NAME enforce-first-as``. And vice-versa if a global
+   enforcement is disabled, you can override this behavior per neighbor too.
+
+   Default: enabled.
+
+.. note::
+
+   If you have a peering to RS (Route-Server), most likely you MUST disable the
+   first AS enforcement.
+
 Suppress duplicate updates
 --------------------------
 
@@ -1526,7 +1547,10 @@ Configuring Peers
    Discard updates received from the specified (eBGP) peer if the AS_PATH
    attribute does not contain the PEER's ASN as the first AS_PATH segment.
 
-   Default: disabled.
+   You can enable or disable this enforcement globally too using
+   ``bgp enforce-first-as`` command.
+
+   Default: enabled.
 
 .. clicmd:: neighbor PEER extended-optional-parameters
 

diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c
@@ -282,11 +282,6 @@ static struct test_peer_attr test_peer_attrs[] = {
 		.u.flag = PEER_FLAG_DONT_CAPABILITY,
 		.type = PEER_AT_GLOBAL_FLAG,
 	},
-	{
-		.cmd = "enforce-first-as",
-		.u.flag = PEER_FLAG_ENFORCE_FIRST_AS,
-		.type = PEER_AT_GLOBAL_FLAG,
-	},
 	{
 		.cmd = "local-as",
 		.peer_cmd = "local-as 1",

diff --git a/tests/bgpd/test_peer_attr.py b/tests/bgpd/test_peer_attr.py
@@ -15,7 +15,6 @@ class TestFlag(frrtest.TestMultiOut):
 TestFlag.okfail("peer\\description")
 TestFlag.okfail("peer\\disable-connected-check")
 TestFlag.okfail("peer\\dont-capability-negotiate")
-TestFlag.okfail("peer\\enforce-first-as")
 TestFlag.okfail("peer\\local-as")
 TestFlag.okfail("peer\\local-as 1 no-prepend")
 TestFlag.okfail("peer\\local-as 1 no-prepend replace-as")

diff --git a/tests/topotests/bgp_route_server_client/r1/bgpd.conf b/tests/topotests/bgp_route_server_client/r1/bgpd.conf
@@ -2,6 +2,7 @@
 router bgp 65001
  bgp router-id 10.10.10.1
  no bgp ebgp-requires-policy
+ no bgp enforce-first-as
  neighbor 2001:db8:1::1 remote-as external
  neighbor 2001:db8:1::1 timers 3 10
  neighbor 2001:db8:1::1 timers connect 5

diff --git a/tests/topotests/bgp_vpnv4_asbr/r1/bgpd.conf b/tests/topotests/bgp_vpnv4_asbr/r1/bgpd.conf
@@ -1,6 +1,7 @@
 router bgp 65500
  bgp router-id 192.0.2.1
  no bgp ebgp-requires-policy
+ no bgp enforce-first-as
  neighbor 192.0.2.100 remote-as 65500
  neighbor 192.0.2.100 update-source lo
  neighbor 192.168.0.100 remote-as 65500

diff --git a/tests/topotests/bgp_vpnv4_asbr/r2/bgpd.conf b/tests/topotests/bgp_vpnv4_asbr/r2/bgpd.conf
@@ -1,9 +1,10 @@
-debug bgp nht
-debug bgp zebra
-debug bgp labelpool
+!debug bgp nht
+!debug bgp zebra
+!debug bgp labelpool
 router bgp 65500
  bgp router-id 192.0.2.2
  no bgp ebgp-requires-policy
+ no bgp enforce-first-as
  neighbor 192.0.2.100 remote-as 65500
  neighbor 192.0.2.100 update-source lo
  neighbor 192.168.0.100 remote-as 65500

diff --git a/tests/topotests/bgp_vpnv4_asbr/r3/bgpd.conf b/tests/topotests/bgp_vpnv4_asbr/r3/bgpd.conf
@@ -1,6 +1,7 @@
 router bgp 65501
  bgp router-id 192.0.2.3
  no bgp ebgp-requires-policy
+ no bgp enforce-first-as
  neighbor 192.168.1.200 remote-as 65502
  address-family ipv4 unicast
   no neighbor 192.168.1.200 activate

diff --git a/yang/frr-bgp-neighbor.yang b/yang/frr-bgp-neighbor.yang
@@ -76,7 +76,7 @@ submodule frr-bgp-neighbor {
 
     leaf enforce-first-as {
       type boolean;
-      default "false";
+      default "true";
       description
         "When set to 'true' it will enforce the first AS for EBGP routes.";
     }