Skip to content

ndncert: fix v0.3 session cryptography - #236

Closed
markverick wants to merge 3 commits into
named-data:mainfrom
markverick:fix/ndncert-v03-crypto-201
Closed

markverick wants to merge 3 commits into
named-data:mainfrom
markverick:fix/ndncert-v03-crypto-201

Conversation

@markverick

Copy link
Copy Markdown
Collaborator

Fixes #201.

The client currently encodes IV counters in little-endian order,
increments them before encryption, and does not track received IVs.
Reusing a Client also reuses its ephemeral ECDH key across requests.

  • Generate fresh ECDH keys and IV state for each NEW request.
  • Encode counters in big-endian order, starting at zero, and advance
    after encryption using integer plaintext block counts.
  • Reject malformed IVs and tags, changing or colliding IV prefixes,
    replayed or overlapping counter ranges, and counter exhaustion.
  • Update receive state only after authentication and IV validation.

Low-level callers must now pass the session's AeadCounter to
AeadDecrypt and handle errors returned by AeadMessage.FromTLV.
Peers using the previous IV format need coordinated updates.

Regression tests cover sequential sessions, counter encoding and
exhaustion, malformed messages, signature validation, and two-round
CHALLENGE exchanges with replay rejection.

Validation:

  • go test ./...
  • go test -race ./std/security/ndncert/...
  • go vet ./std/security/ndncert/...
  • WebAssembly test compilation

Initialize ECDH and IV state for every NEW request rather than once per Client. Cover sequential reuse, failed NEW attempts, and CA signature validation with distinct CA and requester keys.
Start IV counters at zero, encode them in big-endian order, and advance after encryption using integer block counts. Reject empty plaintext, oversized GCM messages, and exhausted counters without wrapping.

Add reference-encryption, block arithmetic, and send-exhaustion regression tests.
Reject malformed cipher messages, changing or colliding IV prefixes, replayed or overlapping block ranges, and receive-counter wraparound. Update peer state only after authentication and validation, and decrypt into a private buffer without modifying the packet.

Cover authenticated receive-state transitions and a two-round client CHALLENGE exchange against an independent AES-GCM peer, including replay rejection.

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.

🔵 Needs a closer look

Security-sensitive session-state changes and intentional wire-format and API incompatibilities warrant human security and interoperability review.

No open issues 🎉

What changed in this PR

This PR addresses #201 by aligning NDNCERT client session cryptography with v0.3 counter encoding, IV validation, and ephemeral-key requirements.

Changes:

  • Generate fresh ECDH keys and IV state for each NEW request.
  • Use big-endian, post-encryption counters and reject malformed messages, replayed or overlapping ranges, prefix violations, and counter exhaustion.
  • Update decryption callers and add regression tests for session reuse, authentication, and multi-round CHALLENGE exchanges.
File Description
std/​security/​ndncert/​crypto_aes.go Corrects counter handling and adds session-aware IV validation.
std/​security/​ndncert/​crypto_aes_test.go Tests encoding, exhaustion, authentication, replay rejection, and buffer preservation.
std/​security/​ndncert/​client.go Adds fresh key and IV state initialization per session.
std/​security/​ndncert/​client_session_test.go Tests sequential sessions, CHALLENGE exchanges, and signature validation.
std/​security/​ndncert/​client_protocol.go Initializes sessions during NEW and handles stateful decryption errors.

Review effort: Balanced

@markverick

Copy link
Copy Markdown
Collaborator Author

will merge in a branch instead for a proper stacked PR

@markverick markverick closed this Oct 26, 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.

[ndncert] Cryptographic non-compliance

2 participants