On Fri, Mar 30, 2018 at 09:50:01PM +0200, Tom Beard wrote:
> The commit below to bgpd/rde_attr.c introduced a check for AS0 in the
> AS-PATH received in an UPDATE message:
>
> >revision 1.98
> >date: 2017/05/26 20:55:30; author: phessler; state: Exp; lines: +8 -2;
> >commitid: p82SYNnm0pVdbRCn;
> >AS 0 is special and should be considered an error.
> >
> >Drop the session if it shows during OPEN or CAPA, or mark as invalid if it
> >is part of an Update.
> >
> >required by RFC 7607
>
> This works fine if the peer has 4-byte AS capability but fails if not.
>
> aspath_verify() happens before aspath_inflate() fixes up the 2-byte AS path.
> As a result when iterating over the path in aspath_verify(),
> aspath_extract() is returning incorrect AS number values, sometimes this
> will return a 0 causing aspath_verify() to drop the update with message "bad
> ASPATH, path invalidated and prefix withdrawn".
>
> The patch below flips the order so that aspath_inflate() for a peer without
> 4-byte capability happens before aspath_verify(). This also means there is
> no need to pass the 4-byte capability indicator to aspath_verify() as
> it???ll only ever be looking at a path containing 4 byte AS numbers.
>
> Appreciate the work on bgpd. Hope this is of some use.
>
Thanks for this diff. Yes, you right there is a problem with calling
aspath_extract() in aspath_verify(). Now calling aspath_inflate() before
aspath_verify() should not be done since aspath_verify() ensures that the
aspath is valid. So calling aspath_inflate() before is not the right
approach. I commited the following alternative instead.
--
:wq Claudio
Index: rde_attr.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.100
diff -u -p -r1.100 rde_attr.c
--- rde_attr.c 31 May 2017 10:44:00 -0000 1.100
+++ rde_attr.c 1 Apr 2018 09:46:38 -0000
@@ -426,7 +426,7 @@ aspath_verify(void *data, u_int16_t len,
u_int8_t *seg = data;
u_int16_t seg_size, as_size = 2;
u_int8_t seg_len, seg_type;
- int i, error = 0;
+ int error = 0;
if (len & 1)
/* odd length aspath are invalid */
@@ -436,6 +436,9 @@ aspath_verify(void *data, u_int16_t len,
as_size = 4;
for (; len > 0; len -= seg_size, seg += seg_size) {
+ const u_char *ptr;
+ int pos;
+
if (len < 2) /* header length check */
return (AS_ERR_BAD);
seg_type = seg[0];
@@ -461,9 +464,14 @@ aspath_verify(void *data, u_int16_t len,
/* empty aspath segments are not allowed */
return (AS_ERR_BAD);
- /* RFC 7607 - AS 0 is considered malformed */
- for (i = 0; i < seg_len; i++) {
- if (aspath_extract(seg, i) == 0)
+ /* RFC 7607 - AS 0 is considered malformed */
+ ptr = seg + 2;
+ for (pos = 0; pos < seg_len; pos++) {
+ u_int32_t as = 0;
+
+ ptr += as_size;
+ memcpy(&as, ptr, as_size);
+ if (as == 0)
return (AS_ERR_SOFT);
}
}
