automotive, j1939: scanning for CAs by BenGardiner · Pull Request #5164 · secdev/scapy

BenGardiner · 2026-09-10T15:32:44Z

Description

This adds j1939 scanning for Controller Applications on-top-of the soft socket support.

The changes aim to introduce only those J1939 value enumeration definitions which can be sourced from freely available locations on the internet. As such, there is not a complete list of the values.

I don't intend any impacts on other parts of the libraries.

fixes #xxx

LLM coding tools were used in the development of this PR: copilot and gemini, various models.

AI-Assisted: yes (GitHub Copilot)

AI-Assisted: no

AI-Assisted: yes (Claude Sonnet 4.6)

AI-Assisted: no

…ostile transfers The soft socket handled the transfers its own tests produce, all of them under 100 bytes from a single well-behaved peer. Everything past that was either dropped or actively harmful, and none of it was visible because no test exceeded one CTS block, ran two senders, or sent a malformed frame. A payload of more than 1785 bytes needs more TP.DT packets than a sequence number can express, so building the announcement raised inside the scheduler thread after the TX state had already been set. The message vanished with a log line and the state machine stayed latched, which silently discarded every later send on that socket. send() now refuses such a payload, _J1939_TP_MAX_DATA finally being used for what it was defined for, and a failure anywhere in _begin_send resets the machine instead of wedging it. Reception was a single set of rx_* attributes, so a second announcement threw away the transfer in progress. Since a busy J1939 bus has several ECUs broadcasting at once, a monitor built on this socket lost most of what it saw. Sessions now live in a dict keyed by the (source address, destination) pair the protocol itself uses, capped so a hostile bus cannot grow it without bound, and a peer that asks for a second PGN while one is running is refused with an abort rather than displacing it. Frames from the bus are no longer taken at face value. An announcement whose size and packet count cannot describe a message is refused instead of delivering an empty payload; a CTS naming a packet outside the message is aborted instead of indexing the buffer backwards and emitting sequence number 0; and CTS, acknowledgement and abort frames must now name the PGN of the session they claim to be part of. tx_peer_sa is cleared when a session ends, so a node that took part in an earlier transfer can no longer abort an unrelated broadcast. The rest are smaller: the receiver honours the max_packets of a request and issues a CTS per block instead of authorising everything at once, close() derives its drain budget from what is left to send rather than truncating any broadcast longer than two seconds while __del__ no longer drains at all, basecls is used for delivered messages and recv_raw returns the payload, transport frames carry the caller's priority as single frames already did, a source address of 0xFF warns because it cannot legally appear on the wire, and a CAN socket that goes away closes the J1939 socket instead of leaving a caller blocked in recv() forever. The twelve new cases in the campaign each fail on the code before this commit and pass after it. The existing 185 are untouched and still pass. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

Four things the first pass got wrong or left rough. send() measured the payload differently from the code that transmits it, so the size guard and the wire could disagree for a message whose data was not bytes; both now go through one helper. The check for a CAN socket that has gone away was written twice in can_recv, once at each end. The new basecls parameter and the per-peer session model were undocumented. And the priority a caller asks for now reaches the TP.CM and TP.DT frames of a multi-packet message, which is a deliberate change of default from the 7 the code used to hardcode for TP.DT, so the docstring says so. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

… hid Four handlers cancelled a scheduler timeout inside try/except/pass, which Codacy flags and which hides a real failure as readily as the expected one. The expected one is narrow: TimeoutScheduler raises Scapy_Exception when a timeout has already fired or been cancelled, which races normally against the state machine dropping it. One _cancel helper now does that in the five places that needed it, logging anything else at debug level, and send() sets sent_time behind an isinstance check rather than catching the AttributeError a non-packet would raise. Building a connection abort was written out five times and refusing a session three times; both are helpers now, which is what made it obvious that the check for a TP.DT past the authorised block can never fire: the next CTS is sent from the same handler that completes a block, so the window it guards does not exist. Writing the test for it is what showed that, and both the branch and the test are gone. The four new cases cover what had no test: the session table filling up and answering with 'system resources', a stalled reception aborting with 'timeout' once its wall-clock ceiling passes, a sender aborting a reception it started, and close(timeout=0) as the way to give up on a transfer on purpose. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

Propagate adapter send failures so TX resets instead of advancing past a frame that never left the host, accept EOM ACK only after every DT was sent with matching size/count, and enforce BAM vs directed control destination rules. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

Drain the CAN hardware RX FIFO at startup and close, poll with zero delay while TP is active, reject new sends while closing, preserve received TP priority, and reassemble into a bytearray. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

Use J1939_NO_PGN as the accept-all sentinel so real PGN 0 is filterable, drop dead TX state, reuse from_can, narrow recv closure handling, and make the portable soft-socket tests not inherit a campaign linux tag. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (GitHub Copilot) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Guarantee _J1939_TP_T1/_J1939_TP_T2 are restored if the inactivity-timeout case fails mid-run, so later tests cannot inherit shortened deadlines. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

Reject J1939SoftSocket(can_socket=None) with Scapy_Exception, assert vcan setup succeeded, and close NativeCANSocket adapters after soft / native interop cases so later vcan tests do not inherit leaked FDs. AI-Assisted: yes (Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

BenGardiner · 2026-09-10T15:33:45Z

@polybassa

polybassa · 2026-09-10T16:01:16Z

+    MANUFACTURERS,
+    J1939NameDecoder,
+    J1939_NAME,
+    decode_j1939_name,


Could you unify this to j1939_decode_name

I'm happy to comply. But I don't think I grok what the change requested is.

You want me to rename decode_j1939_name -> j1939_decode_name ?

polybassa · 2026-09-10T16:01:48Z

+    pgn_from_fields,
+    dst_from_fields,
+    pgn_is_pdu1,
+    log_j1939,


polybassa · 2026-09-10T16:04:21Z

+        # Declared in big-endian (MSB-first) order for BitField processing.
+        # do_dissect / do_build reverse the 4 bytes to convert between
+        # J1939 little-endian wire format and Scapy's big-endian BitField.
+        BitField("OC", 0, 7),  # bits 31-25 (MSB side)


If necessary, bit fields can decode LE as well by making the size negative

polybassa · 2026-09-10T16:04:48Z

+    def do_dissect(self, s):
+        # type: (bytes) -> bytes
+        """Dissect a 4-byte LE DTC from *s*; return remaining bytes."""
+        if len(s) >= 4:


This schould go away, then

polybassa · 2026-09-10T16:05:00Z

+            return s[4:]
+        return b""
+
+    def do_build(self):


polybassa · 2026-09-10T16:07:37Z

+        BitEnumField("pl_flash", 3, 2, _LAMP_STATUS),
+    ]
+
+    def __init__(self, *args, **kwargs):


Remove and use a packetListField instead

polybassa · 2026-09-10T16:07:59Z

+        # Trailing bytes shorter than a full DTC (< 4 bytes) are treated as
+        # 0xFF padding and silently ignored, per J1939-21 single-frame rules.
+        self.dtcs = []
+        while len(remain) >= 4:


PacketListField

polybassa · 2026-09-10T16:08:55Z

+        dtc_bytes = b"".join(bytes(dtc) for dtc in self.dtcs)
+        result = lamp_bytes + dtc_bytes
+        if len(result) < 8:
+            result += b"\xff" * (8 - len(result))


Why is this necessary?

This can become a StrFixedLenField("padding",.... )

polybassa · 2026-09-10T16:10:28Z

+        # type: (bytes) -> Tuple[bytes, bytes]
+        return b"", s
+
+    def __repr__(self):


Is it really necessary to overwrite repr?

no. I shouldn't do that here

polybassa · 2026-09-10T16:12:14Z

+    #: PGN for DM13 Stop/Start Broadcast
+    PGN = PGN_DM13
+
+    _hold_signal_enum = {0xFE: "start", 0xFF: "stop"}


polybassa · 2026-09-10T16:24:21Z

