[PATCH] Wrong calculation in as_path_getlen, and check_aspa improvement suggestion

Alexander Zubkov green at qrator.net
Mon Sep 1 09:31:39 CEST 2025


Hello,

Sorry, I wasn't much attentive reading the original message, it is all
there too. :)

On Sun, Aug 31, 2025 at 8:27 PM Evann DREUMONT <bird at evann.dev> wrote:

> Hello!
>
> Yes, I can confirm that, especially with the new RFC 9774 standard, see
> https://datatracker.ietf.org/doc/html/rfc9774#section-4.
> That something we address in the second patch.
>
> Originally, the segfault was handled by returning ASPA_INVALID on AS_PATH
> that contained an AS\_SET, but we discovered that this was not the real
> problem, but rather a bad allocation due to an incorrect calculation of the
> AS path length. We mitigated this issue with our first patch.
>
> Best,
> --
> Evann & Alarig
>
> On 8/31/25 8:02 PM, Alexander Zubkov via Bird-users wrote:
>
> Hi,
>
> AFAIK, ASPA RFC forbid AS sets and considers such announces invalid:
>
>
> https://www.ietf.org/archive/id/draft-ietf-sidrops-aspa-verification-22.html#section-6.2-3.3.1
>
> > If the AS_PATH has an AS_SET, then the procedure halts with the outcome
> "Invalid".
>
> Regards,
> Alexander
>
> On Sun, Aug 31, 2025, 18:11 Alarig Le Lay via Bird-users <
> bird-users at network.cz> wrote:
>
>> Hello,
>>
>> We (Evann and I) found a bug related to as_path_getlen() when used by
>> aspa_check(). When a route contains an AS_SET segment type, the length
>> returned by as_path_getlen() is incorrect. The function assumes that the
>> length of an AS_PATH_SET is a single AS (1), while in reality an
>> AS_PATH_SET is an unordered set of ASN (as described here
>> https://www.rfc-editor.org/rfc/rfc4271#section-9.2.2.1).
>>
>> See the following update:
>>         2025-08-31 15:35:15.134 <INFO> Checking prefix 76.165.0.0/16
>> (path 208627 29075 174 32440 {2055 10349 22985 23207 23294 23366 26002
>> 26303 26333 30564 54529 396992 401290}) IN from bgp_alarig_ipv4
>>
>> Using gdb we can inspect the path object:
>>         (gdb) p path->length
>>         $101 = 72
>>         (gdb) x /72xb path->data
>>         0x555555725e54:    0x02    0x04    0x00    0x03    0x2e    0xf3
>>   0x00    0x00
>>         0x555555725e5c:    0x71    0x93    0x00    0x00    0x00    0xae
>>   0x00    0x00
>>         0x555555725e64:    0x7e    0xb8    0x01    0x0d    0x00    0x00
>>   0x08    0x07
>>         0x555555725e6c:    0x00    0x00    0x28    0x6d    0x00    0x00
>>   0x59    0xc9
>>         0x555555725e74:    0x00    0x00    0x5a    0xa7    0x00    0x00
>>   0x5a    0xfe
>>         0x555555725e7c:    0x00    0x00    0x5b    0x46    0x00    0x00
>>   0x65    0x92
>>         0x555555725e84:    0x00    0x00    0x66    0xbf    0x00    0x00
>>   0x66    0xdd
>>         0x555555725e8c:    0x00    0x00    0x77    0x64    0x00    0x00
>>   0xd5    0x01
>>         0x555555725e94:    0x00    0x06    0x0e    0xc0    0x00    0x06
>>   0x1f    0x8a
>>
>> In this example, we have a route with an AS_PATH that contain:
>>     - an AS_PATH_SEQUENCE (0x02) with a length of 4 (0x04): (208627
>>       29075 174 32440);
>>     - an AS_PATH_SET (0x01) with a length of 13 (0x0d): {2055 10349
>>       22985 23207 23294 23366 26002 26303 26333 30564 54529 396992
>>       401290}.
>> The total length of this update is then 17, but if we dump the function
>> result, we can see that the actual computed length is 5 (4 + 1 for the
>> AS_PATH_SET).
>>         (gdb) p len
>>         $103 = 5
>>
>> This leads to a too small memory allocation, when normalizing the AS
>> Path in aspa_check():
>>         /* Normalize the AS Path: drop stuffings */
>>         u32 *asns = alloca(sizeof(u32) * len);
>> Causing a SEGFAULT during the as path walk. Since as_path_walk()
>> considers each element of the AS_PATH_SET as a step. In the while
>> (as_path_walk(path, &ppos, &asns[nsz])), the asns object should have a
>> size of 17 and not 5 resulting in overwriting memory and finally
>> triggering a SEGFAULT. (However we've seen this working when the AS_SET
>> is small, for example, it's working for the following route, but this is
>> mostly luck and could lead to weird behaviors):
>>         Checking prefix 104.141.0.0/16 (path 208627 29075 174 32440
>> {400943}) IN from bgp_alarig_ipv4
>>
>> Here is the gdb output showing this behaviour:
>>         2025-08-31 15:35:15.134 <TRACE> bgp_alarig_ipv4: Got UPDATE
>>         2025-08-31 15:35:15.134 <INFO> Checking prefix 76.165.0.0/16
>> (path 208627 29075 174 32440 {2055 10349 22985 23207 23294 23366 26002
>> 26303 26333 30564 54529 396992 401290}) IN from bgp_alarig_ipv4
>>
>>         Thread 1 "bird" received signal SIGSEGV, Segmentation fault.
>>         0x00005555555d68ac in as_path_walk (path=0x5555000066dd,
>> pos=0x7fffffffd15c,
>>             val=0x7fffffffd144) at nest/a-path.c:702
>>         702     const u8 *q = p + path->length;
>>         (gdb) p path->data
>>         $1 = 0x5555000066e1 <error: Cannot access memory at address
>> 0x5555000066e1>
>>
>> And here is a dump of asns just before the segfault :
>>         (gdb) p *asns at nsz+1
>>         $57 = {208627, 29075, 174, 32440, 2055, 10349, 22985, 23207,
>> 23294, 23366, 26002, 26303,
>>           26333}
>>
>> We propose to set the AS_PATH_SET length to the announced length in the
>> AS_PATH data instead of 1, see
>> 0001-NEST-correct-as_path-len-calculation.patch.
>>
>> Furthermore, as per
>>
>> https://datatracker.ietf.org/doc/html/rfc9774#name-updates-to-the-requirements
>> (BGP speakers MUST use the "treat-as-withdraw" error handling behavior
>> per [RFC7606] upon reception of BGP UPDATE messages containing AS_SETs
>> or AS_CONFED_SETs in the AS_PATH or AS4_PATH [RFC6793]) and even if
>>
>> https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-aspa-verification-22#name-as_path-verification
>> changes it to a SHOULD, another improvement we propose is to check for
>> AS_PATH_SET the same way it’s already done for AS_PATH_CONFED_SEQUENCE
>> and AS_PATH_CONFED_SET at the beginning of the aspa_check() (see
>> 0002-NEST-return-ASPA_INVALID-for-path-containing-AS_SET.patch). The
>> proposed patch is only for ASPA, not for ROV, in order to avoid dropping
>> routes for too much people, and the patch only drop a few amounts of
>> routes (including a few routes dropped for invalid ROV) :
>>     Routes:         1031692 imported, 212 filtered, 0 exported, 1031692
>> preferred
>>
>> Don’t hesitate to discuss the patch if needed,
>> --
>> Alarig and Evann
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20250901/368c09b3/attachment.htm>


More information about the Bird-users mailing list