From a5b053356e5fcdb3ec82f911a7589f49c292c922 Mon Sep 17 00:00:00 2001 From: Nils Weiss Date: Wed, 8 Jan 2025 20:52:27 +0100 Subject: [PATCH 1/5] Fix #4635: Enhance ISO-TP Soft Socket Implementation to properly handle CANFD packets --- scapy/contrib/isotp/isotp_soft_socket.py | 41 +++++++++++++----------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/scapy/contrib/isotp/isotp_soft_socket.py b/scapy/contrib/isotp/isotp_soft_socket.py index 81c6d3fe874..09780a0fd68 100644 --- a/scapy/contrib/isotp/isotp_soft_socket.py +++ b/scapy/contrib/isotp/isotp_soft_socket.py @@ -4,29 +4,16 @@ # Copyright (C) Nils Weiss # Copyright (C) Enrico Pozzobon +import heapq # scapy.contrib.description = ISO-TP (ISO 15765-2) Soft Socket Library # scapy.contrib.status = library import logging +import socket import struct import time import traceback -import heapq -import socket - -from threading import Thread, Event, RLock from bisect import bisect_left - -from scapy.packet import Packet -from scapy.layers.can import CAN -from scapy.error import Scapy_Exception -from scapy.supersocket import SuperSocket -from scapy.config import conf -from scapy.consts import LINUX -from scapy.utils import EDecimal -from scapy.automaton import ObjectPipe, select_objects -from scapy.contrib.isotp.isotp_packet import ISOTP, CAN_MAX_DLEN, N_PCI_SF, \ - N_PCI_CF, N_PCI_FC, N_PCI_FF, ISOTP_MAX_DLEN, ISOTP_MAX_DLEN_2015, CAN_FD_MAX_DLEN - +from threading import Thread, Event, RLock # Typing imports from typing import ( Optional, @@ -40,6 +27,17 @@ TYPE_CHECKING, ) +from scapy.automaton import ObjectPipe, select_objects +from scapy.config import conf +from scapy.consts import LINUX +from scapy.contrib.isotp.isotp_packet import ISOTP, CAN_MAX_DLEN, N_PCI_SF, \ + N_PCI_CF, N_PCI_FC, N_PCI_FF, ISOTP_MAX_DLEN, ISOTP_MAX_DLEN_2015, CAN_FD_MAX_DLEN +from scapy.error import Scapy_Exception +from scapy.layers.can import CAN, CANFD +from scapy.packet import Packet +from scapy.supersocket import SuperSocket +from scapy.utils import EDecimal + if TYPE_CHECKING: from scapy.contrib.cansocket import CANSocket @@ -579,13 +577,18 @@ def _get_padding_size(pl_size): return fd_accepted_sizes[-1] return fd_accepted_sizes[pos] + if self.fd: + pkt_cls = CANFD + else: + pkt_cls = CAN + if self.padding: load += b"\xCC" * (_get_padding_size(len(load)) - len(load)) if self.tx_id is None or self.tx_id <= 0x7ff: - self.can_socket.send(CAN(identifier=self.tx_id, data=load)) + self.can_socket.send(pkt_cls(identifier=self.tx_id, data=load)) else: - self.can_socket.send(CAN(identifier=self.tx_id, flags="extended", - data=load)) + self.can_socket.send(pkt_cls(identifier=self.tx_id, flags="extended", + data=load)) def can_recv(self): # type: () -> None From fd1c036d2eab46a82ec305ec8f1cd360f75baa1a Mon Sep 17 00:00:00 2001 From: Nils Weiss Date: Thu, 9 Jan 2025 20:42:19 +0100 Subject: [PATCH 2/5] fix unit tests --- scapy/contrib/isotp/isotp_packet.py | 42 ++++++++++++++--------------- test/contrib/isotp_soft_socket.uts | 27 ++++++++++++++----- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/scapy/contrib/isotp/isotp_packet.py b/scapy/contrib/isotp/isotp_packet.py index 6f62614ca2b..3da14956dda 100644 --- a/scapy/contrib/isotp/isotp_packet.py +++ b/scapy/contrib/isotp/isotp_packet.py @@ -6,19 +6,8 @@ # scapy.contrib.description = ISO-TP (ISO 15765-2) Packet Definitions # scapy.contrib.status = library -import struct import logging - -from scapy.config import conf -from scapy.packet import Packet -from scapy.fields import BitField, FlagsField, StrLenField, \ - ThreeBytesField, XBitField, ConditionalField, \ - BitEnumField, ByteField, XByteField, BitFieldLenField, StrField, \ - FieldLenField, IntField, ShortField -from scapy.compat import chb, orb -from scapy.layers.can import CAN, CAN_FD_MAX_DLEN as CAN_FD_MAX_DLEN -from scapy.error import Scapy_Exception - +import struct # Typing imports from typing import ( Optional, @@ -29,6 +18,16 @@ cast, ) +from scapy.compat import chb, orb +from scapy.config import conf +from scapy.error import Scapy_Exception +from scapy.fields import BitField, FlagsField, StrLenField, \ + ThreeBytesField, XBitField, ConditionalField, \ + BitEnumField, ByteField, XByteField, BitFieldLenField, StrField, \ + FieldLenField, IntField, ShortField +from scapy.layers.can import CAN, CAN_FD_MAX_DLEN as CAN_FD_MAX_DLEN, CANFD +from scapy.packet import Packet + log_isotp = logging.getLogger("scapy.contrib.isotp") CAN_MAX_IDENTIFIER = (1 << 29) - 1 # Maximum 29-bit identifier @@ -103,6 +102,7 @@ def fragment(self, *args, **kargs): """ fd = kargs.pop("fd", False) + pkt_cls = CANFD if fd else CAN def _get_data_len(): # type: () -> int @@ -122,10 +122,10 @@ def _get_data_len(): frame_data = struct.pack('B', self.rx_ext_address) + frame_data if self.rx_id is None or self.rx_id <= 0x7ff: - pkt = CAN(identifier=self.rx_id, data=frame_data) + pkt = pkt_cls(identifier=self.rx_id, data=frame_data) else: - pkt = CAN(identifier=self.rx_id, flags="extended", - data=frame_data) + pkt = pkt_cls(identifier=self.rx_id, flags="extended", + data=frame_data) return [pkt] # Construct the first frame @@ -138,10 +138,10 @@ def _get_data_len(): idx = _get_data_len() - len(frame_header) frame_data = self.data[0:idx] if self.rx_id is None or self.rx_id <= 0x7ff: - frame = CAN(identifier=self.rx_id, data=frame_header + frame_data) + frame = pkt_cls(identifier=self.rx_id, data=frame_header + frame_data) else: - frame = CAN(identifier=self.rx_id, flags="extended", - data=frame_header + frame_data) + frame = pkt_cls(identifier=self.rx_id, flags="extended", + data=frame_header + frame_data) # Construct consecutive frames n = 1 @@ -156,10 +156,10 @@ def _get_data_len(): if self.rx_ext_address: frame_header = struct.pack('B', self.rx_ext_address) + frame_header # noqa: E501 if self.rx_id is None or self.rx_id <= 0x7ff: - pkt = CAN(identifier=self.rx_id, data=frame_header + frame_data) # noqa: E501 + pkt = pkt_cls(identifier=self.rx_id, data=frame_header + frame_data) # noqa: E501 else: - pkt = CAN(identifier=self.rx_id, flags="extended", - data=frame_header + frame_data) + pkt = pkt_cls(identifier=self.rx_id, flags="extended", + data=frame_header + frame_data) pkts.append(pkt) return cast(List[Packet], pkts) diff --git a/test/contrib/isotp_soft_socket.uts b/test/contrib/isotp_soft_socket.uts index ece7ef5f621..bb641d4b7dc 100644 --- a/test/contrib/isotp_soft_socket.uts +++ b/test/contrib/isotp_soft_socket.uts @@ -138,7 +138,7 @@ with TestSocket(CANFD) as cans, TestSocket(CANFD) as stim, ISOTPSoftSocket(cans, s.failure_analysis() raise Scapy_Exception("ERROR") msg = pkts[0] - assert msg.data == dhex(data_str) + assert dhex(data_str) in msg.data = Two frame receive @@ -235,7 +235,8 @@ def testfd(): cfs = stim.sniff(timeout=20, count=len(fragments) - 1, started_callback=lambda: stim.send(ack)) for fragment, cf in zip(fragments, ff + cfs): - assert (bytes(fragment) == bytes(cf)) + print(bytes(fragment), bytes(cf)) + assert (bytes(fragment) in bytes(cf)) testfd() @@ -370,7 +371,7 @@ with TestSocket(CANFD) as isocan, ISOTPSoftSocket(isocan, tx_id=0x641, rx_id=0x2 assert can[0].data == dhex("10 {} {}".format("%02X" % size_to_send, " ".join(["%02X" % x for x in range(max_pl_size)]))) can = cans.sniff(timeout=1, count=1, started_callback=lambda: cans.send(CANFD(identifier = 0x241, data=dhex("30 00 00")))) assert can[0].identifier == 0x641 - assert can[0].data == dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) + assert dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) in can[0].data = Send single frame ISOTP message with TestSocket(CAN) as cans, TestSocket(CAN) as isocan, ISOTPSoftSocket(isocan, tx_id=0x641, rx_id=0x241) as s: @@ -468,7 +469,7 @@ with TestSocket(CANFD) as isocan, ISOTPSoftSocket(isocan, tx_id=0x641, rx_id=0x2 raise Scapy_Exception("ERROR") can = pkts[0] assert can.identifier == 0x641 - assert can.data == dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) + assert dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) in can.data thread.join(15) acks.close() @@ -557,7 +558,7 @@ with TestSocket(CANFD) as isocan, ISOTPSoftSocket(isocan, tx_id=0x641, rx_id=0x2 raise Scapy_Exception("ERROR") can = pkts[0] assert can.identifier == 0x641 - assert can.data == dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) + assert dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) in can.data thread.join(15) acks.close() @@ -644,7 +645,7 @@ with TestSocket(CANFD) as isocan, ISOTPSoftSocket(isocan, tx_id=0x641, rx_id=0x2 raise Scapy_Exception("ERROR") can = pkts[0] assert can.identifier == 0x641 - assert can.data == dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) + assert dhex("21 {}".format(" ".join(["%02X" % x for x in range(max_pl_size, size_to_send)]))) in can.data thread.join(15) acks.close() @@ -943,6 +944,20 @@ assert rx == msg assert rx2 is not None assert rx2 == msg += ISOTPSoftSocket sr1 timeout +msg = ISOTP(b'\x11\x22\x33\x11\x22\x33\x11\x22\x33\x11\x22\x33') + +with TestSocket(CAN) as isocan_tx, ISOTPSoftSocket(isocan_tx, 0x123, 0x321) as sock_tx, \ + TestSocket(CAN) as isocan_rx, ISOTPSoftSocket(isocan_rx, 0x321, 0x123) as sock_rx: + isocan_rx.pair(isocan_tx) + sniffer = AsyncSniffer(opened_socket=sock_rx, timeout=1, count=1, prn=lambda x: sock_rx.send(msg)) + sniffer.start() + time.sleep(1) + sniffer.join(timeout=1) + rx = sniffer.results + +assert len(rx) == 0 + = ISOTPSoftSocket sniff msg = ISOTP(b'\x11\x22\x33\x11\x22\x33\x11\x22\x33\x11\x22\x33') From 57a2f6a7b7d59e947212634d02fad35d1bef4497 Mon Sep 17 00:00:00 2001 From: Nils Weiss Date: Thu, 9 Jan 2025 20:46:23 +0100 Subject: [PATCH 3/5] fix unit tests --- scapy/contrib/isotp/isotp_soft_socket.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scapy/contrib/isotp/isotp_soft_socket.py b/scapy/contrib/isotp/isotp_soft_socket.py index 09780a0fd68..a04d98c671b 100644 --- a/scapy/contrib/isotp/isotp_soft_socket.py +++ b/scapy/contrib/isotp/isotp_soft_socket.py @@ -577,10 +577,7 @@ def _get_padding_size(pl_size): return fd_accepted_sizes[-1] return fd_accepted_sizes[pos] - if self.fd: - pkt_cls = CANFD - else: - pkt_cls = CAN + pkt_cls = CANFD if self.fd else CAN if self.padding: load += b"\xCC" * (_get_padding_size(len(load)) - len(load)) From 5cd1d6a473757fcd7f135dff9facdf92d8d68c7b Mon Sep 17 00:00:00 2001 From: Nils Weiss Date: Wed, 15 Jan 2025 08:34:32 +0100 Subject: [PATCH 4/5] update unit test and fix bug --- scapy/contrib/isotp/isotp_soft_socket.py | 1 + test/contrib/isotp_soft_socket.uts | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/scapy/contrib/isotp/isotp_soft_socket.py b/scapy/contrib/isotp/isotp_soft_socket.py index a04d98c671b..90bde267828 100644 --- a/scapy/contrib/isotp/isotp_soft_socket.py +++ b/scapy/contrib/isotp/isotp_soft_socket.py @@ -209,6 +209,7 @@ def select(sockets, remain=None): """ obj_pipes = [x.impl.rx_queue for x in sockets if isinstance(x, ISOTPSoftSocket) and not x.closed] + obj_pipes += [x for x in sockets if isinstance(x, ObjectPipe) and not x.closed] ready_pipes = select_objects(obj_pipes, remain) diff --git a/test/contrib/isotp_soft_socket.uts b/test/contrib/isotp_soft_socket.uts index bb641d4b7dc..2e7bdfaeccf 100644 --- a/test/contrib/isotp_soft_socket.uts +++ b/test/contrib/isotp_soft_socket.uts @@ -950,13 +950,9 @@ msg = ISOTP(b'\x11\x22\x33\x11\x22\x33\x11\x22\x33\x11\x22\x33') with TestSocket(CAN) as isocan_tx, ISOTPSoftSocket(isocan_tx, 0x123, 0x321) as sock_tx, \ TestSocket(CAN) as isocan_rx, ISOTPSoftSocket(isocan_rx, 0x321, 0x123) as sock_rx: isocan_rx.pair(isocan_tx) - sniffer = AsyncSniffer(opened_socket=sock_rx, timeout=1, count=1, prn=lambda x: sock_rx.send(msg)) - sniffer.start() - time.sleep(1) - sniffer.join(timeout=1) - rx = sniffer.results + rx2 = sock_tx.sr1(msg, timeout=1, verbose=True) -assert len(rx) == 0 +assert rx2 is None = ISOTPSoftSocket sniff From b8e334ff28cabcb853ec869a0ba5ba84ffd5abf8 Mon Sep 17 00:00:00 2001 From: Nils Weiss Date: Wed, 15 Jan 2025 08:43:56 +0100 Subject: [PATCH 5/5] fix flake and mypy --- scapy/contrib/isotp/isotp_soft_socket.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scapy/contrib/isotp/isotp_soft_socket.py b/scapy/contrib/isotp/isotp_soft_socket.py index 90bde267828..4182d445336 100644 --- a/scapy/contrib/isotp/isotp_soft_socket.py +++ b/scapy/contrib/isotp/isotp_soft_socket.py @@ -207,8 +207,9 @@ def select(sockets, remain=None): """This function is called during sendrecv() routine to wait for sockets to be ready to receive """ - obj_pipes = [x.impl.rx_queue for x in sockets if - isinstance(x, ISOTPSoftSocket) and not x.closed] + obj_pipes: List[Union[SuperSocket, ObjectPipe[Tuple[bytes, Union[float, EDecimal]]]]] = [ # noqa: E501 + x.impl.rx_queue for x in sockets if + isinstance(x, ISOTPSoftSocket) and not x.closed] obj_pipes += [x for x in sockets if isinstance(x, ObjectPipe) and not x.closed] ready_pipes = select_objects(obj_pipes, remain)