Skip to content

Fix streaming validation, FFI lifetime soundness, error code alignment, and doc accuracy#183

Closed
Copilot wants to merge 3 commits intousers/jstatia/native_primitivesfrom
copilot/sub-pr-181
Closed

Fix streaming validation, FFI lifetime soundness, error code alignment, and doc accuracy#183
Copilot wants to merge 3 commits intousers/jstatia/native_primitivesfrom
copilot/sub-pr-181

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 20, 2026

PR review identified bugs in the streaming signing path, unsound FFI lifetime annotations, misaligned C/Rust error codes, and stale documentation across the Layer 1 primitives.

Streaming payload validation (builder.rs)

  • Added usize::try_from(payload_len) overflow guard before Vec::with_capacity — a silent truncation on 32-bit platforms would over-allocate or panic
  • Added PayloadError::LengthMismatch checks after streaming reads in both detached and embedded paths — a mismatch between the declared size() and bytes actually read would silently produce a signature over a truncated payload

FFI lifetime soundness (ffi/src/types.rs, ffi/src/error.rs)

  • All *_handle_to_inner functions returned &'static Inner, which is an unsound claim — changed to explicit &'a so the lifetime is tied to the input pointer and not accidentally extended past the handle's validity

Error code fixes

  • PayloadError(_) was mapped to FFI_ERR_PAYLOAD_MISSING (-4), colliding with PayloadMissing — C callers had no way to distinguish "detached payload not provided" from "payload I/O / length error". Now maps to FFI_ERR_INVALID_ARGUMENT (-5)
  • sign1.h error code values were completely out of sync with the Rust FFI exports (e.g., INVALID_ARGUMENT was -2 in C, -5 in Rust). Fixed to match the authoritative Rust values; added comment for the non-sequential COSE_SIGN1_ERR_PANIC = -99
  • Fixed RFC reference: 9338 → 9052 (COSE_Sign1 is RFC 9052, not 9338)

Config / packaging

  • cbindgen.toml: Removed prefix = "cosesign1_" — would double-prefix already-named exports like cose_sign1_error_messagecosesign1_cose_sign1_error_message
  • ffi/Cargo.toml: Removed rlib from crate-type — FFI projection crates only need staticlib + cdylib; no workspace crate depends on this as a library

Documentation

  • sign1/README.md: Rewrote — old version referenced wrong crate name (cosesign1_primitives), described a caller-supplied CborProvider model that no longer exists, and showed API signatures (sign(&provider, ...)) that don't compile
  • troubleshooting.md: LARGE_STREAM_THRESHOLDLARGE_PAYLOAD_THRESHOLD (actual constant name)
  • cose.h: Removed ambiguous dual free-function reference; canonical free is cose_string_free()

CI

  • Restored workflow_dispatch support for all three jobs (dotnet, native-rust, native-c-cpp) — the prior change dropped it, causing manual CI runs to skip all jobs silently

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Jstatia and others added 2 commits March 20, 2026 16:09
…ing support

Rust crates (8):
- cbor_primitives: CBOR encoding/decoding traits
- cbor_primitives_everparse: EverParse backend + streaming CborStreamDecoder
- crypto_primitives: Crypto signer/verifier traits, JWK types
- cose_primitives: RFC 9052 COSE types, CoseData (Buffered/Streamed),
  LazyHeaderMap, ArcSlice/ArcStr zero-copy header values
- cose_sign1_primitives: COSE_Sign1 message, builder, sig_structure,
  streaming parse/sign/verify
- cose_sign1_crypto_openssl: OpenSSL EC/RSA/EdDSA/ML-DSA provider
- cose_sign1_primitives_ffi: C-ABI for Sign1 operations
- cose_sign1_crypto_openssl_ffi: C-ABI for crypto provider

Zero-copy architecture:
- CoseSign1Message owns single Arc<[u8]> backing buffer
- Protected headers, payload, signature are Range<usize> slices
- CoseHeaderValue::Bytes/Text use ArcSlice/ArcStr (share backing Arc)
- LazyHeaderMap defers parsing via OnceLock until first access
- Clone is O(1) for payload/signature (Arc pointer copy)

Streaming support:
- CborStreamDecoder: parse CBOR from Read+Seek streams
- CoseData::Streamed: headers buffered (~1KB), payload by offset
- parse_stream(): parse 10GB files with ~1.4KB memory
- sign_streaming(): 64KB peak for detached payloads
- verify_payload_streaming(): 64KB peak for ECDSA/RSA verification
- Ed25519/ML-DSA fall back to buffered (documented)

Single-pass sign+embed:
- Detached: 64KB chunk buffer, no payload retention
- Embedded: read into embed buffer, signer reads same buffer
- MemoryPayload uses Arc<[u8]> (open() is pointer copy, not clone)

C/C++ projections:
- cose.h/.hpp, sign1.h/.hpp, crypto/openssl.h/.hpp
- C/C++ API docs and architecture documentation

Documentation:
- native/rust/docs/memory-characteristics.md
- native/docs/ARCHITECTURE.md and getting started guides
- Per-crate READMEs

221 tests, 90%+ line coverage, zero clippy warnings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, docs, CI workflow_dispatch

Co-authored-by: JeromySt <12549141+JeromySt@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/CoseSignTool/sessions/855b756d-2986-49b6-8843-1bf96ef5a6f5
Copilot AI changed the title [WIP] Native Layer 1: Zero-copy COSE primitives with streaming parse/sign/verify Fix streaming validation, FFI lifetime soundness, error code alignment, and doc accuracy Mar 21, 2026
Copilot AI requested a review from JeromySt March 21, 2026 00:05
@JeromySt JeromySt force-pushed the users/jstatia/native_primitives branch 2 times, most recently from 63ec316 to 4350e90 Compare March 21, 2026 00:27
@JeromySt
Copy link
Copy Markdown
Member

Closing as duplicate. All fixes cherry-picked into PR #181: FFI lifetime soundness, overflow guard, error codes, doc corrections, cbindgen prefix.

@JeromySt JeromySt closed this Mar 21, 2026
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