From e0fd8df4226d4eabdcd404c1c88b39f4febca5fd Mon Sep 17 00:00:00 2001 From: JingMatrix Date: Fri, 21 Aug 2026 15:56:49 +0200 Subject: [PATCH] Delete the RKP-only properties instead of flipping them to false The two remote_provisioning.*.rkp_only properties gate whether keystore2 provisions attestation keys only through RKP or may fall back to a batch key; letting the batch key through is what routes a target app's key generation onto our keybox. They are plain system properties, not persist.*, and a vendor .prop that init re-applies each boot can reassert the on default (#236), so a live resetprop reverts on the next boot (#244) -- and on some devices a delayed config sync reasserts it hours later with no reboot at all (#262). Writing false also leaves an anomalous value behind: a device that ships true now reads false, which is itself a signal. So instead of flipping the property, the user now deletes it. resetprop --delete removes it entirely; keystore2 and our hook both read an unset property as false, so the keybox path is reached exactly as before, but the device now looks like one that never declared RKP-only enforcement rather than one with a suspiciously flipped value. The choice is recorded as a delete intent in /data/adb/teesim/rkp.json ({ property: value-at-deletion }) and re-enforced at daemon start and on every push (applyRkpDeletions): a property the vendor .prop brought back is simply deleted again. Only the two known rkp_only names are ever deleted or persisted, so a stale or hand-edited rkp.json can never steer resetprop at an arbitrary property. Because a deleted property reads back as unset -- indistinguishable from one the device never shipped -- the daemon exposes its delete-intent list (GET /rkp) so the WebUI can tell the two apart: a present property shows a trash control that deletes it, a deleted one shows a Restore control (carrying the value remembered at deletion) that sets it back and clears the intent, and a property that is merely absent is hidden. Delete and restore run in the daemon under the same lock as the boot/push re-enforce (POST /rkp/delete, POST /rkp/restore), so a live change can never interleave with it. Each of them also re-pushes the config, because the push is what carries the delete-intent set to the hook: the data-dir watcher only fires for config.json and keyboxes, so without it a fresh intent would not reach the hook until some unrelated push happened, and until then a reasserted property would read as a genuine rkp_only=true. Restoring a property that was already unset when it was deleted only drops the intent -- writing a plausible-looking true onto a device that never declared the knob would leave exactly the anomalous reading deletion exists to avoid. enable_rkpd is persist.device_config and already survives on its own, so it is no longer routed through the daemon or persisted here -- it stays a plain WebUI toggle. Some devices reassert the property hours after boot, between pushes, with no reboot at all (#262), which the eager pass alone would miss. The daemon pushes the delete-intent set down to the hook; when the hook reads an rkp_only prop live despite that intent, it still allows that one lookup -- denying an rkp-only level fails key generation outright, worse than the single check it races with -- but notes the reassertion, which rides back on the getUsage reply. The daemon then re-deletes the property live, so the next provisioning reads it unset and takes the keybox path. keystore1 never resolves a remotely-provisioned key, so it no-ops both ends. Also fixes the NDK that rust/build.sh resolves, which the clippy step now trips over. It took the newest NDK under $ANDROID_HOME/ndk, and on a CI runner that is a preinstalled one rather than the version the workflow installs; NDK 30 rejects the unversioned target triple cargo-ndk passes, so openssl-sys's bindgen dies on its cdefs.h. It now honours ANDROID_NDK_HOME and ANDROID_NDK_ROOT first, which is how the workflow already passes the pin. CMake always passed NDK_HOME explicitly, so only a direct build.sh call was ever affected. Closes #244. --- app/src/main/java/org/matrix/teesim/App.kt | 130 ++++++++++++++++++ app/src/main/java/org/matrix/teesim/Const.kt | 8 ++ .../main/java/org/matrix/teesim/Control.kt | 32 +++++ .../main/java/org/matrix/teesim/KeyAdmin.kt | 75 ++++++++-- .../main/java/org/matrix/teesim/Resolver.kt | 9 ++ .../main/java/org/matrix/teesim/RkpStore.kt | 119 ++++++++++++++++ .../main/java/org/matrix/teesim/SysProp.kt | 52 +++++-- common/control.cpp | 18 +++ common/control.h | 13 ++ keymint/keymint_hook.cpp | 27 +++- keymint/keymint_router.cpp | 55 ++++++++ keystore/keystore_router.cpp | 6 + module/webroot/css/app.css | 8 ++ .../js/controllers/keybox-controller.js | 20 ++- module/webroot/js/data/keyadmin.js | 19 +++ module/webroot/js/data/rkp-io.js | 94 ++++++++++--- module/webroot/js/ui/rkp-view.js | 92 +++++++++++-- rust/build.sh | 13 +- 18 files changed, 727 insertions(+), 63 deletions(-) create mode 100644 app/src/main/java/org/matrix/teesim/RkpStore.kt diff --git a/app/src/main/java/org/matrix/teesim/App.kt b/app/src/main/java/org/matrix/teesim/App.kt index 6ac6bea33..3f80ed403 100644 --- a/app/src/main/java/org/matrix/teesim/App.kt +++ b/app/src/main/java/org/matrix/teesim/App.kt @@ -185,6 +185,10 @@ object App { if (!restarting) ReAttest.run(cfg) } } + // The hook reports an rkp_only prop it saw reasserted despite a delete intent (#262); + // re-delete it live here, off the control reader thread, so the next provisioning reads + // it unset and takes the keybox path. + Control.onRkpReassert = { names -> reEnforceRkpDeletions(names) } Control.start() // Freeze the auto-include baseline (known_packages.json) at a known moment, before the // first resolve reads it — so "future installs only" is measured from daemon-start, not @@ -199,6 +203,17 @@ object App { // no-ops from 34 on). So the set of installed apps is only ever re-read here, on a // config change, and at daemon start. KeyAdmin.onRescan = { resolveAndPush() } + // The WebUI deletes/restores an RKP-only property through the daemon (POST /rkp/*), so + // the live resetprop, the rkp.json delete-intent and the re-push that hands the new + // intent set to the hook form one atomic, lock-ordered step with the boot/push + // re-enforce. + KeyAdmin.onRkp = { action, name -> + when (action) { + "delete" -> deleteRkpProp(name) + "restore" -> restoreRkpProp(name) + else -> false + } + } startUsagePoll() SystemLogger.info("App: daemon initialised; entering main loop") @@ -352,6 +367,7 @@ object App { harvest = Harvester.applyUserOverrides(capturedBase, OverrideStore.load()) KeyAdmin.updateHarvest(harvest) applyBootProps(harvest) + applyRkpDeletions() try { lastGoodConfig = ConfigStore.load() } catch (e: ConfigStore.ConfigException) { @@ -518,6 +534,120 @@ object App { LogTail.start(File(moduleDir, "$abi/libteesim_logcat.so")) } + /** + * Enforce the user's RKP-only *delete intents* (rkp.json): delete any `remote_provisioning.*` + * knob that is marked deleted yet reads back present. Those props are plain (not `persist.*`) + * and a device can ship them `true` in a vendor `.prop` that init re-applies every boot + * (OnePlus Android 16, #236), so a deleted prop reappears on the next boot. Runs from + * resolveAndPush — its first call is daemon start, and it runs again on every later push, so a + * reappearance is cleared at the next push. keystore2 and our hook both read an unset prop as + * false, so the keybox path holds with no retrigger. A device that re-asserts the prop between + * pushes, hours after boot (#262), is handled by [reEnforceRkpDeletions] off the hook's report + * instead. + */ + private fun applyRkpDeletions() { + for ((name, _) in RkpStore.load()) { + val live = DeviceProps.prop(name, "") + if (live.isEmpty()) continue // already absent + if (SysProp.delete(name)) + SystemLogger.info("App: rkp prop re-deleted $name (reappeared as '$live')") + else SystemLogger.warning("App: rkp prop $name reappeared as '$live' but delete failed") + } + } + + /** + * Delete the RKP-only knobs the hook reports it saw present despite a delete intent (#262), + * reported on the getUsage reply. Runs under the push lock, so it cannot interleave with + * [applyRkpDeletions] or a delete/restore. Keys off the intent store, not the reported names: a + * knob the user restored between the hook's report and this call carries no intent and is left + * alone. + */ + @Synchronized + fun reEnforceRkpDeletions(reported: List) { + val intents = RkpStore.load() + for (name in reported) { + if (name !in intents) { + SystemLogger.info("App: rkp lazy re-delete skipped $name (no delete intent)") + continue + } + val live = DeviceProps.prop(name, "") + if (live.isEmpty()) continue // already absent + if (SysProp.delete(name)) + SystemLogger.info("App: rkp lazy re-delete $name (reasserted to '$live')") + else SystemLogger.warning("App: rkp lazy re-delete $name failed (was '$live')") + } + } + + /** + * Delete one RKP-only property live and record the delete intent, as one atomic step. Reached + * from the WebUI through `KeyAdmin`'s `POST /rkp/delete`. Because it is `@Synchronized` on the + * same monitor as [resolveAndPush] (and therefore [applyRkpDeletions]), the live delete and the + * persist can never interleave with a boot/re-push re-enforce. Captures the property's current + * value first so [restoreRkpProp] can put exactly that back. Records the intent only when the + * live delete took, so the file mirrors the live state. [name] is validated against + * [RkpStore.KNOWN] by the caller. Returns whether the property was deleted live. + */ + @Synchronized + fun deleteRkpProp(name: String): Boolean { + val was = DeviceProps.prop(name, "") + val ok = SysProp.delete(name) + if (ok) { + RkpStore.recordDelete(name, was) + SystemLogger.info( + "App: rkp prop deleted $name (was '${was.ifEmpty { "unset" }}'); intent persisted" + ) + pushRkpIntents("delete $name") + } else { + SystemLogger.warning("App: could not delete rkp prop $name live; not recording intent") + } + return ok + } + + /** + * Restore one deleted RKP-only property and clear its intent, atomically. Reached through `POST + * /rkp/restore`. Sets the property back to the value remembered at deletion, then drops the + * intent so [applyRkpDeletions] leaves it alone. When nothing was remembered the property was + * already absent when the intent was recorded, so restoring it means leaving it absent: the + * intent is dropped and no value is invented — writing a plausible-looking `true` onto a device + * that never declared the knob would leave exactly the anomalous reading deletion exists to + * avoid. Clears the intent only when the live set took. Returns whether the restore took. + */ + @Synchronized + fun restoreRkpProp(name: String): Boolean { + val remembered = RkpStore.load()[name].orEmpty() + if (remembered.isEmpty()) { + RkpStore.clear(name) + SystemLogger.info( + "App: rkp prop $name was already unset when deleted; intent cleared, leaving it unset" + ) + pushRkpIntents("restore $name") + return true + } + val ok = SysProp.set(name, remembered) + if (ok) { + RkpStore.clear(name) + SystemLogger.info("App: rkp prop restored $name=$remembered; intent cleared") + pushRkpIntents("restore $name") + } else { + SystemLogger.warning("App: could not restore rkp prop $name live; intent kept") + } + return ok + } + + /** + * Re-resolve and push after rkp.json changed, so the hook's delete-intent set (pushed as the + * config's `rkpDeleted`) matches the file again. Nothing else would: the DATA_DIR watcher only + * fires for config.json and keyboxes, so without this a fresh delete intent would not reach the + * hook until some unrelated push, and until then a reasserted property (#262) would read as a + * genuine rkp_only=true and never be reported for a lazy re-delete. Safe to call from the + * `@Synchronized` delete/restore: the monitor is reentrant, and by here rkp.json is already + * written, so the push carries the new intent set. [why] only labels the line in the log. + */ + private fun pushRkpIntents(why: String) { + SystemLogger.info("App: re-pushing config after rkp intent change ($why)") + resolveAndPush() + } + /** Where the inject binary + native libs live: args[0], else the dex dir, else default. */ private fun resolveModuleDir(args: Array): File { args.firstOrNull()?.let { diff --git a/app/src/main/java/org/matrix/teesim/Const.kt b/app/src/main/java/org/matrix/teesim/Const.kt index b5a0fd7ac..2437bff00 100644 --- a/app/src/main/java/org/matrix/teesim/Const.kt +++ b/app/src/main/java/org/matrix/teesim/Const.kt @@ -14,6 +14,14 @@ object Const { * key). Written by the WebUI, merged over the frozen captured harvest on every push. */ val overridesFile = File(DATA_DIR, "overrides.json") + /** + * The user's persisted RKP-only delete intents ({ property: value-at-deletion }). The two + * rkp_only props are non-persist and a vendor .prop can re-apply a default (often true) each + * boot (#236), so the daemon re-deletes an intent-marked prop that reappears; the stored value + * lets a later restore put exactly that back. enable_rkpd is persist.device_config, survives on + * its own, and is not managed here (the WebUI keeps it as a plain toggle). + */ + val rkpFile = File(DATA_DIR, "rkp.json") val adminTokenFile = File(DATA_DIR, "admin.token") /** diff --git a/app/src/main/java/org/matrix/teesim/Control.kt b/app/src/main/java/org/matrix/teesim/Control.kt index db96fa10d..abe4d1e6b 100644 --- a/app/src/main/java/org/matrix/teesim/Control.kt +++ b/app/src/main/java/org/matrix/teesim/Control.kt @@ -50,6 +50,11 @@ object Control { // Invoked (on [commitExecutor]) after the lib acks a config commit, so the daemon can re-attest // pre-existing keys against the just-committed profile set. Coalesced by [commitPending]. @Volatile var onCommitted: (() -> Unit)? = null + + // Invoked (on [commitExecutor]) when a getUsage reply reports rkp_only props the hook saw + // reasserted despite a delete intent (#262), so the daemon can re-delete them live. The names + // are always a subset of the known rkp_only knobs; the daemon re-validates. + @Volatile var onRkpReassert: ((List) -> Unit)? = null private val commitExecutor = Executors.newSingleThreadExecutor { r -> Thread(r, "teesim-reattest").apply { isDaemon = true } } @@ -262,11 +267,38 @@ object Control { // on PackageManager. usageReplies.clear() usageReplies.offer(msg) + // A reasserted rkp_only prop (the hook saw it live despite a delete intent) rides + // the same reply; re-delete off the reader thread so a resetprop can't stall it. + dispatchRkpReassert(msg.optJSONArray("rkpReassert")) } "pong" -> logPong(msg.optLong("epoch")) } } + // Hand any reasserted rkp_only prop names from a getUsage reply to [onRkpReassert], on the + // commit executor rather than this reader thread, so the daemon's resetprop --delete cannot + // stall frame reading. A no-op when the array is absent/empty or no handler is set. + private fun dispatchRkpReassert(arr: JSONArray?) { + if (arr == null || arr.length() == 0) return + val names = ArrayList(arr.length()) + for (i in 0 until arr.length()) { + val name = arr.optString(i) + if (name.isNotEmpty()) names.add(name) + } + if (names.isEmpty()) return + val handler = onRkpReassert ?: return + SystemLogger.info( + "Control: rkp reassert reported for ${names.joinToString(",")}; re-deleting" + ) + commitExecutor.execute { + try { + handler(names) + } catch (e: Throwable) { + SystemLogger.warning("Control: rkp reassert handler failed: ${e.message}") + } + } + } + // The keepalive answers every 30s for as long as the channel is up. A pong that reports a new // epoch is logged; an unchanged one only every [PONG_LOG_EVERY]th, to show the channel is // alive. diff --git a/app/src/main/java/org/matrix/teesim/KeyAdmin.kt b/app/src/main/java/org/matrix/teesim/KeyAdmin.kt index 0634b95fb..df4526edf 100644 --- a/app/src/main/java/org/matrix/teesim/KeyAdmin.kt +++ b/app/src/main/java/org/matrix/teesim/KeyAdmin.kt @@ -54,20 +54,23 @@ import org.json.JSONObject * place auto-included uids are visible, since the rule needs the root-only known_packages.json * baseline. Empty profiles[] with epoch 0 before the first push) POST /rescan -> { ok, uids } * (re-resolve against the live device and re-push; how a newly installed app is discovered, there - * being no package watcher) GET /packages -> { ok, firstAppUid, apps:[ {uid, packages:[..], label, - * system, launchable, enabled, installTime, freq, lastUsed, recent} ] } (every installed app, one - * entry per uid, for the Scope picker: installTime = epoch ms of first install; freq = persistent - * key-request count; lastUsed = epoch ms of last request; recent = requested a key since this boot) - * GET /icon?pkg=P&token=T -> raw image/png (query-token auth, like /logs/download; 404 when the - * package has no icon) POST /usage/clear -> { ok, cleared } (wipes the frequency memory) POST - * /keys/db/delete?ids=1,2,3 -> { ok, deleted, requested } (removes those keyentry ids from - * keystore2, marker- and target-verified) GET /keys/inspect?alias=A -> { ok, alias, - * attestation{...} | null } POST /keys/delete?alias=A -> { ok, deleted } GET /logs?after=N&max=M -> - * { ok, lines:[{seq,level,tag,text}], nextAfter } GET /keybox/inspect?name=F -> { ok, name, - * deviceId, revocationListAvailable, keys:[{algorithm, privateKeyPresent, chainLength, linkage, - * rootAuthority(google|aosp|knox|unknown|none), googleSigned, chainVerified, revoked, - * revocationChecked, certs:[{index, subject, issuer, serial, notBefore, notAfter, expired, sigAlg, - * keyAlgorithm, keySize, isCa, selfSigned, signatureValid?, revocationChecked?, revoked?, + * being no package watcher) GET /rkp -> { ok, deleted:{ prop:value } } (the user's RKP-only delete + * intents, for the Restore control) POST /rkp/delete?name=P and POST /rkp/restore?name=P -> { ok, + * name, action } (delete one RKP-only prop + persist the intent, or restore it + clear the intent, + * atomically; name must be a known knob) GET /packages -> { ok, firstAppUid, apps:[ {uid, + * packages:[..], label, system, launchable, enabled, installTime, freq, lastUsed, recent} ] } + * (every installed app, one entry per uid, for the Scope picker: installTime = epoch ms of first + * install; freq = persistent key-request count; lastUsed = epoch ms of last request; recent = + * requested a key since this boot) GET /icon?pkg=P&token=T -> raw image/png (query-token auth, like + * /logs/download; 404 when the package has no icon) POST /usage/clear -> { ok, cleared } (wipes the + * frequency memory) POST /keys/db/delete?ids=1,2,3 -> { ok, deleted, requested } (removes those + * keyentry ids from keystore2, marker- and target-verified) GET /keys/inspect?alias=A -> { ok, + * alias, attestation{...} | null } POST /keys/delete?alias=A -> { ok, deleted } GET + * /logs?after=N&max=M -> { ok, lines:[{seq,level,tag,text}], nextAfter } GET /keybox/inspect?name=F + * -> { ok, name, deviceId, revocationListAvailable, keys:[{algorithm, privateKeyPresent, + * chainLength, linkage, rootAuthority(google|aosp|knox|unknown|none), googleSigned, chainVerified, + * revoked, revocationChecked, certs:[{index, subject, issuer, serial, notBefore, notAfter, expired, + * sigAlg, keyAlgorithm, keySize, isCa, selfSigned, signatureValid?, revocationChecked?, revoked?, * revocationStatus?, revocationReason?, rootAuthority?}]}] } GET /canary -> { ok, currentCode, * latest{...}|null, updateAvailable } POST /canary/install?tag=&variant= -> { ok, message } */ @@ -100,6 +103,15 @@ object KeyAdmin { */ @Volatile var onRescan: (() -> Int)? = null + /** + * Set by [App] to its delete-and-restore for one RKP-only property, invoked by `POST + * /rkp/delete` and `POST /rkp/restore`. Running the live `resetprop`, the delete-intent write + * and the re-push that carries the new intent set to the hook in the daemon keeps all three + * atomic with the boot/push re-enforce. Takes the action ("delete"|"restore") and the real + * property name; returns whether the live change took. + */ + @Volatile var onRkp: ((String, String) -> Boolean)? = null + fun start(record: Harvester.Record) { harvest = record token = newToken() @@ -431,6 +443,9 @@ object KeyAdmin { method == "GET" && path == "/scope" -> scope() method == "GET" && path == "/packages" -> packages() method == "POST" && path == "/rescan" -> rescan() + method == "GET" && path == "/rkp" -> rkpList() + method == "POST" && path == "/rkp/delete" -> rkpAction("delete", query) + method == "POST" && path == "/rkp/restore" -> rkpAction("restore", query) method == "POST" && path == "/usage/clear" -> usageClear() method == "POST" && path == "/keys/db/delete" -> deleteDbKeys(query) method == "GET" && path == "/keys/inspect" -> @@ -703,6 +718,38 @@ object KeyAdmin { return JSONObject().put("ok", true).put("uids", uids) } + /** + * `GET /rkp` — the user's current RKP-only delete intents, `{ ok, deleted:{ : + * } }`, so the WebUI can render a Restore control for each deleted knob + * (the property itself reads back as unset, so the live value alone cannot tell "deleted" from + * "never shipped"). Read-only; a dirty read only feeds a repaint, so it does not take the App + * monitor. + */ + private fun rkpList(): JSONObject { + val deleted = JSONObject() + for ((name, value) in RkpStore.load()) deleted.put(name, value) + return JSONObject().put("ok", true).put("deleted", deleted) + } + + /** + * `POST /rkp/delete?name=` and `POST /rkp/restore?name=` — delete one + * RKP-only property (and persist the intent) or restore it (and clear the intent), as one + * atomic step in [App.deleteRkpProp]/[App.restoreRkpProp]. The property name is checked against + * [RkpStore.KNOWN] here so a malformed or hostile request can never drive `resetprop` at an + * arbitrary property. + */ + private fun rkpAction(action: String, query: Map): JSONObject { + val name = query["name"] ?: error("name required") + if (name !in RkpStore.KNOWN) + return JSONObject().put("ok", false).put("error", "unknown rkp property") + val hook = onRkp ?: return JSONObject().put("ok", false).put("error", "daemon not ready") + if (!hook(action, name)) + return JSONObject() + .put("ok", false) + .put("error", "could not $action property (no working resetprop)") + return JSONObject().put("ok", true).put("name", name).put("action", action) + } + /** * Stream the rendered PNG icon for `?pkg=`, looked up in `?user=` (default 0, so an app that * only exists in a work profile still resolves). Validates the package shape before touching diff --git a/app/src/main/java/org/matrix/teesim/Resolver.kt b/app/src/main/java/org/matrix/teesim/Resolver.kt index f899f86d7..cb9b01f98 100644 --- a/app/src/main/java/org/matrix/teesim/Resolver.kt +++ b/app/src/main/java/org/matrix/teesim/Resolver.kt @@ -94,6 +94,15 @@ object Resolver { profiles.put(resolveProfile(p, scope, harvest)) } msg.put("profiles", profiles) + + // The rkp_only props the user deleted, so the hook can tell a genuine rkp_only=true from + // one a vendor .prop reasserted after a delete, and ask the daemon (via the getUsage + // reply) to re-delete it. Device-wide, full replace each push; empty when nothing is + // deleted. + val rkpDeleted = JSONArray() + for (name in RkpStore.load().keys) rkpDeleted.put(name) + msg.put("rkpDeleted", rkpDeleted) + // Published as the last act of building the message, so a resolve that throws part-way // publishes nothing. There is no "push succeeded" moment to wait for instead: Control.push // only stages the JSON for the connection thread, and actual delivery is confirmed much diff --git a/app/src/main/java/org/matrix/teesim/RkpStore.kt b/app/src/main/java/org/matrix/teesim/RkpStore.kt new file mode 100644 index 000000000..9f4c83d0f --- /dev/null +++ b/app/src/main/java/org/matrix/teesim/RkpStore.kt @@ -0,0 +1,119 @@ +package org.matrix.teesim + +import java.io.File +import org.json.JSONObject + +/** + * The user's persisted RKP-only *delete intents*, at [Const.rkpFile]. A flat `{ property: "" }` map, keyed by the real system-property name (only the two + * `remote_provisioning.*.rkp_only` knobs). An entry means "the user deleted this property; keep it + * gone." The stored value is what the property read just before deletion, so a later restore can + * put exactly that back. + * + * The two `rkp_only` props are plain (not `persist.*`) and a device can ship them `true` in a + * vendor `.prop` that init re-applies every boot (#236), so a deleted prop reappears on the next + * boot and the intent is what tells the daemon to delete it again. keystore2 and our hook both read + * an unset prop as false, so the keybox path holds while it is absent. `enable_rkpd` is **not** + * managed here — it is `persist.device_config` and survives on its own, so the WebUI keeps it as a + * plain toggle. + * + * The daemon owns the file: [App.deleteRkpProp]/[App.restoreRkpProp] write it (live `resetprop` + + * this store, atomically) and [App.applyRkpDeletions] re-enforces it at boot. Not internally + * locked: every read ([load]) and write happens on the App monitor (`resolveAndPush`, + * `deleteRkpProp` and `restoreRkpProp` are all `@Synchronized`), so access is already serialized — + * except [load] from [KeyAdmin]'s read-only `GET /rkp`, where a dirty read only feeds a UI repaint. + */ +object RkpStore { + + /** + * The only property names we will ever delete, persist an intent for, or restore — a + * hostile/stale key in rkp.json is ignored, so [App.applyRkpDeletions] can never be steered to + * delete an arbitrary property. Mirrors the WebUI's deletable list. + */ + val KNOWN = + setOf( + "remote_provisioning.tee.rkp_only", + "remote_provisioning.strongbox.rkp_only", + ) + + /** + * The persisted delete intents ({ property: value-at-deletion }), or empty when the file is + * absent/unreadable/malformed. Never throws: a broken rkp.json must not take the daemon's push + * path down — it just means "no delete intents". Keys outside [KNOWN] are dropped on read, so a + * stale/hand-edited entry can never reach [App.applyRkpDeletions]. + */ + fun load(): Map { + val f = Const.rkpFile + if (!f.exists()) return emptyMap() + return try { + val o = JSONObject(f.readText()) + val m = LinkedHashMap() + for (k in o.keys()) if (k in KNOWN) m[k] = o.optString(k, "") + // Verbose, not info: this runs on every push and on every WebUI /rkp repaint, and the + // answer is "(none)" on all but the handful of devices that ever delete a knob. The + // transitions that matter — a recorded, cleared or re-enforced intent — are info. + SystemLogger.verbose( + "RkpStore: loaded delete intents ${m.keys.joinToString(",").ifEmpty { "(none)" }}" + ) + m + } catch (e: Exception) { + SystemLogger.warning("RkpStore: cannot read rkp.json; ignoring delete intents", e) + emptyMap() + } + } + + /** + * Record a delete intent for [name], remembering [valueAtDeletion] for a faithful restore, and + * preserving any other intent. Called only from [App.deleteRkpProp], after the live `resetprop + * --delete` succeeded. Rejects a name outside [KNOWN] rather than persist something [load] + * would just drop. + */ + fun recordDelete(name: String, valueAtDeletion: String) { + if (name !in KNOWN) { + SystemLogger.warning( + "RkpStore: refusing to record delete intent for unknown knob '$name'" + ) + return + } + val m = LinkedHashMap(load()) + m[name] = valueAtDeletion + writeAll(m) + SystemLogger.info( + "RkpStore: delete intent recorded $name (was '${valueAtDeletion.ifEmpty { "unset" }}')" + ) + } + + /** + * Drop the delete intent for [name] (the user restored it), preserving the others. Called only + * from [App.restoreRkpProp], after the live `resetprop` put the value back. + */ + fun clear(name: String) { + val m = LinkedHashMap(load()) + if (m.remove(name) == null) return + writeAll(m) + SystemLogger.info("RkpStore: delete intent cleared $name") + } + + /** + * Overwrite the whole file atomically (temp + rename) so a crash mid-write can never leave a + * torn rkp.json. Never throws — a failed write is logged; the live state is already correct. + */ + private fun writeAll(intents: Map) { + try { + val o = JSONObject() + for ((k, v) in intents) o.put(k, v) + val f = Const.rkpFile + val tmp = File(f.parentFile, "${f.name}.tmp") + tmp.writeText(o.toString(2) + "\n") + if (!tmp.renameTo(f)) { + f.writeText(o.toString(2) + "\n") + tmp.delete() + } + } catch (e: Exception) { + SystemLogger.warning( + "RkpStore: cannot persist rkp.json; intent may not survive a reboot", + e, + ) + } + } +} diff --git a/app/src/main/java/org/matrix/teesim/SysProp.kt b/app/src/main/java/org/matrix/teesim/SysProp.kt index c65a0a115..184563f3b 100644 --- a/app/src/main/java/org/matrix/teesim/SysProp.kt +++ b/app/src/main/java/org/matrix/teesim/SysProp.kt @@ -1,11 +1,16 @@ package org.matrix.teesim /** - * Best-effort setter for read-only (`ro.*`) system properties. Plain `setprop` cannot touch `ro.*`, - * so this shells out to Magisk's `resetprop -n` (the `-n` skips re-triggering property_service, - * which is what lets a read-only prop be overwritten). Used when a user overrides the verified-boot - * key/hash so the matching `ro.boot.vbmeta.*` property reflects the spoofed value for anything that - * reads it directly. + * Best-effort writer for system properties the platform will not let us change the ordinary way, + * via Magisk's `resetprop`. Two uses: + * - [set] overwrites a read-only (`ro.*`) prop that plain `setprop` cannot touch. It passes `-n`, + * which skips re-triggering property_service — that is what lets a read-only prop be overwritten + * at all. Used when a user overrides the verified-boot key/hash, so the matching + * `ro.boot.vbmeta.*` property reflects the spoofed value for anything that reads it directly. + * - [delete] removes a prop outright with `resetprop --delete`, for the + * `remote_provisioning.*.rkp_only` knobs: an absent prop reads as a device that never declared + * RKP-only enforcement, where a flipped value would read as an anomaly. [App] drives it from the + * user's delete intents ([RkpStore]). * * Never throws — a device without resetprop just leaves the property as-is, logged. The daemon's * own attestation does not depend on these props (it uses the pushed config); this only keeps the @@ -17,23 +22,48 @@ object SysProp { * Overwrite [name] with [value] via the first resetprop invocation that succeeds. Returns * whether any candidate reported success. Idempotent — safe to call on every push. */ - fun set(name: String, value: String): Boolean { - val candidates = + fun set(name: String, value: String): Boolean = + run( + "set", + name, listOf( listOf("resetprop", "-n", name, value), listOf("magisk", "resetprop", "-n", name, value), - ) + ), + ) + + /** + * Remove [name] entirely via `resetprop --delete`, so the property reads back as *unset* rather + * than as a spoofed value. Used for the `remote_provisioning.*.rkp_only` knobs: an absent prop + * looks like a device that never declared RKP-only enforcement, whereas a flipped `false` on a + * device that ships `true` is an anomalous value. keystore2 and our hook both treat unset as + * false, so deletion yields the same keybox path as a `false` set, with a smaller footprint. + * Returns whether any candidate reported success. Idempotent — deleting an absent prop + * succeeds. + */ + fun delete(name: String): Boolean = + run( + "delete", + name, + listOf( + listOf("resetprop", "--delete", name), + listOf("magisk", "resetprop", "--delete", name), + ), + ) + + /** Run the first candidate argv that exits 0; [verb] labels the action in the logs. */ + private fun run(verb: String, name: String, candidates: List>): Boolean { for (cmd in candidates) { try { val p = ProcessBuilder(cmd).redirectErrorStream(true).start() val out = p.inputStream.bufferedReader().readText().trim() val code = p.waitFor() if (code == 0) { - SystemLogger.info("SysProp: set $name via '${cmd.first()}'") + SystemLogger.info("SysProp: $verb $name via '${cmd.first()}'") return true } SystemLogger.info( - "SysProp: '${cmd.first()}' exited $code for $name${if (out.isEmpty()) "" else " ($out)"}" + "SysProp: '${cmd.first()}' exited $code for $verb $name${if (out.isEmpty()) "" else " ($out)"}" ) } catch (e: Exception) { SystemLogger.info( @@ -41,7 +71,7 @@ object SysProp { ) } } - SystemLogger.warning("SysProp: could not set $name (no working resetprop)") + SystemLogger.warning("SysProp: could not $verb $name (no working resetprop)") return false } } diff --git a/common/control.cpp b/common/control.cpp index 3ea70ecb4..45fcbb881 100644 --- a/common/control.cpp +++ b/common/control.cpp @@ -223,6 +223,20 @@ void ApplyConfig(const tjson::Value &msg, uint64_t &epoch, int &applied, int &to boot.module_hash_len = module_hash.size(); teesim_cfg_begin(&boot); + // The rkp_only props the user deleted (device-wide, independent of the profile set). Replaces the + // hook's set on every push, so a restore drops out of it. An absent/empty array clears it. + std::vector rkp_deleted; + std::vector rkp_deleted_ptrs; + if (const tjson::Value *rd = msg.get("rkpDeleted"); rd && rd->is_array()) + for (size_t i = 0; i < rd->size(); ++i) { + // as_string() yields "" for a non-string element; an empty name would only pollute the + // router's set, so drop it here rather than teach the router to ignore it. + const std::string &name = rd->at(i).as_string(); + if (!name.empty()) rkp_deleted.push_back(name); + } + for (const auto &s : rkp_deleted) rkp_deleted_ptrs.push_back(s.c_str()); + teesim_cfg_set_rkp_deleted(rkp_deleted_ptrs.data(), rkp_deleted_ptrs.size()); + const tjson::Value *profiles = msg.get("profiles"); if (profiles && profiles->is_array()) { total = static_cast(profiles->size()); @@ -383,10 +397,14 @@ void HandleConnection(int fd) { // Poll the router's per-caller key-usage snapshot (see control.h). The router owns the JSON // array; we wrap it in the reply envelope and free it. char* j = teesim_usage_json_alloc(); + char* rr = teesim_rkp_reassert_json_alloc(); std::string resp = "{\"type\":\"usage\",\"apps\":"; resp += (j ? j : "[]"); + resp += ",\"rkpReassert\":"; + resp += (rr ? rr : "[]"); resp += "}"; free(j); + free(rr); WriteFrame(fd, resp); } else if (t == "ping") { const tjson::Value *e = msg.get("epoch"); diff --git a/common/control.h b/common/control.h index cb7bb6172..a52f9181a 100644 --- a/common/control.h +++ b/common/control.h @@ -110,6 +110,19 @@ char *teesim_usage_json_alloc(void); // shifted its transaction ordinals and grew finish()'s signature between 10 and 11. int teesim_android_api(void); +// The set of remote_provisioning.*.rkp_only property names the user has deleted, pushed with each +// config (full replace; names borrowed for the call). It lets the keymint hook tell a genuine +// rkp_only=true — leave it, allow the lookup — from one a vendor .prop reasserted after a delete, +// which it notes for the daemon to re-delete. The keymint router stores it; keystore1 no-ops (RKP is +// a keystore2-only construct). +void teesim_cfg_set_rkp_deleted(const char *const *names, size_t n); + +// Drain the rkp_only property names the hook saw reasserted (present while under a delete intent) +// since the last call, as a malloc'd NUL-terminated JSON array the caller must free() — +// ["remote_provisioning.tee.rkp_only"], or "[]" when none. The daemon polls this on the getUsage +// reply and re-deletes each live. The keymint router fills it; keystore1 returns "[]". +char *teesim_rkp_reassert_json_alloc(void); + // --- control server, implemented in common/control.cpp ----------------------- // Bind the control socket (its path lives in control.cpp), listen, and serve diff --git a/keymint/keymint_hook.cpp b/keymint/keymint_hook.cpp index 4a51cf59d..cb9fc26eb 100644 --- a/keymint/keymint_hook.cpp +++ b/keymint/keymint_hook.cpp @@ -36,6 +36,10 @@ using aidl::android::hardware::security::keymint::IKeyMintDevice; extern "C" AIBinder* teesim_router_new_device(int32_t security_level, AIBinder* real_binder); // True if the calling uid belongs to a live target profile (keymint_router.cpp). extern "C" bool teesim_is_target_uid(int32_t uid); +// RKP-only delete-intent tracking (keymint_router.cpp): is this prop one the user deleted, and note +// that we saw it live again so the daemon re-deletes it. +extern "C" bool teesim_rkp_is_delete_intent(const char* name); +extern "C" void teesim_rkp_note_reassert(const char* name); namespace { @@ -300,10 +304,25 @@ binder_status_t HookedTransact(AIBinder* binder, transaction_code_t code, AParce bool rkp_only = ReadRkpOnly(prop, raw); const char* val = raw[0] ? raw : ""; if (rkp_only) { - tls_rkp_verdict = "allowed-rkp-only-level"; - LOGI("HookedTransact: RKP NOT denying %s getRegistration for target uid=%d (irpcName=%s, " - "%s=%s; denial would fail key generation on an rkp-only level)", - level, uid, irpc, prop, val); + // The prop reads rkp_only=true. If the user deleted it, this is a reassertion (a vendor + // .prop or a delayed config sync, #262) the daemon must undo — note it so the next + // getUsage poll re-deletes it. We still ALLOW this one lookup: denying an rkp-only level + // fails key generation outright (there is no batch fallback yet), which is worse than the + // one leaked check this races with. Once the daemon re-deletes, the prop reads unset and + // the ordinary deny -> keybox path takes over on the next provisioning. + if (teesim_rkp_is_delete_intent(prop)) { + teesim_rkp_note_reassert(prop); + tls_rkp_verdict = "allowed-reasserted-pending-redelete"; + LOGW("HookedTransact: RKP lazy re-delete — %s reasserted to %s=%s for target uid=%d " + "despite a delete intent; allowing this lookup and asking the daemon to re-delete " + "(this check may leak once; the next one won't)", + level, prop, val, uid); + } else { + tls_rkp_verdict = "allowed-rkp-only-level"; + LOGI("HookedTransact: RKP NOT denying %s getRegistration for target uid=%d (irpcName=%s, " + "%s=%s; denial would fail key generation on an rkp-only level)", + level, uid, irpc, prop, val); + } } else { tls_rkp_verdict = "denied"; // The gate only knows what keystore2 will now do, not how the key will be rooted: a diff --git a/keymint/keymint_router.cpp b/keymint/keymint_router.cpp index cc60052d6..e36ac4a4c 100644 --- a/keymint/keymint_router.cpp +++ b/keymint/keymint_router.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -1670,6 +1671,60 @@ extern "C" bool teesim_is_target_uid(int32_t uid) { return false; } +// --- RKP-only delete-intent tracking ------------------------------------------------------------- +// g_rkp_deleted: the property names the user deleted, pushed with each config (full replace). The +// hook consults it to tell a genuine rkp_only=true from one a vendor .prop reasserted after a delete. +// g_rkp_reassert: names the hook saw reasserted since the last drain; the daemon polls and re-deletes. +// Both guarded by g_rkp_mu — reached from the app's binder thread (hook) and the control thread (push, +// drain), never the keystore hot path otherwise. +static std::mutex g_rkp_mu; +static std::set g_rkp_deleted; +static std::set g_rkp_reassert; + +extern "C" void teesim_cfg_set_rkp_deleted(const char* const* names, size_t n) { + std::lock_guard lk(g_rkp_mu); + g_rkp_deleted.clear(); + for (size_t i = 0; i < n; ++i) + if (names[i]) g_rkp_deleted.insert(names[i]); + // Drop pending notes for props not in the new deleted set, so a restored prop is not re-deleted. + for (auto it = g_rkp_reassert.begin(); it != g_rkp_reassert.end();) + it = g_rkp_deleted.count(*it) ? std::next(it) : g_rkp_reassert.erase(it); +} + +// True if [name] is a property the user deleted — i.e. a live value for it is a reassertion we should +// undo rather than honour. Called by the hook while it holds no other lock. +extern "C" bool teesim_rkp_is_delete_intent(const char* name) { + if (!name) return false; + std::lock_guard lk(g_rkp_mu); + return g_rkp_deleted.count(name) != 0; +} + +// Note that [name] was seen live despite a delete intent, so the daemon re-deletes it on its next +// poll. Idempotent within a drain window; the hook re-notes on every provisioning until it is gone. +extern "C" void teesim_rkp_note_reassert(const char* name) { + if (!name) return; + std::lock_guard lk(g_rkp_mu); + if (g_rkp_deleted.count(name)) g_rkp_reassert.insert(name); +} + +extern "C" char* teesim_rkp_reassert_json_alloc(void) { + std::string out = "["; + { + std::lock_guard lk(g_rkp_mu); + bool first = true; + for (const auto& n : g_rkp_reassert) { + if (!first) out += ","; + first = false; + out += '"'; + out += n; // fixed property names, never user text — no JSON escaping needed + out += '"'; + } + g_rkp_reassert.clear(); + } + out += "]"; + return strdup(out.c_str()); +} + extern "C" bool teesim_cfg_resign(const char* profile_id, const uint8_t* leaf, size_t leaf_len, TsCertSink sink, void* ctx) { if (!profile_id || !leaf || leaf_len == 0 || !sink) return false; diff --git a/keystore/keystore_router.cpp b/keystore/keystore_router.cpp index 353febb19..55d3bf61c 100644 --- a/keystore/keystore_router.cpp +++ b/keystore/keystore_router.cpp @@ -946,6 +946,12 @@ extern "C" const char* teesim_hook_name(void) { return "keystore1"; } // for the scope picker, so this router reports an empty array (see control.h). extern "C" char* teesim_usage_json_alloc(void) { return strdup("[]"); } +// RKP is a keystore2/Android-12+ construct; the keystore1 path never resolves a remotely-provisioned +// key, so there is nothing to track here (see control.h). Accept the pushed set as a no-op and never +// report a reassertion. +extern "C" void teesim_cfg_set_rkp_deleted(const char* const* /*names*/, size_t /*n*/) {} +extern "C" char* teesim_rkp_reassert_json_alloc(void) { return strdup("[]"); } + // Config-staging API (see common/control.h). teesim_cfg_begin/add_profile run on // the control thread; only teesim_cfg_commit touches the live routing tables. extern "C" void teesim_cfg_begin(const TsBootInfo* boot) { diff --git a/module/webroot/css/app.css b/module/webroot/css/app.css index 4cf32fbde..9d30c1e3e 100644 --- a/module/webroot/css/app.css +++ b/module/webroot/css/app.css @@ -520,6 +520,14 @@ html, body { overscroll-behavior-y: contain; } .toggle-main { flex: 1; min-width: 0; display: flex; flex-direction: column; gap: 2px; cursor: pointer; } .toggle-main .field-help { margin: 0; } +/* Remote Key Provision: the two rkp_only knobs are deletable — a present one carries a square trash + button, a deleted one is dimmed with a "removed" chip and a Restore. */ +.rkp-del { flex: none; width: 44px; height: 44px; padding: 0; display: inline-flex; align-items: center; justify-content: center; border-radius: var(--radius-sm); border: 1px solid var(--danger); background: transparent; color: var(--danger); cursor: pointer; } +.rkp-del:active { background: var(--chip); } +.rkp-del:focus-visible { outline: 2px solid var(--danger); outline-offset: 2px; } +.rkp-row.deleted .field-label { color: var(--muted); } +.rkp-row.deleted .chip { align-self: flex-start; margin-top: 2px; } + /* --- scope picker page (drill-in) --------------------------------------- */ /* Search row: a rounded field with a leading search-icon button and a trailing sort-icon button, * both inside the field. Sticky so it rides the top of the scrolling body. */ diff --git a/module/webroot/js/controllers/keybox-controller.js b/module/webroot/js/controllers/keybox-controller.js index 534e5872b..8b47d977e 100644 --- a/module/webroot/js/controllers/keybox-controller.js +++ b/module/webroot/js/controllers/keybox-controller.js @@ -5,7 +5,7 @@ import { listKeyboxes, importKeybox, renameKeybox, deleteKeybox } from "../data/keybox-io.js"; import { keyAdmin } from "../data/keyadmin.js"; -import { readRkpProps, setRkpProp } from "../data/rkp-io.js"; +import { readRkpProps, setRkpProp, deleteRkpProp, restoreRkpProp } from "../data/rkp-io.js"; import { renderKeyboxes, renderKeyboxImport, renderKeyboxInspect, fillKeyboxInspect } from "../ui/keybox-view.js"; import { renderRkpSection } from "../ui/rkp-view.js"; import { el, toast, confirmDialog, promptDialog, openSheet, openOverlay } from "../ui/dom.js"; @@ -169,11 +169,25 @@ export function create(mount) { const rkpActions = { async toggle(name, on) { - // No toast: the switch is its own feedback, and refreshRkp re-reads so a failed write just - // snaps the knob back to the value the device actually took. + // enable_rkpd. No toast: the switch is its own feedback, and refreshRkp re-reads so a failed + // write just snaps the knob back to the value the device actually took. await setRkpProp(name, on); return refreshRkp(); }, + async del(name) { + // Delete an rkp_only prop. refreshRkp re-reads: on success the prop is gone and the row turns + // into a Restore control; on failure the trash row stays. The daemon owns the live delete. + const r = await deleteRkpProp(name); + if (!r.ok) toast("Delete failed: " + (r.error || "unknown error")); + return refreshRkp(); + }, + async restore(name) { + // Put a deleted rkp_only prop back. refreshRkp re-reads: on success the Restore row turns back + // into a trash row; on failure the Restore control stays. + const r = await restoreRkpProp(name); + if (!r.ok) toast("Restore failed: " + (r.error || "unknown error")); + return refreshRkp(); + }, }; attachPullToRefresh(mount, refresh); diff --git a/module/webroot/js/data/keyadmin.js b/module/webroot/js/data/keyadmin.js index 96bb88d1b..b69aeccc8 100644 --- a/module/webroot/js/data/keyadmin.js +++ b/module/webroot/js/data/keyadmin.js @@ -178,6 +178,12 @@ const canaryInstallQuery = (args) => "?tag=" + encodeURIComponent((args && args.tag) || "") + "&variant=" + encodeURIComponent((args && args.variant) || "release"); +// One RKP-only property, by its real name. The daemon re-validates the name against its known set +// and deletes/restores it live + records or clears the delete-intent atomically (POST /rkp/delete, +// POST /rkp/restore), so a boot/push re-enforce can never interleave with the user's action. +const rkpNameQuery = (args) => + "?name=" + encodeURIComponent((args && args.name) || ""); + // The save target: the chosen folder and filename, both encoded so any character is inert. // The log text itself rides in the request body (too large for a query), not here. const logsWriteQuery = (args) => @@ -221,6 +227,19 @@ export async function keyAdmin(action, args = {}) { // app is discovered — there is no package observer in the daemon, so the Profiles screen's // pull-to-refresh is what goes and looks. Resolves to { ok, uids }. return request("POST", "/rescan"); + case "rkpList": + // The user's RKP-only delete intents, so the view can show a Restore control for each deleted + // knob (a deleted prop reads back as unset, indistinguishable from never-shipped). { ok, deleted }. + return request("GET", "/rkp"); + case "rkpDelete": + // Delete one RKP-only property live and persist the delete-intent, one daemon-owned lock-ordered + // step, so it cannot interleave with a boot/push re-enforce. Resolves to { ok, name, action }. + return request("POST", "/rkp/delete" + rkpNameQuery(args)); + case "rkpRestore": + // Restore one deleted RKP-only property: set it back to the value remembered at deletion (or, + // when it was already unset then, just stop re-removing it) and clear the intent, atomically. + // Resolves to { ok, name, action }. + return request("POST", "/rkp/restore" + rkpNameQuery(args)); case "keysDbDelete": return request("POST", "/keys/db/delete" + idsQuery(args)); case "inspect": diff --git a/module/webroot/js/data/rkp-io.js b/module/webroot/js/data/rkp-io.js index 8fd8d752c..4dc6c587e 100644 --- a/module/webroot/js/data/rkp-io.js +++ b/module/webroot/js/data/rkp-io.js @@ -1,52 +1,114 @@ -// Remote-key-provisioning knobs adapter. It reads and writes the handful of system properties that -// decide how keystore2 sources attestation keys, so the Keyboxes screen can show and flip them. The -// property NAMES here are fixed literals, never derived from user input; only the boolean the user -// toggles reaches setProp (as canonical "true"/"false"). No DOM. +// Remote-key-provisioning knobs adapter. It reads the handful of system properties that decide how +// keystore2 sources attestation keys, and changes them through the daemon, so the Keyboxes screen can +// show and manage them. The property NAMES here are fixed literals, never derived from user input; +// the daemon re-validates every name. No DOM. +// +// The two `rkp_only` props are deletable: the daemon removes the prop live and records a persistent +// delete-intent, re-enforced each boot (a vendor .prop can re-apply them). A deleted prop reads back +// as unset — indistinguishable from one the device never shipped — so the daemon's delete-intent list +// is what tells the view to offer a Restore control instead of hiding the row. `enable_rkpd` is +// persist.device_config and survives on its own, so it stays a plain toggle. import { getProp, setProp } from "../bridge/shell.js"; +import { keyAdmin } from "./keyadmin.js"; // The properties we surface, in display order. `key` is a stable id for the view; `name` is the real -// system property; `label`/`help` describe it. The two rkp_only knobs force a security level down the -// RKP-only path (no batch-key fallback); enable_rkpd toggles the native provisioner itself. +// system property; `label`/`help` describe it. `kind` is "deletable" for the two rkp_only knobs (the +// view renders a trash/restore control) or "toggle" for enable_rkpd (an on/off switch). export const RKP_PROPS = [ { key: "teeRkpOnly", name: "remote_provisioning.tee.rkp_only", label: "TEE RKP-only", - help: "When on, the TEE security level provisions attestation keys only via RKP, with no batch-key fallback.", + help: "When present, the TEE level provisions attestation keys only via RKP, with no batch-key fallback. Delete it to let keystore2 fall back to the keybox.", + kind: "deletable", }, { key: "strongboxRkpOnly", name: "remote_provisioning.strongbox.rkp_only", label: "StrongBox RKP-only", - help: "When on, the StrongBox security level provisions attestation keys only via RKP, with no batch-key fallback.", + help: "When present, the StrongBox level provisions attestation keys only via RKP, with no batch-key fallback. Delete it to let keystore2 fall back to the keybox.", + kind: "deletable", }, { key: "enableRkpd", name: "persist.device_config.remote_key_provisioning_native.enable_rkpd", label: "Enable rkpd", help: "Whether the native remote key provisioning daemon (rkpd) runs on this device.", + kind: "toggle", }, ]; const isTrue = (v) => v === "true" || v === "1"; -// Read every knob and return only the ones this device actually defines. A property whose getprop -// value is non-empty is "present" and shown; an empty value means the device does not define it and -// the row (and, if all three are empty, the whole section) is omitted. The returned rows carry the -// raw value plus a derived boolean for the switch. +// The daemon's current RKP-only delete intents, as a { name: valueAtDeletion } map. Empty on any +// failure (daemon not ready, etc.) so a read error just means "nothing is marked deleted". +async function readDeleteIntents() { + try { + const r = await keyAdmin("rkpList"); + return (r && r.ok && r.deleted) || {}; + } catch (_) { + return {}; + } +} + +// Read every knob and return the rows the view should paint. For a "toggle" prop, an empty getprop +// value means the device does not define it, so the row is omitted. For a "deletable" prop, order of +// precedence: a recorded delete-intent => a "deleted" row (Restore control), carrying the value to +// put back; else a present value => an "active" row (trash control); else (absent, no intent) the +// device never shipped it => omitted. So a deleted knob stays visible as Restore, while a knob the +// device genuinely lacks shows nothing. export async function readRkpProps() { + const deleted = await readDeleteIntents(); const rows = []; for (const p of RKP_PROPS) { + if (p.kind === "toggle") { + const value = await getProp(p.name); + if (!value) continue; // absent / unset => not a knob on this device + rows.push({ ...meta(p), kind: "toggle", value, on: isTrue(value) }); + continue; + } + // deletable rkp_only prop + if (Object.prototype.hasOwnProperty.call(deleted, p.name)) { + // The remembered value, "" when the prop was already unset at deletion time — the view says + // so rather than offering to restore a value the device never had. + rows.push({ ...meta(p), kind: "deleted", value: deleted[p.name] || "" }); + continue; + } const value = await getProp(p.name); - if (!value) continue; // absent / unset => not a knob on this device - rows.push({ key: p.key, name: p.name, label: p.label, help: p.help, value, on: isTrue(value) }); + if (!value) continue; // absent and not deleted by us => device never shipped it + rows.push({ ...meta(p), kind: "active", value }); } return rows; } -// Flip one knob, written as canonical "true"/"false". Callers re-read afterwards so the UI reflects -// the value the device actually took (a failed write leaves the old value and the switch snaps back). +const meta = (p) => ({ key: p.key, name: p.name, label: p.label, help: p.help }); + +// Delete one rkp_only property through the daemon (live resetprop --delete + persistent delete-intent, +// one atomic lock-ordered step). Never throws: on any failure returns { ok:false, error } so the +// caller's re-read just leaves the row as it was. +export async function deleteRkpProp(name) { + try { + return await keyAdmin("rkpDelete", { name }); + } catch (e) { + return { ok: false, error: e && e.message }; + } +} + +// Restore one deleted rkp_only property through the daemon: set it back to the value remembered at +// deletion — or, when it was already unset then, simply stop re-removing it — and clear the intent. +// Never throws, same contract as deleteRkpProp. +export async function restoreRkpProp(name) { + try { + return await keyAdmin("rkpRestore", { name }); + } catch (e) { + return { ok: false, error: e && e.message }; + } +} + +// Flip the enable_rkpd toggle, written as canonical "true"/"false". It is persist.device_config, so a +// direct resetprop sticks on its own — no daemon round-trip needed. Callers re-read afterwards so the +// switch reflects the value the device actually took. export async function setRkpProp(name, on) { return setProp(name, on ? "true" : "false"); } diff --git a/module/webroot/js/ui/rkp-view.js b/module/webroot/js/ui/rkp-view.js index 9e6be6862..ee5024241 100644 --- a/module/webroot/js/ui/rkp-view.js +++ b/module/webroot/js/ui/rkp-view.js @@ -1,15 +1,27 @@ // The "Remote Key Provision" section on the Keyboxes screen. It sits at the same title level as -// "Keyboxes": a panel-head with an h1.panel-title, then one card of labelled iOS-style switches, one -// per property the device defines. Stateless — it renders from the rows the controller passes and -// emits toggles through actions.toggle(name, on). When there are no rows it paints nothing, so a -// device without any of these knobs shows no section at all. Pure DOM through el(); all text is -// textContent, never innerHTML. It imports no data/* or bridge/*. +// "Keyboxes": a panel-head with an h1.panel-title, then one card of rows, one per property the device +// defines (or that the user has deleted). Stateless — it renders from the rows the controller passes. +// Pure DOM through el(); all text is textContent, never innerHTML. It imports only ./dom. +// +// Each row is one of three kinds: +// "active" an rkp_only prop that is present -> a trash button that deletes it (actions.del) +// "deleted" an rkp_only prop the user deleted -> a Restore button that puts it back (actions.restore) +// "toggle" enable_rkpd -> an on/off switch (actions.toggle) // // renderRkpSection(host, state, actions) -// state = { rows } rows = [{ key, name, label, help, value, on }] -// actions = { toggle(name, on) } +// state = { rows } rows = [{ key, name, label, help, kind, value?, on? }] +// actions = { del(name), restore(name), toggle(name, on) } + +import { el, clear, svgIcon } from "./dom.js"; -import { el, clear } from "./dom.js"; +// A trash-can glyph in the same 24x24, stroke-only style as dom.js's ICON_SEARCH/ICON_SORT. +const ICON_TRASH = [ + { d: "M3 6h18" }, + { d: "M8 6V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2" }, + { d: "M19 6l-1 14a2 2 0 0 1-2 2H8a2 2 0 0 1-2-2L5 6" }, + { d: "M10 11v6" }, + { d: "M14 11v6" }, +]; export function renderRkpSection(host, state, actions) { clear(host); @@ -21,15 +33,16 @@ export function renderRkpSection(host, state, actions) { ])); // These knobs almost never need touching: the module already handles remote provisioning so the - // keybox stays in effect. Turning them off is an escape hatch for the rare device where keybox - // attestation explicitly fails — hence the calm, informational tone rather than an alarm. - if (rows.some((r) => r.on)) { + // keybox stays in effect. Deleting them is an escape hatch for the rare device where keybox + // attestation explicitly fails — hence the calm, informational tone. Only shown while there is a + // still-present rkp_only prop left to delete. + if (rows.some((r) => r.kind === "active")) { host.appendChild(el("div", { class: "card" }, [el("div", { class: "banner" }, [ el("div", { text: "You usually don't need to change these." }), el("div", { class: "muted small", text: - "The module handles remote provisioning while these stay on. Only if a device explicitly fails " + - "keybox attestation — a rare case — should you toggle them all off to force keystore2 onto the " + - "keybox." }), + "The module handles remote provisioning while these stay in place. Only if a device explicitly " + + "fails keybox attestation — a rare case — should you delete the rkp-only properties to force " + + "keystore2 onto the keybox. A deleted property is re-removed on every boot until you restore it." }), ])])); } @@ -39,6 +52,13 @@ export function renderRkpSection(host, state, actions) { } function rkpRow(r, actions) { + if (r.kind === "toggle") return toggleRow(r, actions); + if (r.kind === "deleted") return deletedRow(r, actions); + return activeRow(r, actions); +} + +// enable_rkpd — an iOS-style on/off switch. +function toggleRow(r, actions) { const id = "rkp-" + r.key; const input = el("input", { id, class: "switch-input", type: "checkbox", checked: r.on, @@ -60,3 +80,47 @@ function rkpRow(r, actions) { ]), ]); } + +// A present rkp_only prop — a square trash button that deletes it. +function activeRow(r, actions) { + const btn = el("button", { + class: "rkp-del", type: "button", + title: "Delete " + r.name, "aria-label": "Delete " + r.name, + onclick: () => actions.del(r.name), + }, [svgIcon(ICON_TRASH, { size: 18 })]); + return el("div", { class: "field toggle-field rkp-row" }, [ + el("div", { class: "toggle-row" }, [ + el("div", { class: "toggle-main" }, [ + el("span", { class: "field-label", text: r.label }), + el("span", { class: "field-help", text: r.help }), + el("span", { class: "field-help mono", text: r.name + " = " + r.value }), + ]), + btn, + ]), + ]); +} + +// An rkp_only prop the user deleted — dimmed, with a "removed" chip and a Restore button. Restore +// puts back the value remembered at deletion; when the prop was already unset then (r.value is +// empty) there is no value to put back, so it only stops the re-removal. +function deletedRow(r, actions) { + const help = r.value + ? "Re-removed on every boot. Restore to set " + r.name + " back to \"" + r.value + "\"." + : "Re-removed on every boot. It was already unset when removed, so Restore just stops that."; + const btn = el("button", { + class: "btn small ghost", type: "button", + title: "Restore " + r.name, "aria-label": "Restore " + r.name, + onclick: () => actions.restore(r.name), + text: "Restore", + }); + return el("div", { class: "field toggle-field rkp-row deleted" }, [ + el("div", { class: "toggle-row" }, [ + el("div", { class: "toggle-main" }, [ + el("span", { class: "field-label", text: r.label }), + el("span", { class: "field-help", text: help }), + el("span", { class: "chip warn small", text: "removed" }), + ]), + btn, + ]), + ]); +} diff --git a/rust/build.sh b/rust/build.sh index ddb2ebcb9..3d15f5971 100755 --- a/rust/build.sh +++ b/rust/build.sh @@ -4,7 +4,8 @@ # The crate is a static library that resolves BoringSSL at runtime inside # keystore2, so the build needs only BoringSSL's headers (openssl-sys bindgens the # FFI from them) — no libcrypto.so of any ABI. Configure via environment: -# NDK_HOME Android NDK (default: newest under $ANDROID_HOME/ndk) +# NDK_HOME Android NDK (default: $ANDROID_NDK_HOME or $ANDROID_NDK_ROOT, else the +# newest under $ANDROID_HOME/ndk) # ABI Android ABI (default: arm64-v8a) # API platform level (default: 29) # BORINGSSL BoringSSL source dir with include/ (cloned if unset/missing) @@ -24,10 +25,20 @@ case "$ABI" in esac ENVPREFIX="$(echo "$TRIPLE" | tr '[:lower:]-' '[:upper:]_')" +# Honour the standard NDK variables before falling back to the newest installed one. A CI runner +# installs the NDK the workflow pins but keeps a newer one preinstalled, and "newest wins" picks +# that newer one: NDK 30 rejects the unversioned target triple cargo-ndk passes ("Unversioned +# target triples are not supported!" out of its cdefs.h) and openssl-sys's bindgen dies on it. +# CMake always passes NDK_HOME explicitly, so only a direct call relies on this — notably CI's +# `build.sh clippy`, which gets the pin through ANDROID_NDK_HOME. +if [ -z "${NDK_HOME:-}" ]; then + NDK_HOME="${ANDROID_NDK_HOME:-${ANDROID_NDK_ROOT:-}}" +fi if [ -z "${NDK_HOME:-}" ]; then NDK_HOME="$(ls -d "${ANDROID_HOME:-$HOME/Android/Sdk}"/ndk/* 2>/dev/null | sort -V | tail -1)" fi [ -d "$NDK_HOME" ] || { echo "NDK not found; set NDK_HOME" >&2; exit 1; } +echo "build.sh: using NDK $NDK_HOME (ABI $ABI, API $API)" # Set every NDK variable cargo-ndk consults to the one NDK we resolved, so it neither warns about a # mismatch nor silently falls back to a different preinstalled NDK (CI runners preset ANDROID_NDK_ROOT). export ANDROID_NDK_HOME="$NDK_HOME" ANDROID_NDK_ROOT="$NDK_HOME" ANDROID_NDK="$NDK_HOME"