Skip to content

Commit d73bbc1

Browse files
bkayrancigpotter2guedou
authored
Support Multiple Comments in Packet Object (#4798)
* Rename 'comment' to 'comments' in Packet class and related classes for consistency * Add support for 'comment' parameter in GenericPcapWriter and RawPcapWriter classes for back compatibility * fix typo * Add test and support for 'comment' parameter for back compatibility * Refactor type annotations to comply with mypy type checking * Remove unnecessary code related to comments definition * Remove redundant fallback to comment for compatibility handling * add comment for option codes * Remove legacy 'comment' field when writing packet; use 'comments' instead * remove casting and update comment property to take first item of list * fix linting and typing issues * Reorder fields in PacketMetadata tuple for consistency and update unpacking in PcapNgReader * Apply formatting suggestion by guedou Co-authored-by: Guillaume Valadon <guillaume@valadon.net> --------- Co-authored-by: gpotter2 <10530980+gpotter2@users.noreply.github.com> Co-authored-by: Guillaume Valadon <guillaume@valadon.net>
1 parent 7aea5cf commit d73bbc1

File tree

3 files changed

+78
-36
lines changed

3 files changed

+78
-36
lines changed

‎scapy/packet.py‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class Packet(
101101
"direction", "sniffed_on",
102102
# handle snaplen Vs real length
103103
"wirelen",
104-
"comment",
104+
"comments",
105105
"process_information"
106106
]
107107
name = None
@@ -179,7 +179,7 @@ def __init__(self,
179179
self.wirelen = None # type: Optional[int]
180180
self.direction = None # type: Optional[int]
181181
self.sniffed_on = None # type: Optional[_GlobInterfaceType]
182-
self.comment = None # type: Optional[bytes]
182+
self.comments = None # type: Optional[List[bytes]]
183183
self.process_information = None # type: Optional[Dict[str, Any]]
184184
self.stop_dissection_after = stop_dissection_after
185185
if _pkt:
@@ -223,6 +223,26 @@ def __init__(self,
223223
Optional[bytes],
224224
]
225225

226+
@property
227+
def comment(self):
228+
# type: () -> Optional[bytes]
229+
"""Get the comment of the packet"""
230+
if self.comments and len(self.comments):
231+
return self.comments[0]
232+
return None
233+
234+
@comment.setter
235+
def comment(self, value):
236+
# type: (Optional[bytes]) -> None
237+
"""
238+
Set the comment of the packet.
239+
If value is None, it will clear the comments.
240+
"""
241+
if value is not None:
242+
self.comments = [value]
243+
else:
244+
self.comments = None
245+
226246
def __reduce__(self):
227247
# type: () -> Tuple[Type[Packet], Tuple[bytes], Packet._PickleType]
228248
"""Used by pickling methods"""
@@ -435,7 +455,7 @@ def copy(self) -> Self:
435455
clone.payload = self.payload.copy()
436456
clone.payload.add_underlayer(clone)
437457
clone.time = self.time
438-
clone.comment = self.comment
458+
clone.comments = self.comments
439459
clone.direction = self.direction
440460
clone.sniffed_on = self.sniffed_on
441461
return clone
@@ -1145,7 +1165,7 @@ def clone_with(self, payload=None, **kargs):
11451165
self.raw_packet_cache_fields
11461166
)
11471167
pkt.wirelen = self.wirelen
1148-
pkt.comment = self.comment
1168+
pkt.comments = self.comments
11491169
pkt.sniffed_on = self.sniffed_on
11501170
pkt.direction = self.direction
11511171
if payload is not None:

‎scapy/utils.py‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,7 +1652,7 @@ class RawPcapNgReader(RawPcapReader):
16521652
PacketMetadata = collections.namedtuple("PacketMetadataNg", # type: ignore
16531653
["linktype", "tsresol",
16541654
"tshigh", "tslow", "wirelen",
1655-
"comment", "ifname", "direction",
1655+
"comments", "ifname", "direction",
16561656
"process_information"])
16571657

16581658
def __init__(self, filename, fdesc=None, magic=None): # type: ignore
@@ -1796,8 +1796,8 @@ def _read_packet(self, size=MTU): # type: ignore
17961796
return res
17971797

17981798
def _read_options(self, options):
1799-
# type: (bytes) -> Dict[int, bytes]
1800-
opts = dict()
1799+
# type: (bytes) -> Dict[int, Union[bytes, List[bytes]]]
1800+
opts = dict() # type: Dict[int, Union[bytes, List[bytes]]]
18011801
while len(options) >= 4:
18021802
try:
18031803
code, length = struct.unpack(self.endian + "HH", options[:4])
@@ -1806,7 +1806,13 @@ def _read_options(self, options):
18061806
"%d !" % len(options))
18071807
raise EOFError
18081808
if code != 0 and 4 + length <= len(options):
1809-
opts[code] = options[4:4 + length]
1809+
# https://www.ietf.org/archive/id/draft-tuexen-opsawg-pcapng-05.html#name-options-format
1810+
if code in [1, 2988, 2989, 19372, 19373]:
1811+
if code not in opts:
1812+
opts[code] = []
1813+
opts[code].append(options[4:4 + length]) # type: ignore
1814+
else:
1815+
opts[code] = options[4:4 + length]
18101816
if code == 0:
18111817
if length != 0:
18121818
warning("PcapNg: invalid option "
@@ -1825,6 +1831,12 @@ def _read_block_idb(self, block, _):
18251831
options_raw = self._read_options(block[8:])
18261832
options = self.default_options.copy() # type: Dict[str, Any]
18271833
for c, v in options_raw.items():
1834+
if isinstance(v, list):
1835+
# Spec allows multiple occurrences (see
1836+
# https://www.ietf.org/archive/id/draft-tuexen-opsawg-pcapng-05.html#section-4.2-8.6)
1837+
# but does not define which to use. We take the first for
1838+
# backward compatibility.
1839+
v = v[0]
18281840
if c == 9:
18291841
length = len(v)
18301842
if length == 1:
@@ -1880,11 +1892,13 @@ def _read_block_epb(self, block, size):
18801892

18811893
process_information = {}
18821894
for code, value in options.items():
1883-
if code in [0x8001, 0x8003]: # PCAPNG_EPB_PIB_INDEX, PCAPNG_EPB_E_PIB_INDEX
1895+
# PCAPNG_EPB_PIB_INDEX, PCAPNG_EPB_E_PIB_INDEX
1896+
if code in [0x8001, 0x8003]:
18841897
try:
1885-
proc_index = struct.unpack(self.endian + "I", value)[0]
1898+
proc_index = struct.unpack(
1899+
self.endian + "I", value)[0] # type: ignore
18861900
except struct.error:
1887-
warning("PcapNg: EPB invalid proc index"
1901+
warning("PcapNg: EPB invalid proc index "
18881902
"(expected 4 bytes, got %d) !" % len(value))
18891903
raise EOFError
18901904
if proc_index < len(self.process_information):
@@ -1894,9 +1908,9 @@ def _read_block_epb(self, block, size):
18941908
warning("PcapNg: EPB invalid process information index "
18951909
"(%d/%d) !" % (proc_index, len(self.process_information)))
18961910

1897-
comment = options.get(1, None)
1911+
comments = options.get(1, None)
18981912
epb_flags_raw = options.get(2, None)
1899-
if epb_flags_raw:
1913+
if epb_flags_raw and isinstance(epb_flags_raw, bytes):
19001914
try:
19011915
epb_flags, = struct.unpack(self.endian + "I", epb_flags_raw)
19021916
except struct.error:
@@ -1917,10 +1931,10 @@ def _read_block_epb(self, block, size):
19171931
tshigh=tshigh,
19181932
tslow=tslow,
19191933
wirelen=wirelen,
1920-
comment=comment,
19211934
ifname=ifname,
19221935
direction=direction,
1923-
process_information=process_information))
1936+
process_information=process_information,
1937+
comments=comments))
19241938

19251939
def _read_block_spb(self, block, size):
19261940
# type: (bytes, int) -> Tuple[bytes, RawPcapNgReader.PacketMetadata]
@@ -1944,10 +1958,10 @@ def _read_block_spb(self, block, size):
19441958
tshigh=None,
19451959
tslow=None,
19461960
wirelen=wirelen,
1947-
comment=None,
19481961
ifname=None,
19491962
direction=None,
1950-
process_information={}))
1963+
process_information={},
1964+
comments=None))
19511965

19521966
def _read_block_pkt(self, block, size):
19531967
# type: (bytes, int) -> Tuple[bytes, RawPcapNgReader.PacketMetadata]
@@ -1968,10 +1982,10 @@ def _read_block_pkt(self, block, size):
19681982
tshigh=tshigh,
19691983
tslow=tslow,
19701984
wirelen=wirelen,
1971-
comment=None,
19721985
ifname=None,
19731986
direction=None,
1974-
process_information={}))
1987+
process_information={},
1988+
comments=None))
19751989

19761990
def _read_block_dsb(self, block, size):
19771991
# type: (bytes, int) -> None
@@ -2043,10 +2057,11 @@ def _read_block_pib(self, block, _):
20432057
options = self._read_options(block)
20442058
for code, value in options.items():
20452059
if code == 2:
2046-
process_information["name"] = value.decode("ascii", "backslashreplace")
2060+
process_information["name"] = value.decode( # type: ignore
2061+
"ascii", "backslashreplace")
20472062
elif code == 4:
20482063
if len(value) == 16:
2049-
process_information["uuid"] = str(UUID(bytes=value))
2064+
process_information["uuid"] = str(UUID(bytes=value)) # type: ignore
20502065
else:
20512066
warning("PcapNg: DPEB UUID length is invalid (%d)!",
20522067
len(value))
@@ -2072,7 +2087,7 @@ def read_packet(self, size=MTU, **kwargs):
20722087
rp = super(PcapNgReader, self)._read_packet(size=size)
20732088
if rp is None:
20742089
raise EOFError
2075-
s, (linktype, tsresol, tshigh, tslow, wirelen, comment, ifname, direction, process_information) = rp # noqa: E501
2090+
s, (linktype, tsresol, tshigh, tslow, wirelen, comments, ifname, direction, process_information) = rp # noqa: E501
20762091
try:
20772092
cls = conf.l2types.num2layer[linktype] # type: Type[Packet]
20782093
p = cls(s, **kwargs) # type: Packet
@@ -2088,7 +2103,7 @@ def read_packet(self, size=MTU, **kwargs):
20882103
if tshigh is not None:
20892104
p.time = EDecimal((tshigh << 32) + tslow) / tsresol
20902105
p.wirelen = wirelen
2091-
p.comment = comment
2106+
p.comments = comments
20922107
p.direction = direction
20932108
p.process_information = process_information.copy()
20942109
if ifname is not None:
@@ -2114,9 +2129,9 @@ def _write_packet(self,
21142129
usec=None, # type: Optional[int]
21152130
caplen=None, # type: Optional[int]
21162131
wirelen=None, # type: Optional[int]
2117-
comment=None, # type: Optional[bytes]
21182132
ifname=None, # type: Optional[bytes]
21192133
direction=None, # type: Optional[int]
2134+
comments=None, # type: Optional[List[bytes]]
21202135
):
21212136
# type: (...) -> None
21222137
raise NotImplementedError
@@ -2197,7 +2212,7 @@ def write_packet(self,
21972212
if wirelen is None:
21982213
wirelen = caplen
21992214

2200-
comment = getattr(packet, "comment", None)
2215+
comments = getattr(packet, "comments", None)
22012216
ifname = getattr(packet, "sniffed_on", None)
22022217
direction = getattr(packet, "direction", None)
22032218
if not isinstance(packet, bytes):
@@ -2212,10 +2227,10 @@ def write_packet(self,
22122227
rawpkt,
22132228
sec=f_sec, usec=usec,
22142229
caplen=caplen, wirelen=wirelen,
2215-
comment=comment,
22162230
ifname=ifname,
22172231
direction=direction,
2218-
linktype=linktype
2232+
linktype=linktype,
2233+
comments=comments,
22192234
)
22202235

22212236

@@ -2367,9 +2382,9 @@ def _write_packet(self,
23672382
usec=None, # type: Optional[int]
23682383
caplen=None, # type: Optional[int]
23692384
wirelen=None, # type: Optional[int]
2370-
comment=None, # type: Optional[bytes]
23712385
ifname=None, # type: Optional[bytes]
23722386
direction=None, # type: Optional[int]
2387+
comments=None, # type: Optional[List[bytes]]
23732388
):
23742389
# type: (...) -> None
23752390
"""
@@ -2545,7 +2560,7 @@ def _write_block_epb(self,
25452560
timestamp=None, # type: Optional[Union[EDecimal, float]] # noqa: E501
25462561
caplen=None, # type: Optional[int]
25472562
orglen=None, # type: Optional[int]
2548-
comment=None, # type: Optional[bytes]
2563+
comments=None, # type: Optional[List[bytes]]
25492564
flags=None, # type: Optional[int]
25502565
):
25512566
# type: (...) -> None
@@ -2580,11 +2595,12 @@ def _write_block_epb(self,
25802595

25812596
# Options
25822597
opts = b''
2583-
if comment is not None:
2584-
comment = bytes_encode(comment)
2585-
opts += struct.pack(self.endian + "HH", 1, len(comment))
2586-
# Pad Option Value to 32 bits
2587-
opts += self._add_padding(comment)
2598+
if comments and len(comments):
2599+
for c in comments:
2600+
comment = bytes_encode(c)
2601+
opts += struct.pack(self.endian + "HH", 1, len(comment))
2602+
# Pad Option Value to 32 bits
2603+
opts += self._add_padding(comment)
25882604
if type(flags) == int:
25892605
opts += struct.pack(self.endian + "HH", 2, 4)
25902606
opts += struct.pack(self.endian + "I", flags)
@@ -2601,9 +2617,9 @@ def _write_packet(self, # type: ignore
26012617
usec=None, # type: Optional[int]
26022618
caplen=None, # type: Optional[int]
26032619
wirelen=None, # type: Optional[int]
2604-
comment=None, # type: Optional[bytes]
26052620
ifname=None, # type: Optional[bytes]
26062621
direction=None, # type: Optional[int]
2622+
comments=None, # type: Optional[List[bytes]]
26072623
):
26082624
# type: (...) -> None
26092625
"""
@@ -2659,7 +2675,7 @@ def _write_packet(self, # type: ignore
26592675
flags = None
26602676

26612677
self._write_block_epb(packet, timestamp=sec, caplen=caplen,
2662-
orglen=wirelen, comment=comment, ifid=ifid, flags=flags)
2678+
orglen=wirelen, comments=comments, ifid=ifid, flags=flags)
26632679
if self.sync:
26642680
self.f.flush()
26652681

‎test/regression.uts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2259,6 +2259,12 @@ wrpcapng(tmpfile, p)
22592259
l = rdpcap(tmpfile)
22602260
assert l[0].comment == p.comment
22612261

2262+
p = Ether() / IPv6() / TCP()
2263+
p.comments = [b"Hello!", b"Scapy!", b"Pcapng!"]
2264+
wrpcapng(tmpfile, p)
2265+
l = rdpcap(tmpfile)
2266+
assert l[0].comments == p.comments
2267+
22622268
= rdpcap on fifo
22632269
~ linux
22642270
f = get_temp_file()

0 commit comments

Comments
 (0)