From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Mon, 9 Mar 2020 13:50:18 +0000 (-0400)
Subject: zebra: Prevent awful misconfiguration in vrf's
X-Git-Tag: base_7.4~48^2
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=refs%2Fpull%2F5944%2Fhead;p=mirror%2Ffrr.git

zebra: Prevent awful misconfiguration in vrf's

Currently the linux kernel allows you to specify the same
table id -> multiple vrf's.  While I am arguing with
the kernel people about proper behavior here let's
just remove this as a possiblity from happening and
mark it a zebra stopable misconfiguration.

(Effectively we are preventing a crash down the line
as that all over FRR we assume it's a unique
mapping not a many to one).

Why fail hard?  Because we hope to get the person
who misconfigured it to actually notice immediately
not hours or days down the line when shit hits the fan.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---

diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index 78155b1455..17b6edfed0 100644
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -275,7 +275,7 @@ static void netlink_determine_zebra_iftype(const char *kind,
 	netlink_parse_rtattr((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta))
 
 static void netlink_vrf_change(struct nlmsghdr *h, struct rtattr *tb,
-			       const char *name)
+			       uint32_t ns_id, const char *name)
 {
 	struct ifinfomsg *ifi;
 	struct rtattr *linkinfo[IFLA_INFO_MAX + 1];
@@ -310,10 +310,22 @@ static void netlink_vrf_change(struct nlmsghdr *h, struct rtattr *tb,
 	nl_table_id = *(uint32_t *)RTA_DATA(attr[IFLA_VRF_TABLE]);
 
 	if (h->nlmsg_type == RTM_NEWLINK) {
+		vrf_id_t exist_id;
+
 		if (IS_ZEBRA_DEBUG_KERNEL)
 			zlog_debug("RTM_NEWLINK for VRF %s(%u) table %u", name,
 				   ifi->ifi_index, nl_table_id);
 
+		exist_id = vrf_lookup_by_table(nl_table_id, ns_id);
+		if (exist_id != VRF_DEFAULT) {
+			vrf = vrf_lookup_by_id(exist_id);
+
+			flog_err(
+				EC_ZEBRA_VRF_MISCONFIGURED,
+				"VRF %s id %u table id overlaps existing vrf %s, misconfiguration exiting",
+				name, ifi->ifi_index, vrf->name);
+			exit(-1);
+		}
 		/*
 		 * vrf_get is implied creation if it does not exist
 		 */
@@ -664,7 +676,7 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 
 	/* If VRF, create the VRF structure itself. */
 	if (zif_type == ZEBRA_IF_VRF && !vrf_is_backend_netns()) {
-		netlink_vrf_change(h, tb[IFLA_LINKINFO], name);
+		netlink_vrf_change(h, tb[IFLA_LINKINFO], ns_id, name);
 		vrf_id = (vrf_id_t)ifi->ifi_index;
 	}
 
@@ -1226,7 +1238,7 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 
 	/* If VRF, create or update the VRF structure itself. */
 	if (zif_type == ZEBRA_IF_VRF && !vrf_is_backend_netns()) {
-		netlink_vrf_change(h, tb[IFLA_LINKINFO], name);
+		netlink_vrf_change(h, tb[IFLA_LINKINFO], ns_id, name);
 		vrf_id = (vrf_id_t)ifi->ifi_index;
 	}
 
diff --git a/zebra/zebra_errors.c b/zebra/zebra_errors.c
index ef792d14c2..b75708031e 100644
--- a/zebra/zebra_errors.c
+++ b/zebra/zebra_errors.c
@@ -785,6 +785,12 @@ static struct log_ref ferr_zebra_err[] = {
 		.suggestion =
 			"See if the nexthop you are trying to add is already present in the fib."
 	},
+	{
+		.code = EC_ZEBRA_VRF_MISCONFIGURED,
+		.title = "Duplicate VRF table id detected",
+		.description = "Zebra has detected a situation where there are two vrf devices with the exact same tableid.  This is considered a complete misconfiguration of VRF devices and breaks a fundamental assumption in FRR about how VRF's work",
+		.suggestion = "Use different table id's for the VRF's in question"
+	},
 	{
 		.code = END_FERR,
 	}
diff --git a/zebra/zebra_errors.h b/zebra/zebra_errors.h
index 4625a03ae6..395004d0bb 100644
--- a/zebra/zebra_errors.h
+++ b/zebra/zebra_errors.h
@@ -132,6 +132,7 @@ enum zebra_log_refs {
 	EC_ZEBRA_DUP_IP_DETECTED,
 	EC_ZEBRA_BAD_NHG_MESSAGE,
 	EC_ZEBRA_DUPLICATE_NHG_MESSAGE,
+	EC_ZEBRA_VRF_MISCONFIGURED,
 };
 
 void zebra_error_init(void);