Skip to content

feat: add HPKE e2e encryption on relay for Bonjour-paired peers#958

Draft
ritave wants to merge 3 commits into
mainfrom
ritave/hpke
Draft

feat: add HPKE e2e encryption on relay for Bonjour-paired peers#958
ritave wants to merge 3 commits into
mainfrom
ritave/hpke

Conversation

@ritave
Copy link
Copy Markdown
Contributor

@ritave ritave commented Apr 27, 2026

Summary

Explain the motivation and the changes. Link issues (e.g., Closes #123).

Changes

  • Behavior change
  • UI change (screenshots below)
  • Refactor / chore
  • Tests
  • Docs

Test Plan

Steps to verify locally (commands, screenshots, recordings). Include model used.

Screenshots

If UI updated, add before/after.

Checklist

  • I have read CONTRIBUTING.md
  • I added/updated tests where reasonable
  • I updated docs/README as needed
  • I verified build on macOS with Xcode 16.4+

ritave added 3 commits April 29, 2026 17:10
Fixed:

1. HPKEKeyStore.swift — public-key caching. publicKeyBytes and publicKeyEncoded were re-deriving from the private key on every call, which BonjourAdvertiser hits per agent on every TXT republish. Now cached alongside _privateKey, with cache invalidation on warmUp and reset.
2. HPKEKeyStore.swift — zeroize derived bytes in warmUp. The 32-byte derived key is now wiped from the local stack after writing to keychain, matching the security hygiene MasterKey.generate() already enforces on its own bytes.
3. HPKEKeyStore.swift — fixed inverted comment. Renamed privateKeyUnlocked → privateKeyLocked (caller holds lock) and corrected the docstring.
4. HPKEEncryption.swift — single-shot sender. _sender is now Optional, consumed (set to nil) on first sealRequestBody call. A second call throws sealFailed instead of silently producing counter-1 ciphertext the server can't open.
5. HPKEEncryption.swift — dropped unused HPKEClientContext.suite field. It mirrored the singleton constant and wasn't read anywhere.
6. HTTPHandler.swift — skip body copy on plaintext requests. The encryption block now reads the body buffer only inside the contains(name:) branch, so plaintext traffic doesn't pay for an extra Data allocation per request.
7. HTTPHandler.swift — defensive context clear in .end cleanup. stateRef.value.encryptionContext = nil after dispatch, matching the existing requestHead/requestBodyBuffer cleanup. Stops a prior request's context from leaking forward if HTTP keep-alive is ever enabled (today's Connection: close keeps it 1:1, but the cleanup is one line of insurance).
8. RemoteProviderService.swift — fixed O(n²) buffer scan in makeDecryptingChunkStream. Tracks a searchStart index that advances past already-scanned bytes (with a 1-byte overlap so a separator straddling the boundary still matches). Drops worst-case from O(n²) to O(n) over a long stream.

Skipped (deliberately):
- Removing hpkeSuite from the model — agent flagged it as dead state, but it's the wire-format gate for future suite negotiation, not pure dead code.
- Extracting a shared HMAC-SHA512 KDF helper across AgentKey/PairingKey/HPKEKeyStore — out of scope; would touch unrelated identity code.
- Protocol-ifying var encryption across the four ResponseWriter classes — small churn for limited win; the current shared emitEvent free function is clear.
- Struct-based encryption-header parser — only one suite exists today.
- Test isolation refactor (HPKEKeyStore(testMode:)) — current .serialized works fine for 7 fast tests.
@ritave ritave force-pushed the ritave/hpke branch 2 times, most recently from 1b00c35 to d63bdca Compare April 29, 2026 20:34
@mimeding
Copy link
Copy Markdown
Contributor

mimeding commented May 1, 2026

Status note from the PR cleanup pass: the latest visible checks are green, but this PR is still draft and currently marked DIRTY with merge conflicts. I am not mutating community branches from this account. Next actionable step is to resolve conflicts against current main, keep the PR draft until that is done, and rerun the standard checks before review.

@mimeding
Copy link
Copy Markdown
Contributor

mimeding commented May 4, 2026

@tpae separate CI note:

This PR's failure is not the provider/model-picker race affecting the newer red PRs. Its test-core run is failing much earlier with module-resolution errors in the EventSource dependency chain (CAsyncHTTPClient, CNIOLLHTTP, CNIOExtrasZlib, CNIOPosix, _NumericsShims).

Recommended next step: keep this draft blocked until it is rebased onto current main after #1025 lands, then rerun CI. If the same module-resolution errors remain, this needs dependency/toolchain investigation specific to this branch before any security review.

Why: this PR touches relay encryption/identity-adjacent behavior, so we should avoid reviewing or merging it while the build cannot prove the branch even compiles cleanly.

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.

2 participants