From ed1e5f387f21fb890d7942a290d779465b7472e9 Mon Sep 17 00:00:00 2001 From: Claudio Jeker Date: Sun, 28 Dec 2008 15:19:22 +0000 Subject: 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. --- usr.sbin/bgpd/rde.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'usr.sbin/bgpd/rde.c') 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 @@ -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; -- cgit v1.2.3