Skip to content

Conversation

T-recks
Copy link

@T-recks T-recks commented Aug 20, 2025

This PR adds two new layers to scapy/contrib, implementing the PDUs for the following Delay Tolerant Networking (DTN) protocols: TCP Convergence Layer version 4 (RFC 9174) and Bundle Protocol version 7 (RFC 9171) with support for BPSec (RFC 9172). These protocols are fundamental to the DTN suite and included in nearly all implementations of DTN, which is widely used by NASA, ESA, etc. for experimental purposes and research on internetworking in space communications.

Unit tests are currently not included in this PR but the code has been unit tested in NASA's dtn-test-framework repository, which depends on these two layers and is used internally at NASA for formal software requirements verification. If required by the maintainers, I am willing to modify and import these unit tests into Scapy.

The implementation of these layers depends on two Python libraries: crcmod for computing checksums and flynn for decoding CBOR. I added some lines to the pyproject.toml to provide these as optional dependencies. The layers are implemented in the scapy.contrib.dtn directory and are otherwise self-contained. Please let me know if these optional dependencies or methods of organizing the code are not acceptable to the maintainers...

@T-recks T-recks changed the title Hdtn New BPv7 and TCPCL layers Aug 20, 2025
@T-recks T-recks changed the title New BPv7 and TCPCL layers Add BPv7 and TCPCL layers Aug 20, 2025
@gpotter2
Copy link
Member

Hi there, thanks for your interest in Scapy !

Two quick things before I start a deeper review :

  • For very specific use cases, we typically only use runtime dependencies (that prompt an error when trying to import the layer), rather than adding them to pyproject.toml
  • Having a layer with no unit tests for a very specific protocol basically dooms it to breaking one day. While I understand that it really isn't fun to write them, we impose at least a few unit tests for new layers (dissecting a few packets, building a few packets and checking that the output is the same, etc.).

@gpotter2
Copy link
Member

Also do not hesitate to run black on your files.

Copy link

codecov bot commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 76.78832% with 159 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.87%. Comparing base (449142a) to head (4812c62).

Files with missing lines Patch % Lines
scapy/contrib/dtn/bpv7.py 68.88% 103 Missing ⚠️
scapy/contrib/dtn/cbor.py 71.08% 48 Missing ⚠️
scapy/contrib/dtn/tcpcl.py 95.91% 4 Missing ⚠️
scapy/contrib/dtn/tcpcl_session.py 95.71% 3 Missing ⚠️
scapy/contrib/dtn/common.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4824      +/-   ##
==========================================
- Coverage   80.91%   80.87%   -0.04%     
==========================================
  Files         366      371       +5     
  Lines       90130    90815     +685     
==========================================
+ Hits        72927    73451     +524     
- Misses      17203    17364     +161     
Files with missing lines Coverage Δ
scapy/contrib/dtn/common.py 95.00% <95.00%> (ø)
scapy/contrib/dtn/tcpcl_session.py 95.71% <95.71%> (ø)
scapy/contrib/dtn/tcpcl.py 95.91% <95.91%> (ø)
scapy/contrib/dtn/cbor.py 71.08% <71.08%> (ø)
scapy/contrib/dtn/bpv7.py 68.88% <68.88%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@T-recks
Copy link
Author

T-recks commented Aug 22, 2025

I removed the extra lines from pyproject.toml so it will just raise an import error e.g. if you evaluate import scapy.contrib.dtn.bpv7 when flynn or crcmod is not available.

I am reading the UTScapy documentation and will update the PR with unit tests when I figure everything out...

@T-recks T-recks marked this pull request as draft August 23, 2025 19:10
@gpotter2
Copy link
Member

Btw you can add dependencies for the tests over here

scapy/tox.ini

Line 35 in 449142a

python-can
!

@T-recks
Copy link
Author

T-recks commented Aug 24, 2025

I added unit tests. I believe the runtime dependency import errors should be fixed now for the CI checks, and also the formatting / code quality checks and SPDX checks.

@T-recks
Copy link
Author

T-recks commented Aug 24, 2025

One unit test is failing on Windows. Not sure why... appears to be a module unrelated to my PR. Fixed in latest run

@T-recks T-recks marked this pull request as ready for review August 24, 2025 21:47
return crcfun(pkt).to_bytes(size, "big"), crc_index


class PacketFieldWithRemain(PacketField):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see why you did that, but this isn't how we usually do it.

PacketField returns the remaining bytes only when they are contained in a Padding layer. Typically you just have to add the following to the inner packet (which is contained in a PacketField)

def default_payload_class(self):
    return conf.padding_layer

This isn't super intuitive, but you'll find many examples in Scapy.

def default_payload_class(self, _):
return conf.padding_layer

Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi ! Sorry for the delay of reviewing !
It's looking pretty good although I have a few suggestions

),
]

def dissect(self, s: bytes):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why you need this? Thanks

def __str__(self):
return f"ipn:{self.node_id}.{self.service_number}" # noqa: E231

def __eq__(self, other):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expected this to work without __eq__ :/

return self.cls(decoded_m + remain)


class CBORPacketFieldWithRemain(CBORPacketField):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, I think this could be worked around

from typing import List


class Session:
Copy link
Member

@gpotter2 gpotter2 Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this could be implemented using Scapy's "session", which allows users to have their own prn while still performing actions when receiving a packet in a sniff, rdpcap or other packet source.

There's a bit of documentation here
https://scapy.readthedocs.io/en/latest/usage.html#advanced-sniffing-sniffing-sessions
and you can have a look at the source https://github.com/secdev/scapy/blob/master/scapy/sessions.py

Typically you can extend IPSession if you want it to handle IP fragmentation, or TCPSession (with a few tricks) to handle TCP reassembly. Feel free to ask if anything's unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants