From f12eb4e0632b04ce1779bebf2e26ba590f22be51 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 13 Oct 2026 08:57:33 -0700 Subject: [PATCH] integrate android audit into build flow --- .mise.toml | 4 - README.md | 8 +- android/README.md | 5 +- android/SETUP.md | 16 +-- android/audit-libs.sh | 32 ----- build.zig | 67 ++++++++++- build/audit_android.zig | 180 +++++++++++++++++++++++++++++ build/audit_android_dependency.zig | 6 + build/audit_android_fixture.zig | 18 +++ 9 files changed, 282 insertions(+), 54 deletions(-) delete mode 100755 android/audit-libs.sh create mode 100644 build/audit_android.zig create mode 100644 build/audit_android_dependency.zig create mode 100644 build/audit_android_fixture.zig diff --git a/.mise.toml b/.mise.toml index 7256364..879fc93 100644 --- a/.mise.toml +++ b/.mise.toml @@ -59,10 +59,6 @@ adb shell am start -n dev.lerch.tally/.MainActivity description = "Boot the tally35 AVD - does not install the app (add -- -no-window for headless)" run = 'ANDROID_AVD_HOME="$MISE_PROJECT_ROOT/.tmp/avd" emulator -avd tally35 -no-audio -no-boot-anim -gpu swiftshader_indirect' -[tasks.android-audit] -description = "Check the Android libraries define every symbol they use (CI runs this too)" -run = 'zig build android && "$MISE_PROJECT_ROOT/android/audit-libs.sh" "$MISE_PROJECT_ROOT/zig-out/android"' - [tasks.apk-signer-test] description = "Build a release APK and sign it with the CI's HSM signer (action-hsm-sign), against SoftHSM (needs docker or podman)" run = ''' diff --git a/README.md b/README.md index 21752e9..fa0c8f2 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ mise run android-install # all of the above, then install a | `zig build test` | engine, CLI, TUI, C ABI and JNI tests, and a C program linked through `tally.h` | | `zig build coverage` | the same tests under kcov, one report per tree; `-Dcoverage-threshold=80` fails below 80% | | `zig build jvm-test` | the JNI layer loaded into a real JVM (needs a JDK; mise provides one) | -| `zig build android` | `zig-out/android/{arm64-v8a,x86_64,armeabi-v7a}/libtally.so`, ReleaseSmall | +| `zig build android` | `zig-out/android/{arm64-v8a,x86_64,armeabi-v7a}/libtally.so`, ReleaseSmall, audited | ### mise tasks @@ -221,7 +221,6 @@ mise run android-install # all of the above, then install a | `android-emulator`, `android-avd` | install the emulator and a 16 KB-page image, create the `tally35` AVD in `.tmp/avd` | | `android-run` | boot that AVD | | `android-install` | build the libraries and APK, install on the attached device, launch it | -| `android-audit` | check the Android libraries define every symbol they use (no libc, no TLS, no `DT_NEEDED`) | | `apk-signer-test` | build a release APK and sign it with CI's HSM signer, against SoftHSM (needs docker or podman) | ## How it is put together @@ -252,7 +251,8 @@ A few decisions shape everything else, and the design document explains each: layer whose function-table indices are proved against the running JVM in `JNI_OnLoad`, and against a desktop JVM by `zig build jvm-test`. - **The Android library links against nothing.** No libc, no NDK. That is why - `android-audit` exists: an undefined symbol links fine and fails in `dlopen` on a phone. + `zig build android` audits each library before installing it: an undefined symbol + links fine and fails in `dlopen` on a phone. - **Display budgets belong to the frontend.** The engine renders to whatever digit budget it is given; a phone, a terminal and a clipboard want different ones. @@ -287,7 +287,7 @@ punctuation: the source is ASCII. | Job | Runner | What | |---------------------|--------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | Engine, CLI and TUI | `ubuntu-latest` | `zig fmt --check`, zlint, build, `zig build test` in Debug and each release mode, `zig build jvm-test` in Debug and ReleaseSmall, then release builds for x86_64 and aarch64 Linux and aarch64 macOS | -| Android | `ubuntu-latest` | `zig build android`, the library audit, the JVM unit tests, debug and unsigned release APKs (with lint's release checks) | +| Android | `ubuntu-latest` | `zig build android` (which audits the libraries), the JVM unit tests, debug and unsigned release APKs (with lint's release checks) | | Sign | `ubuntu-latest-with-hsm` | master only: signs the APK with the HSM, then every release file with a detached signature logged to sigstore | | Publish | `ubuntu-latest` | master only: uploads the signed files to the generic package registry under the short SHA and `latest` | | F-Droid repo | `ubuntu-latest-with-hsm` | master only: adds the signed APK to the F-Droid repository on the HSM runner's host and regenerates its index | diff --git a/android/README.md b/android/README.md index 8291cfe..bac225b 100644 --- a/android/README.md +++ b/android/README.md @@ -63,8 +63,9 @@ device: `gradle testDebugUnitTest`. Verified on a physical device too: a Galaxy S23+ (arm64-v8a, 4 KB pages, Android 16). arm64-v8a is the interesting ABI: it is the one phones actually use, and the only one -where the page-size bug in design 6.3 showed up. `mise run android-audit` checks every -ABI's library for the symbols that would make `dlopen` fail. +where the page-size bug in design 6.3 showed up. `zig build android` checks every ABI's +library for the symbols that would make `dlopen` fail, and refuses to install one that +has them. ## Release builds diff --git a/android/SETUP.md b/android/SETUP.md index 84ef25d..86614db 100644 --- a/android/SETUP.md +++ b/android/SETUP.md @@ -184,16 +184,12 @@ The `android-avd` task sidesteps it with `ANDROID_AVD_HOME`; deleting the dead s also works. **`UnsatisfiedLinkError: dlopen failed: cannot locate symbol ...`** - the native library -referenced something Bionic provides and this library does not link. See design 6.3; the -audit is zero `DT_NEEDED`, zero undefined dynamic symbols, no TLS segment, per ABI: - -``` -readelf -d zig-out/android/arm64-v8a/libtally.so | grep NEEDED # expect nothing -readelf --dyn-syms -W zig-out/android/arm64-v8a/libtally.so | awk '$7=="UND"' -``` - -Check all three ABIs. One instance of this reproduced on arm64-v8a alone, so an x86_64 -emulator passed while phones would have failed. +referenced something Bionic provides and this library does not link. See design 6.3. +`zig build android` should have refused it: it audits all three ABIs before installing +(zero `DT_NEEDED`, zero undefined dynamic symbols, no TLS segment, 16 KiB `LOAD` +alignment) and names each problem it finds, so an APK carrying such a library was built +from a `zig-out/android` older than the audit. One instance of this reproduced on +arm64-v8a alone, so an x86_64 emulator passed while phones would have failed. **The app runs but answers look stale** - `zig build android` was not re-run, so the APK carries an old `libtally.so`. diff --git a/android/audit-libs.sh b/android/audit-libs.sh deleted file mode 100755 index fff50d2..0000000 --- a/android/audit-libs.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/bin/sh -# Check the Android libraries `zig build android` produced will load on a device. -# -# android/audit-libs.sh [zig-out/android] -# -# libtally.so is linked against nothing (design 6.2): no libc, no NDK. So every symbol it -# references has to be one it defines, it may not have a TLS segment (only Bionic can -# satisfy `__tls_get_addr`), and it may not name a library it needs. Each of those -# links on a desktop and fails in `dlopen` on a phone, before any code here runs - which -# is how the first build failed, and why this runs on every build rather than once. -set -eu - -dir="${1:-zig-out/android}" -fail=0 -for abi in arm64-v8a x86_64 armeabi-v7a; do - lib="$dir/$abi/libtally.so" - if [ ! -f "$lib" ]; then - echo "$abi: missing $lib" - fail=1 - continue - fi - undefined="$(readelf --dyn-syms -W "$lib" | awk '$7 == "UND" && $8 != ""' | wc -l)" - tls="$(readelf -l -W "$lib" | grep -c ' TLS ' || true)" - needed="$(readelf -d "$lib" | grep -c NEEDED || true)" - exports="$(readelf --dyn-syms -W "$lib" | grep -c 'Java_dev_lerch_tally_TallyEngine_' || true)" - printf '%-12s undefined=%s tls=%s needed=%s jni-exports=%s\n' "$abi" "$undefined" "$tls" "$needed" "$exports" - if [ "$undefined" -ne 0 ] || [ "$tls" -ne 0 ] || [ "$needed" -ne 0 ] || [ "$exports" -eq 0 ]; then - fail=1 - fi -done -[ "$fail" -eq 0 ] && echo "all Android libraries are self-contained" -exit "$fail" diff --git a/build.zig b/build.zig index a910c0f..72ecefa 100644 --- a/build.zig +++ b/build.zig @@ -275,14 +275,31 @@ pub fn build(b: *std.Build) void { // Always ReleaseSmall. A debug build of this library is twenty times the size and // would be a strange thing to ship inside an APK; `zig build` without this step // still produces a host library at whatever `-Doptimize` says. + // + // Each library is audited before it is installed (`build/audit_android.zig`): no + // undefined symbols, no TLS, no `DT_NEEDED`, 16 KiB `LOAD` alignment. Any of those + // links fine and fails in `dlopen` on a phone, and one of them once did so on + // arm64-v8a only, which an x86_64 emulator would never have shown. A library that + // fails is never in `zig-out/android` for Gradle to package. { const android_step = b.step("android", "Cross-compile libtally.so for Android ABIs"); + const auditor = b.addExecutable(.{ + .name = "audit-android", + .root_module = b.createModule(.{ + .root_source_file = b.path("build/audit_android.zig"), + .target = b.graph.host, + }), + }); + const audit = b.addRunArtifact(auditor); + audit.expectExitCode(0); + const abis = [_]struct { triple: []const u8, dir: []const u8 }{ .{ .triple = "aarch64-linux-android", .dir = "arm64-v8a" }, .{ .triple = "x86_64-linux-android", .dir = "x86_64" }, .{ .triple = "arm-linux-androideabi", .dir = "armeabi-v7a" }, }; - for (abis) |abi| { + var installs: [abis.len]*std.Build.Step.InstallArtifact = undefined; + for (abis, &installs) |abi, *install| { const query = std.Target.Query.parse(.{ .arch_os_abi = abi.triple }) catch @panic("bad Android target triple"); const lib = b.addLibrary(.{ @@ -298,11 +315,57 @@ pub fn build(b: *std.Build) void { // libraries to be aligned for them. The default here is 4KB, which loads // fine on a 4KB device and is rejected at submission. lib.link_z_max_page_size = 16384; - const install = b.addInstallArtifact(lib, .{ + audit.addArg(abi.dir); + audit.addArtifactArg(lib); + install.* = b.addInstallArtifact(lib, .{ .dest_dir = .{ .override = .{ .custom = b.fmt("android/{s}", .{abi.dir}) } }, }); + } + // One audit of all three, ahead of every install: a problem in any ABI stops + // all of them, so the directory never holds a mix of checked and unchecked. + for (installs) |install| { + install.step.dependOn(&audit.step); android_step.dependOn(&install.step); } + + // And the audit is held to failing. The fixture breaks every rule, and the + // auditor must say so for each one. Part of `test`: it needs only Zig, and an + // audit that passed everything would look exactly like one that works. + const fixture_target = b.resolveTargetQuery(std.Target.Query.parse(.{ .arch_os_abi = "x86_64-linux-android" }) catch + @panic("bad Android target triple")); + const dependency = b.addLibrary(.{ + .linkage = .dynamic, + .name = "audit-dependency", + .root_module = b.createModule(.{ + .root_source_file = b.path("build/audit_android_dependency.zig"), + .target = fixture_target, + .optimize = .ReleaseSmall, + }), + }); + const fixture = b.addLibrary(.{ + .linkage = .dynamic, + .name = "audit-fixture", + .root_module = b.createModule(.{ + .root_source_file = b.path("build/audit_android_fixture.zig"), + .target = fixture_target, + .optimize = .ReleaseSmall, + }), + }); + fixture.root_module.linkLibrary(dependency); + const refuse = b.addRunArtifact(auditor); + refuse.addArg("x86_64"); + refuse.addArtifactArg(fixture); + refuse.expectExitCode(1); + for ([_][]const u8{ + "references puts and does not define it", + "references __tls_get_addr and does not define it", + "has a TLS segment", + "needs libaudit-dependency.so, and it may need no library", + "a LOAD segment is aligned to 0x1000", + "exports no Java_dev_lerch_tally_TallyEngine_* entry points", + "does not export JNI_OnLoad", + }) |problem| refuse.expectStdErrMatch(problem); + test_step.dependOn(&refuse.step); } // -- The JNI layer against a real JVM -- diff --git a/build/audit_android.zig b/build/audit_android.zig new file mode 100644 index 0000000..fc9ffb9 --- /dev/null +++ b/build/audit_android.zig @@ -0,0 +1,180 @@ +//! Checks that each Android `libtally.so` will load on a device (design 6.3). +//! +//! The library links against nothing: no libc, no NDK. So every symbol it references +//! has to be one it defines, it may not have a TLS segment (only Bionic's libc can +//! satisfy `__tls_get_addr`), and it may not name a library it needs. Each of those +//! links on a desktop and fails in `dlopen` on a phone, before any code in the library +//! runs. The first APK failed exactly that way, and a later instance reproduced on +//! arm64-v8a alone, so an x86_64 emulator would have passed while phones failed. The +//! code that pulls such a symbol in never has to run for it to matter: `BIND_NOW` is +//! set, so even an unreferenced PLT entry is resolved at load. +//! +//! Also checked: every `LOAD` segment is aligned for 16 KiB pages, which Android 15 +//! introduced and Play requires (a 4 KiB alignment loads on a 4 KiB device and is +//! refused at submission), the library is for the architecture its directory names, +//! and the JNI entry points are exported. +//! +//! This replaced `android/audit-libs.sh`, a `readelf` pipeline that CI ran as a step of +//! its own after the build, and that needed binutils installed to do it. Here it is part +//! of `zig build android`, on every machine that builds the libraries, and a library +//! that fails it is never installed for Gradle to package. +//! +//! Usage: audit-android [ ...] +//! Prints nothing when every library passes; otherwise every problem found, and +//! exits non-zero. + +const std = @import("std"); +const elf = std.elf; + +/// Android's page size since version 15, and what Play holds libraries to. +const page_alignment = 16 * 1024; + +/// Every JNI entry point is named for the Kotlin class it belongs to. +const jni_prefix = "Java_dev_lerch_tally_TallyEngine_"; + +pub fn main(init: std.process.Init) !u8 { + const allocator = init.arena.allocator(); + const args = try init.minimal.args.toSlice(allocator); + if (args.len < 3 or (args.len - 1) % 2 != 0) { + std.log.err("usage: audit-android [ ...]", .{}); + return 2; + } + + var stderr_buf: [4096]u8 = undefined; + var stderr = std.Io.File.stderr().writerStreaming(init.io, &stderr_buf); + const out = &stderr.interface; + + var failed = false; + var i: usize = 1; + while (i < args.len) : (i += 2) { + const abi = args[i]; + const path = args[i + 1]; + const bytes = std.Io.Dir.cwd().readFileAlloc(init.io, path, allocator, .limited(64 * 1024 * 1024)) catch |err| { + try out.print("{s}: cannot read {s}: {t}\n", .{ abi, path, err }); + failed = true; + continue; + }; + var report: Report = .{ .abi = abi, .out = out }; + audit(allocator, abi, bytes, &report) catch |err| { + try out.print("{s}: {s} is not an ELF file this can read: {t}\n", .{ abi, path, err }); + failed = true; + continue; + }; + if (report.problems > 0) failed = true; + } + try out.flush(); + return if (failed) 1 else 0; +} + +/// Problems found in one library, written as they are found. +const Report = struct { + abi: []const u8, + out: *std.Io.Writer, + problems: usize = 0, + + fn problem(self: *Report, comptime fmt: []const u8, args: anytype) !void { + self.problems += 1; + try self.out.print("{s}: " ++ fmt ++ "\n", .{self.abi} ++ args); + } +}; + +/// The ELF machine each Android ABI directory holds. +fn expectedMachine(abi: []const u8) ?elf.EM { + if (std.mem.eql(u8, abi, "arm64-v8a")) return .AARCH64; + if (std.mem.eql(u8, abi, "x86_64")) return .X86_64; + if (std.mem.eql(u8, abi, "armeabi-v7a")) return .ARM; + return null; +} + +fn audit(allocator: std.mem.Allocator, abi: []const u8, bytes: []const u8, report: *Report) !void { + var reader: std.Io.Reader = .fixed(bytes); + const header = try elf.Header.read(&reader); + + if (expectedMachine(abi)) |machine| { + if (header.machine != machine) try report.problem("built for {t}, not {t}", .{ header.machine, machine }); + } else { + try report.problem("not an Android ABI this knows", .{}); + } + + // Program headers: no TLS, every LOAD aligned for 16 KiB pages, and where the + // dynamic section is. + var dynamic: ?struct { offset: u64, size: u64 } = null; + var phdrs = header.iterateProgramHeadersBuffer(bytes); + while (try phdrs.next()) |phdr| switch (phdr.p_type) { + elf.PT_TLS => try report.problem("has a TLS segment, which needs __tls_get_addr from a libc", .{}), + elf.PT_LOAD => if (phdr.p_align < page_alignment) { + try report.problem("a LOAD segment is aligned to 0x{x}, not the 0x{x} 16 KiB pages need", .{ phdr.p_align, page_alignment }); + }, + elf.PT_DYNAMIC => dynamic = .{ .offset = phdr.p_offset, .size = phdr.p_filesz }, + else => {}, + }; + + // The dynamic symbol table and its strings, from the section headers. + var dynsym: ?elf.Elf64_Shdr = null; + var dynstr: ?elf.Elf64_Shdr = null; + var sections = header.iterateSectionHeadersBuffer(bytes); + var all_sections: std.ArrayList(elf.Elf64_Shdr) = .empty; + while (try sections.next()) |shdr| try all_sections.append(allocator, shdr); + for (all_sections.items) |shdr| { + if (shdr.sh_type == elf.SHT_DYNSYM) { + dynsym = shdr; + if (shdr.sh_link < all_sections.items.len) dynstr = all_sections.items[shdr.sh_link]; + } + } + const symbols = dynsym orelse return report.problem("has no dynamic symbol table, so it exports nothing", .{}); + const strings = dynstr orelse return report.problem("has no string table for its dynamic symbols", .{}); + const names = try slice(bytes, strings.sh_offset, strings.sh_size); + + // Dynamic entries: no DT_NEEDED. + if (dynamic) |d| { + var entries = header.iterateDynamicSectionBuffer(bytes, d.offset, d.size); + while (try entries.next()) |entry| { + if (entry.d_tag == elf.DT_NULL) break; + if (entry.d_tag == elf.DT_NEEDED) { + try report.problem("needs {s}, and it may need no library", .{name(names, entry.d_val)}); + } + } + } + + // Symbols: none undefined, and the JNI entry points defined. + const entry_size: u64 = if (header.is_64) @sizeOf(elf.Elf64_Sym) else @sizeOf(elf.Elf32_Sym); + const table = try slice(bytes, symbols.sh_offset, symbols.sh_size); + var jni_exports: usize = 0; + var has_onload = false; + var at: usize = 0; + while (at + entry_size <= table.len) : (at += @intCast(entry_size)) { + var sym_reader: std.Io.Reader = .fixed(table[at..]); + const sym_name: u32, const section: u16 = if (header.is_64) blk: { + const sym = try sym_reader.takeStruct(elf.Elf64_Sym, header.endian); + break :blk .{ sym.st_name, sym.st_shndx }; + } else blk: { + const sym = try sym_reader.takeStruct(elf.Elf32_Sym, header.endian); + break :blk .{ sym.st_name, sym.st_shndx }; + }; + const text = name(names, sym_name); + // The first entry is the null symbol, which has no name and is not a reference. + if (text.len == 0) continue; + if (section == elf.SHN_UNDEF) { + try report.problem("references {s} and does not define it", .{text}); + } else if (std.mem.startsWith(u8, text, jni_prefix)) { + jni_exports += 1; + } else if (std.mem.eql(u8, text, "JNI_OnLoad")) { + has_onload = true; + } + } + if (jni_exports == 0) try report.problem("exports no {s}* entry points", .{jni_prefix}); + if (!has_onload) try report.problem("does not export JNI_OnLoad", .{}); +} + +/// `len` bytes at `offset`, or an error if the file is shorter than its headers say. +fn slice(bytes: []const u8, offset: u64, len: u64) error{EndOfStream}![]const u8 { + if (offset > bytes.len or len > bytes.len - offset) return error.EndOfStream; + return bytes[@intCast(offset)..][0..@intCast(len)]; +} + +/// The NUL-terminated string at `offset` in a string table. +fn name(strings: []const u8, offset: u64) []const u8 { + if (offset >= strings.len) return ""; + const rest = strings[@intCast(offset)..]; + return rest[0 .. std.mem.indexOfScalar(u8, rest, 0) orelse rest.len]; +} diff --git a/build/audit_android_dependency.zig b/build/audit_android_dependency.zig new file mode 100644 index 0000000..384fe46 --- /dev/null +++ b/build/audit_android_dependency.zig @@ -0,0 +1,6 @@ +//! The library `audit_android_fixture.zig` is linked against, so the fixture has a +//! `DT_NEEDED` entry for the auditor to refuse. + +export fn tally_audit_dependency() c_int { + return 0; +} diff --git a/build/audit_android_fixture.zig b/build/audit_android_fixture.zig new file mode 100644 index 0000000..66449f6 --- /dev/null +++ b/build/audit_android_fixture.zig @@ -0,0 +1,18 @@ +//! A library that breaks every rule `audit_android.zig` checks, so `zig build test` can +//! hold the auditor to failing on it. An audit that passes everything would be +//! indistinguishable from a working one by looking at the real libraries. +//! +//! It references a symbol it does not define, has a thread-local (a TLS segment, and a +//! reference to `__tls_get_addr`), is linked at the default 4 KiB page alignment, +//! needs another library (`build.zig` links it against one), and exports no JNI entry +//! points. + +extern fn puts(s: [*:0]const u8) c_int; +extern fn tally_audit_dependency() c_int; + +threadlocal var calls: u32 = 0; + +export fn tally_audit_fixture() c_int { + calls += 1; + return puts("never called: this library exists to be refused") + tally_audit_dependency(); +}