BGP (v4) Open typed packet failing to parse. #118

kbandla · 2015-06-04T01:48:05Z

From Mawr...@gmail.com on June 22, 2012 11:38:15

What steps will reproduce the problem? 1. Packet received was:

\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00'\x01\x04\xfd\xe8\x00\xb4\xc0\xa8\x02\x01\n\x02\x08\x01\x04\x00\x01\x00\x01\x02\x00

call dpkt.bgp.BGP(buf)

Raises exception:

Traceback (most recent call last):
File "bgp_dump.py", line 11, in
p = bgp.BGP(data)
File "/usr/local/lib/python2.7/site-packages/dpkt/dpkt.py", line 75, in init
self.unpack(args[0])
File "/usr/local/lib/python2.7/site-packages/dpkt/bgp.py", line 130, in unpack
self.data = self.open = self.Open(self.data)
File "/usr/local/lib/python2.7/site-packages/dpkt/dpkt.py", line 75, in init
self.unpack(args[0])
File "/usr/local/lib/python2.7/site-packages/dpkt/bgp.py", line 157, in unpack
param = self.Parameter(self.data)
File "/usr/local/lib/python2.7/site-packages/dpkt/dpkt.py", line 75, in init
self.unpack(args[0])
File "/usr/local/lib/python2.7/site-packages/dpkt/bgp.py", line 188, in unpack
self.data = self.capability = self.Capability(self.data)
File "/usr/local/lib/python2.7/site-packages/dpkt/dpkt.py", line 78, in init
raise NeedData
dpkt.dpkt.NeedData What is the expected output? What do you see instead? The expected output is that the packet is parsed, with the ROUTE REFRESH capability enabled. What version of the product are you using? On what operating system? # $Id: bgp.py 52 2008-08-25 22:22:34Z jon.oberheide $

On Ubuntu, but I don't think it's a platform dependent issue. Please provide any additional information below. I believe the cause to be is if the packet contains a zero-length capabilities (route refresh), the resulting data is 0, but it tries to unpack it anyways, causing the exception.

According to the RFC ( http://tools.ietf.org/html/rfc2918 ) the zero-length capabilities is expected.

A solution I came up with was to check if self.data is length 0, and if so, return before calling unpack. Added this to line 183 in bgp.py:

            if len(self.data) == 0:
                return

Original issue: http://code.google.com/p/dpkt/issues/detail?id=91

The text was updated successfully, but these errors were encountered:

amgadhanafy · 2020-07-08T17:58:37Z

I have the same error
@kbandla did you find any solution for that

amgadhanafy · 2020-07-09T21:54:31Z

data from https://packetlife.net/captures/protocol/bgp/
file bgplu.cap
https://packetlife.net/media/captures/bgplu.cap
frame 8

buf = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x41\x01\x04\x00\x01\x00\xb4\x0a\x01\x01\x01\x24\x02\x22\x01\x04\x00\x01\x00\x01\x01\x04\x00\x01\x00\x04\x02\x00\x40\x02\x01\x2c\x41\x04\x00\x00\x00\x01\x45\x08\x00\x01\x01\x01\x00\x01\x04\x01'
dpkt.bgp.BGP(buf)

error thrown

Traceback (most recent call last):
  File "C:\Python\Python38\lib\site-packages\dpkt\dpkt.py", line 89, in __init__
    self.unpack(args[0])
  File "C:\Python\Python38\lib\site-packages\dpkt\bgp.py", line 218, in unpack
    dpkt.Packet.unpack(self, buf)
  File "C:\Python\Python38\lib\site-packages\dpkt\dpkt.py", line 171, in unpack
    struct.unpack(self.__hdr_fmt__, buf[:self.__hdr_len__])):
struct.error: unpack requires a buffer of 2 bytes

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:/Users/Amgad/PycharmProjects/amgad/bgp_test.py", line 50, in print_packets
    bgp_element = dpkt.bgp.BGP(bgp_data)
  File "C:\Python\Python38\lib\site-packages\dpkt\dpkt.py", line 89, in __init__
    self.unpack(args[0])
  File "C:\Python\Python38\lib\site-packages\dpkt\bgp.py", line 150, in unpack
    self.data = self.open = self.Open(self.data)
  File "C:\Python\Python38\lib\site-packages\dpkt\dpkt.py", line 89, in __init__
    self.unpack(args[0])
  File "C:\Python\Python38\lib\site-packages\dpkt\bgp.py", line 177, in unpack
    param = self.Parameter(self.data)
  File "C:\Python\Python38\lib\site-packages\dpkt\dpkt.py", line 89, in __init__
    self.unpack(args[0])
  File "C:\Python\Python38\lib\site-packages\dpkt\bgp.py", line 204, in unpack
    self.data = self.capability = self.Capability(self.data)
  File "C:\Python\Python38\lib\site-packages\dpkt\dpkt.py", line 92, in __init__
    raise NeedData
dpkt.dpkt.NeedData

same frame when opened using Wireshark is opened properly and shows capabilities

kbandla · 2020-07-10T02:42:54Z

ack. looking into this

kbandla · 2020-07-13T04:10:52Z

