Skip to content

Asn1 oer uper contrib - #5050

Open
polybassa wants to merge 16 commits into
secdev:masterfrom
polybassa:asn1-oer-uper-contrib
Open

Asn1 oer uper contrib#5050
polybassa wants to merge 16 commits into
secdev:masterfrom
polybassa:asn1-oer-uper-contrib

Conversation

@polybassa

Copy link
Copy Markdown
Contributor

No description provided.

Nils Weiss added 2 commits August 7, 2026 18:31
Move OER/UPER codec implementations to scapy.contrib and wire
asn1fields for OER/PER using the pluggable tagging/kwargs hooks.

AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
@polybassa
polybassa force-pushed the asn1-oer-uper-contrib branch from e7bc1d3 to 4cdc2de Compare August 7, 2026 18:45
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.58855% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.83%. Comparing base (d8f7b27) to head (83cffbf).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
scapy/contrib/oer.py 96.19% 19 Missing ⚠️
scapy/contrib/uper.py 98.33% 12 Missing ⚠️
scapy/asn1fields.py 99.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5050      +/-   ##
==========================================
+ Coverage   80.48%   80.83%   +0.34%     
==========================================
  Files         390      392       +2     
  Lines       96810    98108    +1298     
==========================================
+ Hits        77917    79305    +1388     
+ Misses      18893    18803      -90     
Files with missing lines Coverage Δ
scapy/asn1/asn1.py 87.24% <100.00%> (+0.39%) ⬆️
scapy/asn1/ber.py 96.69% <100.00%> (+0.50%) ⬆️
scapy/asn1fields.py 95.53% <99.00%> (+7.65%) ⬆️
scapy/contrib/uper.py 98.33% <98.33%> (ø)
scapy/contrib/oer.py 96.19% <96.19%> (ø)

... and 13 files with indirect coverage changes

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

Nils Weiss added 4 commits August 8, 2026 07:11
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
Comment thread scapy/asn1/ber.py Outdated
codec = ASN1_Codecs.BER
tag = ASN1_Class_UNIVERSAL.ANY
skip_tagging = False
tagging_enc = staticmethod(BER_tagging_enc)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we removed those in the previous PR and moved them to 'self.codec'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed... now this PR is ready for review....

Nils Weiss and others added 9 commits August 10, 2026 14:33
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
A SEQUENCE with OPTIONAL/DEFAULT components was encoded without the
preamble required by X.696 16.2.2, so peers could not tell which
components were present. Fixed-size BIT STRINGs kept their length
determinant and unused-bit count, and fixed-size OCTET STRINGs of 1, 2,
4 or 8 bytes encoded without a length determinant but were decoded
expecting one, so they could be built but never parsed.

Encodings now match asn1tools byte for byte in both directions.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
An unconstrained BIT STRING counted its length determinant in octets
instead of bits and padded the content to a whole octet, which also
shifted every field encoded after it, so peers read back the wrong bits.

Content of 16K units or more emitted a fragment header and then the
whole content in one go, without the per-fragment determinants and the
terminating one required by X.691 11.9.3.8: a conformant peer silently
decoded a truncated value. Fragmentation is now implemented on both
sides for OCTET STRING, BIT STRING, OBJECT IDENTIFIER and SEQUENCE OF,
and append_length_determinant refuses lengths it cannot express rather
than clamping them.

A fixed or range constrained BIT STRING silently padded or truncated a
value whose length violated the constraint, where OER already raised.

On the OER side the integer encoder picked the width and the signedness
from the value rather than from the declared type, so 200 in a field
declared INTEGER (-128..127) encoded as 0xc8 and read back as -56, and
an unbounded value with a zero lower bound gained a spurious leading
zero octet. Out of range values now raise OER_Encoding_Error instead of
struct.error quoting the bounds of the wrong format.

Both decoders raised ValueError on an integer with an empty length
determinant, which escaped the dissector as a non-ASN.1 exception.

Encodings now match asn1tools byte for byte in both directions, over
random schemas as well as the vectors added here.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
The UPER encoder and decoder each carried an align_always method, which
is meaningless for an unaligned codec and had no caller. A family of
standalone helpers (join_encodings, optional_presence_enc, count_enc,
count_dec, constrained_int_dec, unconstrained_int_dec and boolean_dec)
was likewise reachable only from its own tests, and OER kept copies of
the BER check_type and check_type_get_len that only BER itself calls.

UPERcodec_SEQUENCE spliced a raw byte string into the bitstream by
guessing how many of its trailing zero bits were padding, which drops
bits from a sequence that legitimately ends in zeroes. Nothing reaches
it, as sequences are encoded through the ASN1F_SEQUENCE hooks, so it now
refuses the input like its decoding counterpart already did rather than
corrupting it silently.

The surviving helpers took an optional encoder and returned either the
finished bytes or b"", while every caller passed one and the decoding
side passed b"" as a dummy first argument; they now take the encoder or
the decoder directly, as encode_into already did. The minimal two's
complement sizing, the bit to byte packing and the decode-and-check-
remainder wrapper were each written out several times over, and codec
methods declared options they never read, which hid the ones they do
honour.

No encoding changes: the ASN.1 suites, the asn1tools differential
fuzzing and the malformed input fuzzing pass unchanged.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
The enumeration index followed the order the values were declared in
rather than their ascending order, as X.691 14.1 requires, so an
ENUMERATED { c(2), a(0), b(1) } encoded a as index 1 where a conformant
peer reads b. Enumerations written in ascending order, which is the
usual case, were already correct.

An extensible enumerated dropped the one bit prefix of 14.3, shifting
every field encoded after it. The option could not be reached anyway,
as ASN1F_enum_INTEGER was the one field class that did not forward its
codec options, so uper_extensible= raised a TypeError instead of
constraining the field.

An OCTET STRING ignored its SIZE constraint while encoding, where BIT
STRING and OER already raised: a two octet value in a SIZE(4) field
emitted two octets, and in a SIZE(2..4) field an eight octet value wrote
a determinant that wrapped, so the peer read a different length and lost
everything that followed.

Without an enumeration list and without declared bounds, the enumerated
encoder took the upper bound from the value at hand, making the width
depend on the value while the decoder refused the same case; a size_len
of zero was also read as an upper bound of zero, which encoded every
value in no bits at all.

Encodings match asn1tools byte for byte, over the vectors added here as
well as random schemas.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
A CHOICE whose alternatives are ASN1F_PACKET instances, which is how a
choice between tagged sequences is written and what BER already
supports, could not be encoded: the alternative lookup only recognised
packet classes and basic field classes. UPER refused the value outright
and OER dropped the alternative tag, emitting bytes it could not read
back.

OER also let the BER constructed bit into the tag number it emitted, so
an alternative tagged [0] went out as tag number 32 and an untagged
SEQUENCE alternative as universal 48 instead of 16. The encoding round
tripped with itself and with nothing else.

Tags of components were encoded at all, where X.696 encodes none
whatever the tagging environment of the module: the only tag on the wire
is the one of the chosen CHOICE alternative. OER_tagging_enc and
OER_tagging_dec are now the identity, and the alternative tag is emitted
by the choice hook alone.

ASN1F_DEFAULT was defined by importing scapy.contrib.uper, although a
DEFAULT component is not specific to a codec and the OER documentation
refers to it, which left OER users with a name they could not import and
OER with a getattr fallback for the absent set_absent. It now lives in
asn1fields, next to ASN1F_optional, and both codecs re-export it. BER
gains from it too: ASN1F_optional.build asked the wrapped field whether
it was empty, so a DEFAULT component holding its default value was
encoded where DER omits it.

Alternative tags and the encodings of the sequences behind them match
asn1tools byte for byte.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
A constrained INTEGER is written on the width of its range, so a value
outside it cannot be expressed: 100 in an INTEGER (0..7) went out as a
byte that reads back as 4, and -3 as one that reads back as 5. The same
hole let a SEQUENCE OF with a SIZE(1..3) constraint encode an empty list
as index -1. UPER_constrained_int_enc now refuses such a value, as the
string and bit string encoders already do; an extensible type still
takes its extension path before coming here. Building a SEQUENCE OF also
had a branch of its own for an unset field, which wrote a length
determinant of zero past the size constraint, where an empty list went
through the constrained count.

The two codecs each defined a BadTag decoding error that nothing raises,
along with the except branch catching it, and OER kept a
check_type_check_len that only the fields it hooks would call and an
OER_id_dec merging the tag class into the tag number, the lossy pattern
just removed from the choice path. The UPER encoder and decoder also
carried a number_of_bytes and a consume_input with no caller.

Neither OER nor PER puts the tag of a field on the wire, so ASN1Codec
now defaults to identity tagging and only BER registers its own. The
alternative lookup of a CHOICE, copied verbatim in both codecs, becomes
ASN1F_CHOICE.alternative_index, and the scan for optional components
becomes an ASN1F_SEQUENCE.optionals tuple built once. Eleven copies of
the OER length check and the two UPER size checks each collapse into one
helper, with the same messages, and the OER use_object_enc hook returned
exactly what asn1fields does without it.

Coverage of the three modules over the ASN.1 suites goes from 89% to 97%
for OER and from 95% to 99% for UPER, the added tests covering the long
form of an OER tag, the untyped codec fallbacks, a dissect of an empty
encoding, a pre-encoded value, a choice with an unknown tag, an unknown
index, a single alternative or packet class alternatives, and the two
constraint fixes above.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
Tagging had a registration mechanism of its own, next to the field
hooks, and every codec paid for it: ASN1Codec carried an identity
tagging_enc and tagging_dec so that OER and PER, which encode no tag at
all, would not have to register anything. Tagging is just another field
operation a codec does its own way, so BER now registers it among its
field hooks, where a missing entry already means the default behaviour,
and asn1fields leaves the encoding alone when no codec hooks it.

register_tagging, tagging_enc and tagging_dec go away with it, as does
unregister_field_hooks, which nothing called.

Dissecting and building a BER sequence of tagged fields takes the same
time as before.

AI-Assisted: yes (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
@polybassa
polybassa marked this pull request as ready for review August 11, 2026 19:03
@polybassa
polybassa requested a lite review from Copilot August 11, 2026 19:03

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.

Pull request overview

Adds ASN.1 OER and UPER (registered on ASN1_Codecs.OER and ASN1_Codecs.PER) plus a new “field hooks” mechanism to let codecs override compound-field behavior (tagging, SEQUENCE/CHOICE/SEQUENCE OF) while keeping BER as the default behavior.

Changes:

  • Introduces ASN1Codec.register_field_hooks() / field_hook() and updates ASN.1 fields to consult codec-specific hooks.
  • Adds new contrib codecs: scapy.contrib.oer (OER) and scapy.contrib.uper (UPER/PER).
  • Expands/creates UTS coverage for cross-codec build/dissect and OER vectors/fuzzing.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/scapy/layers/ber.uts Updates tests for the new BER tagging hooks + adds BER build/dissect test coverage.
test/scapy/layers/asn1.uts Adds cross-codec (BER/OER/PER) build/dissect tests and codec-opts/default-component checks.
test/contrib/oer.uts New OER-focused test suite (vectors, fuzzing, interop, conformance checks).
scapy/contrib/uper.py New UPER implementation and PER field hooks for bitstream-oriented encoding/decoding.
scapy/contrib/oer.py New OER implementation and OER field hooks for preamble/CHOICE-tag behavior.
scapy/asn1fields.py Adds codec_opts plumbing and consults codec field hooks for tagging/compound-field operations.
scapy/asn1/ber.py Registers BER field hooks (tagging) via the new hook mechanism.
scapy/asn1/asn1.py Adds codec-level field hook registration + safe default for _field_hooks.
.config/codespell_ignore.txt Adds OER/UPER-related ignore words.
Suppressed comments (1)

test/scapy/layers/asn1.uts:402

  • Duplicate helper function: _roundtrip is defined twice back-to-back here. One of them should be removed to avoid confusion and reduce noise in the test file.
def _roundtrip(cls, pkt):
    # type: (type, ASN1_Packet) -> ASN1_Packet
    return cls(raw(pkt))

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

Comment thread test/scapy/layers/ber.uts
Comment on lines +601 to +603
def _asn1_int(val):
# type: (Any) -> int
return val.val if hasattr(val, "val") else val
Comment thread test/scapy/layers/ber.uts
Comment on lines +625 to +627
def _roundtrip(cls, pkt):
# type: (type, ASN1_Packet) -> ASN1_Packet
return cls(raw(pkt))
Comment on lines +243 to +245
def _roundtrip(cls, pkt):
# type: (type, ASN1_Packet) -> ASN1_Packet
return cls(raw(pkt))
Comment on lines +418 to +420
def _asn1_int(val):
# type: (Any) -> int
return val.val if hasattr(val, "val") else val
Comment thread test/contrib/oer.uts
Comment on lines +3 to +6
#
# Try me with:
# bash test/run_tests -t test/scapy/layers/oer.uts -F

Comment thread scapy/asn1/ber.py Outdated

ASN1_Codecs.BER.register_stem(BERcodec_Object)
ASN1_Codecs.BER.register_tagging(BER_tagging_enc, BER_tagging_dec)
ASN1_Codecs.BER.register_field_hooks(_BER_FieldHooks)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Heh I still dislike this API. It would be simpler to have
register_enc(enc=<the_enc_function>, dec=<the_dec_function>) and store all of that into a dictionary into ASN1_Codecs, sorted by codec.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for your feedback

A codec that does a field operation its own way had to declare a class of
static methods and hand it to register_field_hooks, where the attribute
names of that class silently defined the hook points, and each codec kept
its own class in a _field_hooks attribute. The functions are now named as
keyword arguments of register_hooks, and they all land in
ASN1_Codecs.hooks, a dictionary by codec then by field operation, so what
a codec overrides reads at the call site and lives in one place.

The three hook classes become plain module functions, which is what UPER
already needed anyway to bolt its bitstream helpers onto the asn1fields
classes, and the enumerated codec kwargs simply ask whether the packet is
a PER one instead of comparing hook classes.

Dissecting and building a BER sequence of tagged fields takes the same
time as before.

AI-Assisted: yes (Cursor)
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.

3 participants