integrate android audit into build flow

.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 = '''

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                                                                                        |

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
 

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`.

android/audit-libs.sh

@@ -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"

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 --

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 <abi> <path/to/libtally.so> [<abi> <path> ...]
+//! 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 <abi> <libtally.so> [<abi> <libtally.so> ...]", .{});
+        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];
+}

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;
+}

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();
+}