fix(desktop): implement AES-KW instead of BoringSSL-missing aes-256-wrap - #575
Conversation
Creating a sync passphrase failed with `Error: Unknown cipher`.
`encryptionService.wrapKey()` called `createCipheriv('aes-256-wrap', ...)`.
That cipher exists in OpenSSL but not in BoringSSL, which is what Electron
ships — so the call always threw in the main process and the CEK could
never be wrapped. Vitest never caught it because it runs on Node/OpenSSL,
where the cipher is available.
Implement RFC 3394 directly over the raw AES block primitive BoringSSL does
expose. Output is byte-identical to `aes-256-wrap`, verified against the
RFC 3394 §4.6 known-answer vector, so keys wrapped by earlier builds still
unwrap.
Verified under the Electron 41.7.1 binary (BoringSSL): wrap matches the
vector, round-trip succeeds, and a wrong wrapping key is rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR adds an RFC 3394 AES-256 key-wrap implementation using BoringSSL-compatible AES-ECB operations. The encryption service now uses the new helpers, with tests covering compatibility, integrity checks, and invalid keys. ChangesAES key-wrap implementation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR replaces the unavailable desktop cipher implementation with a compatible AES key-wrapping implementation while preserving existing wrapped-key output; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
🎉 This PR is included in version 0.17.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Problem
Creating a sync passphrase fails with
Error: Unknown cipher.encryptionService.wrapKey()calledcreateCipheriv('aes-256-wrap', ...). That cipher exists in OpenSSL but not in BoringSSL, which is what Electron ships. The call therefore always threw in the main process and the CEK could never be wrapped.Reproduced directly:
aes-256-wrapprocess.versions.openssl === '0.0.0')Unknown cipherVitest never caught this because it runs on Node/OpenSSL, where the cipher exists.
Fix
New
aesKeyWrap.tsimplements RFC 3394 over the raw AES block primitive BoringSSL does expose.encryptionServicedelegates to it.Output is byte-identical to
aes-256-wrap, so keys wrapped by earlier builds still unwrap — pinned by the RFC 3394 §4.6 known-answer vector (which I generated from OpenSSL's ownaes-256-wrapbefore writing the implementation).aes-256-ecbhere is the single-block AES call the spec is defined on, with padding disabled — not a mode for bulk data. The chaining, counter and integrity check are RFC 3394's. This is documented at the top of the module.Neither
webcrypto.subtleAES-KW (Unrecognized algorithm name) nor any other built-in wrap is available in Electron, so reimplementing was the only option that preserves the on-disk format.Verification
aes-256-wrap.ELECTRON_RUN_AS_NODE=1): wrap matches the vector, round-trip succeeds, wrong key rejected.pnpm test— 19/19 tasks green.pnpm --filter @dripnex/desktop typecheck— clean.eslint— clean.Note for a separate PR
.nvmrcpins Node 20, butpackages/mcp-serverneeds >= 22.5 (node:sqlite) andpackages/apineeds >= 22 (wrangler). On Node 20pnpm testandpnpm devboth fail for reasons unrelated to this change.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests