OpenBGPd / Juniper 'bug' / BGP session flapping

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

OpenBGPd / Juniper 'bug' / BGP session flapping

Laurent Caron (Mobile)
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 ?

Thanks

Reply | Threaded
Open this post in threaded view
|

Re: OpenBGPd / Juniper 'bug' / BGP session flapping

Claudio Jeker
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);

Reply | Threaded
Open this post in threaded view
|

Re: OpenBGPd / Juniper 'bug' / BGP session flapping

Laurent Caron (Mobile)
Claudio Jeker <[hidden email]> a écrit :

>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);

Hi,

Thanks Claudio.

Gonna try the patch u did provide.

Since i needed a quick fix i did apply the patch I provided which allowed my bgp sessions to come back up.

Cheers

Laurent

Reply | Threaded
Open this post in threaded view
|

Re: OpenBGPd / Juniper 'bug' / BGP session flapping

Laurent Caron (Mobile)
In reply to this post by Claudio Jeker
On Mon, Aug 06, 2012 at 11:15:13PM +0200, Claudio Jeker wrote:
> Only compile tested for now.

Hi Claudio,

I did compile/install a patched version of OpenBGPd from the patch you
provided. It seems stable so far. Will deploy it on 3 other boxes later
on today.

Cheers,

Laurent

Reply | Threaded
Open this post in threaded view
|

Re: OpenBGPd / Juniper 'bug' / BGP session flapping

Laurent Caron (Mobile)
In reply to this post by Claudio Jeker
On Mon, Aug 06, 2012 at 11:15:13PM +0200, Claudio Jeker wrote:
> 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.

Hi,

I confirm the bgpd daemons are functional and stable so far.

Cheers,

Laurent

Reply | Threaded
Open this post in threaded view
|

Re: OpenBGPd / Juniper 'bug' / BGP session flapping

James Shupe-4
In reply to this post by Claudio Jeker
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/6/2012 4:15 PM, Claudio Jeker wrote:

> 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.
>

I ran across this today after AboveNET upgraded some routers (I would
have appreciated a maintenance notice...)

I applied Claudio's patch and the sessions came back up and have been
stable for the last half hour. I'll check back in if there are any issues.

We have both IPv4 and IPv6 sessions with them, and the IPv6 sessions
were unaffected (for what it's worth.) This patch is running on two of
our routers.

Thank you,
- --
James Shupe
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBAgAGBQJQtwhLAAoJECPibMsISQ9adq0QANQIPOXa7yqyDhRs4poH2Tis
AlPZBhRTPHtn54rCVKRMcqGJk/xy0bGHSiwgsZMXj29lxrkFPKG312SXT9VgSMnC
XqKfV0c9NDA9NDD57K7z0bFUvmO0MKr6S4v5/jZTDddikpDjcuGzFTdLpbE+8DfN
4VAXEUu/Ug8h6ZuR9TNYSkup78dQP9W7han+cBsW5PNqa40CM3T944D/QiZiTuP2
kpmEWPyALWzQMldPXaVTLoSyaI3ijxu6tC9iEXMKtQ/IEuF5z/xBHtwj7Vkmc/La
lkL5muRv862eSONdVPvCf4atbUivSTvV3ZjYyOCldzQiVQlZPUf9XdkfAx8FxIrR
ycMMDMCJC0IYtGdjnkJtEP4fgvjGY4/Uxzw2PaYRY6QxWJ09v2mLOfEeA70uZNFy
L2+cBouR3l/8fMPfRwTdqR65JEfkke5TRwtsBi6wWsMla7gK3/2Z4vHLp0LdD5Pu
sIWirQqoE9tCiDzFLyn49Xpfk+M42kJu3cXiDGvdDep3taE/zSHBbCiimgVMPxK7
9eO6o14W9yZxL0C/NTV2f7z1k3wJCG4tvcGznuw5M5K0LdpW89Wy7uBQ1KZstU3p
PlnqVBhBbpcrO+/rOSPiV/AuGMJPfKNnrJSF6Bncdu4dA2i3xWE3taa9JQ7A3JqA
0CojuAbNFQml66wsTJv4
=/Xzi
-----END PGP SIGNATURE-----

Reply | Threaded
Open this post in threaded view
|

Re: OpenBGPd / Juniper 'bug' / BGP session flapping

Laurent Caron (Mobile)
On 29/11/2012 08:01, James Shupe wrote:
> I ran across this today after AboveNET upgraded some routers (I would
> have appreciated a maintenance notice...)
>
> I applied Claudio's patch and the sessions came back up and have been
> stable for the last half hour. I'll check back in if there are any issues.
>
> We have both IPv4 and IPv6 sessions with them, and the IPv6 sessions
> were unaffected (for what it's worth.) This patch is running on two of
> our routers.

I also applied this patch on OpenBSD 5.2's release of OpenBGPd without
any side effect.