kbandla / dpkt

fast, simple packet creation / parsing, with definitions for the basic TCP/IP protocols
Other
1.09k stars 270 forks source link

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

Open kbandla opened 9 years ago

kbandla commented 9 years ago

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

amgadhanafy commented 4 years ago

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

amgadhanafy commented 4 years ago

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

kbandla commented 4 years ago

ack. looking into this

kbandla commented 4 years ago

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.