Skip to content

feat(pq-key-encoder): phase 2 - ASN.1 primitives (ENG-1310, ENG-1311, ENG-1312, ENG-1313, ENG-1314, ENG-1315, ENG-1316)#8

Merged
eacet merged 1 commit intomainfrom
feature/eng-1310
Feb 11, 2026
Merged

feat(pq-key-encoder): phase 2 - ASN.1 primitives (ENG-1310, ENG-1311, ENG-1312, ENG-1313, ENG-1314, ENG-1315, ENG-1316)#8
eacet merged 1 commit intomainfrom
feature/eng-1310

Conversation

@eacet
Copy link
Copy Markdown
Member

@eacet eacet commented Feb 11, 2026

Summary

  • Implement ASN.1/DER encoding and decoding primitives for the pq-key-encoder Rust
    crate
  • Add tag constants, length encoding/decoding, TLV primitives, and AlgorithmIdentifier
    encode/decode
  • 33 new unit tests covering all ASN.1 functions including roundtrip for all 18 PQ
    algorithms
  • All functions are pub(crate) and will be consumed by Phase 3 (SPKI/PKCS8 encoding)

Linear Issues

ENG-1310, ENG-1311, ENG-1312, ENG-1313, ENG-1314, ENG-1315, ENG-1316

Test plan

  • cargo test — 47 tests pass (33 new ASN.1 + 14 existing)
  • cargo build --no-default-features — no_std builds
  • cargo clippy --all-targets --all-features -- -D warnings — clean
  • cargo fmt --check — clean

@linear
Copy link
Copy Markdown

linear bot commented Feb 11, 2026

ENG-1310 2.1: asn1/tags.rs — DER tag constants

Create src/asn1/tags.rs with pub(crate) constants:

  • TAG_INTEGER (0x02), TAG_BIT_STRING (0x03), TAG_OCTET_STRING (0x04), TAG_NULL (0x05), TAG_OBJECT_IDENTIFIER (0x06), TAG_SEQUENCE (0x30), TAG_CONTEXT_0 (0xA0), TAG_CONTEXT_1 (0xA1)

ENG-1311 2.2: asn1/length.rs — DER length encode/decode

Create src/asn1/length.rs with:

  • encode_length(len, &mut Vec<u8>) — short form (< 128) and long form (>= 128)
  • decode_length(&[u8], offset) -> Result<(usize, usize)> — with validation: reject indefinite length, non-minimal encoding, leading zeros, > 6 bytes
  • encoded_length_size(len) -> usize — for pre-computing output size
  • Unit tests: short form (0, 1, 127), long form (128, 255, 256, 65535), round-trip, error cases

ENG-1312 2.3: asn1/encode.rs — TLV encoding primitives

Create src/asn1/encode.rs with pub(crate) functions:

  • encode_tlv(tag, value, out) — write tag + length + value
  • encode_sequence(elements: &[&[u8]], out) — wrap elements in SEQUENCE
  • encode_octet_string(data, out) — TAG_OCTET_STRING wrapper
  • encode_bit_string(data, out) — TAG_BIT_STRING with 0x00 unused bits prefix
  • encode_integer_zero(out) — fixed bytes [0x02, 0x01, 0x00]
  • Unit tests: verify exact byte output for each function

ENG-1313 2.4: asn1/decode.rs — TLV decoding (zero-copy)

Create src/asn1/decode.rs with zero-copy decoding:

  • Tlv<'a> struct: tag, value: &'a [u8], bytes_read
  • read_tlv(input, offset) -> Result<Tlv<'_>> — reads one TLV element borrowing from input
  • decode_oid(bytes) -> Result<String> — delegates to pq_oid::decode_oid()
  • Unit tests: parse known TLV structures, error on truncated input

ENG-1314 2.5: asn1/algorithm.rs — AlgorithmIdentifier encode/decode

Create src/asn1/algorithm.rs:

  • encode_algorithm_identifier(algorithm, out) — writes SEQUENCE { OID } (no NULL parameter)
  • decode_algorithm_identifier(input, offset) -> Result<(Algorithm, usize)> — accepts both absent and NULL parameters for interop
  • Uses pq_oid::encode_oid_to() for raw OID bytes, then wraps with tag+length
  • Unit tests: encode/decode round-trip for ML-KEM-512, ML-DSA-44, SLH-DSA-SHA2-128s

ENG-1315 2.6: asn1/mod.rs — Module declarations

Create src/asn1/mod.rs with pub(crate) module declarations for: tags, length, encode, decode, algorithm.

