packet/dhcp: Support fragmented options

Currently, if you set fragmented option data
for the packet library of dhcp, Traceback occurs.
This patch fixes to analyze up to options not corrupting data.
The remaining corrupting data will be appended
at the end of option list.

Signed-off-by: Shinpei Muraoka <shinpei.muraoka@gmail.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

1 files changed, 81 insertions, 87 deletions

diff --git a/ryu/lib/packet/dhcp.py b/ryu/lib/packet/dhcp.py
index c8e849fa..327c8925 100644
--- a/ryu/lib/packet/dhcp.py
+++ b/ryu/lib/packet/dhcp.py
@@ -15,51 +15,49 @@
 
 """
 DHCP packet parser/serializer
-
-RFC 2131
-DHCP packet format
-
-    0                   1                   2                   3
-    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |     op (1)    |   htype (1)   |   hlen (1)    |   hops (1)    |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                            xid (4)                            |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |           secs (2)            |           flags (2)           |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                          ciaddr  (4)                          |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                          yiaddr  (4)                          |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                          siaddr  (4)                          |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                          giaddr  (4)                          |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                                                               |
-    |                          chaddr  (16)                         |
-    |                                                               |
-    |                                                               |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                                                               |
-    |                          sname   (64)                         |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                                                               |
-    |                          file    (128)                        |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    |                                                               |
-    |                          options (variable)                   |
-    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-
 """
-import binascii
+# RFC 2131
+# DHCP packet format
+#  0                   1                   2                   3
+#  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |     op (1)    |   htype (1)   |   hlen (1)    |   hops (1)    |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                            xid (4)                            |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |           secs (2)            |           flags (2)           |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                          ciaddr  (4)                          |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                          yiaddr  (4)                          |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                          siaddr  (4)                          |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                          giaddr  (4)                          |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                                                               |
+#  |                          chaddr  (16)                         |
+#  |                                                               |
+#  |                                                               |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                                                               |
+#  |                          sname   (64)                         |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                                                               |
+#  |                          file    (128)                        |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+#  |                                                               |
+#  |                          options (variable)                   |
+#  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+
 import random
 import struct
 
-from . import packet_base
+import netaddr
+
 from ryu.lib import addrconv
 from ryu.lib import stringify
-
+from . import packet_base
 
 DHCP_BOOT_REQUEST = 1
 DHCP_BOOT_REPLY = 2
