Skip to content

fix(android): drop setUnlockedDeviceRequired from rootkey wrapper key#57

Merged
gmaclennan merged 2 commits intomainfrom
fix/rootkey-no-screen-lock
May 7, 2026
Merged

fix(android): drop setUnlockedDeviceRequired from rootkey wrapper key#57
gmaclennan merged 2 commits intomainfrom
fix/rootkey-no-screen-lock

Conversation

@gmaclennan
Copy link
Copy Markdown
Member

@gmaclennan gmaclennan commented May 7, 2026

Summary

  • Drops setUnlockedDeviceRequired(true) from the AndroidKeyStore-backed rootkey wrapper key generated by RootKeyStore.

Why

setUnlockedDeviceRequired(true) has two failure modes that are unacceptable for a key whose loss equals identity loss:

  1. Generation requires a configured secure lock screen. Devices without a PIN/pattern/password fail to generate the wrapper key, and the FGS bricks at startup with a rootkey error. Many CoMapeo users — community/conservation contexts, often on shared or no-lock devices — would simply not be able to use the app.
  2. Permanent invalidation on lock-screen toggle. Per KeyPermanentlyInvalidatedException and OWASP MASTG-KNOW-0043, keys are "permanently and irreversibly invalidated once the secure lock screen is disabled". Re-adding the lock screen does not recover the key. A user who briefly disables their PIN loses their CoMapeo identity in every project they participate in.

What we keep

The wrapper key remains:

  • Hardware-backed (StrongBox if available, TEE otherwise)
  • AES-256 GCM, non-extractable
  • Scoped to this app's signing certificate
  • Persisted as an encrypted envelope in SharedPreferences

The only attack surface gained: code execution as our app while the device sits in the post-boot, pre-first-unlock state could decrypt the rootkey envelope. There is no practical path for that on current Android even without the gate, and it's a vanishing concern relative to the identity-loss failure modes above.

Prior art

expo-secure-store — the de-facto standard secret store on Expo/RN Android — makes the same trade and omits this flag. Its default-case wrapper key uses only setUserAuthenticationRequired(false); no lock-screen-tied attributes.

Test plan

  • Existing RootKeyStoreTest instrumented tests still pass on a device with a configured screen lock (no behavior change for that path).
  • Fresh install on an emulator/device without a configured screen lock: FGS reaches STARTED, wrapper key generates, rootkey persists, second launch returns the same bytes.
  • Fresh install on a device with a configured screen lock, then disable the lock: subsequent launches still load the existing wrapper key (no invalidation, since fresh keys generated under this PR don't have the unlock-required gate). Re-enable the lock: still loads.
  • Manual sanity that the comment block accurately describes the rationale (no separate runtime check needed).

gmaclennan and others added 2 commits May 6, 2026 23:45
`setUnlockedDeviceRequired(true)` requires a configured secure lock
screen at key generation and permanently invalidates the key if the
user later disables their lock screen — even briefly, with no
recovery path. For CoMapeo the rootkey IS the user's identity in
every project they participate in, so either failure mode is
identity loss; the marginal at-rest gain over baseline AndroidKeyStore
hardware-backed AES-GCM doesn't justify it. Matches the trade-off
expo-secure-store makes for the same reason.

Existing wrapper keys keep working unchanged; this only affects
fresh generations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d gate

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gmaclennan gmaclennan enabled auto-merge (squash) May 7, 2026 08:26
@gmaclennan gmaclennan merged commit 1078fa1 into main May 7, 2026
8 of 9 checks passed
@gmaclennan gmaclennan deleted the fix/rootkey-no-screen-lock branch May 7, 2026 08:53
gmaclennan added a commit that referenced this pull request May 7, 2026
Reduces the production-code touch points exposed for non-production
consumers (the bench app being the only one) down to a single override
on each platform plus the existing nodejs-mobile stdout-redirect gate.

- Drop `comapeoBackendArgs` (Gradle property + BuildConfig field +
  Kotlin parsing on Android; Info.plist key + Swift parsing on iOS).
  Was speculative surface for future telemetry-sink overrides; nothing
  in this PR populates it. The `--device=<tag>` argv injection the
  native loader does unconditionally is unaffected — production
  backend ignores unknown flags and Sentry tagging will read it.

- Rename `comapeoBackendDir` → `comapeoEntryFile`. Override is now a
  filename inside `nodejs-project/` rather than a sibling directory.
  Bench plugin drops the bench entry into the consumer's
  `nodejs-project/` and lets AGP's asset merge (Android) / a
  Run Script Phase (iOS) co-locate it with the production bundle's
  `index.mjs`. Bench bundle's rollup output is renamed to
  `index.bench.mjs` and no longer ships a `package.json` (the
  production bundle's already does, in the same directory).

- Drop `comapeoStubRootKey` end-to-end now that #57 (drop
  setUnlockedDeviceRequired from rootkey wrapper key) has landed on
  main. The stub existed only to work around BrowserStack stock
  no-screen-lock devices failing key generation; the real keystore
  path now succeeds for them, the bench backend's relaxed init
  handler ignores the rootkey bytes it receives, and the production
  branch in the FGS loader simplifies back to a single
  RootKeyStore.loadOrInitialize() call.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant