summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorClaudio Jeker <claudio@cvs.openbsd.org>2008-12-28 15:19:22 +0000
committerClaudio Jeker <claudio@cvs.openbsd.org>2008-12-28 15:19:22 +0000
commited1e5f387f21fb890d7942a290d779465b7472e9 (patch)
tree4cd36292f47538ce75e8fb4da30041292a457cce
parent15bcab8f8003869faf111d93216766fa46a439e5 (diff)
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.
-rw-r--r--usr.sbin/bgpd/rde.c20
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;