Accept but ignore (treat as withdraw) updates with AS_CONFED_* path

segments. Bgpd does not support confederations but it is too extreme
to close a session because a path contained such elements.
OK henning@, sthen@

4 files changed, 73 insertions, 23 deletions

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 7a4ae4503fe..aba3bc2df5b 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.300 2010/11/10 15:14:36 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.301 2010/11/18 12:18:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -1337,6 +1337,7 @@ rde_attr_parse(u_char *p, u_int16_t len, struct rde_peer *peer,
 	struct bgpd_addr nexthop;
 	u_char		*op = p, *npath;
 	u_int32_t	 tmp32;
+	int		 err;
 	u_int16_t	 attr_len, nlen;
 	u_int16_t	 plen = 0;
 	u_int8_t	 flags;
@@ -1396,7 +1397,17 @@ bad_flags:
 	case ATTR_ASPATH:
 		if (!CHECK_FLAGS(flags, ATTR_WELL_KNOWN, 0))
 			goto bad_flags;
-		if (aspath_verify(p, attr_len, rde_as4byte(peer)) != 0) {
+		err = aspath_verify(p, attr_len, rde_as4byte(peer));
+		if (err == AS_ERR_SOFT) {
+			/*
+			 * soft errors like unexpected segment types are
+			 * not considered fatal and the path is just
+			 * marked invalid.
+			 */
+			a->flags |= F_ATTR_PARSE_ERR;
+			log_peer_warnx(&peer->conf, "bad ASPATH, "
+			    "path invalidated and prefix withdrawn");
+		} else if (err != 0) {
 			rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH,
 			    NULL, 0);
 			return (-1);
@@ -1603,7 +1614,7 @@ bad_flags:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
 		    ATTR_PARTIAL))
 			goto bad_flags;
-		if (aspath_verify(p, attr_len, 1) != 0) {
+		if ((err = aspath_verify(p, attr_len, 1)) != 0) {
 			/*
 			 * XXX RFC does not specify how to handle errors.
 			 * XXX Instead of dropping the session because of a
@@ -1613,8 +1624,9 @@ bad_flags:
 			 * XXX or redistribution.
 			 * XXX We follow draft-ietf-idr-optional-transitive
 			 * XXX by looking at the partial bit.
+			 * XXX Consider soft errors similar to a partial attr.
 			 */
-			if (flags & ATTR_PARTIAL) {
+			if (flags & ATTR_PARTIAL || err == AS_ERR_SOFT) {
 				a->flags |= F_ATTR_PARSE_ERR;
 				log_peer_warnx(&peer->conf, "bad AS4_PATH, "
 				    "path invalidated and prefix withdrawn");
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index 3c0db096f4e..d2a594b9696 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.137 2010/05/26 13:56:07 nicm Exp $ */
+/*	$OpenBSD: rde.h,v 1.138 2010/11/18 12:18:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -79,6 +79,8 @@ struct rde_peer {
 
 #define AS_SET			1
 #define AS_SEQUENCE		2
+#define AS_CONFED_SEQUENCE	3
+#define AS_CONFED_SET		4
 #define ASPATH_HEADER_SIZE	(sizeof(struct aspath) - sizeof(u_char))
 
 LIST_HEAD(aspath_list, aspath);
@@ -331,6 +333,7 @@ int		 aspath_verify(void *, u_int16_t, int);
 #define		 AS_ERR_LEN	-1
 #define		 AS_ERR_TYPE	-2
 #define		 AS_ERR_BAD	-3
+#define		 AS_ERR_SOFT	-4
 void		 aspath_init(u_int32_t);
 void		 aspath_shutdown(void);
 struct aspath	*aspath_get(void *, u_int16_t);
diff --git a/usr.sbin/bgpd/rde_attr.c b/usr.sbin/bgpd/rde_attr.c
index b30cba6be37..753d1a2e85c 100644
--- a/usr.sbin/bgpd/rde_attr.c
+++ b/usr.sbin/bgpd/rde_attr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_attr.c,v 1.86 2010/10/22 12:37:32 claudio Exp $ */
+/*	$OpenBSD: rde_attr.c,v 1.87 2010/11/18 12:18:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -408,6 +408,7 @@ aspath_verify(void *data, u_int16_t len, int as4byte)
 	u_int8_t	*seg = data;
 	u_int16_t	 seg_size, as_size = 2;
 	u_int8_t	 seg_len, seg_type;
+	int		 err = 0;
 
 	if (len & 1)
 		/* odd length aspath are invalid */
@@ -422,7 +423,15 @@ aspath_verify(void *data, u_int16_t len, int as4byte)
 		seg_type = seg[0];
 		seg_len = seg[1];
 
-		if (seg_type != AS_SET && seg_type != AS_SEQUENCE)
+		/*
+		 * BGP confederations should not show up but consider them
+		 * as a soft error which invalidates the path but keeps the
+		 * bgp session running.
+		 */
+		if (seg_type == AS_CONFED_SEQUENCE || seg_type == AS_CONFED_SET)
+			err = AS_ERR_SOFT;
+		if (seg_type != AS_SET && seg_type != AS_SEQUENCE &&
+		    seg_type != AS_CONFED_SEQUENCE && seg_type != AS_CONFED_SET)
 			return (AS_ERR_TYPE);
 
 		seg_size = 2 + as_size * seg_len;
@@ -434,7 +443,7 @@ aspath_verify(void *data, u_int16_t len, int as4byte)
 			/* empty aspath segments are not allowed */
 			return (AS_ERR_BAD);
 	}
-	return (0);	/* aspath is valid but probably not loop free */
+	return (err);	/* aspath is valid but probably not loop free */
 }
 
 void
diff --git a/usr.sbin/bgpd/util.c b/usr.sbin/bgpd/util.c
index b4138711824..412bf3b3345 100644
--- a/usr.sbin/bgpd/util.c
+++ b/usr.sbin/bgpd/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.12 2010/10/24 17:19:35 deraadt Exp $ */
+/*	$OpenBSD: util.c,v 1.13 2010/11/18 12:18:31 claudio Exp $ */
 
 /*
  * Copyright (c) 2006 Claudio Jeker <claudio@openbsd.org>
@@ -28,6 +28,8 @@
 #include "bgpd.h"
 #include "rde.h"
 
+const char	*aspath_delim(u_int8_t, int);
+
 const char *
 log_addr(const struct bgpd_addr *addr)
 {
@@ -161,6 +163,38 @@ log_ext_subtype(u_int8_t subtype)
 	}
 }
 
+const char *
+aspath_delim(u_int8_t seg_type, int closing)
+{
+	static char db[8];
+
+	switch (seg_type) {
+	case AS_SET:
+		if (!closing)
+			return ("{ ");
+		else
+			return (" }");
+	case AS_SEQUENCE:
+		return ("");
+	case AS_CONFED_SEQUENCE:
+		if (!closing)
+			return ("( ");
+		else
+			return (" )");
+	case AS_CONFED_SET:
+		if (!closing)
+			return ("[ ");
+		else
+			return (" ]");
+	default:
+		if (!closing)
+			snprintf(db, sizeof(db), "!%u ", seg_type);
+		else
+			snprintf(db, sizeof(db), " !%u", seg_type);
+		return (db);
+	}
+}
+
 int
 aspath_snprint(char *buf, size_t size, void *data, u_int16_t len)
 {
@@ -189,16 +223,10 @@ aspath_snprint(char *buf, size_t size, void *data, u_int16_t len)
 		seg_len = seg[1];
 		seg_size = 2 + sizeof(u_int32_t) * seg_len;
 
-		if (seg_type == AS_SET) {
-			if (total_size != 0)
-				r = snprintf(buf, size, " { ");
-			else
-				r = snprintf(buf, size, "{ ");
-			UPDATE();
-		} else if (total_size != 0) {
-			r = snprintf(buf, size, " ");
-			UPDATE();
-		}
+		r = snprintf(buf, size, "%s%s",
+		    total_size != 0 ? " " : "",
+		    aspath_delim(seg_type, 0));
+		UPDATE();
 
 		for (i = 0; i < seg_len; i++) {
 			r = snprintf(buf, size, "%s",
@@ -209,10 +237,8 @@ aspath_snprint(char *buf, size_t size, void *data, u_int16_t len)
 				UPDATE();
 			}
 		}
-		if (seg_type == AS_SET) {
-			r = snprintf(buf, size, " }");
-			UPDATE();
-		}
+		r = snprintf(buf, size, "%s", aspath_delim(seg_type, 1));
+		UPDATE();
 	}
 	/* ensure that we have a valid C-string especially for empty as path */
 	if (size > 0)