digidem / digidem/comapeo-core-react-native
refactor: read the rootkey and master key through a native addon instead of the control socket
Nobody has claimed this yet.
- Dominant language
- Kotlin
- Stars
- 1
- Forks
- 0
- Avg merge
- 8h 24m
- Merged PRs (30d)
- 9
Description
Node and the native key stores are co-resident on both platforms — on iOS Node runs on a thread in the app process, and on Android it runs inside the :ComapeoCore FGS process alongside the Kotlin RootKeyStore. Despite that, the rootkey and the cached master key travel between them as base64 over control.sock, with validation, scrubbing, and replay machinery on both sides.
This issue introduces a thin N-API addon so Node calls the existing native stores directly, and takes both keys off the wire.
It also establishes the addon itself, which #244 and (indirectly) #245 build on. Sequence this first.
Why
The init frame is the reason control.sock must bind before comapeo.sock — native has to hand the rootkey over before MapeoManager can be constructed, which is one of the three justifications in ARCHITECTURE §3.2 for having two sockets at all. Removing the secret from the wire removes that ordering constraint.
Concretely, this deletes:
init.rootKeyandinit.masterKey, and their validation inbackend/lib/parse-init.js(the strict-base64 regexes, the trailing-bits round-trip check, the 16/32-byte length gates).- The
master-keyframe end to end:MasterKeyFrame.kt,backend/lib/master-key-frame.js,ControlFrame.MasterKey, the iOS equivalent, and the paired-validator test that locks the inbound and outbound gates together. pendingRootKeyFingerprintand the fingerprint-binding dance insendInitFrame/storeMasterKey, which exists only because the rootkey is zeroed before the reply arrives.initReplyPortand the "target the connection that sent init, don't broadcast" special case inbackend/index.js.- A meaningful share of the scrubbing burden.
sentry-scrub.ts,test-support/scrubber-cases.jsand the tripwire carry the weight they do because secrets cross a channel that gets logged. Defence that no longer has to hold is better than defence that holds.
What it does not change: the storage format, the envelope crypto, the AndroidKeyStore/Keychain interaction, the legacy expo-secure-store migration, or the caching semantics from #228. Those stay in Kotlin and Swift exactly as they are.
Cost
A first-party addon, which the build pipeline has not had before — every current addon is a third-party prebuild fetched by scripts/build-backend.ts. Details in the plan; on Android it is unusually cheap because the host .so already exists.
Keystore decrypt and StrongBox writes can exceed 100 ms, so the calls must not block Node's loop thread. They need napi_create_async_work, which also means the backend awaits a promise rather than awaiting a frame.
Performance is not a motivation here and should not be used to justify design choices. Two calls per boot, 16 and 32 bytes. The one real timing effect is that the boot round trip collapses — today Node binds the socket, broadcasts started, native wakes, decrypts, and replies, where a pull is just the keystore decrypt. That is a few ms on a low-end device, not a throughput story.
The rootkey load moves from "native pushes on started" to "Node pulls when it needs it". Failure attribution changes shape: today a rootkey failure is reported via error-native before Node has built anything, and the FGS-side boot.rootkey-load span wraps it. After this change the failure surfaces as a rejected promise inside Node's boot. Both paths must still land in ERROR with phase: "rootkey".
Plan
1. Addon skeleton
Create native-bridge/ at the repo root — C sources plus per-platform packaging. It exposes N-API functions that forward to platform entry points; it contains no crypto and no keystore logic.
Android: add napi_register_module_v1 to the existing libcomapeo-core-react-native.so. That target is already built in-repo by android/CMakeLists.txt from android/src/main/cpp/jni-bridge.cpp, already has a working JNI_OnLoad (fbjni, line 162), and is already loaded by System.loadLibrary("comapeo-core-react-native") at NodeJSService.kt:215. Node then does process.dlopen(mod, 'libcomapeo-core-react-native.so') — bare name against the APK mmap region, the same mechanism __loadAddon uses for the other addons (BUILD.md §4). Because Kotlin loaded it first, dlopen returns the existing handle and the cached JavaVM is already there. One .so, two entry points, no new artifact.
The addon needs an Application Context for SharedPreferences. Add a JNI entry point that Kotlin calls before starting Node (alongside the existing initialize(dataDir)) to stash a global ref.
iOS: build a small comapeo-native__<version>.xcframework containing only the N-API glue. It must not link against the app, so it resolves the Swift side with dlsym(RTLD_DEFAULT, "comapeo_get_root_key") etc. — the @_cdecl symbols live in the host binary and RTLD_DEFAULT searches globally. Package it under ios/Frameworks/ like the others; AppLifecycleDelegate.swift:52 already exports NATIVE_LIB_DIR and the loader already resolves <name>__<version>.framework/<name>.
Note the version in the filename is part of the existing scheme, not decoration — keep it and bump it when the ABI changes.
2. Native entry points
Swift, in ios/RootKeyStore.swift or a small adjacent file, as @_cdecl C functions. Kotlin, as @JvmStatic methods on a new object that the JNI glue calls. Both wrap the store APIs that already exist and are identical in shape across platforms:
loadOrInitialize() -> RootKeyResult { key, generated } // throws
loadMasterKey(rootKey) -> ByteArray? / Data?
storeMasterKey(masterKey, rootKeyFingerprint)
fingerprintOf(rootKey) -> ByteArray / Data // static
3. Addon JS surface
Three functions, all async, all returning/accepting Buffer:
getRootKey() // -> { rootKey: Buffer, generated: boolean }
getMasterKey(rootKey) // -> Buffer | null
putMasterKey(masterKey, rootKeyFingerprint) // -> void
Keep fingerprintOf native rather than reimplementing it in JS — it is already RootKeyStore.fingerprintOf / RootKeyStore.fingerprint(of:) on both platforms, and a second implementation is a second thing to keep in agreement. Either return the fingerprint alongside the rootkey from getRootKey(), or expose it as a fourth function; returning it from getRootKey() avoids handing the rootkey back across the boundary a second time and is preferred.
All three go through napi_create_async_work. None may throw into Node's loop; failures reject.
3a. Copy discipline — deliberately not zero-copy
Related work on the Sentry path (#244) minimises copies across the addon boundary. That goal inverts here and the difference must be preserved.
These are secrets. The objective is few tracked, zeroable copies, not few copies. A borrowed V8 buffer is neither reliably zeroable (V8 may have made internal copies during any string or GC operation) nor ours to zero, since N-API cannot take ownership of a backing store. At 16 and 32 bytes the copy count is irrelevant to performance.
So: copy explicitly at every hop, keep each copy in a buffer we own, and fill(0) / resetBytes every one on the way out — which is what the current native code already does. Do not "optimise" this path to borrow or transfer, and do not let a future pass at #244's copy discipline leak into it.
4. Backend wiring
backend/lib/create-key-manager.js is already split out and documented as unit-testable "without sockets or a manager", so it needs no change — only its inputs move.
Replace await initPromise in backend/index.js with a call through the addon:
const { rootKey, fingerprint } = await bridge.getRootKey()
const masterKey = await bridge.getMasterKey(rootKey) // null on first boot
const keyManager = await createKeyManager({ rootKey, masterKey, withSpan })
and replace the initReplyPort.postMessage(masterKeyFrame(...)) block with await bridge.putMasterKey(keyManager.getMasterKey(), fingerprint), keeping the existing zeroing and the never-fatal semantics (a failed write costs the next boot a derivation, exactly as today).
Keep the boot.master-key-derive span in createKeyManager — it remains the degraded-device signal. Add a span around getRootKey() so the native boot.rootkey-load timing is still visible from the Node side now that it is on Node's critical path.
Delete parse-init.js, master-key-frame.js, and their tests. Remove init and the master-key handling from the SimpleRpcServer method table and from ControlFrame.{kt,swift}. SimpleRpcServer keeps shutdown and error-native for now — those go in #245.
5. Native cleanup
Delete sendInitFrame() and storeMasterKey() from both NodeJSService.kt and NodeJSService.swift, along with pendingRootKeyFingerprint. The started frame no longer triggers anything except the node-spawn span close and backendState = .controlBound; leave both in place.
RootKeyStore itself is untouched on both platforms.
6. Boot-order check
Once init is gone, control.sock no longer needs to bind before comapeo.sock. Do not reorder them in this issue — just note it in ARCHITECTURE §3.2, since the constraint is what #245 builds on.
Testing
The existing store tests are the ones that matter and they should not move: RootKeyStoreTest.kt (the 19-case on-device storage matrix), RootKeyStoreTests.swift, and the @comapeo/crypto vectors. If those still pass, the storage layer is unchanged by construction.
New:
- A device-level test per platform that drives the three addon functions from Node and asserts a round trip — put a master key, read it back, confirm a fingerprint mismatch returns null. This is the contract that replaces the paired-validator test being deleted, and it must run on-device because it exercises the real keystore.
- Backend unit tests for the boot path with a faked bridge: cache hit (no derivation), cache miss (one derivation, one
putMasterKey),getRootKey()rejecting (boot fails withphase: "rootkey"), andputMasterKeyrejecting (boot succeeds). - Confirm the mutation-checked derivation-count assertions from #228 still hold.
MockBackend.swift's handshake currently reads the init frame and asserts on receivedRootKey. That assertion moves to the addon test; the mock keeps the started/ready handshake minus the init step.
Risks
dlopen of a library already loaded by System.loadLibrary returning the same handle is the mechanism this depends on for the Android JavaVM. It is standard Bionic behaviour but should be smoke-tested early — if it does not hold, the fallback is JNI_GetCreatedJavaVMs.
RTLD_DEFAULT symbol resolution on iOS depends on the Swift @_cdecl symbols being exported from the app binary and not stripped in Release. Verify against a Release build, not just Debug.
Async work on the FGS process means the keystore call can outlive a service stop. Guard the completion path against a torn-down service.
Out of scope
Removing control.sock, moving the Sentry traffic, and the FGS↔main lifecycle channel. Sequenced separately.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the addon packaging points in native-bridge/, android/CMakeLists.txt, NodeJSService.kt, and AppLifecycleDelegate.swift, then trace backend/index.js and create-key-manager.js. Run RootKeyStoreTest.kt, RootKeyStoreTests.swift, and the new device-level addon tests while checking the backend bridge tests. Done means all three async addon calls work on both platforms, the boot-path cases pass, and init/master-key socket handling is removed without changing storage behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, c, ios, kotlin, node.js, swift
- Domain
- backend, build-system, mobile
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100