Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BGP: Handle multiple capabilities in one Optional Parameter #480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

BGP: Handle multiple capabilities in one Optional Parameter #480

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 42 additions & 3 deletions dpkt/bgp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@
# Capability Types
CAP_MULTIPROTOCOL = 1
CAP_ROUTE_REFRESH = 2
CAP_GRACEFUL_RESTART = 64
CAP_SUPPORT_4OCTETAS = 65
CAP_ADD_PATH = 69

# NOTIFICATION Error Codes
MESSAGE_HEADER_ERROR = 1
Expand Down Expand Up @@ -200,7 +203,22 @@ def unpack(self, buf):
if self.type == AUTHENTICATION:
self.data = self.authentication = self.Authentication(self.data)
elif self.type == CAPABILITY:
self.data = self.capability = self.Capability(self.data)
l = []
while self.data:
capability = self.Capability(self.data)
l.append(capability)
self.data = self.data[self.__hdr_len__+capability.len:]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe pep8 would want whitespace around +

self.data = self.capabilities = l
# define capability to keep backward compatibility
self.capability = self.capabilities[0]

def __len__(self):
return self.__hdr_len__ + sum(map(len, self.capabilities))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling len() on each instance of capability would cause serialization to bytes, then taking the resulting length. would something like sum(cap.len for cap in self.capabilities) achieve the same result?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively you could override the __len__ method to return that header element?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't cover the case where the type is AUTHENTICATION, as capabilities won't exist (and it doesn't count the length of the authentication data).


def __bytes__(self):
caps = b''.join(map(bytes, self.capabilities))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also fail if this is an authentication parameter. Perhaps some variant of caps = bytes(self.data) if not isinstance(self.data, list) else b''.join(map(bytes, self.data))?

self.cap_len = len(caps)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?
also prob shouldn't create new fields inside this method

return self.pack_hdr() + caps

class Authentication(dpkt.Packet):
__hdr__ = (
Expand All @@ -217,6 +235,7 @@ def unpack(self, buf):
dpkt.Packet.unpack(self, buf)
self.data = self.data[:self.len]


class Update(dpkt.Packet):
__hdr_defaults__ = {
'withdrawn': [],
Expand Down Expand Up @@ -738,6 +757,8 @@ def __bytes__(self):
__bgp2 = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x63\x02\x00\x00\x00\x48\x40\x01\x01\x00\x40\x02\x0a\x01\x02\x01\xf4\x01\xf4\x02\x01\xfe\xbb\x40\x03\x04\xc0\xa8\x00\x0f\x40\x05\x04\x00\x00\x00\x64\x40\x06\x00\xc0\x07\x06\xfe\xba\xc0\xa8\x00\x0a\xc0\x08\x0c\xfe\xbf\x00\x01\x03\x16\x00\x04\x01\x54\x00\xfa\x80\x09\x04\xc0\xa8\x00\x0f\x80\x0a\x04\xc0\xa8\x00\xfa\x16\xc0\xa8\x04'
__bgp3 = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x79\x02\x00\x00\x00\x62\x40\x01\x01\x00\x40\x02\x00\x40\x05\x04\x00\x00\x00\x64\xc0\x10\x08\x00\x02\x01\x2c\x00\x00\x01\x2c\xc0\x80\x24\x00\x00\xfd\xe9\x40\x01\x01\x00\x40\x02\x04\x02\x01\x15\xb3\x40\x05\x04\x00\x00\x00\x2c\x80\x09\x04\x16\x05\x05\x05\x80\x0a\x04\x16\x05\x05\x05\x90\x0e\x00\x1e\x00\x01\x80\x0c\x00\x00\x00\x00\x00\x00\x00\x00\x0c\x04\x04\x04\x00\x60\x18\x77\x01\x00\x00\x01\xf4\x00\x00\x01\xf4\x85'
__bgp4 = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x2d\x01\x04\x00\xed\x00\x5a\xc6\x6e\x83\x7d\x10\x02\x06\x01\x04\x00\x01\x00\x01\x02\x02\x80\x00\x02\x02\x02\x00'
__bgp10 = 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'


# BGP-EVPN type 1-4 packets for testing.
__bgp5 = b'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x60\x02\x00\x00\x00\x49\x40\x01\x01\x00\x40\x02\x00\x40\x05\x04\x00\x00\x00\x64\xc0\x10\x10\x03\x0c\x00\x00\x00\x00\x00\x08\x00\x02\x03\xe8\x00\x00\x00\x02\x90\x0e\x00\x24\x00\x19\x46\x04\x01\x01\x01\x02\x00\x01\x19\x00\x01\x01\x01\x01\x02\x00\x02\x05\x00\x00\x03\xe8\x00\x00\x04\x00\x00\x00\x00\x00\x02\x00\x00\x02'
Expand All @@ -757,6 +778,7 @@ def test_pack():
assert (__bgp7 == bytes(BGP(__bgp7)))
assert (__bgp8 == bytes(BGP(__bgp8)))
assert (__bgp9 == bytes(BGP(__bgp9)))
assert (__bgp10 == bytes(BGP(__bgp10)))


def test_unpack():
Expand Down Expand Up @@ -828,11 +850,11 @@ def test_unpack():
p = b4.open.parameters[0]
assert (p.type == CAPABILITY)
assert (p.len == 6)
c = p.capability
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].capability
c = b4.open.parameters[2].capabilities[0]
assert (c.code == CAP_ROUTE_REFRESH)
assert (c.len == 0)

Expand Down Expand Up @@ -934,6 +956,23 @@ def test_unpack():
assert (r.ip_address == b'\xc0\xb4\x01\x02\xc0\xb4\x01\x02\xc0\xb4\x01\x02\xc0\xb4\x01\x02')
assert (r.mpls_label_stack == b'\x00\x00\x02\x00\x00\x00')

b10 = BGP(__bgp10)
assert (b10.len == 65)
assert (b10.type == OPEN)
assert (b10.open.asn == 1)
assert (b10.open.param_len == 36)
assert (len(b10.open.parameters) == 1)
p = b10.open.parameters[0]
assert (p.type == CAPABILITY)
assert (p.len == 34)
c = p.capabilities[0]
assert (c.code == CAP_MULTIPROTOCOL)
assert (c.len == 4)
assert (c.data == b'\x00\x01\x00\x01')
c = b10.open.parameters[0].capabilities[3]
assert (c.code == CAP_GRACEFUL_RESTART)
assert (c.len == 2)


if __name__ == '__main__':
test_pack()
Expand Down