From: claudio <claudio@openbsd.org>
Date: Wed, 5 Sep 2018 07:31:29 +0000 (+0000)
Subject: Change verification of communities a bit. Move the flag checking first since
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=65ba595f63ea191b09693c662ba1f5ad23af7597;p=openbsd

Change verification of communities a bit. Move the flag checking first since
it is currently terminal and then do the length check. If that one triggers
do a treat-as-withdraw but at the same time drop the bad attribute since it
is better to not have invalid attributes in the Adj-RIB-In since most code
does not expect that.
OK benno@
---

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 207281f71a0..a9a33451bb4 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.416 2018/08/29 19:47:47 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.417 2018/09/05 07:31:29 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -1583,6 +1583,9 @@ bad_flags:
 		/* 4-byte ready server take the default route */
 		goto optattr;
 	case ATTR_COMMUNITIES:
+		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
+		    ATTR_PARTIAL))
+			goto bad_flags;
 		if (attr_len == 0 || attr_len % 4 != 0) {
 			/*
 			 * mark update as bad and withdraw all routes as per
@@ -1591,12 +1594,13 @@ bad_flags:
 			a->flags |= F_ATTR_PARSE_ERR;
 			log_peer_warnx(&peer->conf, "bad COMMUNITIES, "
 			    "path invalidated and prefix withdrawn");
+			break;
 		}
+		goto optattr;
+	case ATTR_LARGE_COMMUNITIES:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
 		    ATTR_PARTIAL))
 			goto bad_flags;
-		goto optattr;
-	case ATTR_LARGE_COMMUNITIES:
 		if (attr_len == 0 || attr_len % 12 != 0) {
 			/*
 			 * mark update as bad and withdraw all routes as per
@@ -1605,12 +1609,13 @@ bad_flags:
 			a->flags |= F_ATTR_PARSE_ERR;
 			log_peer_warnx(&peer->conf, "bad LARGE COMMUNITIES, "
 			    "path invalidated and prefix withdrawn");
+			break;
 		}
+		goto optattr;
+	case ATTR_EXT_COMMUNITIES:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
 		    ATTR_PARTIAL))
 			goto bad_flags;
-		goto optattr;
-	case ATTR_EXT_COMMUNITIES:
 		if (attr_len == 0 || attr_len % 8 != 0) {
 			/*
 			 * mark update as bad and withdraw all routes as per
@@ -1619,10 +1624,8 @@ bad_flags:
 			a->flags |= F_ATTR_PARSE_ERR;
 			log_peer_warnx(&peer->conf, "bad EXT_COMMUNITIES, "
 			    "path invalidated and prefix withdrawn");
+			break;
 		}
-		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
-		    ATTR_PARTIAL))
-			goto bad_flags;
 		goto optattr;
 	case ATTR_ORIGINATOR_ID:
 		if (attr_len != 4)