Add a ugly workaround for the problem where an invalid AS4_PATH is passed

over mulitple hops and causes bgpd to close the connection. This is what
the RFC requires us to do but the result is a DoS against all OpenBGPD
routers when somebody injects such a bad optional transitive attribute
because the intermediate routers don't give a damn about it.
As a result we now ignore such bad prefixes and don't allow them in the
decision process. The handling of optional transitive attributes needs to
be rethinked because all of them can be abused in such a way.
Idea OK by a few + henning@, tested myself against my crappy regress test
suite that needs way more work.

1 files changed, 14 insertions, 6 deletions

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 07d7679d292..579cdb3bc55 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.233 2008/12/12 16:02:49 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.234 2008/12/28 15:19:21 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -797,8 +797,10 @@ rde_update_dispatch(struct imsg *imsg)
 		/*
 		 * if either ATTR_NEW_AGGREGATOR or ATTR_NEW_ASPATH is present
 		 * try to fixup the attributes.
+		 * XXX do not fixup if F_ATTR_LOOP is set.
 		 */
-		if (asp->flags & F_ATTR_AS4BYTE_NEW)
+		if (asp->flags & F_ATTR_AS4BYTE_NEW &&
+		    !(asp->flags & F_ATTR_LOOP))
 			rde_as4byte_fixup(peer, asp);
 
 		/* enforce remote AS if requested */
@@ -1347,10 +1349,16 @@ bad_flags:
 		    ATTR_PARTIAL))
 			goto bad_flags;
 		if (aspath_verify(p, attr_len, 1) != 0) {
-			/* XXX draft does not specify how to handle errors */
-			rde_update_err(peer, ERR_UPDATE, ERR_UPD_OPTATTR,
-			    op, len);
-			return (-1);
+			/*
+			 * XXX RFC does not specify how to handle errors.
+			 * XXX Instead of dropping the session because of a
+			 * XXX bad path just mark the full update as not
+			 * XXX loop-free the update is no longer eligible and
+			 * XXX will not be considered for routing or
+			 * XXX redistribution. Something better is needed.
+			 */
+			a->flags |= F_ATTR_LOOP;
+			goto optattr;
 		}
 		a->flags |= F_ATTR_AS4BYTE_NEW;
 		goto optattr;