So after spending some time looking at this issue, it looks like the packet has multiple capabilities in one Optional Parameter, which the code is not able to handle.

Details

The Open message packet has an optional parameter type called Capabilities (Parameter Type 2).

The parameter contains one or more triples <Capability Code, 
Capability Length, Capability Value>, where each triple is 
encoded as shown below:
      +------------------------------+
          | Capability Code (1 octet)    |
          +------------------------------+
          | Capability Length (1 octet)  |
          +------------------------------+
          | Capability Value (variable)  |
          ~                              ~
          +------------------------------+

You can see this in the wireshark screenshot you posted, where the optional parameter length is 36 bytes, which includes 6 capabilities, but the current code when parsing Open parameters only consumes the first capability:

    def unpack(self, buf):
        dpkt.Packet.unpack(self, buf)
        self.data = self.data[:self.len]

        if self.type == AUTHENTICATION:
            self.data = self.authentication = self.Authentication(self.data)
        elif self.type == CAPABILITY:
            self.data = self.capability = self.Capability(self.data)

            class Authentication(dpkt.Packet):
                __hdr__ = (
                    ('code', 'B', 0),
                )

            class Capability(dpkt.Packet):
                __hdr__ = (
                    ('code', 'B', 0),
                    ('len', 'B', 0)
                )

                def unpack(self, buf):
                    dpkt.Packet.unpack(self, buf)
                    self.data = self.data[:self.len]

Even consuming this one capabilty has a bug while calculating the length, which is the bug you filed. Add a __len__ to the Capability will get us past the error, but the result is only partially correct (only 1 of 6 capabilities are read):


BGP(marker=b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff', len=65, data=Open(asn=1, holdtime=180, identifier=167837953, param_len=36, parameters=[Parameter(type=2, len=34, data=Capability(code=1, len=4, data=b'\x00\x01\x00\x01'))], data=[Parameter(type=2, len=34, data=Capability(code=1, len=4, data=b'\x00\x01\x00\x01'))]))
+ BGP data length: 46
+ Number of parameters: 1
+ Length of parameter 1: 36
Parameter(type=2, len=34, data=Capability(code=1, len=4, data=b'\x00\x01\x00\x01'))
Capability(code=1, len=4, data=b'\x00\x01\x00\x01')

Real Fix

So the fix needs updating the code to parse all the mulitiple capability fields.
However, this involves some changes to Open properties, especially Parameter
unpacking code, and also needs updating the unittests for the bgp module.

The correct output should be:

Number of Parameters: 1
  Parameter 1 has 6 capabilities
    Capability(code=1, len=4, data=b'\x00\x01\x00\x01')
    Capability(code=1, len=4, data=b'\x00\x01\x00\x04')
    Capability(code=2)
    Capability(code=64, len=2, data=b'\x01,')
    Capability(code=65, len=4, data=b'\x00\x00\x00\x01')
    Capability(code=69, len=8, data=b'\x00\x01\x01\x01\x00\x01\x04\x01')

Parsing the __bgp4 packet in the bgp.py file would then result in :

Number of Parameters: 3
        Parameter 1 has 1 capabilities
                Capability(code=1, len=4, data=b'\x00\x01\x00\x01')
        Parameter 2 has 1 capabilities
                Capability(code=128)
        Parameter 3 has 1 capabilities
                Capability(code=2)

and the corresponding unittest would then become:

b4 = dpkt.bgp.BGP(__bgp4)
assert (b4.len == 45)
assert (b4.type == OPEN)
assert (b4.open.asn == 237)
assert (b4.open.param_len == 16)
assert (len(b4.open.parameters) == 3)
p = b4.open.parameters[0]
assert (p.type == CAPABILITY)
assert (p.len == 6)
c = p.capabilities[0]
assert (c.code == CAP_MULTIPROTOCOL)
assert (c.len == 4)
assert (c.data == b'\x00\x01\x00\x01')
c = b4.open.parameters[2].capabilities[0]
assert (c.code == CAP_ROUTE_REFRESH)
assert (c.len == 0)

PR coming with the changes.

* Fixes #118 (please see issue for details) * Introduce `capabilities` property (for cases where there are >1 capablity) * Keep older `capability` property for backward compatibility * Added `__bgp10` unittest, which broke the original parser

kbandla added bug imported labels Jun 4, 2015

kbandla linked a pull request Jul 13, 2020 that will close this issue

BGP: Handle multiple capabilities in one Optional Parameter #480

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BGP (v4) Open typed packet failing to parse. #118

BGP (v4) Open typed packet failing to parse. #118

kbandla commented Jun 4, 2015

amgadhanafy commented Jul 8, 2020

amgadhanafy commented Jul 9, 2020 •

edited

Loading

kbandla commented Jul 10, 2020

kbandla commented Jul 13, 2020

BGP (v4) Open typed packet failing to parse. #118

BGP (v4) Open typed packet failing to parse. #118

Comments

kbandla commented Jun 4, 2015

amgadhanafy commented Jul 8, 2020

amgadhanafy commented Jul 9, 2020 • edited Loading

kbandla commented Jul 10, 2020

kbandla commented Jul 13, 2020

Details

Real Fix

amgadhanafy commented Jul 9, 2020 •

edited

Loading