docs(e2ee): Option A E2EE design spec for sign-off #6
Conversation
|
The files' contents are under analysis for test generation. |
|
Review these changes at https://app.gitnotebooks.com/OneFineStarstuff/OneFineStarstuff.github.io/pull/6 |
|
Review changes with Changed Files
|
Reviewer's GuideIntroduces a comprehensive end-to-end encryption design specification (Option A) for sign-off, detailing threat model, key lifecycle state machines, cryptographic primitives, messaging and file encryption flows, capability tokens, server APIs, storage schema, operational guardrails and test plan. Sequence diagram for device enrollment and attestationsequenceDiagram
actor User
participant NewDevice
participant TrustedDevice
participant Server
User->>NewDevice: Generate Ed25519/X25519 keys
NewDevice->>TrustedDevice: Show QR with pubkeys, nonce
TrustedDevice->>NewDevice: Scan QR, verify SAS
TrustedDevice->>NewDevice: Sign attestation
NewDevice->>Server: POST /devices/attest
Server->>NewDevice: Record attestation, mark verified
Sequence diagram for file encryption and sharingsequenceDiagram
participant SenderDevice
participant RecipientDevice
participant Server
SenderDevice->>SenderDevice: Generate random DEK
loop For each chunk
SenderDevice->>SenderDevice: Derive nonce, encrypt chunk (AES-256-GCM)
SenderDevice->>SenderDevice: Compute BLAKE3 digest
end
SenderDevice->>SenderDevice: Create manifest, sign with Ed25519
loop For each recipient device
SenderDevice->>RecipientDevice: Wrap DEK with X25519 sealed box
SenderDevice->>Server: Store key_wraps
end
SenderDevice->>Server: Upload encrypted file and manifest
RecipientDevice->>Server: Download encrypted file and manifest
RecipientDevice->>RecipientDevice: Unwrap DEK, decrypt chunks
Sequence diagram for capability token issuance and usagesequenceDiagram
participant Client
participant Server
Client->>Server: Request capability token (POST /capabilities)
Server->>Server: Issue PASETO v4.public token (Ed25519-signed)
Server->>Client: Return capability token
Client->>Server: Access resource (GET/PUT /objects/:id) with token
Server->>Server: Validate token, check claims
Server->>Client: Grant or deny access
Class diagram for identity and device key state machinesclassDiagram
class IdentityKey {
+uninitialized
+generated
+backed_up
+compromised
generate()
backup()
revoke()
}
class Device {
+new
+pending_attestation
+verified
+revoked
provision()
attest()
verify()
revoke()
}
IdentityKey <|-- Device: "bound to"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
View changes in DiffLens |
![reviewabot[bot]](https://avatars.githubusercontent.com/in/1023405?s=60&v=4){kind=small}
There was a problem hiding this comment.
The document is well-structured and comprehensive. However, there are a few points that need attention:
-
The document should not contain TODO comments or open items. The section "## 17. Open Items / Future Work" should be removed or moved to a separate issue tracker or project management tool.
-
There are too many consecutive line breaks in some sections. For example, between sections 1 and 2, and sections 2 and 3. Reducing these to a single line break would improve readability.
-
The file should end with a newline. The absence of a newline at the end of the file is indicated by 'No newline at end of file'.
Please address these issues to improve the quality and readability of the document.
|
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🧰 Additional context used🪛 LanguageToolDESIGN-E2EE.md[grammar] ~3-~3: There might be a mistake here. (QB_NEW_EN) [grammar] ~12-~12: There might be a mistake here. (QB_NEW_EN) [grammar] ~13-~13: There might be a mistake here. (QB_NEW_EN) [grammar] ~15-~15: There might be a mistake here. (QB_NEW_EN) [grammar] ~20-~20: There might be a mistake here. (QB_NEW_EN) [grammar] ~21-~21: There might be a mistake here. (QB_NEW_EN) [grammar] ~22-~22: There might be a mistake here. (QB_NEW_EN) [grammar] ~34-~34: There might be a mistake here. (QB_NEW_EN) [grammar] ~35-~35: There might be a mistake here. (QB_NEW_EN) [grammar] ~36-~36: There might be a mistake here. (QB_NEW_EN) [grammar] ~37-~37: There might be a mistake here. (QB_NEW_EN) [grammar] ~38-~38: There might be a mistake here. (QB_NEW_EN) [grammar] ~39-~39: There might be a mistake here. (QB_NEW_EN) [grammar] ~42-~42: There might be a mistake here. (QB_NEW_EN) [grammar] ~43-~43: There might be a mistake here. (QB_NEW_EN) [grammar] ~44-~44: There might be a mistake here. (QB_NEW_EN) [grammar] ~45-~45: There might be a mistake here. (QB_NEW_EN) [grammar] ~46-~46: There might be a mistake here. (QB_NEW_EN) [grammar] ~47-~47: There might be a mistake here. (QB_NEW_EN) [grammar] ~48-~48: There might be a mistake here. (QB_NEW_EN) [grammar] ~56-~56: There might be a mistake here. (QB_NEW_EN) [grammar] ~66-~66: There might be a mistake here. (QB_NEW_EN) [grammar] ~86-~86: There might be a mistake here. (QB_NEW_EN) [grammar] ~87-~87: There might be a mistake here. (QB_NEW_EN) [grammar] ~87-~87: There might be a mistake here. (QB_NEW_EN) [grammar] ~93-~93: There might be a mistake here. (QB_NEW_EN) [grammar] ~94-~94: There might be a mistake here. (QB_NEW_EN) [grammar] ~95-~95: There might be a mistake here. (QB_NEW_EN) [grammar] ~96-~96: There might be a mistake here. (QB_NEW_EN) [grammar] ~97-~97: There might be a mistake here. (QB_NEW_EN) [grammar] ~98-~98: There might be a mistake here. (QB_NEW_EN) [grammar] ~99-~99: There might be a mistake here. (QB_NEW_EN) [grammar] ~102-~102: There might be a mistake here. (QB_NEW_EN) [grammar] ~103-~103: There might be a mistake here. (QB_NEW_EN) [grammar] ~104-~104: There might be a mistake here. (QB_NEW_EN) [grammar] ~105-~105: There might be a mistake here. (QB_NEW_EN) [grammar] ~106-~106: There might be a mistake here. (QB_NEW_EN) [grammar] ~107-~107: There might be a mistake here. (QB_NEW_EN) [grammar] ~108-~108: There might be a mistake here. (QB_NEW_EN) [grammar] ~109-~109: There might be a mistake here. (QB_NEW_EN) [grammar] ~110-~110: There might be a mistake here. (QB_NEW_EN) [grammar] ~115-~115: There might be a mistake here. (QB_NEW_EN) [grammar] ~116-~116: There might be a mistake here. (QB_NEW_EN) [grammar] ~117-~117: There might be a mistake here. (QB_NEW_EN) [grammar] ~118-~118: There might be a mistake here. (QB_NEW_EN) [grammar] ~119-~119: There might be a mistake here. (QB_NEW_EN) [grammar] ~120-~120: There might be a mistake here. (QB_NEW_EN) [grammar] ~121-~121: There might be a mistake here. (QB_NEW_EN) [grammar] ~131-~131: There might be a mistake here. (QB_NEW_EN) [grammar] ~149-~149: There might be a mistake here. (QB_NEW_EN) [grammar] ~152-~152: There might be a mistake here. (QB_NEW_EN) [grammar] ~153-~153: There might be a mistake here. (QB_NEW_EN) [grammar] ~154-~154: There might be a mistake here. (QB_NEW_EN) [grammar] ~157-~157: There might be a mistake here. (QB_NEW_EN) [grammar] ~158-~158: There might be a mistake here. (QB_NEW_EN) 🪛 markdownlint-cli2 (0.17.2)DESIGN-E2EE.md66-66: Reversed link syntax (MD011, no-reversed-links) 70-70: Fenced code blocks should have a language specified (MD040, fenced-code-language) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (12)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gstraccini[bot]](https://avatars.githubusercontent.com/in/480132?s=60&v=4){kind=small}
|
View changes in DiffLens |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Please define the canonical JSON serialization rules used for manifest signing and PASETO claims to ensure interoperability and prevent signature mismatches.
- The adaptive chunk size range (512 KB–2 MB) should specify how the client decides chunk boundaries (e.g., network conditions or buffer fullness) to guarantee consistent streaming performance.
- Clarify the 2-of-3 Shamir secret sharing recovery flow with concrete details on the approval workflow, HSM escrow process, and roles of user vs. admin to avoid ambiguity during key recovery.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Please define the canonical JSON serialization rules used for manifest signing and PASETO claims to ensure interoperability and prevent signature mismatches.
- The adaptive chunk size range (512 KB–2 MB) should specify how the client decides chunk boundaries (e.g., network conditions or buffer fullness) to guarantee consistent streaming performance.
- Clarify the 2-of-3 Shamir secret sharing recovery flow with concrete details on the approval workflow, HSM escrow process, and roles of user vs. admin to avoid ambiguity during key recovery.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
View changes in DiffLens |
❌ Deploy Preview for onefinestarstuff failed.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
DESIGN-E2EE.md (6)
79-87: PASETO v4.public: add kid, issuer/audience, footer assertions, and revocation guidancev4.public tokens are signed-only. Add standard claims and a key id to support rotation; use footers/implicit assertions for tenant/region and bind tokens to device when appropriate. Document revocation via short TTL + denylist for tid.
Apply this diff:
-## 7. Capability Tokens -- Format: PASETO v4.public (Ed25519-signed by server capability key). -Claims: -- sub: user or device id -- scope: [object:get|put, room:read, room:write, membership:manage] -- resource: URI or prefix (e.g., s3://bucket/path/object-id) -- exp: expiry; iat/nbf -- region: data residency constraint -- tid/nonce: unique token id to prevent replay +## 7. Capability Tokens +- Format: PASETO v4.public (Ed25519-signed by server capability key). +- Include footer/implicit assertions for {tenant, region} and key id (kid) to support rotation. +Claims: +- iss (issuer), aud (audience) +- sub: user or device id +- scope: [object:get|put, room:read, room:write, membership:manage] +- resource: URI or prefix (e.g., s3://bucket/path/object-id) +- exp: expiry; iat/nbf; nbf skew ≤ 2 min +- region: data residency constraint (also duplicated in footer) +- tid (unique token id): server enforces one-time use or short TTL with denylist window
35-43: Device attestation: specify anti-replay and channel-binding detailsGood flow. Clarify SAS/QR contents (ephemeral challenge, attester identity fingerprint), server anti-replay, and proof-of-possession to prevent device injection via the server.
Recommended additions:
- QR includes: device_pubkeys, device_kid, attester_identity_fingerprint, attestation_nonce, expires_at.
- Trusted device verifies SAS/QR and signs attestation over all fields; server validates nonce freshness (single-use) and binds attested keys to user.
- Proof-of-possession: new device demonstrates private key ownership via server challenge before “verified”.
99-108: Storage schema: add uniqueness/foreign-key constraints and minimal indexesDefine constraints to prevent integrity gaps and to support hot paths.
Suggested DDL refinements:
- devices: UNIQUE(user_id, ed25519_pub), UNIQUE(user_id, id); status CHECK IN ('new','verified','revoked').
- memberships: PK(room_id, device_id); FOREIGN KEYs to rooms(id), devices(id); INDEX on (room_id, status).
- sender_keys: UNIQUE(room_id, epoch); INDEX(room_id, created_at DESC).
- objects: UNIQUE(id); INDEX(room_id), INDEX(owner), INDEX(blake3_digest).
- key_wraps: PK(object_id, device_id); FOREIGN KEYs; INDEX(device_id).
- audit_events: INDEX(actor, ts), INDEX(type, ts); ensure append-only semantics (immutable columns).
18-26: Crypto library choices: pin implementations and randomness sourcesStrong primitive choices. Document implementations per platform and RNG sources to avoid footguns in Web/WASM.
- Web: WebCrypto for AES-GCM/HKDF; libsodium.js or wasm-sodium for X/Ed25519; CSPRNG from crypto.getRandomValues().
- WASM libsignal-client: ensure secure entropy bridge and durable storage for prekeys (IndexedDB).
- Mobile: platform Keystore/Keychain-backed keys where possible.
109-112: Shamir recovery: require policy guardrails to preserve E2EE posture2-of-3 with “admin escrow” weakens user-only control if operationalized loosely.
Add:
- Split-knowledge and role separation for escrow shares; M-of-N approvals with auditable policy.
- Out-of-band user consent and mandatory delay window for recovery.
- Shares stored in HSM-backed services with access logging and anomaly alerts.
1-5: Front-matter polishConsider YAML front-matter for status/owner for consistency with other docs; not blocking.
Example:
title: End-to-End Encryption (E2EE) Design — Option A
status: Draft for review
owner: Kyaw
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
DESIGN-E2EE.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
DESIGN-E2EE.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...ion A Sign-off Status: Draft for review Owner: Kyaw ## 1. Scope and Goals - Web...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...or non-E2EE content. ## 2. Threat Model - Adversaries: curious/compromised servers...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ITM), malicious clients, stolen devices. - Trust boundaries: Only clients see plain...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...r messages; verifiable membership/state. - Out of scope initial: traffic analysis r...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... ## 4. Identity & Device State Machines ### 4.1 Identity Keys States: uninitialized ...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...ice State Machines ### 4.1 Identity Keys States: uninitialized -> generated -> ba...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ed_up (optional) -> compromised(revoked) Transitions: - generate: create Ed25519 ...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...al) -> compromised(revoked) Transitions: - generate: create Ed25519 identity key pa...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...nerate: create Ed25519 identity key pair - backup: wrap private key with Argon2id-d...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...id-derived KEK; store vault in IndexedDB - revoke: mark identity compromised; re-en...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...nroll devices ### 4.2 Device Enrollment States: new -> pending_attestation -> ve...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...nding_attestation -> verified -> revoked Transitions: - new: device generates Ed2...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...tion -> verified -> revoked Transitions: - new: device generates Ed25519 (sign) + X...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...e generates Ed25519 (sign) + X25519 (DH) - provision: QR shows {device_pubkeys, non...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...}; trusted device scans and verifies SAS - attest: trusted device signs attestation...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...s attestation binding device to identity - verify: server records attestation; devi...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...rds attestation; device becomes verified - revoke: immediate revocation; triggers r...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...info="file-chunk" || chunk_index)[0..12] - AES-256-GCM over each chunk; produce per...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...t_without_sig)) ``` ### 6.3 DEK Sharing - For each recipient device X25519 pubkey,...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...or each recipient device X25519 pubkey, create sealed box of DEK. - Store wraps: key_w...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...X25519 pubkey, create sealed box of DEK. - Store wraps: key_wraps(object_id, device...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...s(object_id, device_id, wrap_ciphertext) - Rekey on membership change; rewrap to ac...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...igned by server capability key). Claims: - sub: user or device id - scope: [object:...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...y key). Claims: - sub: user or device id - scope: [object:get|put, room:read, room:...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...oom:read, room:write, membership:manage] - resource: URI or prefix (e.g., s3://buck...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...refix (e.g., s3://bucket/path/object-id) - exp: expiry; iat/nbf - region: data resi...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...t/path/object-id) - exp: expiry; iat/nbf - region: data residency constraint - tid/...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: .../nbf - region: data residency constraint - tid/nonce: unique token id to prevent re...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...d to prevent replay ## 8. APIs (Server) - POST /devices/attest - POST /devices/rev...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ... 8. APIs (Server) - POST /devices/attest - POST /devices/revoke - POST /rooms - POS...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...T /devices/attest - POST /devices/revoke - POST /rooms - POST /rooms/:id/members - ...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...est - POST /devices/revoke - POST /rooms - POST /rooms/:id/members - POST /rooms/:i...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ... - POST /rooms - POST /rooms/:id/members - POST /rooms/:id/rotate - POST /capabilit...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...oms/:id/members - POST /rooms/:id/rotate - POST /capabilities - PUT /objects/:id (r...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...T /rooms/:id/rotate - POST /capabilities - PUT /objects/:id (requires capability) -...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...- PUT /objects/:id (requires capability) - GET /objects/:id (requires capability) -...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...- GET /objects/:id (requires capability) - WS /events ## 9. Storage Schema (Postgr...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...torage Schema (Postgres + S3-compatible) - users(id, identity_pubkey_hash, oidc_sub...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ... identity_pubkey_hash, oidc_sub, region) - devices(id, user_id, ed25519_pub, x25519...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...ub, x25519_pub, attestation_sig, status) - rooms(id, created_by, policy) - membersh...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... status) - rooms(id, created_by, policy) - memberships(room_id, device_id, role, si...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...room_id, device_id, role, since, status) - sender_keys(room_id, epoch, key_id, wrap...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ... key_id, wrapped_keys jsonb, created_at) - objects(id, owner, room_id, bucket, path...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ..._digest, size, manifest_sig, created_at) - key_wraps(object_id, device_id, wrap_cip...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...s(object_id, device_id, wrap_ciphertext) - audit_events(id, actor, type, target, ts...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...and audit. ## 11. Metadata Minimization - Store hashed identity references; coarse...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...encrypted membership maps when feasible. - Avoid plaintext titles/tags. No plaintex...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...L, SCA, SBOM (Syft), container scanning. - Signed commits and releases. ## 16. Tes...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...es. ## 16. Test Plan (Acceptance Gates) - Unit and property tests for: keygen, pro...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...ASETO claims/validation, rotation flows. - Integration: 1:1 E2EE chat, file upload/...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...bership change triggers rewrap/rotation. - Data residency pinning tests; GDPR DSR e...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...rcises. ## 17. Open Items / Future Work - Evaluate MLS migration path for large ro...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...uate MLS migration path for large rooms. - HPKE support behind wrapping abstraction...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
DESIGN-E2EE.md
53-53: Reversed link syntax
(DEK, info="file-chunk" || chunk_index)[0..12]
(MD011, no-reversed-links)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: gitStream.cm
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build
- GitHub Check: build (22.x)
- GitHub Check: run-lint
- GitHub Check: build (4.1.1)
- GitHub Check: build (3.6.3)
- GitHub Check: Analysis Results
🔇 Additional comments (2)
DESIGN-E2EE.md (2)
44-48: Revocation propagation and sender-key rotation triggers“Peers refuse messages from revoked devices” requires timely revocation distribution. Call out the revocation feed and rotation triggers to avoid stale sender keys.
Add:
- A server WS “revocation/events” feed; clients persist last event id and refuse sessions absent revocation sync ≤ N minutes.
- On device revoke: rotate room sender keys immediately and invalidate pending prekeys from the revoked device.
Would you like a concrete event schema proposal?
121-124: Throughput target likely dominated by JS/WASM + I/O; validate with a bench planp95 decrypt < 120 ms for 10 MB is aggressive in a browser with streaming + integrity checks. Recommend publishing a quick micro/macro bench plan across desktop/mobile profiles.
Proposed plan:
- Micro: WebCrypto AES-GCM decrypt of 10 MB ArrayBuffer; measure end-to-end with AAD + nonce derivation.
- Macro: Streaming decrypt + BLAKE3 verify in a Worker with 512 KB vs 2 MB chunks; record p50/p95 on mid-tier laptop and Android mid-range.
- Capture CPU %, GC pauses, and backpressure behavior.
| - Nonce derivation: nonce_i = HKDF(DEK, info="file-chunk" || chunk_index)[0..12] | ||
| - AES-256-GCM over each chunk; produce per-chunk BLAKE3 and cumulative whole-file BLAKE3. | ||
|
|
There was a problem hiding this comment.
Clarify and harden AES-GCM nonce derivation; bind AAD to prevent misuse
The current nonce formula is ambiguous and risks incorrect implementations. For AES-GCM, nonce uniqueness per DEK is critical. Also, include AAD to bind chunk context to the ciphertext.
Apply this diff to specify an unambiguous, misuse-resistant construction and fix the markdown “reversed link” warning:
- - Nonce derivation: nonce_i = HKDF(DEK, info="file-chunk" || chunk_index)[0..12]
- - AES-256-GCM over each chunk; produce per-chunk BLAKE3 and cumulative whole-file BLAKE3.
+ - Nonce derivation (96-bit): Let PRK = HKDF-Extract(salt=blake3_file, IKM=DEK). For each chunk index i:
+ nonce_i = HKDF-Expand(PRK, info = "nonce:file-chunk:v1" || LE64(i), L = 12) # first 12 bytes only (0..11)
+ Notes:
+ - Use LE64(i) as an 8-byte little-endian value. i must be < 2^32 in practice.
+ - "||" denotes byte concatenation.
+ - AES-256-GCM over each chunk with AAD = concat("e2ee-file:v1", version, chunk_index, object_id, algo, chunk_size)
+ to bind context and detect cross-context replays.
+ - Produce per-chunk BLAKE3 and cumulative whole-file BLAKE3 (for content addressing; not relied upon for AEAD integrity).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Nonce derivation: nonce_i = HKDF(DEK, info="file-chunk" || chunk_index)[0..12] | |
| - AES-256-GCM over each chunk; produce per-chunk BLAKE3 and cumulative whole-file BLAKE3. | |
| - Nonce derivation (96-bit): Let PRK = HKDF-Extract(salt=blake3_file, IKM=DEK). For each chunk index i: | |
| nonce_i = HKDF-Expand(PRK, info = "nonce:file-chunk:v1" || LE64(i), L = 12) # first 12 bytes only (0..11) | |
| Notes: | |
| - Use LE64(i) as an 8-byte little-endian value. i must be < 2^32 in practice. | |
| - "||" denotes byte concatenation. | |
| - AES-256-GCM over each chunk with AAD = concat("e2ee-file:v1", version, chunk_index, object_id, algo, chunk_size) | |
| to bind context and detect cross-context replays. | |
| - Produce per-chunk BLAKE3 and cumulative whole-file BLAKE3 (for content addressing; not relied upon for AEAD integrity). |
🧰 Tools
🪛 LanguageTool
[grammar] ~53-~53: There might be a mistake here.
Context: ...info="file-chunk" || chunk_index)[0..12] - AES-256-GCM over each chunk; produce per...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
53-53: Reversed link syntax
(DEK, info="file-chunk" || chunk_index)[0..12]
(MD011, no-reversed-links)
🤖 Prompt for AI Agents
In DESIGN-E2EE.md around lines 53–55, the nonce derivation and AAD are ambiguous
and the markdown has a reversed-link warning; change the spec to derive a 96-bit
(12-byte) GCM nonce by HKDF-Expand using the file DEK as input keying material,
an explicit salt (or zero salt if none), and an info string formed
deterministically (e.g. "file-chunk-v1" || chunk_index || 0x01) and take the
first 12 bytes as the nonce to guarantee uniqueness per DEK+chunk; require
AES-256-GCM to use Additional Authenticated Data that binds chunk_index and a
file identifier (and version/algorithm tag) to the ciphertext (e.g. AAD =
"file-chunk-v1" || file_id || chunk_index) so chunk context is authenticated;
update the text to specify HKDF parameters (hash, salt behavior, info
composition), the exact nonce length (12 bytes), and that implementations must
fail if nonce collisions would occur; and fix the reversed-markdown link by
swapping the link text and URL into the canonical [text](url) form.
DESIGN-E2EE.md
Outdated
| ### 6.2 Manifest Format (signed by device Ed25519) | ||
| ``` | ||
| version: 1 | ||
| algo: aes-256-gcm | ||
| chunk_size: <bytes> | ||
| length: <bytes> | ||
| blake3_file: <hex> | ||
| chunks: | ||
| - index: 0 | ||
| offset: 0 | ||
| size: <bytes> | ||
| blake3: <hex> | ||
| - ... | ||
| key_wraps: omitted in manifest; stored adjacent by object_id | ||
| sig: ed25519(signing_device_pubkey, canonical_json(manifest_without_sig)) | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Manifest: add language tag, key id (kid), and a clear canonicalization standard
Specify YAML for linting, include a key identifier to locate the verifying key, and name the canonicalization algorithm to avoid ambiguity.
Apply this diff:
-```
+```yaml
version: 1
+kid: <signing_device_key_id> # stable identifier or fingerprint of the Ed25519 signing key
algo: aes-256-gcm
chunk_size: <bytes>
length: <bytes>
blake3_file: <hex>
chunks:
- index: 0
offset: 0
size: <bytes>
blake3: <hex>
- ...
key_wraps: omitted in manifest; stored adjacent by object_id
-sig: ed25519(signing_device_pubkey, canonical_json(manifest_without_sig))
+sig: ed25519(signing_device_pubkey, JCS(manifest_without_sig)) # JCS = RFC 8785 canonical JSON
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In DESIGN-E2EE.md around lines 56 to 71, update the manifest format to include a
key identifier and an explicit canonicalization algorithm: add a "kid:
<signing_device_key_id>" field under version, and change the sig line to specify
JCS (RFC 8785) canonical JSON (e.g. sig: ed25519(signing_device_pubkey,
JCS(manifest_without_sig))); also mark the example as YAML for linting/clarity.
Ensure the new kid field is documented as a stable identifier or fingerprint of
the Ed25519 signing key and that the canonicalization change replaces the vague
"canonical_json" wording with "JCS(manifest_without_sig)".
</details>
<!-- fingerprinting:phantom:triton:chinchilla -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ### 6.3 DEK Sharing | ||
| - For each recipient device X25519 pubkey, create sealed box of DEK. | ||
| - Store wraps: key_wraps(object_id, device_id, wrap_ciphertext) | ||
| - Rekey on membership change; rewrap to active devices. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bind DEK wraps to object and recipient; prefer HPKE-ready structure over bare sealed boxes
Sealed boxes don’t carry AAD; pairing a ciphertext with a device_id in the DB without binding risks substitution by a malicious server. Define a wrap payload that includes object_id, alg, and recipient key id, and migrate to HPKE where AAD is first-class.
Apply this diff:
-### 6.3 DEK Sharing
-- For each recipient device X25519 pubkey, create sealed box of DEK.
-- Store wraps: key_wraps(object_id, device_id, wrap_ciphertext)
-- Rekey on membership change; rewrap to active devices.
+### 6.3 DEK Sharing
+- For each recipient device (recipient_kid, X25519 pubkey), create a wrap over:
+ wrap_payload = CBOR({
+ "v": 1,
+ "object_id": <object_id>,
+ "alg": "aes-256-gcm",
+ "dek": <32 bytes>,
+ "recipient_kid": <recipient_kid>
+ })
+ Prefer HPKE(KEM=X25519, KDF=HKDF-SHA256, AEAD=ChaCha20-Poly1305 or AES-GCM) so that AAD = object_id || "dek-wrap:v1"
+ is authenticated. If using libsodium sealed boxes as a stopgap, include the above fields inside the sealed payload.
+- Store wraps: key_wraps(object_id, recipient_kid, wrap_ciphertext, alg="hpke-v1")
+- Rekey on membership change; rewrap to active devices only.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### 6.3 DEK Sharing | |
| - For each recipient device X25519 pubkey, create sealed box of DEK. | |
| - Store wraps: key_wraps(object_id, device_id, wrap_ciphertext) | |
| - Rekey on membership change; rewrap to active devices. | |
| ### 6.3 DEK Sharing | |
| - For each recipient device (recipient_kid, X25519 pubkey), create a wrap over: | |
| wrap_payload = CBOR({ | |
| "v": 1, | |
| "object_id": <object_id>, | |
| "alg": "aes-256-gcm", | |
| "dek": <32 bytes>, | |
| "recipient_kid": <recipient_kid> | |
| }) | |
| Prefer HPKE(KEM=X25519, KDF=HKDF-SHA256, AEAD=ChaCha20-Poly1305 or AES-GCM) so that AAD = object_id || "dek-wrap:v1" | |
| is authenticated. If using libsodium sealed boxes as a stopgap, include the above fields inside the sealed payload. | |
| - Store wraps: key_wraps(object_id, recipient_kid, wrap_ciphertext, alg="hpke-v1") | |
| - Rekey on membership change; rewrap to active devices only. |
🧰 Tools
🪛 LanguageTool
[grammar] ~73-~73: There might be a mistake here.
Context: ...t_without_sig)) ``` ### 6.3 DEK Sharing - For each recipient device X25519 pubkey,...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...or each recipient device X25519 pubkey, create sealed box of DEK. - Store wraps: key_w...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...X25519 pubkey, create sealed box of DEK. - Store wraps: key_wraps(object_id, device...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...s(object_id, device_id, wrap_ciphertext) - Rekey on membership change; rewrap to ac...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In DESIGN-E2EE.md around lines 73 to 77, the DEK wrap scheme currently suggests
storing bare sealed-box ciphertexts keyed only by device_id, which allows
server-side substitution and lacks AAD; update the design to bind each wrap to
both object and recipient by defining a wrap payload structure that contains
object_id, algorithm identifier (alg), and recipient_key_id and to replace
sealed boxes with an HPKE-ready envelope that uses AAD; specify that stored rows
(key_wraps) must include object_id, device_id, recipient_key_id, alg, and
wrap_ciphertext (HPKE-encrypted blob) and that unwrapping must verify AAD
matches the object_id/recipient_key_id/alg before accepting the DEK; update
rekey/migration notes to rewrap existing sealed-box entries into the new HPKE
payload format and validate integrity on migration.
| ## 12. Request Signing & Replay Protection | ||
| - Client signs sensitive requests with device Ed25519 over canonical payload + timestamp. | ||
| - Server enforces skew window and tid uniqueness. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Request signing: specify canonicalization, covered fields, and replay window
Define canonical JSON (e.g., JCS), exact covered headers/query params, and the replay/skew windows to ensure interoperability and security.
Apply this diff:
-## 12. Request Signing & Replay Protection
-- Client signs sensitive requests with device Ed25519 over canonical payload + timestamp.
-- Server enforces skew window and tid uniqueness.
+## 12. Request Signing & Replay Protection
+- Canonicalization: JCS (RFC 8785) over a payload that includes method, path, sorted query, selected headers (host, content-type, content-length), body hash (BLAKE3), timestamp, and tid.
+- Signature: Ed25519 device key over sign_payload; header: X-Signature: base64(sig), X-Signature-KID: <device_kid>.
+- Server enforces clock skew window ≤ 2 minutes and tid uniqueness for 10 minutes (per tenant).
+- GET/HEAD requests sign an empty body hash; all signed fields must be verified before processing.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 12. Request Signing & Replay Protection | |
| - Client signs sensitive requests with device Ed25519 over canonical payload + timestamp. | |
| - Server enforces skew window and tid uniqueness. | |
| ## 12. Request Signing & Replay Protection | |
| - Canonicalization: JCS (RFC 8785) over a payload that includes method, path, sorted query, selected headers (host, content-type, content-length), body hash (BLAKE3), timestamp, and tid. | |
| - Signature: Ed25519 device key over sign_payload; header: X-Signature: base64(sig), X-Signature-KID: <device_kid>. | |
| - Server enforces clock skew window ≤ 2 minutes and tid uniqueness for 10 minutes (per tenant). | |
| - GET/HEAD requests sign an empty body hash; all signed fields must be verified before processing. |
…ize, rotations, TTLs, residency, audit, SAS/QR)
…e, and rekey flows
…), and PASETO payload example
|
View changes in DiffLens |
1 similar comment
|
View changes in DiffLens |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
DESIGN-E2EE.md (4)
73-77: DEK sharing: bind to object and recipient; prefer HPKE envelope over bare sealed boxesAuthenticated AAD avoids server-side substitution of wraps.
-### 6.3 DEK Sharing -- For each recipient device X25519 pubkey, create sealed box of DEK. -- Store wraps: key_wraps(object_id, device_id, wrap_ciphertext) -- Rekey on membership change; rewrap to active devices. +### 6.3 DEK Sharing +- For each recipient device (recipient_kid, X25519 pubkey), create a wrap: + wrap_payload = CBOR({ "v":1, "object_id": <object_id>, "alg":"aes-256-gcm", "dek": <32 bytes>, "recipient_kid": <recipient_kid> }) + Prefer HPKE (KEM=X25519, KDF=HKDF-SHA256, AEAD=ChaCha20-Poly1305 or AES-GCM) with AAD = "dek-wrap:v1" || object_id || recipient_kid. + If using sealed boxes short-term, embed the above payload inside the sealed ciphertext. +- Store wraps: key_wraps(object_id, recipient_kid, wrap_ciphertext, alg="hpke-v1") +- Rekey on membership change; rewrap to active devices only.
53-55: Clarify nonce derivation and require AAD; fix reversed-link lintNonce uniqueness per DEK is critical for GCM; current formula is ambiguous and triggers MD011. Bind chunk context via AAD.
-- Nonce derivation: nonce_i = HKDF(DEK, info="file-chunk" || chunk_index)[0..12] -- AES-256-GCM over each chunk; produce per-chunk BLAKE3 and cumulative whole-file BLAKE3. +- Nonce derivation (96-bit): + Let PRK = HKDF-Extract(salt = blake3_file, IKM = DEK). + For each chunk index i: + nonce_i = HKDF-Expand(PRK, info = "nonce:file-chunk:v1" || LE64(i), L = 12) # bytes 0..11 + Notes: + - Use LE64(i) as an 8-byte little-endian value. i must be < 2^32 in practice. + - "||" denotes byte concatenation. +- AES-256-GCM over each chunk with AAD = concat("e2ee-file:v1", version, chunk_index, object_id, algo, chunk_size) + to bind context and detect cross-context replays. +- Produce per-chunk BLAKE3 (of ciphertext) and cumulative whole-file BLAKE3 (also of ciphertext) for content addressing.
56-71: Manifest example: add kid, specify JCS canonicalization, and mark code block languageThis improves verifiability and fixes MD040.
-``` +```yaml version: 1 +kid: <signing_device_key_id> # identifier/fingerprint of the Ed25519 signing key algo: aes-256-gcm chunk_size: <bytes> length: <bytes> blake3_file: <hex> chunks: - index: 0 offset: 0 size: <bytes> blake3: <hex> - ... key_wraps: omitted in manifest; stored adjacent by object_id -sig: ed25519(signing_device_pubkey, canonical_json(manifest_without_sig)) +sig: ed25519(signing_device_pubkey, JCS(manifest_without_sig)) # RFC 8785 canonical JSON--- `121-124`: **Request signing: specify canonicalization, covered fields, and replay/skew windows** Interoperability and security benefit from precise rules. ```diff -## 12. Request Signing & Replay Protection -- Client signs sensitive requests with device Ed25519 over canonical payload + timestamp. -- Server enforces skew window and tid uniqueness. +## 12. Request Signing & Replay Protection +- Canonicalization: JCS (RFC 8785) over a payload that includes method, path, sorted query, selected headers (host, content-type, content-length), body hash (BLAKE3), timestamp, and tid. +- Signature: Ed25519 device key over sign_payload; headers: X-Signature: base64(sig), X-Signature-KID: <device_kid>. +- Server enforces clock skew window ≤ 2 minutes and tid uniqueness for 10 minutes (per tenant). +- GET/HEAD requests sign an empty body hash; all signed fields must be verified before processing.
🧹 Nitpick comments (3)
docs/sequences/e2ee-sequences.md (1)
23-26: Wrap DEK with AAD-bound envelope; prefer HPKE-ready over bare sealed_boxBare sealed boxes don’t authenticate object_id or recipient binding. Include object_id and recipient_kid in the wrapped payload and authenticate as AAD (HPKE preferred).
- - wrap = sealed_box(DEK, pk_X) - - POST key_wraps(object_id, device_id, wrap) + - wrap_payload = CBOR({ "v":1, "object_id": <object_id>, "alg":"aes-256-gcm", "dek": <32 bytes>, "recipient_kid": <kid_X> }) + - wrap = HPKE_Encaps_Seal(pk_X, aad = "dek-wrap:v1" || object_id || kid_X, plaintext = wrap_payload) + - POST key_wraps(object_id, recipient_kid, wrap, alg="hpke-v1")docs/schemas/capability.schema.json (1)
5-16: Constrain claims surface: disallow extras, validate tid, and bound timestamp fieldsLocking down the payload helps prevent misuse and improves validation.
Apply:
"title": "Capability Claims (PASETO v4.public payload)", "type": "object", "required": ["sub", "scope", "resource", "exp", "region", "tid"], + "additionalProperties": false, "properties": { "sub": {"type": "string"}, "scope": {"type": "string", "enum": ["object:get", "object:put", "room:read", "room:write", "membership:manage"]}, "resource": {"type": "string"}, - "exp": {"type": "integer"}, - "iat": {"type": "integer"}, - "nbf": {"type": "integer"}, + "exp": {"type": "integer", "minimum": 0, "description": "seconds since epoch"}, + "iat": {"type": "integer", "minimum": 0, "description": "seconds since epoch"}, + "nbf": {"type": "integer", "minimum": 0, "description": "seconds since epoch"}, "region": {"type": "string", "description": "enforced data residency region (controls bucket/prefix routing)"}, - "tid": {"type": "string"} + "tid": { + "description": "unique transaction/token id", + "oneOf": [ + {"type": "string", "pattern": "^[0-9A-HJKMNP-TV-Z]{26}$"}, // ULID + {"type": "string", "pattern": "^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$"} // UUID v4 + ] + } } }DESIGN-E2EE.md (1)
107-109: Storage schema: extend key_wraps to include recipient_kid and algTo support AAD-bound wraps and migration to HPKE, add recipient_kid and alg columns.
-- key_wraps(object_id, device_id, wrap_ciphertext) +- key_wraps(object_id, recipient_kid, wrap_ciphertext, alg)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
DESIGN-E2EE.md(1 hunks)docs/examples/paseto-v4-public.json(1 hunks)docs/schemas/capability.schema.json(1 hunks)docs/schemas/manifest.schema.json(1 hunks)docs/sequences/e2ee-sequences.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/examples/paseto-v4-public.json
🧰 Additional context used
🪛 LanguageTool
DESIGN-E2EE.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...n A Sign-off Status: Ready for sign-off Owner: Kyaw ## 1. Scope and Goals - Web...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...or non-E2EE content. ## 2. Threat Model - Adversaries: curious/compromised servers...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ITM), malicious clients, stolen devices. - Trust boundaries: Only clients see plain...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...r messages; verifiable membership/state. - Out of scope initial: traffic analysis r...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... ## 4. Identity & Device State Machines ### 4.1 Identity Keys States: uninitialized ...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...ice State Machines ### 4.1 Identity Keys States: uninitialized -> generated -> ba...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ed_up (optional) -> compromised(revoked) Transitions: - generate: create Ed25519 ...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...al) -> compromised(revoked) Transitions: - generate: create Ed25519 identity key pa...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...nerate: create Ed25519 identity key pair - backup: wrap private key with Argon2id-d...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...id-derived KEK; store vault in IndexedDB - revoke: mark identity compromised; re-en...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...nroll devices ### 4.2 Device Enrollment States: new -> pending_attestation -> ve...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...nding_attestation -> verified -> revoked Transitions: - new: device generates Ed2...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...tion -> verified -> revoked Transitions: - new: device generates Ed25519 (sign) + X...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...e generates Ed25519 (sign) + X25519 (DH) - provision: QR shows {device_pubkeys, non...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...}; trusted device scans and verifies SAS - attest: trusted device signs attestation...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...s attestation binding device to identity - verify: server records attestation; devi...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...rds attestation; device becomes verified - revoke: immediate revocation; triggers r...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...info="file-chunk" || chunk_index)[0..12] - AES-256-GCM over each chunk; produce per...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...t_without_sig)) ``` ### 6.3 DEK Sharing - For each recipient device X25519 pubkey,...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...or each recipient device X25519 pubkey, create sealed box of DEK. - Store wraps: key_w...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...X25519 pubkey, create sealed box of DEK. - Store wraps: key_wraps(object_id, device...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...igned by server capability key). Claims: - sub: user or device id - scope: [object:...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...y key). Claims: - sub: user or device id - scope: [object:get|put, room:read, room:...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...refix (e.g., s3://bucket/path/object-id) - exp: expiry; iat/nbf - region: enforced ...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...t/path/object-id) - exp: expiry; iat/nbf - region: enforced data residency region (...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ... region (controls bucket/prefix routing) - tid/nonce: unique token id to prevent re...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...nonce: unique token id to prevent replay - TTLs: object GET/PUT 15 minutes; room/ev...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...vent scopes 1 hour. ## 8. APIs (Server) - POST /devices/attest - POST /devices/rev...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ... 8. APIs (Server) - POST /devices/attest - POST /devices/revoke - POST /rooms - POS...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...T /devices/attest - POST /devices/revoke - POST /rooms - POST /rooms/:id/members - ...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...est - POST /devices/revoke - POST /rooms - POST /rooms/:id/members - POST /rooms/:i...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ... - POST /rooms - POST /rooms/:id/members - POST /rooms/:id/rotate - POST /capabilit...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...oms/:id/members - POST /rooms/:id/rotate - POST /capabilities - PUT /objects/:id (r...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...T /rooms/:id/rotate - POST /capabilities - PUT /objects/:id (requires capability) -...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...- PUT /objects/:id (requires capability) - GET /objects/:id (requires capability) -...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...- GET /objects/:id (requires capability) - WS /events ## 9. Storage Schema (Postgr...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... identity_pubkey_hash, oidc_sub, region) - devices(id, user_id, ed25519_pub, x25519...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...ub, x25519_pub, attestation_sig, status) - rooms(id, created_by, policy) - membersh...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ... status) - rooms(id, created_by, policy) - memberships(room_id, device_id, role, si...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...room_id, device_id, role, since, status) - sender_keys(room_id, epoch, key_id, wrap...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ... key_id, wrapped_keys jsonb, created_at) - objects(id, owner, room_id, bucket, path...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ..._digest, size, manifest_sig, created_at) - key_wraps(object_id, device_id, wrap_cip...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...s(object_id, device_id, wrap_ciphertext) - audit_events(id, actor, type, target, ts...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ...encrypted membership maps when feasible. - Avoid plaintext titles/tags. No plaintex...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...chema uses sidecar tables for isolation. - Instant kill-switch disables capability ...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...L, SCA, SBOM (Syft), container scanning. - Signed commits and releases. ## 16. Tes...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...es. ## 16. Test Plan (Acceptance Gates) - Unit and property tests for: keygen, pro...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...ASETO claims/validation, rotation flows. - Integration: 1:1 E2EE chat, file upload/...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...bership change triggers rewrap/rotation. - Data residency pinning tests; GDPR DSR e...
(QB_NEW_EN)
[grammar] ~144-~144: There might be a mistake here.
Context: ...rcises. ## 17. Open Items / Future Work - Evaluate MLS migration path for large ro...
(QB_NEW_EN)
[grammar] ~145-~145: There might be a mistake here.
Context: ...uate MLS migration path for large rooms. - HPKE support behind wrapping abstraction...
(QB_NEW_EN)
docs/sequences/e2ee-sequences.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...l) ## 1) Device Provisioning (QR + SAS) 1. New device: gen (Ed25519_sign_D, X25519_...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...eys, nonce: N, ts, sig: sign_D(N||ts) }) 3. Trusted device scans QR 4. Both devices ...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...r_identity_pub, device_pubkeys, N, ts }) 7. Trusted device -> Server: POST /devices/...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...erver: POST /devices/attest(attestation) 8. Server records device; broadcasts WS eve...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ile Share (Encrypt + Upload + Share DEK) 1. Client picks file; gen random DEK 2. For...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...ile; gen random DEK 2. For each chunk i: - nonce_i = HKDF(DEK, info="file-chunk"||i...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...= HKDF(DEK, info="file-chunk"||i)[0..12] - c_i = AES-256-GCM(plaintext_i, nonce_i) ...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ... c_i = AES-256-GCM(plaintext_i, nonce_i) - blake3_i = BLAKE3(c_i) 3. blake3_file = ...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ..._i, nonce_i) - blake3_i = BLAKE3(c_i) 3. blake3_file = BLAKE3(c_0||c_1||...) 4. M...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...) 3. blake3_file = BLAKE3(c_0||c_1||...) 4. Manifest = { version, algo, chunk_size, ...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...nk_size, length, blake3_file, chunks[] } 5. sig = Ed25519.sign(manifest_without_sig)...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...sig = Ed25519.sign(manifest_without_sig) 6. Upload chunks and manifest (ciphertext o...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...only) 7. For each recipient device pk_X: - wrap = sealed_box(DEK, pk_X) - POST k...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ... pk_X: - wrap = sealed_box(DEK, pk_X) - POST key_wraps(object_id, device_id, wra...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... wrap) ## 3) Membership change -> Rekey 1. Admin changes room members 2. Server emi...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
DESIGN-E2EE.md
53-53: Reversed link syntax
(DEK, info="file-chunk" || chunk_index)[0..12]
(MD011, no-reversed-links)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/sequences/e2ee-sequences.md
16-16: Reversed link syntax
(DEK, info="file-chunk"||i)[0..12]
(MD011, no-reversed-links)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: run-lint
- GitHub Check: build
- GitHub Check: test (2.7)
- GitHub Check: build (3.6.3)
- GitHub Check: build (4.1.1)
- GitHub Check: Analysis Results
🔇 Additional comments (3)
docs/schemas/manifest.schema.json (2)
10-11: Schema/design drift: chunk_size bounds don’t match the designDESIGN-E2EE.md (Line 52) says 512KB–2MB. The schema allows 256KB–4MB. Please reconcile and choose one source of truth.
1-27: Enforce stricter JSON-Schema formanifest.schema.jsonI couldn’t find a design doc that definitively pins the
chunk_sizebounds (please confirm whether it should be 512 KB–2 MB or another range). In the meantime, tighten the schema to prevent drift and ambiguity:• Add
"additionalProperties": falseat the top level
• Require a signing-key identifier (kid) alongsidesig
• Constrainchunk_sizebounds per your specification (e.g. 524288–2097152)
• Enforce at least one chunk and forbid extras in each chunk object:
"minItems": 1onchunks"additionalProperties": falseinside eachchunks.items
• Constrainsigto a precise encoding and document canonicalization (JCS):8c8,10 < "required": ["version", "algo", "chunk_size", "length", "blake3_file", "chunks", "sig"], --- > "required": ["version", "algo", "chunk_size", "length", "blake3_file", "chunks", "kid", "sig"], > "additionalProperties": false, 11c13 < "chunk_size": {"type": "integer", "minimum": 262144, "maximum": 4194304}, --- > "chunk_size": {"type": "integer", "minimum": /* confirm lower bound */, "maximum": /* confirm upper bound */}, 18a21 > "kid": {"type": "string", "description": "identifier of the Ed25519 signing key"}, 22a26,27 > "chunks": { > "type": "array", "minItems": 1, 26a32 > "additionalProperties": false, 36c42,50 < "sig": {"type": "string", "description": "Ed25519 signature over canonicalized manifest without sig"} --- > "sig": { > "description": "Ed25519 signature over JCS(RFC 8785)-canonicalized manifest_without_sig", > "oneOf": [ > {"type": "string","pattern":"^[0-9a-f]{128}$"}, > {"type":"string","pattern":"^[A-Za-z0-9_-]{86}(==)?$"} > ] > }Please confirm the intended
chunk_sizelimits against your spec.DESIGN-E2EE.md (1)
52-52: Align chunk_size bounds with the schemaHere you state 512KB–2MB, while manifest.schema.json allows 256KB–4MB. Please pick one and update the other for consistency.
| - nonce_i = HKDF(DEK, info="file-chunk"||i)[0..12] | ||
| - c_i = AES-256-GCM(plaintext_i, nonce_i) | ||
| - blake3_i = BLAKE3(c_i) | ||
| 3. blake3_file = BLAKE3(c_0||c_1||...) | ||
| 4. Manifest = { version, algo, chunk_size, length, blake3_file, chunks[] } | ||
| 5. sig = Ed25519.sign(manifest_without_sig) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Nonce derivation is ambiguous and MD lint flags reversed-link; specify 96-bit HKDF construction and bind AAD
For AES-GCM, nonce uniqueness is critical. Also bind context via AAD to prevent cross-object replay. Fix the “reversed link” warning by avoiding bracket slicing syntax.
Apply:
- - nonce_i = HKDF(DEK, info="file-chunk"||i)[0..12]
- - c_i = AES-256-GCM(plaintext_i, nonce_i)
+ - Derive 96-bit nonce_i deterministically:
+ Let PRK = HKDF-Extract(salt = blake3_file, IKM = DEK)
+ nonce_i = HKDF-Expand(PRK, info = "nonce:file-chunk:v1" || LE64(i), L = 12) # bytes 0..11
+ - c_i = AES-256-GCM(plaintext_i, nonce_i, AAD = concat("e2ee-file:v1", version, chunk_index=i, object_id, algo, chunk_size))
- blake3_i = BLAKE3(c_i)
-3. blake3_file = BLAKE3(c_0||c_1||...)
+3. blake3_file = BLAKE3(c_0 || c_1 || ...)
-4. Manifest = { version, algo, chunk_size, length, blake3_file, chunks[] }
+4. Manifest = { version, algo, chunk_size, length, blake3_file, chunks[] } # signature excludes sig field
-5. sig = Ed25519.sign(manifest_without_sig)
+5. sig = Ed25519.sign(JCS(manifest_without_sig)) # RFC 8785 canonical JSON📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - nonce_i = HKDF(DEK, info="file-chunk"||i)[0..12] | |
| - c_i = AES-256-GCM(plaintext_i, nonce_i) | |
| - blake3_i = BLAKE3(c_i) | |
| 3. blake3_file = BLAKE3(c_0||c_1||...) | |
| 4. Manifest = { version, algo, chunk_size, length, blake3_file, chunks[] } | |
| 5. sig = Ed25519.sign(manifest_without_sig) | |
| - Derive 96-bit nonce_i deterministically: | |
| Let PRK = HKDF-Extract(salt = blake3_file, IKM = DEK) | |
| nonce_i = HKDF-Expand(PRK, info = "nonce:file-chunk:v1" || LE64(i), L = 12) # bytes 0..11 | |
| - c_i = AES-256-GCM(plaintext_i, nonce_i, | |
| AAD = concat("e2ee-file:v1", version, chunk_index = i, object_id, algo, chunk_size)) | |
| - blake3_i = BLAKE3(c_i) | |
| 3. blake3_file = BLAKE3(c_0 || c_1 || ...) | |
| 4. Manifest = { version, algo, chunk_size, length, blake3_file, chunks[] } # signature excludes sig field | |
| 5. sig = Ed25519.sign(JCS(manifest_without_sig)) # RFC 8785 canonical JSON |
🧰 Tools
🪛 LanguageTool
[grammar] ~16-~16: There might be a mistake here.
Context: ...= HKDF(DEK, info="file-chunk"||i)[0..12] - c_i = AES-256-GCM(plaintext_i, nonce_i) ...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ... c_i = AES-256-GCM(plaintext_i, nonce_i) - blake3_i = BLAKE3(c_i) 3. blake3_file = ...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ..._i, nonce_i) - blake3_i = BLAKE3(c_i) 3. blake3_file = BLAKE3(c_0||c_1||...) 4. M...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...) 3. blake3_file = BLAKE3(c_0||c_1||...) 4. Manifest = { version, algo, chunk_size, ...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...nk_size, length, blake3_file, chunks[] } 5. sig = Ed25519.sign(manifest_without_sig)...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...sig = Ed25519.sign(manifest_without_sig) 6. Upload chunks and manifest (ciphertext o...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
16-16: Reversed link syntax
(DEK, info="file-chunk"||i)[0..12]
(MD011, no-reversed-links)
🤖 Prompt for AI Agents
In docs/sequences/e2ee-sequences.md around lines 16–21, the nonce derivation and
AES-GCM usage are ambiguous and trigger MD lint warnings: replace the
bracket-slice notation and clearly specify a 96-bit (12‑byte) HKDF-based nonce
construction (e.g. use HKDF-Extract/Expand and take the first 12 bytes of
HKDF-Expand output), and explicitly bind additional authenticated data (AAD)
that includes per-object context (for example file identifier, manifest version
or chunk index) to AES-256-GCM to prevent cross-object replay; update the text
to show the HKDF construction name (HKDF-Extract/Expand), specify which input is
salt/IKM and info, remove the [0..12] slice syntax, and note that AAD should
include the manifest (or file-id||version) when encrypting each chunk before
computing BLAKE3 and signing the manifest.
…aid diagrams for key flows
|
View changes in DiffLens |
1 similar comment
|
View changes in DiffLens |
This PR adds the end-to-end encryption design document (Option A) for review and sign-off.\n\nHighlights:\n- Threat model and adversary mapping\n- Identity/device key state machines\n- Provisioning, rotation, revocation, and recovery flows\n- File crypto streaming + manifest spec (AES-256-GCM, HKDF nonces, BLAKE3)\n- Capability tokens via PASETO v4.public\n- Metadata minimization and privacy budget policy\n- Rollout, canary, and kill-switch logic\n- Acceptance test plan and performance targets\n\nPlease review and leave comments inline in DESIGN-E2EE.md.\n
Summary by Sourcery
Add a comprehensive end-to-end encryption design specification (Option A) outlining the architecture, key management, encryption flows, and operational procedures for security review and sign-off.
Documentation:
Summary by CodeRabbit