On Mon, Aug 06, 2012 at 10:34:22PM +0200, Laurent CARON wrote:
> Hi,
>
> I'm hit by a rather nasty OpenBGPd 'bug' causing sessions to flap
> (basically go down/up/...).
>
> One of the prefixes is: 81.169.0.0/17
>
> Description of bug
> https://puck.nether.net/pipermail/juniper-nsp/2012-July/023774.html
>
> Is the included fix
> (((s & 0xf0) & ~(ATTR_EXTLEN | (m))) == (t))
> instead of just
> (((s) & ~(ATTR_EXTLEN | (m))) == (t))
>
> sufficient ?
>
I would prefer something like this. Since then we ensure that we do not
forward crap (as in we regard the RFC and send nothing with reserved bits
set). AFAIK there is nothing out there that started to use the reserved
bits so I'm curious how that happend again.
Only compile tested for now.
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.316
diff -u -p -r1.316 rde.c
--- rde.c 27 May 2012 18:52:07 -0000 1.316
+++ rde.c 6 Aug 2012 20:57:27 -0000
@@ -1382,7 +1382,7 @@ rde_update_withdraw(struct rde_peer *pee
} while (0)
#define CHECK_FLAGS(s, t, m) \
- (((s) & ~(ATTR_EXTLEN | (m))) == (t))
+ (((s) & ~(ATTR_DEFMASK | (m))) == (t))
int
rde_attr_parse(u_char *p, u_int16_t len, struct rde_peer *peer,
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.142
diff -u -p -r1.142 rde.h
--- rde.h 21 Sep 2011 08:59:01 -0000 1.142
+++ rde.h 6 Aug 2012 21:09:02 -0000
@@ -118,6 +118,9 @@ enum attrtypes {
#define ATTR_PARTIAL 0x20
#define ATTR_TRANSITIVE 0x40
#define ATTR_OPTIONAL 0x80
+#define ATTR_RESERVED 0x0f
+/* by default mask the reserved bits and the ext len bit */
+#define ATTR_DEFMASK (ATTR_RESERVED | ATTR_EXTLEN)
/* default attribute flags for well known attributes */
#define ATTR_WELL_KNOWN ATTR_TRANSITIVE
Index: rde_attr.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.90
diff -u -p -r1.90 rde_attr.c
--- rde_attr.c 12 Apr 2012 17:27:20 -0000 1.90
+++ rde_attr.c 6 Aug 2012 21:14:39 -0000
@@ -37,12 +37,12 @@ attr_write(void *p, u_int16_t p_len, u_i
u_char *b = p;
u_int16_t tmp, tot_len = 2; /* attribute header (without len) */
+ flags &= ~ATTR_DEFMASK;
if (data_len > 255) {
tot_len += 2 + data_len;
flags |= ATTR_EXTLEN;
} else {
tot_len += 1 + data_len;
- flags &= ~ATTR_EXTLEN;
}
if (tot_len > p_len)
@@ -69,12 +69,12 @@ attr_writebuf(struct ibuf *buf, u_int8_t
{
u_char hdr[4];
+ flags &= ~ATTR_DEFMASK;
if (data_len > 255) {
flags |= ATTR_EXTLEN;
hdr[2] = (data_len >> 8) & 0xff;
hdr[3] = data_len & 0xff;
} else {
- flags &= ~ATTR_EXTLEN;
hdr[2] = data_len & 0xff;
}
@@ -322,6 +322,7 @@ attr_alloc(u_int8_t flags, u_int8_t type
fatal("attr_optadd");
rdemem.attr_cnt++;
+ flags &= ~ATTR_DEFMASK; /* normalize mask */
a->flags = flags;
a->hash = hash32_buf(&flags, sizeof(flags), HASHINIT);
a->type = type;
@@ -351,6 +352,7 @@ attr_lookup(u_int8_t flags, u_int8_t typ
struct attr *a;
u_int32_t hash;
+ flags &= ~ATTR_DEFMASK; /* normalize mask */
hash = hash32_buf(&flags, sizeof(flags), HASHINIT);
hash = hash32_buf(&type, sizeof(type), hash);
hash = hash32_buf(&len, sizeof(len), hash);
