[quagga-dev 16660] [PATCH] bgpd: fix length calculation of multi-segment AS_PATH UPDATE messages

Andreas Jaggi aj at open.ch
Fri Sep 8 03:53:37 BST 2017


Hi

There is a bug in the length calculation of mutli-segment AS_PATH UPDATE messages, which can make bgpd send out malformed BGP UPDATE packets and in turn causes that BGP sessions are reset.

This thread on quagga-users shows how the problem manifests: https://lists.quagga.net/pipermail/quagga-users/2017-September/014830.html

In our case we had an UPDATE message having an AS_PATH with 567 entries which was split into three segments (255+255+57).
This results in an AS_PATH attribute of the following length: 2274 bytes
(eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 57*4 segment length)
The AS_PATH attribute was written properly into the message, but the length written in the path attribute header by bgpd was 3294 bytes instead of 2274 (thus resulting in an invalid/malformed BGP UPDATE message).

Turns out that the aspath_put function wrongly double-accumulates the written bytes and then ends up with these 3294 bytes instead of 2274.
(eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 510*4 segment length + 2 bytes segment header + 57*4 segment length).


The following patch (against the master branch of Quagga) fixes the length calculation in aspath_put:

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index b7af5e88..d813bfba 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -903,7 +903,7 @@ aspath_put (struct stream *s, struct aspath *as, int use32bit )
               assegment_header_put (s, seg->type, AS_SEGMENT_MAX);
               assegment_data_put (s, seg->as, AS_SEGMENT_MAX, use32bit);
               written += AS_SEGMENT_MAX;
-              bytes += ASSEGMENT_SIZE (written, use32bit);
+              bytes += ASSEGMENT_SIZE (AS_SEGMENT_MAX, use32bit);
             }
           
           /* write the final segment, probably is also the first */


It looks like this bug has been slumbering for 10 years since its introduction in 2007 when it was added with the AS4 support changes (commit 0b2aa3a0).

Which leads us to the following affected versions:
Quagga: 0.99.10 - 0.99.24.1, 1.0.*, 1.1.0, 1.1.1, 1.2.0, 1.2.1
FRR: 2.0, 3.0-rc1


Cheers
Andreas


-- 
andreas jaggi
lead engineer network services

open systems ag
raeffelstrasse 29
ch-8045 zurich
t: +41 58 100 10 10
f: +41 58 100 10 11
aj at open.ch

http://www.open.ch


More information about the Quagga-dev mailing list