+
+
+def sniff_dm1(
+    interface="can0",  # type: str


Maybe it would be more useful to provide a socket here, than the user can specific which kind of socket he wants to use, and we can save the creation and destruction overhead

polybassa · 2026-09-10T16:27:37Z

+# --- Result container
+
+
+class DmScanResult(object):


I recommend to use a dataclass here

polybassa · 2026-09-10T16:43:14Z

+    def do_dissect(self, s: bytes) -> bytes:
+        """Dissect 8 LE bytes into J1939_NAME bitfields (reading LSB first)."""
+        if len(s) >= 8:
+            super(J1939_NAME, self).do_dissect(s[:8][::-1])


Same here, use LE bit fields

polybassa · 2026-09-10T16:57:02Z

+        else:
+            func_desc = f"Reserved State ({func})"
+
+        return {


What about using a dataclass here as well? This would remove the index accesses later in the code

polybassa · 2026-09-10T16:58:32Z

+#: Subfunction 0x00 asks for a response; 0x01 suppresses it (but some ECUs
+#: respond anyway, confirming UDS support).
+_UDS_TESTER_PRESENT_REQS = [
+    b"\x02\x3e\x00\xff\xff\xff\xff\xff",


Use the UDS layer here, maybe via lazy importing

polybassa · 2026-09-10T16:59:16Z

+
+#: XCP CONNECT command payload: command byte 0xFF, mode 0x00 (normal connection),
+#: followed by 6 padding bytes (0xFF) to fill an 8-byte CAN frame.
+_XCP_CONNECT_REQ = b"\xff\x00\xff\xff\xff\xff\xff\xff"


Use the existing xcp definitions

polybassa · 2026-09-10T17:00:47Z

+    # type: (SuperSocket) -> None
+    """Flush the kernel CAN receive buffer before sending a probe.
+
+    On :class:`~scapy.contrib.cansocket_python_can.PythonCANSocket` the


To much comment IMHO

polybassa · 2026-09-10T17:01:27Z

+#: CAN Extended Frame Format flag for socketcan ``CAN_RAW_FILTER`` entries.
+#: Set in the ``can_id`` field of ``struct can_filter`` so the kernel matches
+#: only 29-bit extended identifiers.  Value equals ``socket.CAN_EFF_FLAG``.
+_SOCKETCAN_EFF_FLAG = 0x80000000


This schould already exist as definition

polybassa · 2026-09-10T17:47:15Z

+        return sock, False
+    try:
+        from scapy.contrib.cansocket_native import NativeCANSocket
+        if not isinstance(sock, NativeCANSocket):


Just use CANSocket, then it stays generic

polybassa · 2026-09-10T17:48:34Z

+              *send_sock* is **never** closed by the caller.
+    """
+    if callable(sock_or_factory):
+        probe = sock_or_factory()


How is this SockorFactory defined?

sock_or_factory is a either a CANSocket or a function returning a (newly created) CANSocket

BenGardiner · 2026-09-11T12:53:18Z

thanks @polybassa for the review. I can do almost all of that right now. There's a couple things that are either answering your questions or require me to ask you questions first.

Nils Weiss and others added 20 commits September 8, 2026 14:39

Implement J1939 Soft Socket for SAE J1939 Transport Protocol in Python

d833043

AI-Assisted: yes (GitHub Copilot)

Add debug logging for J1939SoftSocket operations and packet handling

e0626fe

AI-Assisted: yes (GitHub Copilot)

Add debug logging for J1939SoftSocket operations and packet handling

90570fe

AI-Assisted: no

Fix Unit-Tests

e59199f

AI-Assisted: yes (Claude Sonnet 4.6)

Fix Codacy

3414c3b

AI-Assisted: no

Import SuperSocket from scapy.supersocket

cfec25e

AI-Assisted: yes (GitHub Copilot) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'master' into j1939_soft_sockets

d3c482a

automotive: J1939 Diagnostic Messages (J1939-73) and DM scanner

8347e30

automotive: J1939 Controller Application (CA) enumeration scanner

c2063fa

automotive: J1939 package __init__ integration with scapy.contrib.j1939

9917b2c

automotive: J1939 64-bit NAME decoder and protocol support (J1939-81)

93ffd77

Merge branch 'master' into j1939-soft-sockets-again

fbcb5a9

polybassa requested changes Sep 10, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

BenGardiner commented Sep 10, 2026

Description

Uh oh!

BenGardiner commented Sep 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenGardiner commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants