Asn1 oer uper contrib - #5050
Open
polybassa wants to merge 44 commits into
Open
Conversation
polybassa
force-pushed
the
asn1-oer-uper-contrib
branch
from
August 7, 2026 18:45
e7bc1d3 to
4cdc2de
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5050 +/- ##
==========================================
+ Coverage 80.63% 81.09% +0.46%
==========================================
Files 390 401 +11
Lines 96936 98603 +1667
==========================================
+ Hits 78168 79966 +1798
+ Misses 18768 18637 -131
🚀 New features to boost your workflow:
|
gpotter2
reviewed
Aug 10, 2026
Contributor
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.
gpotter2
reviewed
Aug 12, 2026
gpotter2
reviewed
Aug 26, 2026
gpotter2
left a comment
gpotter2
left a comment
Member
There was a problem hiding this comment.
This is very hard to review. I think it needs some cleanup phase where it reduces the number of functions that are used only once.
I also think that eventually it makes more sense to include all of this in scapy/asn1 directly
gpotter2
requested changes
Sep 1, 2026
Comment on lines
+120
to
+129
| def _OER_check_len(name, s, number_of_bytes, offset=0): | ||
| # type: (str, bytes, int, int) -> None | ||
| """Raise unless s carries number_of_bytes octets past its first offset.""" | ||
| available = len(s) - offset | ||
| if available < number_of_bytes: | ||
| raise OER_Decoding_Error( | ||
| "%s: Got %i bytes while expecting %i" % | ||
| (name, available, number_of_bytes), | ||
| remaining=s | ||
| ) |
Member
There was a problem hiding this comment.
Useful as a standalone function? I don't like this pattern of calling a function that might throw an error, it's not clear from the parent code
Comment on lines
+164
to
+208
| def OER_signed_integer_enc(i): | ||
| # type: (int) -> bytes | ||
| # X.696 10.4: the shortest two's complement encoding. A negative value | ||
| # needs one bit less than its magnitude suggests, as -2**(8n-1) still | ||
| # fits in n octets, hence the increment before measuring. | ||
| magnitude = i + 1 if i < 0 else i | ||
| number_of_bytes = (magnitude.bit_length() + 8) // 8 | ||
| value = i & ((1 << (8 * number_of_bytes)) - 1) | ||
| return OER_len_enc(number_of_bytes) + value.to_bytes(number_of_bytes, "big") | ||
|
|
||
|
|
||
| def OER_signed_integer_dec(s): | ||
| # type: (bytes) -> Tuple[int, bytes] | ||
| number_of_bytes, s = OER_len_dec(s) | ||
| _OER_check_len("OER_signed_integer_dec", s, number_of_bytes) | ||
| if number_of_bytes == 0: | ||
| raise OER_Decoding_Error( | ||
| "OER_signed_integer_dec: got an empty length determinant", | ||
| remaining=s | ||
| ) | ||
| value = int.from_bytes(s[:number_of_bytes], "big") | ||
| number_of_bits = 8 * number_of_bytes | ||
| if value & (1 << (number_of_bits - 1)): | ||
| value -= (1 << number_of_bits) - 1 | ||
| value -= 1 | ||
| return value, s[number_of_bytes:] | ||
|
|
||
|
|
||
| def OER_unsigned_integer_enc(i): | ||
| # type: (int) -> bytes | ||
| if i < 0: | ||
| raise OER_Encoding_Error( | ||
| "OER_unsigned_integer_enc: %i is negative" % i | ||
| ) | ||
| number_of_bits = max(i.bit_length(), 1) | ||
| number_of_bytes = (number_of_bits + 7) // 8 | ||
| return OER_len_enc(number_of_bytes) + i.to_bytes(number_of_bytes, "big") | ||
|
|
||
|
|
||
| def OER_unsigned_integer_dec(s): | ||
| # type: (bytes) -> Tuple[int, bytes] | ||
| number_of_bytes, s = OER_len_dec(s) | ||
| _OER_check_len("OER_unsigned_integer_dec", s, number_of_bytes) | ||
| value = int.from_bytes(s[:number_of_bytes], "big") | ||
| return value, s[number_of_bytes:] |
Member
There was a problem hiding this comment.
We don't have all of those for BER? They're part of the classes directly, I think it makes more sense
Member
|
Very sorry @polybassa but this still looks like slop for the most part. This probably means we should try to add more coding guidances to the AI |
polybassa
pushed a commit
to polybassa/scapy-1
that referenced
this pull request
Sep 2, 2026
Restore extension points and correct OID, integer bounds, PER padding, X.509 extract_packet, and BER DEFAULT handling; add regression tests. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
polybassa
pushed a commit
to polybassa/scapy-1
that referenced
this pull request
Sep 2, 2026
Restore extension points and correct OID, integer bounds, PER padding, X.509 extract_packet, and BER DEFAULT handling; add regression tests. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
polybassa
force-pushed
the
asn1-oer-uper-contrib
branch
from
September 2, 2026 05:06
3aba643 to
07160bb
Compare
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)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
AI-Assisted: yes (Cursor)
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>
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>
Unify compound child dispatch on encode_to/decode_from, add OER encoder contexts and PER bit-stream helpers, and read field constraints directly in primitive codecs instead of round-tripping through codec_kwargs. Co-authored-by: Cursor <cursoragent@cursor.com> AI-Assisted: yes (Cursor Agent)
Remove EncodingParams, codec_kwargs, and field codec_opts/_codec_kwargs; codecs resolve size_len and constraints via field= directly. Co-authored-by: Cursor <cursoragent@cursor.com> AI-Assisted: yes (Cursor Agent)
Restore extension points and correct OID, integer bounds, PER padding, X.509 extract_packet, and BER DEFAULT handling; add regression tests. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Remove unused imports, tighten typing for observed tags and codec returns, and satisfy line-length checks on BER override annotations. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Fix UPER trailing-octet handling, OER integer constraints, X.509 underlayer extraction, known-multiplier string typing, and restore codec tagging/_codec_kwargs contracts with regression tests. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
polybassa
force-pushed
the
asn1-oer-uper-contrib
branch
from
September 2, 2026 07:45
071ffa8 to
2f31055
Compare
Rebase onto master picked up secdev#5062; replace remaining orb() uses with direct byte indexing so the contrib codecs import cleanly. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Rewrite the intentional typo kwargs in oer.uts so codespell stays quiet, and flatten the oer_int_wire_params docstring for Sphinx -W. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
ASN1F_PACKET.any2i and UPER nestings set parent only, which broke EncryptedData.encrypt in KerberosSSP; set underlayer and parent together. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Import codecs from scapy.asn1 and use minimum/maximum/extensible/unsigned in tests. Co-authored-by: Cursor <cursoragent@cursor.com> AI-Assisted: yes (Cursor Agent)
Drop unused compound helper re-exports, constraint aliases, and stale smoke tests. Co-authored-by: Cursor <cursoragent@cursor.com> AI-Assisted: yes (Cursor Agent)
Write OER SEQUENCE children into one encoder, resolve UPER bounds once, and centralize two's-complement octet math. Co-authored-by: Cursor <cursoragent@cursor.com> AI-Assisted: yes (Cursor Agent)
Add semi-constrained INTEGER and extensible SIZE paths, honor OER size constraints and named enum encode values, and drop leftover dead helpers. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
polybassa
force-pushed
the
asn1-oer-uper-contrib
branch
from
September 2, 2026 13:09
584768f to
faaa99b
Compare
actions/checkout builds a merge of the PR into the base branch, so rev-list from HEAD was also validating base-branch tips. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Move compound wire rules onto encoder/decoder methods, reject empty OER SEQUENCEs with mandatory fields, shrink BER churn, and canonicalize UPER constraint kwargs. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
2 tasks
Keep the HEAD^1..HEAD^2 restriction on this branch so secdev#5050 CI does not false-fail on master tips until the standalone CI PR lands. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Cast codec.enc results after neutral get_codec typing, and drop obsolete attr-defined ignores. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Bind compound encode/decode hooks directly on BER/OER/UPER contexts, separate field-layer contexts from raw UPER bit streams, and inline trivial constraint getters and single-use compound forwarders. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Drop _codec_kwargs and ASN1Codec.new_encoder/new_decoder wrappers, stream BER SEQUENCE children through a nested context, and avoid SEQUENCE OF fragment list slices. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Keep OPTIONAL child walking in compound.py and move BER/OER/UPER
SEQUENCE, CHOICE, SEQUENCE OF, and PACKET implementations into
compound_{ber,oer,uper}.py.
AI-Assisted: yes (Cursor Agent)
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop dead decoder offset/chunk counters, use int.from_bytes/to_bytes for OER length determinants, and make UPER read_bit avoid redundant arithmetic. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Fold CHOICE/PACKET second-layer helpers into the bound codec hooks, drop sequence_encode_children and UPER set_remainder, and keep only the shared OPTIONAL decode walk. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Store UPER input as a byte buffer with a bit cursor instead of one giant integer, drop per-field kwargs copies, look up CHOICE by tag, and join OER SEQUENCE OF payloads once. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Large OCTET STRING reads at a non-byte offset use one bulk int.from_bytes/shift instead of growing an integer per source byte; delegate whole-octet read_bits to that path and drop duplicate bounds checks on the small-field integer reader. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Reject SEQUENCE/CHOICE/SEQUENCE OF as SEQUENCE OF field elements with an explicit error, keep ASN1F_PACKET (used by Kerberos) via UPER context hooks, and drop misleading compound encode_into/dissect_from_decoder wrappers so only primitives use the raw-bit API. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Replace chunked giant-int finalization with a bytearray plus pending-bit accumulator, speed up Decoder.remaining(), and reject compound SEQUENCE OF elements only on the UPER path so BER/OER keep master construction behavior. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the per-byte _peek_bits_int loop, keep append_bits on the byte path except for a trailing partial octet, bulk-shift unaligned append_bytes, and trim redundant SEQUENCE OF PACKET checks. AI-Assisted: yes (Cursor Agent) Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.