ENG-1316 2.7: Wire up asn1 module in lib.rs

Add mod asn1; to src/lib.rs.

Copy link
Copy Markdown
Member Author

eacet commented Feb 11, 2026

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Feb 11, 2026

Greptile Overview

Greptile Summary

This PR implements foundational ASN.1 encoding/decoding primitives for the post-quantum key encoder. The implementation includes:

  • Length encoding/decoding with DER compliance, rejecting indefinite forms and non-minimal encodings
  • TLV (Tag-Length-Value) parsing with proper bounds checking
  • AlgorithmIdentifier encoding/decoding that accepts both absent and NULL parameters for interoperability
  • Utility functions for common ASN.1 types (SEQUENCE, OCTET STRING, BIT STRING, INTEGER)

The code demonstrates strong attention to security with comprehensive validation and error handling. Test coverage is excellent with edge cases and error scenarios well-tested. One minor overflow risk was identified in the length decoder that should use checked arithmetic.

Confidence Score: 4/5

  • This PR is safe to merge with one minor arithmetic overflow fix recommended
  • The implementation is well-structured with comprehensive test coverage and proper error handling. The DER encoding follows standards with correct validation. The single identified issue is a potential integer overflow in length decoding that should use checked arithmetic for defense in depth, though the existing 6-byte limit provides some protection.
  • Pay attention to packages/pq-key-encoder/rust/src/asn1/length.rs for the overflow fix

Important Files Changed

Filename Overview
packages/pq-key-encoder/rust/src/asn1/algorithm.rs Added AlgorithmIdentifier encoding/decoding for post-quantum algorithms. Handles both absent and NULL parameters for interoperability. Good test coverage including edge cases.
packages/pq-key-encoder/rust/src/asn1/decode.rs Implemented TLV parsing and OID decoding with proper bounds checking and error handling. Well-tested with comprehensive error cases.
packages/pq-key-encoder/rust/src/asn1/encode.rs Added ASN.1 encoding utilities for TLV, SEQUENCE, OCTET STRING, BIT STRING, and INTEGER. Straightforward implementation with good test coverage.
packages/pq-key-encoder/rust/src/asn1/length.rs Implemented DER length encoding/decoding with proper validation for minimal encoding and bounds checking. Minor overflow risk in decode for extremely large lengths.

Sequence Diagram

sequenceDiagram
    participant Client
    participant encode_algorithm_identifier
    participant encode_length
    participant pq_oid
    participant decode_algorithm_identifier
    participant read_tlv
    participant decode_length
    participant decode_oid
    
    Note over Client,decode_oid: Encoding Flow
    Client->>encode_algorithm_identifier: Algorithm
    encode_algorithm_identifier->>pq_oid: encode_oid_to(algorithm.oid())
    pq_oid-->>encode_algorithm_identifier: OID bytes
    encode_algorithm_identifier->>encode_length: OID length
    encode_length-->>encode_algorithm_identifier: encoded length
    encode_algorithm_identifier-->>Client: SEQUENCE { OID }
    
    Note over Client,decode_oid: Decoding Flow
    Client->>decode_algorithm_identifier: DER bytes, offset
    decode_algorithm_identifier->>read_tlv: input, offset
    read_tlv->>decode_length: input, offset+1
    decode_length-->>read_tlv: (length, bytes_consumed)
    read_tlv-->>decode_algorithm_identifier: SEQUENCE TLV
    decode_algorithm_identifier->>read_tlv: sequence.value, 0
    read_tlv-->>decode_algorithm_identifier: OID TLV
    decode_algorithm_identifier->>decode_oid: OID bytes
    decode_oid->>pq_oid: decode_oid(bytes)
    pq_oid-->>decode_oid: OID string
    decode_oid-->>decode_algorithm_identifier: OID string
    decode_algorithm_identifier-->>Client: (Algorithm, bytes_read)
Loading

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

7 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Member Author

eacet commented Feb 11, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 99488f6a74

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@eacet
Copy link
Copy Markdown
Member Author

eacet commented Feb 11, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4e2dfd73d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

… ENG-1312, ENG-1313, ENG-1314, ENG-1315, ENG-1316)
@eacet
Copy link
Copy Markdown
Member Author

eacet commented Feb 11, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@eacet eacet merged commit b1a2fd3 into main Feb 11, 2026
8 checks passed
@eacet eacet deleted the feature/eng-1310 branch February 11, 2026 13:50
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.

1 participant