Skip to content

cbor: Scapy-native fields, codec hardening, and cbor2 CI - #5125

Open
polybassa wants to merge 80 commits into
secdev:masterfrom
polybassa:split/cbor-scapy-native
Open

polybassa wants to merge 80 commits into
secdev:masterfrom
polybassa:split/cbor-scapy-native

Conversation

@polybassa

Copy link
Copy Markdown
Contributor

Summary

  • Rewrite CBOR packet/field APIs toward Scapy ASN.1-style conventions
  • Harden maps, floats, optionals, and raw-cache fidelity
  • Add pinned cbor2 differential coverage behind isolated tox/CI

Stack

PR 2 of 3. Depends on #5124 (CRC presets). Reviewable CBOR-only commits sit on top of that foundation; next is BPv7 contrib.

Test plan

  • ./test/run_tests -t test/scapy/layers/cbor.uts -N
  • tox -e cbor2 (Python 3.12)
  • flake8 / mypy on touched paths

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.24731% with 293 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.42%. Comparing base (b38b2e3) to head (d591ee0).

Files with missing lines Patch % Lines
scapy/cbor/cborfields.py 82.99% 177 Missing ⚠️
scapy/cbor/cborcodec.py 84.14% 72 Missing ⚠️
scapy/cbor/cbor.py 85.25% 41 Missing ⚠️
scapy/cborpacket.py 95.58% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5125      +/-   ##
==========================================
+ Coverage   80.35%   80.42%   +0.07%     
==========================================
  Files         374      374              
  Lines       97578    98979    +1401     
==========================================
+ Hits        78411    79608    +1197     
- Misses      19167    19371     +204     
Files with missing lines Coverage Δ
scapy/cbor/__init__.py 100.00% <ø> (ø)
scapy/packet.py 85.01% <100.00%> (+0.21%) ⬆️
scapy/utils.py 73.64% <100.00%> (+0.05%) ⬆️
scapy/cborpacket.py 96.25% <95.58%> (+7.36%) ⬆️
scapy/cbor/cbor.py 85.42% <85.25%> (+1.87%) ⬆️
scapy/cbor/cborcodec.py 84.99% <84.14%> (-2.75%) ⬇️
scapy/cbor/cborfields.py 83.13% <82.99%> (+5.33%) ⬆️

... and 8 files with indirect coverage changes

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

@polybassa
polybassa force-pushed the split/cbor-scapy-native branch from 5f9b34d to 414ac55 Compare September 1, 2026 11:57
@polybassa
polybassa requested review from gpotter2 and a lite review from Copilot September 1, 2026 12:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It makes wide-reaching changes across CBOR core encoding/decoding and packet cache semantics, which warrants careful human validation beyond static review.

Pull request overview

This PR refactors Scapy’s CBOR support toward Scapy-native field conventions (Python-native values, ASN.1-like hooks), strengthens codec correctness (maps, floats, optionals, nesting/indefinite handling, deterministic encoding), and adds an isolated cbor2-based interoperability/differential test lane in tox and CI.

Changes:

  • Rework CBOR field/container APIs to use Scapy-style any2i/i2m/m2i plus build_result/dissect_result, with improved raw-cache fidelity and parent/child ownership handling.
  • Harden CBOR codec behavior (indefinite containers, duplicate keys, preferred float widths, deterministic map ordering, max nesting) and introduce CBOR map pair preservation (CBORMapData).
  • Add pinned cbor2==6.1.4 differential UTScapy campaign plus a dedicated tox env and GitHub Actions job.
File summaries
File Description
tox.ini Adds a dedicated cbor2 tox env for pinned differential tests; removes cbor2 from main deps.
test/scapy/layers/requirements-cbor2.txt Documents/pins optional cbor2 dependency for interop testing.
test/scapy/layers/generate_cbor2_corpus.py Adds helper script to generate reproducible CBOR corpora using cbor2.
test/scapy/layers/cbor_cbor2_interop.uts Adds extensive cbor2-vs-Scapy differential/interoperability coverage.
test/fields.uts Adds a regression test around ConditionalField.__getattr__.
test/configs/windows2.utsc Excludes the cbor2 interop campaign from default Windows2 runs and marks keyword.
test/configs/windows.utsc Excludes the cbor2 interop campaign from default Windows runs and marks keyword.
test/configs/solaris.utsc Excludes the cbor2 interop campaign from default Solaris runs and marks keyword.
test/configs/linux.utsc Excludes the cbor2 interop campaign from default Linux runs and marks keyword.
test/configs/bsd.utsc Excludes the cbor2 interop campaign from default BSD runs and marks keyword.
scapy/cborpacket.py Adds CBOR-specific packet cache integration, default-copy deepening, and parent reattachment on copy.
scapy/cbor/cborfields.py Major CBOR field API rewrite (native values, optional/conditional semantics, sequences, deterministic map schema behavior).
scapy/cbor/cborcodec.py Hardens low-level codec: nesting limit, indefinite support, strictness checks, float width preference, deterministic encoding helpers.
scapy/cbor/cbor.py Introduces CBORMapData, richer equality/hash semantics, float encoding retention, and native wrapper classes.
scapy/cbor/__init__.py Exposes new CBOR types/fields/results in the public API surface.
README.md Fixes packaging status badge markdown formatting.
.github/workflows/unittests.yml Adds a CI job running tox -e cbor2 on Python 3.12.
Review details
  • Files reviewed: 17/18 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tox.ini
Comment thread test/fields.uts Outdated
polybassa pushed a commit to polybassa/scapy-1 that referenced this pull request Sep 9, 2026
Capture map-key equivalence, deterministic floats, hash/tag field
semantics, and multi-map unknown ownership before changing the
implementation.

Co-authored-by: Cursor <cursoragent@cursor.com>
@polybassa
polybassa force-pushed the split/cbor-scapy-native branch from 414ac55 to 4a552ff Compare September 9, 2026 05:01
polybassa pushed a commit to polybassa/scapy-1 that referenced this pull request Sep 9, 2026
Capture map-key equivalence, deterministic floats, hash/tag field
semantics, and multi-map unknown ownership before changing the
implementation.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
@polybassa
polybassa force-pushed the split/cbor-scapy-native branch from 4a552ff to f9e6104 Compare September 9, 2026 06:32
polybassa pushed a commit to polybassa/scapy-1 that referenced this pull request Sep 10, 2026
Capture map-key equivalence, deterministic floats, hash/tag field
semantics, and multi-map unknown ownership before changing the
implementation.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
@polybassa
polybassa force-pushed the split/cbor-scapy-native branch from b433d7e to 06a065b Compare September 10, 2026 06:30
@polybassa

Copy link
Copy Markdown
Contributor Author

@gpotter2 I've done with a lot of refactorings and a manual review; Could you please have a look?

Nils Weiss and others added 15 commits September 14, 2026 08:41
Rewrite the CBOR packet/field layer toward ASN.1-style Scapy APIs,
harden maps/floats/optionals, and add pinned cbor2 differential coverage
behind an isolated tox/CI job.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
Rename CBORF_PACKET/SEQUENCE_OF/ARRAY_OF constructor kwargs to
pkt_cls so typing.Generic.__new__ does not collide on 3.7, and drop
the BPv7-only BundleEidField unit from the CBOR campaign.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
Capture map-key equivalence, deterministic floats, hash/tag field
semantics, and multi-map unknown ownership before changing the
implementation.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
Use semantic CBOR key equivalence for duplicate detection and lookup,
rebuild deterministic floats from values, drop identity hashing, and
treat schema tag numbers as metadata rather than packet fields.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
Arbitrary CBOR values use the lossless object tree; typed floats stay
plain Python floats with wire fidelity via the packet raw cache.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
Replace packet-global _cbor_unknown_map_pairs with a dedicated mutable
field on each CBORF_MAP so nested and sibling maps keep independent
extensions across copy and rebuild.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
Unknown map state now lives in normal fields, so CBOR packets can use
Scapy's default build path. Keep only parent reattachment in copy().

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
Reject nonterminal unbounded SEQUENCE_OF, support count_from/max_count,
and replace the 1<<30 sentinel with conf.max_list_count.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
…ing packets

Default deterministic scanning rejects indefinite containers, drop internal
result types from the public surface, and simplify BYTE_STRING_PACKET decode.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
Use CBOR_FLOAT, CBOR_SEMANTIC_TAG, CBOR_SIMPLE_VALUE, and a CBOR_UNDEFINED
singleton directly in the object model and update tests accordingly.

Co-authored-by: Cursor <cursoragent@cursor.com>

AI-Assisted: yes (Composer)
RFC 8949 map-key equivalence for floats must not collapse every NaN.
Identity now uses sign plus a width-normalized significand, preferring
CBOR_FLOAT wire bytes when available, while +0.0 and -0.0 still match.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Share NaN component and preferred-width helpers between the
deterministic validator and encoder so CBOR_FLOAT NaNs shorten without
losing payload identity, including unknown-map rebuilds.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Remove unused CBORMapData identity helpers, document equivalence-based
lookup, delete the ConditionalField getsource test, and restore the
unrelated README packaging badge.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Decoded floats keep _encoded until mutated. Clearing it on val
assignment prevents rebuilds from emitting stale float bytes.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Unwrap transparent wrappers before the terminal-sequence schema check so
optional/conditional nesting cannot hide an ambiguous unbounded SEQUENCE_OF.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Nils Weiss and others added 16 commits September 14, 2026 08:41
Promoting ismutable FlagValue defaults into fields shadowed
overloaded_fields, so NBNSHeader built NM_FLAGS=0 and NBNS_am failed.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the CBOR_Packet counted-build fast path so nested PACKET encode
includes post_build/payload, and route ANY through public build() like
NULL/UNDEFINED.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Reuse BYTE_STRING decode for packet wrappers, mark only optionals
absent, drop an unreachable budget raise, override
_raw_packet_cache_is_valid, and dedupe half-float / float-key paths.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
* cbor: reserve structural mins until discriminators are ready

Suffix budgeting uses structural_min_items so default count_from and
conditionals cannot starve leading fields; live mins apply once string
count_from sources are in pkt.fields.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

* cbor: stream unframed SEQUENCE without trailing pre-count

Stop after schema fields so counted SEQUENCE_OF leaves valid, bare
break, and truncated suffixes unchanged; framed ARRAY budgets stay strict.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

* cbor: propagate holds_packets from packet-valued item fields

ARRAY_OF/SEQUENCE_OF with CBORF_PACKET elements register as packet
storage and reuse nested fingerprints while encode/decode still follow
item_field versus packet-class selection.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

* cbor: drop unused packet-level cardinality cache

Remove _cbor_build_counted and _cbor_raw_cache_items; nested encode
keeps exact wire via Packet raw cache and bytes(child).

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

* cbor: share byte-string wire helpers; restore Packet field typing

Extract decode/encode helpers so CBORF_BYTE_STRING_PACKET stays
CBORF_field[Packet] while reusing definite-length byte-string handling.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

* cbor: simplify compound parse budgeting

Move suffix reservation onto reserve_min_items, share field dissect
helpers, and drop the compound-side ready/budget branching.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

* cbor: fix flake8 blank lines after budget simplify

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Nils Weiss <nils.weiss@dissecto.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
regression.uts leaves conf.debug_dissector True, so the nested
constructor test must pin False/True explicitly like BYTE_STRING_PACKET.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop count_from, shrink cardinality to structural_min, isolate
ismutable defaults on CBOR_Packet, and fold encode_value into i2m.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop structural_min_items and skipped-optional validation so active
conditionals and required tails keep ownership of budgeted items.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Skip top-level CBOR_ABSENT fingerprints, require direct-terminal
SEQUENCE_OF, and drop SEQUENCE suffix lookahead.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Fail SEMANTIC_TAG(SEQUENCE_OF) at construction, let optional own
presence matching, and drop the single-use _mark_absent helper.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Use CBOR_KeyKind norms and CBOR_FingerprintKind cache tags, drop
redundant int() enum casts, half floats via struct >e, and make
EnumElement deepcopy a singleton.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Reject nested REMAINDER_OF, require usable CBOR_root for packet
classes, snapshot generic CBOR_Object fingerprints, rename
SEQUENCE/SEQUENCE_OF to ITEMS/REMAINDER_OF, and slim max_items.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Recurse remainder rejection only through unframed ITEMS, apply
conf.max_list_count to MAP, and tighten Packet default-copy coverage.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Align remainder validation and MAP limit tests with the re-review's
preferred form, including ITEMS(ARRAY(...), tail) coverage.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Cover conf.max_list_count exactly-at-limit for unknown MAP keys and
ITEMS wrapping ARRAY_INDEFINITE(REMAINDER_OF).

AI-Assisted: yes (Cursor Grok 4.6)
Co-authored-by: Cursor <cursoragent@cursor.com>
Count preserved unknown MAP members in is_empty, encode nested
ARRAY_OF fields, and reject remainder/kwargs misplacements.

AI-Assisted: yes (Cursor Grok 4.6)
Co-authored-by: Cursor <cursoragent@cursor.com>
Cover CBORF_ITEMS TypeError for unexpected keywords alongside the
existing ARRAY encode_indefinite case.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
@polybassa
polybassa force-pushed the split/cbor-scapy-native branch from 99aba66 to ead3071 Compare September 14, 2026 06:53
Nils Weiss and others added 13 commits September 14, 2026 11:23
Stop conditional suffix reservation from using undecoded
discriminators, initialize zero-budget remainders and nested
optionals, fingerprint float bit patterns, and reject nested
packet-valued homogeneous collections.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Inline unframed ITEMS into array schemas for budgeting, reserve
conditionals only after required predecessors are decoded, and
trim preferred-float helpers.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the positional between-required heuristic with reservation
based on already-decoded field names so unrelated required fields
do not suppress a known active conditional.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Put external_cbor2 on the interop campaign, assert preferred float
wire bytes, fix indefinite budget and ITEMS payload checks, and drop
the non-determinism scanner helper.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Proxy getfieldval and subclass methods on the reservation view,
propagate real predicate errors, cap indefinite min_items work
instead of requiring quadratic growth, and trim interop helpers.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the fake reservation view. Predicates run on the packet;
unread fields do not reserve from defaults. Map dissection uses
field wrappers, and UTS covers greedy optional-before-flag.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop unread-field interception. Suffix reservation uses
min_items on the real packet. Default-on trailing conditionals
over-reserve until the discriminator is dissected.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Only optionals inspect the suffix. Conditionals reserve when named
discriminators are already in pkt.fields, so default-on flags
round-trip instead of rejecting their own encodings.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
Unread default-on conditionals no longer starve a preceding optional
during suffix budgeting.

AI-Assisted: yes (Composer)
Co-authored-by: Cursor <cursoragent@cursor.com>
A skipped conditional is decided but never stored in pkt.fields, so later depends_on treated it as unread.

*This message was written with the help of AI (Cursor Grok 4.6).*

Co-authored-by: Cursor <cursoragent@cursor.com>
depends_on does not delay ConditionalField evaluation. Unknown names include the conditional field in the error.

*This message was written with the help of AI (Cursor Grok 4.6).*

Co-authored-by: Cursor <cursoragent@cursor.com>
*This message was written with the help of AI (Cursor Grok 4.6).*

Co-authored-by: Cursor <cursoragent@cursor.com>
*This message was written with the help of AI (Composer).*

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants