Asn1 oer uper contrib - #5050
Conversation
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)
e7bc1d3 to
4cdc2de
Compare
Codecov Report❌ Patch coverage is
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
🚀 New features to boost your workflow:
|
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
| codec = ASN1_Codecs.BER | ||
| tag = ASN1_Class_UNIVERSAL.ANY | ||
| skip_tagging = False | ||
| tagging_enc = staticmethod(BER_tagging_enc) |
There was a problem hiding this comment.
I think we removed those in the previous PR and moved them to 'self.codec'
There was a problem hiding this comment.
fixed... now this PR is ready for review....
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>
There was a problem hiding this comment.
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) andscapy.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:
_roundtripis 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.
| def _asn1_int(val): | ||
| # type: (Any) -> int | ||
| return val.val if hasattr(val, "val") else val |
| def _roundtrip(cls, pkt): | ||
| # type: (type, ASN1_Packet) -> ASN1_Packet | ||
| return cls(raw(pkt)) |
| def _roundtrip(cls, pkt): | ||
| # type: (type, ASN1_Packet) -> ASN1_Packet | ||
| return cls(raw(pkt)) |
| def _asn1_int(val): | ||
| # type: (Any) -> int | ||
| return val.val if hasattr(val, "val") else val |
| # | ||
| # Try me with: | ||
| # bash test/run_tests -t test/scapy/layers/oer.uts -F | ||
|
|
|
|
||
| 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
No description provided.