Correctly handle parse errors in aspath and prefixes. Also do input aspath

loop detection. OK henning@

3 files changed, 85 insertions, 32 deletions

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 76d5cd95f65..bc02e797c0d 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.80 2004/02/18 16:36:09 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.81 2004/02/18 23:18:16 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -386,6 +386,14 @@ rde_update_dispatch(struct imsg *imsg)
 			    NULL, 0);
 			return (-1);
 		}
+		if (prefixlen > 32) {
+			log_peer_warnx(&peer->conf, "bad prefix %s/%u",
+			    inet_ntoa(prefix.v4), prefixlen);
+			rde_update_err(peer, ERR_UPDATE, ERR_UPD_NETWORK,
+			    NULL, 0);
+			return (-1);
+		}
+
 		p += pos;
 		withdrawn_len -= pos;
 		rde_update_log("withdraw", peer, NULL, &prefix, prefixlen);
@@ -408,7 +416,7 @@ rde_update_dispatch(struct imsg *imsg)
 	attr_init(&attrs);
 	while (attrpath_len > 0) {
 		if ((pos = attr_parse(p, attrpath_len, &attrs,
-		    peer->conf.ebgp, conf->as)) < 0) {
+		    peer->conf.ebgp)) < 0) {
 			emsg = attr_error(p, attrpath_len, &attrs,
 			    &subtype, &size);
 			rde_update_err(peer, ERR_UPDATE, subtype, emsg, size);
@@ -428,6 +436,17 @@ rde_update_dispatch(struct imsg *imsg)
 		return (-1);
 	}
 
+	/* aspath needs to be loop free nota bene this is not a hard error */
+	if (peer->conf.ebgp && !aspath_loopfree(attrs.aspath, conf->as)) {
+		char *s;
+		aspath_asprint(&s, attrs.aspath->data, attrs.aspath->hdr.len);
+		log_peer_warnx(&peer->conf, "AS path loop: %s", s);
+		free(s);
+		aspath_destroy(attrs.aspath);
+		attr_optfree(&attrs);
+		return (0);
+	}
+	
 	while (nlri_len > 0) {
 		if ((pos = rde_update_get_prefix(p, nlri_len, &prefix,
 		    &prefixlen)) == -1) {
@@ -437,6 +456,14 @@ rde_update_dispatch(struct imsg *imsg)
 			attr_optfree(&attrs);
 			return (-1);
 		}
+		if (prefixlen > 32) {
+			log_peer_warnx(&peer->conf, "bad prefix %s/%u",
+			    inet_ntoa(prefix.v4), prefixlen);
+			rde_update_err(peer, ERR_UPDATE, ERR_UPD_NETWORK,
+			    NULL, 0);
+			return (-1);
+		}
+
 		p += pos;
 		nlri_len -= pos;
 		rde_update_log("update", peer, &attrs, &prefix, prefixlen);
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index bb2054fcfb9..b1428b28a1f 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.27 2004/02/18 16:36:09 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.28 2004/02/18 23:18:16 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -214,8 +214,7 @@ u_int16_t	 rde_local_as(void);
 
 /* rde_attr.c */
 void		 attr_init(struct attr_flags *);
-int		 attr_parse(u_char *, u_int16_t, struct attr_flags *, int,
-		     u_int16_t);
+int		 attr_parse(u_char *, u_int16_t, struct attr_flags *, int);
 u_char		*attr_error(u_char *, u_int16_t, struct attr_flags *,
 		     u_int8_t *, u_int16_t *);
 u_int8_t	 attr_missing(struct attr_flags *, int);
@@ -227,11 +226,10 @@ int		 attr_optadd(struct attr_flags *, u_int8_t, u_int8_t,
 		     u_char *, u_int16_t);
 void		 attr_optfree(struct attr_flags *);
 
-int		 aspath_verify(void *, u_int16_t, u_int16_t);
-#define		 AS_ERR_LOOP	-1
-#define		 AS_ERR_LEN	-2
-#define		 AS_ERR_TYPE	-3
-#define		 AS_ERR_BAD	-4
+int		 aspath_verify(void *, u_int16_t);
+#define		 AS_ERR_LEN	-1
+#define		 AS_ERR_TYPE	-2
+#define		 AS_ERR_BAD	-3
 struct aspath	*aspath_create(void *, u_int16_t);
 void		 aspath_destroy(struct aspath *);
 int		 aspath_write(void *, u_int16_t, struct aspath *, u_int16_t,
@@ -241,8 +239,10 @@ u_int16_t	 aspath_length(struct aspath *);
 u_int16_t	 aspath_count(struct aspath *);
 u_int16_t	 aspath_neighbour(struct aspath *);
 u_int32_t	 aspath_hash(struct aspath *);
+int		 aspath_loopfree(struct aspath *, u_int16_t);
 int		 aspath_compare(struct aspath *, struct aspath *);
 int		 aspath_snprint(char *, size_t, void *, u_int16_t);
+int		 aspath_asprint(char **, void *, u_int16_t);
 size_t		 aspath_strlen(void *, u_int16_t);
 int		 aspath_match(struct aspath *, enum as_spec, u_int16_t);
 
diff --git a/usr.sbin/bgpd/rde_attr.c b/usr.sbin/bgpd/rde_attr.c
index 5de4af4b4a1..3dcbdf11cd0 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.8 2004/02/18 16:36:09 claudio Exp $ */
+/*	$OpenBSD: rde_attr.c,v 1.9 2004/02/18 23:18:16 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -59,8 +59,7 @@ attr_init(struct attr_flags *a)
 }
 
 int
-attr_parse(u_char *p, u_int16_t len, struct attr_flags *a, int ebgp,
-    u_int16_t as)
+attr_parse(u_char *p, u_int16_t len, struct attr_flags *a, int ebgp)
 {
 	u_int32_t	 tmp32;
 	u_int16_t	 attr_len;
@@ -68,7 +67,6 @@ attr_parse(u_char *p, u_int16_t len, struct attr_flags *a, int ebgp,
 	u_int8_t	 flags;
 	u_int8_t	 type;
 	u_int8_t	 tmp8;
-	int		 r; /* XXX */
 
 	if (len < 3)
 		return (-1);
@@ -105,14 +103,8 @@ attr_parse(u_char *p, u_int16_t len, struct attr_flags *a, int ebgp,
 	case ATTR_ASPATH:
 		if (!CHECK_FLAGS(flags, ATTR_WELL_KNOWN))
 			return (-1);
-		if ((r = aspath_verify(p, attr_len, as)) != 0) {
-			/* XXX could also be a aspath loop but this
-			 * check should be moved to the filtering. */
-			/* XXX loop detection should be done afterwards
-			 * because this is not an error */
-			log_warnx("XXX aspath_verify failed: error %i", r);
+		if (aspath_verify(p, attr_len) != 0)
 			return (-1);
-		}
 		WFLAG(a->wflags, F_ATTR_ASPATH);
 		a->aspath = aspath_create(p, attr_len);
 		/* XXX enforce remote-as == left most AS if not disabled */
@@ -475,7 +467,6 @@ attr_optfree(struct attr_flags *attr)
 /* aspath specific functions */
 
 /* TODO
- * aspath loop detection (partially done I think),
  * aspath regexp search,
  * aspath to string converter
  */
@@ -502,12 +493,11 @@ aspath_extract(void *seg, int pos)
 }
 
 int
-aspath_verify(void *data, u_int16_t len, u_int16_t myAS)
+aspath_verify(void *data, u_int16_t len)
 {
 	u_int8_t	*seg = data;
-	int		 error = 0;
 	u_int16_t	 seg_size;
-	u_int8_t	 i, seg_len, seg_type;
+	u_int8_t	 seg_len, seg_type;
 
 	if (len & 1)
 		/* odd lenght aspath are invalid */
@@ -527,14 +517,8 @@ aspath_verify(void *data, u_int16_t len, u_int16_t myAS)
 		if (seg_size == 0)
 			/* empty aspath segment are not allowed */
 			return AS_ERR_BAD;
-
-		/* XXX not needed */
-		for (i = 0; i < seg_len; i++) {
-			if (myAS == aspath_extract(seg, i))
-				error = AS_ERR_LOOP;
-		}
 	}
-	return (error);	/* aspath is valid but probably not loop free */
+	return 0;	/* aspath is valid but probably not loop free */
 }
 
 struct aspath *
@@ -695,6 +679,33 @@ aspath_neighbour(struct aspath *aspath)
 	return aspath_extract(aspath->data, 0);
 }
 
+int
+aspath_loopfree(struct aspath *aspath, u_int16_t myAS)
+{
+	u_int8_t	*seg;
+	u_int16_t	 len, seg_size;
+	u_int8_t	 i, seg_len, seg_type;
+
+	if (len & 1)
+		/* odd lenght aspath are invalid */
+		return AS_ERR_BAD;
+
+	seg = aspath->data;
+	for (len = aspath->hdr.len; len > 0; len -= seg_size, seg += seg_size) {
+		seg_type = seg[0];
+		seg_len = seg[1];
+		ENSURE(seg_type == AS_SET || seg_type == AS_SEQUENCE);
+		seg_size = 2 + 2 * seg_len;
+
+		ENSURE(seg_size <= len);
+		for (i = 0; i < seg_len; i++) {
+			if (myAS == aspath_extract(seg, i))
+				return 0;
+		}
+	}
+	return 1;
+}
+
 #define AS_HASH_INITIAL 8271
 
 u_int32_t
@@ -794,6 +805,21 @@ aspath_snprint(char *buf, size_t size, void *data, u_int16_t len)
 #undef UPDATE
 }
 
+int
+aspath_asprint(char **ret, void *data, u_int16_t len)
+{
+	size_t	slen, plen;
+
+	slen = aspath_strlen(data, len) + 1;
+	*ret = malloc(slen);
+	if (*ret == NULL)
+		return (-1);
+	plen = aspath_snprint(*ret, slen, data, len);
+	ENSURE(plen < slen);
+
+	return (plen);
+}
+
 size_t
 aspath_strlen(void *data, u_int16_t len)
 {