summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOndrej Zajicek <santiago@crfreenet.org>2009-06-23 10:50:57 +0200
committerOndrej Zajicek <santiago@crfreenet.org>2009-06-23 10:50:57 +0200
commit29c430f8569b90e1962d92a26f96fad7e72d27ec (patch)
tree4493428721ec3d7c40d7d50f334288791779f62f
parent4323099da9e6e8d53072595009731da9b39e9f19 (diff)
downloadbird-29c430f8569b90e1962d92a26f96fad7e72d27ec.tar
bird-29c430f8569b90e1962d92a26f96fad7e72d27ec.zip
Changes handling of AS_PATH_CONFED_* segments in AS_PATH.
Although standard says that if we receive AS_PATH_CONFED_* (and we are not a part of a confederation) segment, we should drop session, nobody does that and it is unwise to do that. Now we drop session just in case that peer ASN is in AS_PATH_CONFED_* segment (to detect peer that considers BIRD as a part of its confederation).
-rw-r--r--proto/bgp/attrs.c187
1 files changed, 103 insertions, 84 deletions
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 8a849e7..e50f6a9 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -55,25 +55,97 @@ bgp_format_origin(eattr *a, byte *buf, int buflen)
}
static int
-bgp_check_path(byte *a, int len, int bs, int errcode)
+path_segment_contains(byte *p, int bs, u32 asn)
{
- while (len)
+ int i;
+ int len = p[1];
+ p += 2;
+
+ for(i=0; i<len; i++)
{
- DBG("Path segment %02x %02x\n", a[0], a[1]);
- if (len < 2 ||
- (a[0] != AS_PATH_SET && a[0] != AS_PATH_SEQUENCE) ||
- bs * a[1] + 2 > len)
- return errcode;
- len -= bs * a[1] + 2;
- a += bs * a[1] + 2;
+ u32 asn2 = (bs == 4) ? get_u32(p) : get_u16(p);
+ if (asn2 == asn)
+ return 1;
+ p += bs;
}
+
return 0;
}
+/* Validates path attribute, removes AS_CONFED_* segments, and also returns path length */
static int
-bgp_check_as_path(struct bgp_proto *p, byte *a, int len)
+validate_path(struct bgp_proto *p, int as_path, int bs, byte *idata, unsigned int *ilength)
+{
+ int res = 0;
+ u8 *a, *dst;
+ int len, plen, copy;
+
+ dst = a = idata;
+ len = *ilength;
+
+ while (len)
+ {
+ if (len < 2)
+ return -1;
+
+ plen = 2 + bs * a[1];
+ if (len < plen)
+ return -1;
+
+ switch (a[0])
+ {
+ case AS_PATH_SET:
+ copy = 1;
+ res++;
+ break;
+
+ case AS_PATH_SEQUENCE:
+ copy = 1;
+ res += a[1];
+ break;
+
+ case AS_PATH_CONFED_SEQUENCE:
+ case AS_PATH_CONFED_SET:
+ if (as_path && path_segment_contains(a, bs, p->remote_as))
+ {
+ log(L_WARN "%s: AS_CONFED_* segment with peer ASN found, misconfigured confederation?", p->p.name);
+ return -1;
+ }
+
+ log(L_WARN "%s: %s_PATH attribute contains AS_CONFED_* segment, skipping segment",
+ p->p.name, as_path ? "AS" : "AS4");
+ copy = 0;
+ break;
+
+ default:
+ return -1;
+ }
+
+ if (copy)
+ {
+ if (dst != a)
+ memmove(dst, a, plen);
+ dst += plen;
+ }
+
+ len -= plen;
+ a += plen;
+ }
+
+ *ilength = dst - idata;
+ return res;
+}
+
+static inline int
+validate_as_path(struct bgp_proto *p, byte *a, int *len)
{
- return bgp_check_path(a, len, p->as4_session ? 4 : 2, 11);
+ return validate_path(p, 1, p->as4_session ? 4 : 2, a, len);
+}
+
+static inline int
+validate_as4_path(struct bgp_proto *p, struct adata *path)
+{
+ return validate_path(p, 0, 4, path->data, &path->length);
}
static int
@@ -160,7 +232,7 @@ static struct attr_desc bgp_attr_table[] = {
{ "origin", 1, BAF_TRANSITIVE, EAF_TYPE_INT, 1, /* BA_ORIGIN */
bgp_check_origin, bgp_format_origin },
{ "as_path", -1, BAF_TRANSITIVE, EAF_TYPE_AS_PATH, 1, /* BA_AS_PATH */
- bgp_check_as_path, NULL },
+ NULL, NULL }, /* is checked by validate_as_path() as a special case */
{ "next_hop", 4, BAF_TRANSITIVE, EAF_TYPE_IP_ADDRESS, 1, /* BA_NEXT_HOP */
bgp_check_next_hop, NULL },
{ "med", 4, BAF_OPTIONAL, EAF_TYPE_INT, 1, /* BA_MULTI_EXIT_DISC */
@@ -1061,73 +1133,6 @@ as4_aggregator_valid(struct adata *aggr)
return 0;
}
-static int
-as4_path_sanitize_and_get_length(struct adata *path)
-{
- int res = 0;
- u8 *p, *dst;
- int len, plen, copy;
-
- dst = p = path->data;
- len = path->length;
-
- while (len)
- {
- if (len <= 2) /* We do not allow empty segments */
- goto inconsistent_path;
-
- switch (p[0])
- {
- case AS_PATH_SET:
- plen = 2 + 4 * p[1];
- copy = 1;
- res++;
- break;
-
- case AS_PATH_SEQUENCE:
- plen = 2 + 4 * p[1];
- copy = 1;
- res += p[1];
- break;
-
- case AS_PATH_CONFED_SEQUENCE:
- case AS_PATH_CONFED_SET:
- log(L_WARN "BGP: AS4_PATH attribute contains AS_CONFED_* segment, skipping segment");
- plen = 2 + 4 * p[1];
- copy = 0;
- break;
-
- default:
- goto unknown_segment;
- }
-
- if (len < plen)
- goto inconsistent_path;
-
- if (copy)
- {
- if (dst != p)
- memmove(dst, p, plen);
- dst += plen;
- }
-
- len -= plen;
- p += plen;
- }
-
- path->length = dst - path->data;
- return res;
-
- inconsistent_path:
- log(L_WARN "BGP: AS4_PATH attribute is inconsistent, skipping attribute");
- return -1;
-
- unknown_segment:
- log(L_WARN "BGP: AS4_PATH attribute contains unknown segment, skipping attribute");
- return -1;
-}
-
-
/* Reconstruct 4B AS_PATH and AGGREGATOR according to RFC 4893 4.2.3 */
static void
@@ -1141,7 +1146,7 @@ bgp_reconstruct_4b_atts(struct bgp_proto *p, rta *a, struct linpool *pool)
if (a4 && !as4_aggregator_valid(a4->u.ptr))
{
- log(L_WARN "BGP: AS4_AGGREGATOR attribute is invalid, skipping attribute");
+ log(L_WARN "%s: AS4_AGGREGATOR attribute is invalid, skipping attribute", p->p.name);
a4 = NULL;
a4_removed = 1;
}
@@ -1177,15 +1182,18 @@ bgp_reconstruct_4b_atts(struct bgp_proto *p, rta *a, struct linpool *pool)
a2->u.ptr = bgp_aggregator_convert_to_new(a2->u.ptr, pool);
if ((a2_as == AS_TRANS) && !a4_removed)
- log(L_WARN "BGP: AGGREGATOR attribute contain AS_TRANS, but AS4_AGGREGATOR is missing");
+ log(L_WARN "%s: AGGREGATOR attribute contain AS_TRANS, but AS4_AGGREGATOR is missing", p->p.name);
}
}
else
if (a4)
- log(L_WARN "BGP: AS4_AGGREGATOR attribute received, but AGGREGATOR attribute is missing");
+ log(L_WARN "%s: AS4_AGGREGATOR attribute received, but AGGREGATOR attribute is missing", p->p.name);
int p2_len = as_path_getlen(p2->u.ptr);
- int p4_len = p4 ? as4_path_sanitize_and_get_length(p4->u.ptr) : -1;
+ int p4_len = p4 ? validate_as4_path(p, p4->u.ptr) : -1;
+
+ if (p4 && (p4_len < 0))
+ log(L_WARN "%s: AS4_PATH attribute is malformed, skipping attribute", p->p.name);
if ((p4_len <= 0) || (p2_len < p4_len))
p2->u.ptr = bgp_merge_as_paths(p2->u.ptr, NULL, AS_PATH_MAXLEN, pool);
@@ -1200,7 +1208,7 @@ bgp_remove_as4_attrs(struct bgp_proto *p, rta *a)
unsigned id2 = EA_CODE(EAP_BGP, BA_AS4_AGGREGATOR);
ea_list **el = &(a->eattrs);
- /* We know that ea_lists constructed in bgp_decode_attrs have one attribute per ea_list struct */
+ /* We know that ea_lists constructed in bgp_decode attrs have one attribute per ea_list struct */
while (*el != NULL)
{
unsigned fid = (*el)->attrs[0].id;
@@ -1302,6 +1310,12 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct lin
if (errcode < 0)
continue;
}
+ else if (code == BA_AS_PATH)
+ {
+ /* Special case as it might also trim the attribute */
+ if (validate_as_path(bgp, z, &l) < 0)
+ { errcode = 11; goto err; }
+ }
type = desc->type;
}
else /* Unknown attribute */
@@ -1310,6 +1324,11 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct lin
{ errcode = 2; goto err; }
type = EAF_TYPE_OPAQUE;
}
+
+ // Only OPTIONAL and TRANSITIVE attributes may have non-zero PARTIAL flag
+ // if (!((flags & BAF_OPTIONAL) && (flags & BAF_TRANSITIVE)) && (flags & BAF_PARTIAL))
+ // { errcode = 4; goto err; }
+
seen[code/8] |= (1 << (code%8));
ea = lp_alloc(pool, sizeof(ea_list) + sizeof(eattr));
ea->next = a->eattrs;