@@ -136,12 +134,9 @@ class dhcp(packet_base.PacketBase):
                     every DHCP message).
     ============== ====================
     """
-    _MIN_LEN = 236
-    _HLEN_UNPACK_STR = '!BBB'
-    _HLEN_UNPACK_LEN = struct.calcsize(_HLEN_UNPACK_STR)
-    _DHCP_UNPACK_STR = '!BIHH4s4s4s4s%ds%ds64s128s'
     _DHCP_PACK_STR = '!BBBBIHH4s4s4s4s16s64s128s'
-    _DHCP_CHADDR_LEN = 16
+    _MIN_LEN = struct.calcsize(_DHCP_PACK_STR)
+    _MAC_ADDRESS_LEN = 6
     _HARDWARE_TYPE_ETHERNET = 1
     _class_prefixes = ['options']
     _TYPE = {
@@ -150,17 +145,14 @@ class dhcp(packet_base.PacketBase):
         ]
     }
 
-    def __init__(self, op, chaddr, options, htype=_HARDWARE_TYPE_ETHERNET,
+    def __init__(self, op, chaddr, options=None, htype=_HARDWARE_TYPE_ETHERNET,
                  hlen=0, hops=0, xid=None, secs=0, flags=0,
                  ciaddr='0.0.0.0', yiaddr='0.0.0.0', siaddr='0.0.0.0',
                  giaddr='0.0.0.0', sname='', boot_file=b''):
         super(dhcp, self).__init__()
         self.op = op
         self.htype = htype
-        if hlen == 0:
-            self.hlen = len(addrconv.mac.text_to_bin(chaddr))
-        else:
-            self.hlen = hlen
+        self.hlen = hlen
         self.hops = hops
         if xid is None:
             self.xid = random.randint(0, 0xffffffff)
@@ -178,20 +170,20 @@ class dhcp(packet_base.PacketBase):
         self.options = options
 
     @classmethod
-    def _parser(cls, buf):
-        (op, htype, hlen) = struct.unpack_from(cls._HLEN_UNPACK_STR, buf)
-        buf = buf[cls._HLEN_UNPACK_LEN:]
-        unpack_str = cls._DHCP_UNPACK_STR % (hlen,
-                                             (cls._DHCP_CHADDR_LEN - hlen))
-        min_len = struct.calcsize(unpack_str)
-        (hops, xid, secs, flags, ciaddr, yiaddr, siaddr, giaddr, chaddr,
-         dummy, sname, boot_file
-         ) = struct.unpack_from(unpack_str, buf)
-        length = min_len
-        if len(buf) > min_len:
-            parse_opt = options.parser(buf[min_len:])
+    def parser(cls, buf):
+        (op, htype, hlen, hops, xid, secs, flags,
+         ciaddr, yiaddr, siaddr, giaddr, chaddr, sname,
+         boot_file) = struct.unpack_from(cls._DHCP_PACK_STR, buf)
+
+        if hlen == cls._MAC_ADDRESS_LEN:
+            chaddr = addrconv.mac.bin_to_text(chaddr[:cls._MAC_ADDRESS_LEN])
+
+        length = cls._MIN_LEN
+        parse_opt = None
+        if len(buf) > length:
+            parse_opt = options.parser(buf[length:])
             length += parse_opt.options_len
-        return (cls(op, addrconv.mac.bin_to_text(chaddr), parse_opt,
+        return (cls(op, chaddr, parse_opt,
                     htype, hlen, hops, xid, secs, flags,
                     addrconv.ipv4.bin_to_text(ciaddr),
                     addrconv.ipv4.bin_to_text(yiaddr),
@@ -200,25 +192,24 @@ class dhcp(packet_base.PacketBase):
                     sname.decode('ascii'), boot_file),
                 None, buf[length:])
 
-    @classmethod
-    def parser(cls, buf):
-        try:
-            return cls._parser(buf)
-        except:
-            return None, None, buf
-
-    def serialize(self, payload, prev):
-        seri_opt = self.options.serialize()
-        pack_str = '%s%ds' % (self._DHCP_PACK_STR,
-                              self.options.options_len)
-        return struct.pack(pack_str, self.op, self.htype, self.hlen,
+    def serialize(self, _payload=None, _prev=None):
+        opt_buf = bytearray()
+        if self.options is not None:
+            opt_buf = self.options.serialize()
+        if netaddr.valid_mac(self.chaddr):
+            chaddr = addrconv.mac.text_to_bin(self.chaddr)
+        else:
+            chaddr = self.chaddr
+        self.hlen = len(chaddr)
+        return struct.pack(self._DHCP_PACK_STR, self.op, self.htype, self.hlen,
                            self.hops, self.xid, self.secs, self.flags,
                            addrconv.ipv4.text_to_bin(self.ciaddr),
                            addrconv.ipv4.text_to_bin(self.yiaddr),
                            addrconv.ipv4.text_to_bin(self.siaddr),
                            addrconv.ipv4.text_to_bin(self.giaddr),
-                           addrconv.mac.text_to_bin(self.chaddr),
-                           self.sname.encode('ascii'), self.boot_file, seri_opt)
+                           chaddr,
+                           self.sname.encode('ascii'),
+                           self.boot_file) + opt_buf
 
 
 class options(stringify.StringifyMixin):
@@ -258,10 +249,7 @@ class options(stringify.StringifyMixin):
     def __init__(self, option_list=None, options_len=0,
                  magic_cookie=_MAGIC_COOKIE):
         super(options, self).__init__()
-        if option_list is None:
-            self.option_list = []
-        else:
-            self.option_list = option_list
+        self.option_list = option_list or []
         self.options_len = options_len
         self.magic_cookie = magic_cookie
 
@@ -272,7 +260,11 @@ class options(stringify.StringifyMixin):
         magic_cookie = struct.unpack_from(cls._MAGIC_COOKIE_UNPACK_STR, buf)[0]
         while len(buf) > offset:
             opt_buf = buf[offset:]
-            opt = option.parser(opt_buf)
+            try:
+                opt = option.parser(opt_buf)
+            except struct.error:
+                opt_parse_list.append(opt_buf)
+                break
             if opt is None:
                 break
             opt_parse_list.append(opt)
@@ -283,10 +275,13 @@ class options(stringify.StringifyMixin):
     def serialize(self):
         seri_opt = addrconv.ipv4.text_to_bin(self.magic_cookie)
         for opt in self.option_list:
-            seri_opt += opt.serialize()
-        seri_opt += binascii.a2b_hex('%x' % DHCP_END_OPT)
-        if self.options_len == 0:
-            self.options_len = len(seri_opt)
+            if isinstance(opt, option):
+                seri_opt += opt.serialize()
+            else:
+                seri_opt += opt
+        if isinstance(self.option_list[-1], option):
+            seri_opt += b'\xff'
+        self.options_len = len(seri_opt)
         return seri_opt
 
 
@@ -333,7 +328,6 @@ class option(stringify.StringifyMixin):
         return cls(tag, value, length)
 
     def serialize(self):
-        if self.length == 0:
-            self.length = len(self.value)
+        self.length = len(self.value)
         options_pack_str = '!BB%ds' % self.length
         return struct.pack(options_pack_str, self.tag, self.length, self.value)