From 141402bcd274310a4866f6f76a0bcf2f7f0ff2ec Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Tue, 1 Sep 2026 10:51:46 -0400 Subject: [PATCH] Refuse to serve a slim APK signed with the wrong key strip.rs re-signs with this machine's ~/.android/debug.keystore on the assumption it is the key the build was signed with, and never checked. Where it isn't -- a keystore recreated after the APK was built signs as an entirely unrelated certificate -- the phone gets an APK Android refuses, reporting "App not installed" with no cause, which reads as the download rather than the signing. Both certificates are now read with `apksigner verify --print-certs`, compared as sets of digests so a v2 source and a v3 output still match, and a mismatch is refused with both digests and the keystore path in the message. The slim copy is deleted on refusal: nothing would serve it without its stamp, but serveable_now reports the size of whatever slim file is on disk. Verified both ways against tdep-survey's app-dioxus: with a fresh keystore under a throwaway HOME the download 500s with the message and leaves nothing behind, and with the real one it serves the same 52,685,076 bytes as before, signed by the same certificate as the raw build. Raised by the tdep-survey session, which measured that a fresh debug keystore is unrelated to the existing one. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 30 +++++++++++++++-- server/src/strip.rs | 79 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 800e46b..3e022d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -240,6 +240,26 @@ mutable at runtime from the phone. somebody asked. Stop and Uninstall do go through the script, and do strand the phone; the app confirms them rather than hiding them. +- **A stripped copy is only serveable if it is signed by the key the + build was.** `strip.rs` re-signs with `~/.android/debug.keystore`, and + nothing about a debug keystore says which builds it made -- it is per + machine, and one recreated after an APK was built signs as an entirely + unrelated certificate (measured: a fresh AGP-parameter keystore against + the existing one, no relation, as a new RSA keypair should be). Where + they differ Android refuses the package and the phone says "App not + installed" with no cause, which reads as the download rather than the + signing and is among the most expensive sentences here to be handed. So + the two are compared -- `apksigner verify --print-certs` on the source + and on what was just signed, as sets of digests so a v2 source and a v3 + output still match -- and a mismatch is refused with both certificates + and the keystore path named. Measured on the files rather than inferred + from the keystore, so it stays true if the signing step changes. The + slim copy is deleted on refusal: with no stamp beside it nothing would + serve it, but `serveable_now` reports the size of whatever slim file is + on disk, so leaving it would have the card describing a download + nothing can install. Empty digests mean "not compared" rather than "no + signer", and say so in the log rather than refusing -- an APK apksigner + cannot read is one this comparison has no opinion about. - **A 500 answers with its message.** `ApiError::Internal` used to be a bare status with an empty body, on the grounds that an internal cause is not safe to hand back -- but every route here is behind the bearer @@ -839,8 +859,14 @@ session reads. What follows is what that means here. `--avd NAME` or `AVD_NAME=`, and when it cannot find that one it lists what *is* attached instead of guessing which of the three situations it is in. -- The server needs `aapt2` (SDK build-tools) to add an app, and - `llvm-strip` (NDK) only for apps that need stripping. **It has to find +- The server needs `aapt2` (SDK build-tools) to add an app, and, for an + app that needs stripping, `llvm-strip` (NDK), `zipalign` and + `apksigner` (build-tools) **and `~/.android/debug.keystore`** -- the + last is the least guessable of the four, since it belongs to no SDK, + is created as a side effect of any Gradle Android build, and is + consumed by the pipeline's final step. A machine that builds APKs and + has no keystore is odd rather than new: something removed it after the + build, and the APK it produced is signed by a key that is now gone. **It has to find both without an environment**, because the way this server usually starts is from a service manager, and one hands its process a scrubbed environment: measured in the Gentoo guest, an OpenRC user service gets diff --git a/server/src/strip.rs b/server/src/strip.rs index aa173ba..35d5e28 100644 --- a/server/src/strip.rs +++ b/server/src/strip.rs @@ -1,13 +1,16 @@ //! Produces a sibling `.slim.apk` for an APK with native `.so` //! debug symbols stripped out of every `lib/**/*.so` (everything else //! copied byte-for-byte, via `ZipWriter::raw_copy_file` so untouched -//! entries are never decompressed/recompressed), re-signed with the same -//! debug key so it still installs cleanly. +//! entries are never decompressed/recompressed), re-signed with this +//! machine's debug key so it still installs cleanly -- which holds only +//! while that is the key the build was signed with, and is therefore +//! checked rather than assumed. //! //! Cached against the source APK's (mtime, size) in a `.slim-stamp` //! sidecar, so it's only regenerated when the underlying build actually //! changes. +use std::collections::BTreeSet; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -118,6 +121,41 @@ fn strip_debug_symbols(src_path: &Path) -> Result { .arg(&aligned_path), )?; + // Re-signing keeps the copy installable only if this machine's debug + // key *is* the key the build was signed with, and nothing about a + // debug keystore says which builds it made -- it is per machine, and + // one recreated after the APK was built signs as an entirely + // unrelated certificate. Where they differ the phone's answer is + // "App not installed" with no cause shown, because Android refuses a + // package whose signature does not match the one it already has; the + // failure is silent, arrives on the device, and looks like the + // download rather than the signing. + // + // Measured on both files rather than inferred from the keystore, so + // this says what was actually produced -- and it stays true if the + // signing step ever changes which key or scheme it uses. + let built_by = signer_certificates(src_path, build_tools); + let signed_by = signer_certificates(&slim_path, build_tools); + if !built_by.is_empty() && !signed_by.is_empty() && built_by != signed_by { + // Removed rather than left for a later request to find: with no + // stamp beside it nothing would serve it, but `serveable_now` + // reports the size of whatever slim copy is on disk, so leaving + // it would have the card describing a download nothing can + // install. + let _ = std::fs::remove_file(&slim_path); + let _ = std::fs::remove_file(with_suffix(&slim_path, ".idsig")); + anyhow::bail!( + "{} is signed by {}, but the debug keystore at {} signs as {} -- a stripped copy \ + signed with it is a different app to Android, and the phone would refuse it with \ + \"App not installed\". Rebuild this app on this machine, or restore the debug \ + keystore it was built with.", + src_path.display(), + built_by.iter().cloned().collect::>().join(", "), + sdk::debug_keystore_path()?.display(), + signed_by.iter().cloned().collect::>().join(", "), + ); + } + std::fs::write(&stamp_path, &stamp)?; tracing::info!( "stripped -> {} ({} bytes)", @@ -127,6 +165,43 @@ fn strip_debug_symbols(src_path: &Path) -> Result { Ok(slim_path) } +/// Which certificates `apk` is signed with, as the SHA-256 digests +/// apksigner prints -- one line per signature scheme, so an APK signed +/// once under v1 and v2 answers with a set of one. +/// +/// Empty when apksigner will not verify the file or says nothing about +/// its certificates. Deliberately not an error: an APK this cannot read +/// is one this comparison has no opinion about, and refusing over it +/// would break a case that works today. The caller treats empty as "not +/// compared" rather than as "no signer", and the log line is what says +/// which of the two happened. +fn signer_certificates(apk: &Path, build_tools: &Path) -> BTreeSet { + let output = Command::new(build_tools.join("apksigner")) + .arg("verify") + .arg("--print-certs") + .arg(apk) + .output(); + let output = match output { + Ok(output) if output.status.success() => output, + other => { + tracing::warn!( + "could not read the signer of {}, so its signature was not compared: {}", + apk.display(), + match other { + Ok(output) => String::from_utf8_lossy(&output.stderr).trim().to_string(), + Err(err) => err.to_string(), + }, + ); + return BTreeSet::new(); + } + }; + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| line.split_once("certificate SHA-256 digest:")) + .map(|(_, digest)| digest.trim().to_string()) + .collect() +} + /// Rewrites `src_path` into `dst_path`, running every `lib/**/*.so` entry /// through `llvm-strip --strip-debug` and copying everything else as-is. fn strip_native_libs(