PPP: fix default size of protocol field (#4106)

In commit 2f5d9bd ("PPP: protocol field can be limited to one
byte"), a new class PPP_ was added to manage parsing and generation a
PPP header with a one byte PPP protocol field.

This was later reworked by commit 834309f ("Small doc cleanups"),
which removed the PPP_ class, and changed the default behavior of the
PPP class to generate a one byte protocol field by default, when its
value was lower than 0x100.

The RFC states that "by default, all implementations MUST transmit
packets with two octet PPP Protocol fields". A header with a one byte
protocol field is issued by implementations when the compression is
negociated.

This patch reverts to the original behavior, which is to generate a two
bytes protocol field by default, but make it possible to explicitly
generate a one byte protocol by passing the value as bytes(). The PPP
class is still able to parse either a one or two bytes protocol.

Link: https://www.rfc-editor.org/rfc/rfc1661.html#section-6.5
Fixes #3913

Author: olivier-matz-6wind

scapy/layers/ppp.py

@@ -292,6 +292,14 @@ class _PPPProtoField(EnumField):
 
     See RFC 1661 section 2
     <https://tools.ietf.org/html/rfc1661#section-2>
+
+    The generated proto field is two bytes when not specified, or when specified
+    as an integer or a string:
+      PPP()
+      PPP(proto=0x21)
+      PPP(proto="Internet Protocol version 4")
+    To explicitly forge a one byte proto field, use the bytes representation:
+      PPP(proto=b'\x21')
     """
     def getfield(self, pkt, s):
         if ord(s[:1]) & 0x01:
@@ -304,12 +312,18 @@ def getfield(self, pkt, s):
         return super(_PPPProtoField, self).getfield(pkt, s)
 
     def addfield(self, pkt, s, val):
-        if val < 0x100:
-            self.fmt = "!B"
-            self.sz = 1
+        if isinstance(val, bytes):
+            if len(val) == 1:
+                fmt, sz = "!B", 1
+            elif len(val) == 2:
+                fmt, sz = "!H", 2
+            else:
+                raise TypeError('Invalid length for PPP proto')
+            val = struct.Struct(fmt).unpack(val)[0]
         else:
-            self.fmt = "!H"
-            self.sz = 2
+            fmt, sz = "!H", 2
+        self.fmt = fmt
+        self.sz = sz
         self.struct = struct.Struct(self.fmt)
         return super(_PPPProtoField, self).addfield(pkt, s, val)
 

test/scapy/layers/ppp.uts

@@ -112,12 +112,24 @@ assert  raw(p) == raw(q)
 assert  q[PPP_ECP_Option].data == b"ABCDEFG" 
 
 
-= PPP with only one byte for protocol
+= PPP IP check that default protocol length is 2 bytes
+~ ppp ip
+
+p = PPP()/IP()
+p
+r = raw(p)
+r
+assert r.startswith(b'\x00\x21')
+assert len(r) == 22
+
+
+= PPP check parsing with only one byte for protocol
 ~ ppp
 
-assert len(raw(PPP() / IP())) == 21
+assert len(raw(PPP(proto=b'\x21') / IP())) == 21
 
 p = PPP(b'!E\x00\x00<\x00\x00@\x008\x06\xa5\xce\x85wP)\xc0\xa8Va\x01\xbbd\x8a\xe2}r\xb8O\x95\xb5\x84\xa0\x12q \xc8\x08\x00\x00\x02\x04\x02\x18\x04\x02\x08\nQ\xdf\xd6\xb0\x00\x07LH\x01\x03\x03\x07Ao')
 assert IP in p
 assert TCP in p
 
+assert PPP(b"\x00\x21" + raw(IP())) == PPP(b"\x21" + raw(IP()))