neiam
/gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
1
Log in to mark this repository.
Read an APK's manifest, and plan the F-Droid repository
504e101 · Gabriel Morell · 2026-09-12 16:04
Message
{commit_body(@commit)}
Files changed
added
docs/fdroid-registry.md
+371
−0
@@ -0,0 +1,371 @@
| 1 | +# F-Droid app repository — plan | |
| 2 | + | |
| 3 | +Serving an F-Droid repository out of the forge, the way apt already is, | |
| 4 | +so that `pkg/fdroid/<owner>/_all` is one repo line in an F-Droid client | |
| 5 | +and CI publishes into it the same way it publishes `.deb`s today. | |
| 6 | + | |
| 7 | +Spec details are quoted inline deliberately: they were the expensive | |
| 8 | +part to establish, and an implementer shouldn't have to re-derive them. | |
| 9 | + | |
| 10 | +## The framing | |
| 11 | + | |
| 12 | +This is **one more `GitGud.Packages.Format` adapter**, not a subsystem. | |
| 13 | +Slice 70 left more in place than it looks: | |
| 14 | + | |
| 15 | +| Need | Already there | | |
| 16 | +|---|---| | |
| 17 | +| Route | `/pkg/:format/:owner/:name/*rest` — dispatches `fdroid` unchanged | | |
| 18 | +| Namespace view | `Format.namespace_marker/0` (`_all`) + the optional `namespace_index/3` callback, built for exactly this | | |
| 19 | +| Per-namespace signing key | `SigningKey`, minted on first use, private half AES-GCM in Postgres via `GitGud.Secrets` | | |
| 20 | +| Public key endpoint | `/pkg/_keys/:format/:owner` | | |
| 21 | +| Auth | `RegistryToken` + HTTP Basic — which the F-Droid client speaks for private repos | | |
| 22 | +| Blobs, retention, pruning, disk usage, UI | unchanged | | |
| 23 | + | |
| 24 | +`namespace_index/3` already carries the rationale for `_all`: | |
| 25 | + | |
| 26 | +> It earns its place where the namespace is the unit a consumer | |
| 27 | +> actually configures — apt, where five repositories mean five `deb` | |
| 28 | +> lines and five keyring entries for what an administrator thinks of as | |
| 29 | +> one publisher. | |
| 30 | + | |
| 31 | +An F-Droid repo is the same story. A phone should get one repo entry | |
| 32 | +for `neiam`, not one per Android project. | |
| 33 | + | |
| 34 | +## What actually has to be built | |
| 35 | + | |
| 36 | +Four pieces. Two are parsers, one is a signer, one is JSON assembly. | |
| 37 | + | |
| 38 | +--- | |
| 39 | + | |
| 40 | +## Phase A — reading an APK | |
| 41 | + | |
| 42 | +The analogue of `Formats.Deb.Control`, and bigger, because an APK does | |
| 43 | +not carry a text stanza. | |
| 44 | + | |
| 45 | +### AndroidManifest.xml is binary | |
| 46 | + | |
| 47 | +An APK is a zip (`:zip` opens it), but the manifest inside is Android's | |
| 48 | +compiled resource format, not XML text. Layout: | |
| 49 | + | |
| 50 | +``` | |
| 51 | +ResChunk_header type u16, headerSize u16, size u32 | |
| 52 | + RES_XML_TYPE 0x0003 the file itself | |
| 53 | + RES_STRING_POOL_TYPE 0x0001 | |
| 54 | + RES_XML_RESOURCE_MAP_TYPE 0x0180 | |
| 55 | + RES_XML_START_ELEMENT 0x0102 | |
| 56 | + RES_XML_END_ELEMENT 0x0103 | |
| 57 | +``` | |
| 58 | + | |
| 59 | +String pool header adds `stringCount u32`, `styleCount u32`, | |
| 60 | +`flags u32`, `stringsStart u32`, `stylesStart u32`, then a | |
| 61 | +`stringCount`-long u32 offset array, then the data. | |
| 62 | + | |
| 63 | +- `flags &&& 0x100` (`UTF8_FLAG`) picks the encoding. Both appear in | |
| 64 | + the wild — AAPT2 emits UTF-8 for most resources but the manifest of | |
| 65 | + the APK this was written against is **UTF-16**. | |
| 66 | +- UTF-16: `u16` length, then that many UTF-16LE code units, then a | |
| 67 | + `u16` NUL. A length with the high bit set is a two-unit length. | |
| 68 | +- UTF-8: `u8` UTF-16 length, `u8` byte length, bytes, `u8` NUL — each | |
| 69 | + length independently extensible by its own high bit. | |
| 70 | + | |
| 71 | +A start element is `ResXMLTree_node` (`lineNumber u32`, `comment u32`) | |
| 72 | +then `ResXMLTree_attrExt`: | |
| 73 | + | |
| 74 | +``` | |
| 75 | +ns u32, name u32, attributeStart u16, attributeSize u16, | |
| 76 | +attributeCount u16, idIndex u16, classIndex u16, styleIndex u16 | |
| 77 | +``` | |
| 78 | + | |
| 79 | +then `attributeCount` × `ResXMLTree_attribute`: | |
| 80 | + | |
| 81 | +``` | |
| 82 | +ns u32, name u32, rawValue u32, size u16, res0 u8, dataType u8, data u32 | |
| 83 | +``` | |
| 84 | + | |
| 85 | +`ns`/`name`/`rawValue` are string-pool indices, `0xFFFFFFFF` for | |
| 86 | +absent. Data types that matter here: | |
| 87 | + | |
| 88 | +| `dataType` | Meaning | | |
| 89 | +|---|---| | |
| 90 | +| `0x03` `TYPE_STRING` | `data` is a string-pool index | | |
| 91 | +| `0x10` `TYPE_INT_DEC` | `data` is the integer | | |
| 92 | +| `0x11` `TYPE_INT_HEX` | ditto | | |
| 93 | +| `0x12` `TYPE_INT_BOOLEAN` | non-zero is true | | |
| 94 | +| `0x01` `TYPE_REFERENCE` | `@0x…` resource id — not resolvable without `resources.arsc` | | |
| 95 | + | |
| 96 | +**Attributes are matched by name string, not by resource id.** The | |
| 97 | +resource map chunk maps pool index → resource id, and a fully correct | |
| 98 | +reader would use it; the names are in the pool regardless, and matching | |
| 99 | +them avoids carrying a resource-id table. The cost is that an APK built | |
| 100 | +by a toolchain that strips names would read as empty — none do. | |
| 101 | + | |
| 102 | +What gets pulled out: | |
| 103 | + | |
| 104 | +| Field | From | | |
| 105 | +|---|---| | |
| 106 | +| `packageName` | `<manifest package>` | | |
| 107 | +| `versionCode` | `<manifest android:versionCode>` | | |
| 108 | +| `versionName` | `<manifest android:versionName>` | | |
| 109 | +| `minSdkVersion`, `targetSdkVersion` | `<uses-sdk>` | | |
| 110 | +| `uses-permission` | `<uses-permission android:name>` | | |
| 111 | +| `nativecode` | `lib/<abi>/` entries in the zip, not the manifest | | |
| 112 | + | |
| 113 | +`android:label` and `android:icon` are usually `TYPE_REFERENCE` into | |
| 114 | +`resources.arsc`. Resolving those means an ARSC reader, which is a | |
| 115 | +second format of comparable size. **Out of scope**: the index falls | |
| 116 | +back to `packageName` for the name, and the repo serves no icons. An | |
| 117 | +`.arsc` reader can come later if anyone wants pretty listings. | |
| 118 | + | |
| 119 | +### Why this is hand-written rather than `rusty-axml` | |
| 120 | + | |
| 121 | +A Rustler NIF is landing anyway for Phase C, so an AXML crate would | |
| 122 | +cost nothing extra in deployment. [`rusty-axml`][rusty-axml] was the | |
| 123 | +candidate (0.2.1, Apache-2.0, ~16k recent downloads, last release | |
| 124 | +2025-07). It was evaluated by running it against this repo's own | |
| 125 | +manifest rather than by reading its docs, the same way the sequoia | |
| 126 | +backend was chosen: | |
| 127 | + | |
| 128 | +``` | |
| 129 | +<manifest> attrs = {"package": "org.neiam.gitgud.app", | |
| 130 | + "android:versionCode": "(type 0x10) 0x1", | |
| 131 | + "android:versionName": "0.1.0", ...} | |
| 132 | +<uses-sdk> attrs = {"android:minSdkVersion": "(type 0x10) 0x1a", ...} | |
| 133 | +``` | |
| 134 | + | |
| 135 | +**It discards typed values.** `attributes()` is a | |
| 136 | +`HashMap<String, String>`, and `parse_start_element` stringifies an | |
| 137 | +integer into `"(type 0x10) 0x1"` at parse time — the `dataType` is | |
| 138 | +gone, and there is no typed accessor anywhere in the public API. Every | |
| 139 | +number this adapter needs is an integer attribute, and F-Droid compares | |
| 140 | +`versionCode` **numerically** to decide whether something is an update. | |
| 141 | + | |
| 142 | +Recovering `1` from `"(type 0x10) 0x1"` means scraping a debug format: | |
| 143 | +correct today, silently wrong the day that `format!` changes, and wrong | |
| 144 | +in the direction of "the client stops seeing updates" rather than the | |
| 145 | +direction of a crash. | |
| 146 | + | |
| 147 | +Its other helpers (`find_node_by_name` matches `android:name`, not the | |
| 148 | +element type; `get_requested_permissions`) are shaped for security | |
| 149 | +analysis of components, not for indexing. | |
| 150 | + | |
| 151 | +So the parser stays in Elixir: it returns `version_code: 1` typed from | |
| 152 | +`dataType`, it is tested against a real AAPT2 manifest including every | |
| 153 | +truncation of it, and it adds no dependency to a code path that reads | |
| 154 | +attacker-supplied input. | |
| 155 | + | |
| 156 | +[rusty-axml]: https://crates.io/crates/rusty-axml | |
| 157 | + | |
| 158 | +### The signing certificate is not where you'd expect | |
| 159 | + | |
| 160 | +F-Droid's index carries `signer` — the SHA-256 of the APK's signing | |
| 161 | +certificate — and the client refuses an update whose signer differs | |
| 162 | +from what's installed. So it has to be extracted. | |
| 163 | + | |
| 164 | +The obvious place is `META-INF/*.RSA` (v1 / JAR signing). **It is not | |
| 165 | +there.** Checked against this repo's own APK: | |
| 166 | + | |
| 167 | +``` | |
| 168 | +META-INF/*.RSA, MANIFEST.MF → absent | |
| 169 | +APK Sig Block 42 → present, block id 0x7109871a (Scheme v2) | |
| 170 | +``` | |
| 171 | + | |
| 172 | +`minSdk` is 26, and AGP omits v1 signing above 24. So the cert comes | |
| 173 | +from the APK Signing Block, which sits between the last zip entry and | |
| 174 | +the central directory: | |
| 175 | + | |
| 176 | +``` | |
| 177 | +EOCD (PK\x05\x06) → central directory offset | |
| 178 | + [cd_offset - 16, cd_offset) magic "APK Sig Block 42" | |
| 179 | + [cd_offset - 24, cd_offset - 16) u64 size-of-block (repeated at its start) | |
| 180 | + block = u64 size ‖ { u64 len ‖ u32 id ‖ value }* ‖ u64 size ‖ magic | |
| 181 | +``` | |
| 182 | + | |
| 183 | +IDs: `0x7109871a` is Scheme v2, `0xf05368c0` Scheme v3, `0x42726577` | |
| 184 | +is apksigner's padding block. A v2/v3 value is: | |
| 185 | + | |
| 186 | +``` | |
| 187 | +signers: len-prefixed sequence of | |
| 188 | + signer: len-prefixed | |
| 189 | + signed data: len-prefixed | |
| 190 | + signatures: len-prefixed | |
| 191 | + public key: len-prefixed | |
| 192 | + where signed data = digests ‖ certificates ‖ additional attributes | |
| 193 | + and certificates = len-prefixed sequence of len-prefixed X.509 DER | |
| 194 | +``` | |
| 195 | + | |
| 196 | +Every length is a little-endian u32. The first certificate of the first | |
| 197 | +signer is the signing cert; `sha256` of its DER is `signer`. | |
| 198 | + | |
| 199 | +Both paths get implemented — v1 for anything with `minSdk < 24`, the | |
| 200 | +signing block for everything else — and v1 is checked second, because | |
| 201 | +an APK carrying both is signed by the same key either way and the | |
| 202 | +block is authoritative. | |
| 203 | + | |
| 204 | +**Verification is out of scope.** The forge reads the certificate; it | |
| 205 | +does not verify the signature over the APK. The client does that, and | |
| 206 | +Android does it again at install. Re-implementing v2 digest | |
| 207 | +verification would be a third format for no added guarantee. | |
| 208 | + | |
| 209 | +--- | |
| 210 | + | |
| 211 | +## Phase B — the index documents | |
| 212 | + | |
| 213 | +Mechanical assembly from `PackageVersion` rows, the same shape as the | |
| 214 | +Deb adapter's `release_document` / `packages_document`. | |
| 215 | + | |
| 216 | +Serve **index-v2**, plus **index-v1** for older clients: | |
| 217 | + | |
| 218 | +``` | |
| 219 | +/pkg/fdroid/<owner>/_all/entry.jar signed; points at index-v2 | |
| 220 | +/pkg/fdroid/<owner>/_all/index-v2.json verified by hash from entry | |
| 221 | +/pkg/fdroid/<owner>/_all/index-v1.jar signed; legacy clients | |
| 222 | +/pkg/fdroid/<owner>/_all/<apk filename> the artifact | |
| 223 | +``` | |
| 224 | + | |
| 225 | +Per F-Droid's Security Model: index-v1 signs with `SHA1withRSA`, | |
| 226 | +index-v2 with `SHA256withRSA`, and index-v2 accepts "any algorithm | |
| 227 | +supported by apksigner and android-23 and newer". SHA-1 there is not a | |
| 228 | +weakness — the relevant property is second-preimage resistance — but | |
| 229 | +new work signs v2 and only emits v1 for reach. | |
| 230 | + | |
| 231 | +--- | |
| 232 | + | |
| 233 | +## Phase C — signing the index | |
| 234 | + | |
| 235 | +The one genuine unknown, and the same shape of gap OpenPGP was for apt. | |
| 236 | + | |
| 237 | +A signed JAR is a zip containing: | |
| 238 | + | |
| 239 | +``` | |
| 240 | +META-INF/MANIFEST.MF per-entry SHA-256 digests | |
| 241 | +META-INF/CERT.SF digest of the manifest, and of each entry's stanza | |
| 242 | +META-INF/CERT.RSA PKCS#7 / CMS detached SignedData over CERT.SF | |
| 243 | +``` | |
| 244 | + | |
| 245 | +`:public_key` speaks X.509 and PKCS but **not CMS SignedData**. That is | |
| 246 | +precisely the hole `native/gitgud_apt` was built to fill for OpenPGP. | |
| 247 | + | |
| 248 | +**Route taken: a Rustler NIF**, `native/gitgud_apk`, using RustCrypto's | |
| 249 | +`cms`, `x509-cert`, `rsa`, `sha2` and `zip`. All pure Rust. Measured | |
| 250 | +against the standard `GitGud.Packages.Apt.Native` sets out: | |
| 251 | + | |
| 252 | +``` | |
| 253 | +gitgud_apt (sequoia) builder: libclang-dev runtime: — | |
| 254 | +gitgud_apk (RustCrypto) builder: — runtime: — | |
| 255 | +``` | |
| 256 | + | |
| 257 | +Cheaper than the backend already accepted for apt — nothing is added to | |
| 258 | +either image stage. | |
| 259 | + | |
| 260 | +The repo's self-signed X.509 certificate does **not** need the NIF: | |
| 261 | +`:public_key.pkix_sign/2` builds it, over the RSA keypair | |
| 262 | +`Packages.Token.generate_keypair/0` already produces. So | |
| 263 | +`generate_keypair(owner, "fdroid")` is one new clause returning PEM, | |
| 264 | +and the NIF only does CMS. | |
| 265 | + | |
| 266 | +### Sign at publish, not at read | |
| 267 | + | |
| 268 | +The Deb adapter signs `InRelease` **on every request**. At NIF speed | |
| 269 | +that is free. It would not be at any other speed, and F-Droid clients | |
| 270 | +poll indexes. | |
| 271 | + | |
| 272 | +So `fdroid` materialises `entry.jar` and `index-v1.jar` into | |
| 273 | +`GitGud.Storage` when an APK is published and serves them as blobs — | |
| 274 | +`{:blob, package_file}` is already an `index_result` variant, so the | |
| 275 | +adapter contract needs nothing new. Signing then runs once per publish | |
| 276 | +instead of once per poll. | |
| 277 | + | |
| 278 | +This also keeps the fallback open (below) without a rewrite. | |
| 279 | + | |
| 280 | +### If RustCrypto goes wrong | |
| 281 | + | |
| 282 | +`jarsigner`/`apksigner` in their own container, the way zot is the | |
| 283 | +fallback-shaped dependency for OCI and aptly was named for apt. It | |
| 284 | +works, and sign-at-publish makes the latency irrelevant — but it is a | |
| 285 | +worse trade *here* than aptly was for apt: | |
| 286 | + | |
| 287 | +- aptly replaces an entire repository format. | |
| 288 | +- A signer sidecar replaces only the CMS step. Phases A, B and the zip | |
| 289 | + assembly stay. | |
| 290 | + | |
| 291 | +And it costs a key-custody split: the private key lives encrypted in | |
| 292 | +Postgres today, and a JVM wants a keystore file. If it happens, the key | |
| 293 | +stays in Postgres and is handed over loopback per request, never | |
| 294 | +persisted in the container, which must have no ingress — it will sign | |
| 295 | +whatever it is given. | |
| 296 | + | |
| 297 | +**Any shim written for this is a small Rust daemon, not a JVM | |
| 298 | +wrapper.** The JVM is the thing being avoided; adding one to escape 150 | |
| 299 | +lines of Rust is the wrong direction. | |
| 300 | + | |
| 301 | +--- | |
| 302 | + | |
| 303 | +## Phase D — publishing from CI | |
| 304 | + | |
| 305 | +The existing `deb` job is the template, down to the verify step: | |
| 306 | + | |
| 307 | +```yaml | |
| 308 | +- name: Publish the APK | |
| 309 | + run: | | |
| 310 | + curl --fail-with-body -sS \ | |
| 311 | + -H "Authorization: Bearer ${{ github.token }}" \ | |
| 312 | + -H "Content-Type: application/vnd.android.package-archive" \ | |
| 313 | + --upload-file android/app/build/outputs/apk/release/app-release.apk \ | |
| 314 | + "$FORGE_URL/pkg/fdroid/$REPOSITORY/upload" | |
| 315 | +``` | |
| 316 | + | |
| 317 | +Consuming it is one repo entry: | |
| 318 | + | |
| 319 | +``` | |
| 320 | +$FORGE_URL/pkg/fdroid/<owner>/_all?fingerprint=<sha256 of the repo cert> | |
| 321 | +``` | |
| 322 | + | |
| 323 | +**Open question — confirm before building UI on it.** The `fingerprint` | |
| 324 | +query parameter is how `fdroidserver` hands out repo URLs and how the | |
| 325 | +QR path works, but neither `All_our_APIs` nor `Setup_an_F-Droid_App_Repo` | |
| 326 | +documents it (the latter is stale — it still describes `index.xml`). | |
| 327 | +Confirm against `fdroidserver` source. If it holds, the packages page | |
| 328 | +should emit a **QR** as well as the URL: F-Droid adds repos by scan, and | |
| 329 | +`EQRCode` is already a dependency. | |
| 330 | + | |
| 331 | +### Prerequisite: a release keystore | |
| 332 | + | |
| 333 | +`android/README.md` notes there isn't one. A debug-signed APK can be | |
| 334 | +published but not meaningfully consumed — and the signer fingerprint | |
| 335 | +would be the shared Android debug key, which every developer has. | |
| 336 | + | |
| 337 | +This is the **APK** signing key, distinct from the **index** signing | |
| 338 | +key. Keep them apart: | |
| 339 | + | |
| 340 | +| Key | Lives | Signs | | |
| 341 | +|---|---|---| | |
| 342 | +| APK release keystore | CI secrets (`GitGud.Secrets`) | the app | | |
| 343 | +| Repo index key | `SigningKey`, per namespace | the index | | |
| 344 | + | |
| 345 | +## Key custody, stated deliberately | |
| 346 | + | |
| 347 | +F-Droid's docs say: *"the repo signing keys should not ever be on a | |
| 348 | +public server."* The forge holds the index key, so this departs from | |
| 349 | +that advice — knowingly, and for the same reason the apt PGP key is | |
| 350 | +held: a registry that cannot sign its own index is not a registry. | |
| 351 | + | |
| 352 | +The mitigation is that it is the **lesser** key. It signs metadata, not | |
| 353 | +apps. An attacker with it can lie about what versions exist; they | |
| 354 | +cannot forge an app, because Android verifies the APK signature at | |
| 355 | +install against the key in `signer`, which lives in CI. | |
| 356 | + | |
| 357 | +## Order | |
| 358 | + | |
| 359 | +**A → B → C → D.** Extraction first, because it is provable against an | |
| 360 | +APK this repo already builds. Then the index documents, served | |
| 361 | +unsigned, checked with `curl`. Then signing, last, because it is the | |
| 362 | +piece with a fallback. CI publishing is the smallest step and lands | |
| 363 | +once there is something to publish into. | |
| 364 | + | |
| 365 | +## Sources | |
| 366 | + | |
| 367 | +- [F-Droid: All our APIs](https://f-droid.org/docs/All_our_APIs/) | |
| 368 | +- [F-Droid: Security Model](https://f-droid.org/en/docs/Security_Model/) | |
| 369 | +- [APK Signature Scheme v2](https://source.android.com/docs/security/features/apksigning/v2) | |
| 370 | +- [JAR file specification — signed JARs](https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html) | |
| 371 | +- `frameworks/base/libs/androidfw/include/androidfw/ResourceTypes.h` — the chunk layouts above |
added
lib/git_gud/packages/formats/fdroid/manifest.ex
+346
−0
@@ -0,0 +1,346 @@
| 1 | +defmodule GitGud.Packages.Formats.Fdroid.Manifest do | |
| 2 | + @moduledoc """ | |
| 3 | + Reads `AndroidManifest.xml` out of an APK. | |
| 4 | + | |
| 5 | + This is the `Formats.Deb.Control` of the F-Droid adapter, and it is | |
| 6 | + bigger, because an APK carries no text stanza. The manifest inside an | |
| 7 | + APK is Android's compiled binary resource format — a chunk tree over | |
| 8 | + a string pool — so there is a small parser here rather than an XML | |
| 9 | + library call. | |
| 10 | + | |
| 11 | + Only what the index needs is extracted: package name, version, SDK | |
| 12 | + levels and permissions. See `docs/fdroid-registry.md` for the chunk | |
| 13 | + layouts and for why `android:label` / `android:icon` are deliberately | |
| 14 | + left alone (they are usually `TYPE_REFERENCE` into `resources.arsc`, | |
| 15 | + and an ARSC reader is a second format of comparable size). | |
| 16 | + | |
| 17 | + Attributes are matched by their **name string**, not by resource id. | |
| 18 | + The resource map chunk maps pool index to resource id and a fully | |
| 19 | + correct reader would consult it; the names are in the pool either | |
| 20 | + way, and matching them avoids carrying a resource-id table. An APK | |
| 21 | + built by a toolchain that stripped attribute names would read as | |
| 22 | + empty — none do. | |
| 23 | + | |
| 24 | + Everything here is defensive. An APK is attacker-supplied input from | |
| 25 | + the registry's point of view, so a malformed chunk returns an error | |
| 26 | + rather than raising, and no length read out of the file is trusted to | |
| 27 | + be in bounds. | |
| 28 | + """ | |
| 29 | + | |
| 30 | + # Chunk types, from ResourceTypes.h. | |
| 31 | + @res_string_pool 0x0001 | |
| 32 | + @res_xml 0x0003 | |
| 33 | + @res_xml_start_element 0x0102 | |
| 34 | + | |
| 35 | + # String pool flags. | |
| 36 | + @utf8_flag 0x100 | |
| 37 | + | |
| 38 | + # Typed-value data types. | |
| 39 | + @type_string 0x03 | |
| 40 | + @type_int_dec 0x10 | |
| 41 | + @type_int_hex 0x11 | |
| 42 | + @type_int_boolean 0x12 | |
| 43 | + | |
| 44 | + @absent 0xFFFFFFFF | |
| 45 | + | |
| 46 | + # A manifest far larger than this is not a manifest. The real one for | |
| 47 | + # this repo's own app is 8 KB; AOSP's largest are in the low hundreds. | |
| 48 | + @max_size 4 * 1024 * 1024 | |
| 49 | + | |
| 50 | + @type attrs :: %{String.t() => String.t() | integer() | boolean()} | |
| 51 | + | |
| 52 | + @type info :: %{ | |
| 53 | + package: String.t() | nil, | |
| 54 | + version_code: integer() | nil, | |
| 55 | + version_name: String.t() | nil, | |
| 56 | + min_sdk: integer() | nil, | |
| 57 | + target_sdk: integer() | nil, | |
| 58 | + permissions: [String.t()] | |
| 59 | + } | |
| 60 | + | |
| 61 | + @doc """ | |
| 62 | + Parse a binary `AndroidManifest.xml`. | |
| 63 | + | |
| 64 | + Returns `{:ok, info}` or `{:error, {status, message}}` shaped for the | |
| 65 | + registry controller. | |
| 66 | + """ | |
| 67 | + @spec parse(binary()) :: {:ok, info()} | {:error, {integer(), String.t()}} | |
| 68 | + def parse(data) when is_binary(data) and byte_size(data) > @max_size, | |
| 69 | + do: {:error, {422, "AndroidManifest.xml is implausibly large"}} | |
| 70 | + | |
| 71 | + def parse(<<@res_xml::little-16, header_size::little-16, size::little-32, _::binary>> = data) | |
| 72 | + when header_size >= 8 do | |
| 73 | + # The declared size may be shorter than the file (trailing padding | |
| 74 | + # is legal); longer means the file is truncated. | |
| 75 | + cond do | |
| 76 | + size > byte_size(data) -> | |
| 77 | + {:error, {422, "AndroidManifest.xml is truncated"}} | |
| 78 | + | |
| 79 | + true -> | |
| 80 | + body = binary_part(data, header_size, size - header_size) | |
| 81 | + | |
| 82 | + with {:ok, pool} <- find_string_pool(body), | |
| 83 | + {:ok, elements} <- collect_elements(body, pool) do | |
| 84 | + {:ok, assemble(elements)} | |
| 85 | + end | |
| 86 | + end | |
| 87 | + end | |
| 88 | + | |
| 89 | + def parse(_), do: {:error, {422, "not a binary AndroidManifest.xml"}} | |
| 90 | + | |
| 91 | + # ── chunk walking ──────────────────────────────────────────────────── | |
| 92 | + | |
| 93 | + # Chunks are `type, headerSize, size` then `size - headerSize` of | |
| 94 | + # payload. A zero or oversized `size` would loop forever or read past | |
| 95 | + # the end, so both stop the walk. | |
| 96 | + defp chunks(<<>>, acc), do: {:ok, Enum.reverse(acc)} | |
| 97 | + | |
| 98 | + defp chunks( | |
| 99 | + <<type::little-16, header_size::little-16, size::little-32, _::binary>> = data, | |
| 100 | + acc | |
| 101 | + ) | |
| 102 | + when size >= 8 and header_size >= 8 and header_size <= size do | |
| 103 | + if size > byte_size(data) do | |
| 104 | + # Trailing garbage after the last well-formed chunk: take what we | |
| 105 | + # have rather than failing the whole manifest. | |
| 106 | + {:ok, Enum.reverse(acc)} | |
| 107 | + else | |
| 108 | + chunk = binary_part(data, 0, size) | |
| 109 | + rest = binary_part(data, size, byte_size(data) - size) | |
| 110 | + chunks(rest, [{type, header_size, chunk} | acc]) | |
| 111 | + end | |
| 112 | + end | |
| 113 | + | |
| 114 | + defp chunks(_trailing, acc), do: {:ok, Enum.reverse(acc)} | |
| 115 | + | |
| 116 | + defp find_string_pool(body) do | |
| 117 | + with {:ok, list} <- chunks(body, []) do | |
| 118 | + case Enum.find(list, fn {type, _, _} -> type == @res_string_pool end) do | |
| 119 | + {_, header_size, chunk} -> parse_string_pool(chunk, header_size) | |
| 120 | + nil -> {:error, {422, "AndroidManifest.xml has no string pool"}} | |
| 121 | + end | |
| 122 | + end | |
| 123 | + end | |
| 124 | + | |
| 125 | + defp collect_elements(body, pool) do | |
| 126 | + with {:ok, list} <- chunks(body, []) do | |
| 127 | + elements = | |
| 128 | + list | |
| 129 | + |> Enum.filter(fn {type, _, _} -> type == @res_xml_start_element end) | |
| 130 | + |> Enum.flat_map(fn {_, header_size, chunk} -> | |
| 131 | + case parse_start_element(chunk, header_size, pool) do | |
| 132 | + {:ok, name, attrs} -> [{name, attrs}] | |
| 133 | + :skip -> [] | |
| 134 | + end | |
| 135 | + end) | |
| 136 | + | |
| 137 | + {:ok, elements} | |
| 138 | + end | |
| 139 | + end | |
| 140 | + | |
| 141 | + # ── string pool ────────────────────────────────────────────────────── | |
| 142 | + | |
| 143 | + defp parse_string_pool(chunk, header_size) do | |
| 144 | + case chunk do | |
| 145 | + <<_::binary-size(8), count::little-32, _styles::little-32, flags::little-32, | |
| 146 | + strings_start::little-32, _styles_start::little-32, _::binary>> -> | |
| 147 | + utf8? = Bitwise.band(flags, @utf8_flag) != 0 | |
| 148 | + | |
| 149 | + with {:ok, offsets} <- pool_offsets(chunk, header_size, count) do | |
| 150 | + data_len = byte_size(chunk) - strings_start | |
| 151 | + | |
| 152 | + if strings_start > byte_size(chunk) or data_len < 0 do | |
| 153 | + {:error, {422, "string pool points outside its chunk"}} | |
| 154 | + else | |
| 155 | + data = binary_part(chunk, strings_start, data_len) | |
| 156 | + | |
| 157 | + strings = | |
| 158 | + offsets | |
| 159 | + |> Enum.map(&read_string(data, &1, utf8?)) | |
| 160 | + |> List.to_tuple() | |
| 161 | + | |
| 162 | + {:ok, strings} | |
| 163 | + end | |
| 164 | + end | |
| 165 | + | |
| 166 | + _ -> | |
| 167 | + {:error, {422, "malformed string pool header"}} | |
| 168 | + end | |
| 169 | + end | |
| 170 | + | |
| 171 | + defp pool_offsets(chunk, header_size, count) do | |
| 172 | + need = count * 4 | |
| 173 | + | |
| 174 | + if header_size + need > byte_size(chunk) do | |
| 175 | + {:error, {422, "string pool offset table is truncated"}} | |
| 176 | + else | |
| 177 | + offsets = | |
| 178 | + for <<off::little-32 <- binary_part(chunk, header_size, need)>>, do: off | |
| 179 | + | |
| 180 | + {:ok, offsets} | |
| 181 | + end | |
| 182 | + end | |
| 183 | + | |
| 184 | + # Out-of-range offsets yield "" rather than failing: one unreadable | |
| 185 | + # string in a pool of eighty shouldn't cost the whole manifest, and | |
| 186 | + # every field we care about is checked for presence afterwards. | |
| 187 | + defp read_string(data, offset, _utf8?) when offset >= byte_size(data), do: "" | |
| 188 | + | |
| 189 | + defp read_string(data, offset, true) do | |
| 190 | + rest = binary_part(data, offset, byte_size(data) - offset) | |
| 191 | + | |
| 192 | + # UTF-8 entries carry two independently-extensible lengths: the | |
| 193 | + # UTF-16 length first, then the byte length. Only the second says | |
| 194 | + # how much to read. | |
| 195 | + with {_utf16_len, rest} <- utf8_len(rest), | |
| 196 | + {byte_len, rest} <- utf8_len(rest), | |
| 197 | + true <- byte_len <= byte_size(rest) do | |
| 198 | + binary_part(rest, 0, byte_len) | |
| 199 | + else | |
| 200 | + _ -> "" | |
| 201 | + end | |
| 202 | + end | |
| 203 | + | |
| 204 | + defp read_string(data, offset, false) do | |
| 205 | + rest = binary_part(data, offset, byte_size(data) - offset) | |
| 206 | + | |
| 207 | + with {len, rest} <- utf16_len(rest), | |
| 208 | + bytes = len * 2, | |
| 209 | + true <- bytes <= byte_size(rest) do | |
| 210 | + rest | |
| 211 | + |> binary_part(0, bytes) | |
| 212 | + |> :unicode.characters_to_binary({:utf16, :little}) | |
| 213 | + |> case do | |
| 214 | + s when is_binary(s) -> s | |
| 215 | + _ -> "" | |
| 216 | + end | |
| 217 | + else | |
| 218 | + _ -> "" | |
| 219 | + end | |
| 220 | + end | |
| 221 | + | |
| 222 | + # A high bit set means the length spans two units, high half first. | |
| 223 | + defp utf8_len(<<hi, lo, rest::binary>>) when Bitwise.band(hi, 0x80) != 0, | |
| 224 | + do: {Bitwise.bsl(Bitwise.band(hi, 0x7F), 8) + lo, rest} | |
| 225 | + | |
| 226 | + defp utf8_len(<<len, rest::binary>>), do: {len, rest} | |
| 227 | + defp utf8_len(_), do: :error | |
| 228 | + | |
| 229 | + defp utf16_len(<<hi::little-16, lo::little-16, rest::binary>>) | |
| 230 | + when Bitwise.band(hi, 0x8000) != 0, | |
| 231 | + do: {Bitwise.bsl(Bitwise.band(hi, 0x7FFF), 16) + lo, rest} | |
| 232 | + | |
| 233 | + defp utf16_len(<<len::little-16, rest::binary>>), do: {len, rest} | |
| 234 | + defp utf16_len(_), do: :error | |
| 235 | + | |
| 236 | + defp pool_at(pool, @absent), do: nil | |
| 237 | + | |
| 238 | + defp pool_at(pool, index) when index < tuple_size(pool), do: elem(pool, index) | |
| 239 | + defp pool_at(_pool, _index), do: nil | |
| 240 | + | |
| 241 | + # ── elements ───────────────────────────────────────────────────────── | |
| 242 | + | |
| 243 | + defp parse_start_element(chunk, header_size, pool) do | |
| 244 | + # `header_size` covers ResChunk_header + ResXMLTree_node; the | |
| 245 | + # attrExt follows it. | |
| 246 | + case chunk do | |
| 247 | + <<_::binary-size(header_size), _ns::little-32, name::little-32, attr_start::little-16, | |
| 248 | + attr_size::little-16, attr_count::little-16, _id::little-16, _class::little-16, | |
| 249 | + _style::little-16, _::binary>> -> | |
| 250 | + case pool_at(pool, name) do | |
| 251 | + nil -> | |
| 252 | + :skip | |
| 253 | + | |
| 254 | + tag -> | |
| 255 | + # `attr_start` is relative to the start of the attrExt, not | |
| 256 | + # to the chunk. | |
| 257 | + base = header_size + attr_start | |
| 258 | + {:ok, tag, read_attrs(chunk, base, attr_size, attr_count, pool, %{})} | |
| 259 | + end | |
| 260 | + | |
| 261 | + _ -> | |
| 262 | + :skip | |
| 263 | + end | |
| 264 | + end | |
| 265 | + | |
| 266 | + defp read_attrs(_chunk, _offset, _size, 0, _pool, acc), do: acc | |
| 267 | + | |
| 268 | + defp read_attrs(chunk, offset, size, count, pool, acc) when size >= 20 do | |
| 269 | + if offset + size > byte_size(chunk) do | |
| 270 | + acc | |
| 271 | + else | |
| 272 | + <<_ns::little-32, name::little-32, raw::little-32, _size::little-16, _res0, data_type, | |
| 273 | + data::little-32, _::binary>> = binary_part(chunk, offset, size) | |
| 274 | + | |
| 275 | + acc = | |
| 276 | + case pool_at(pool, name) do | |
| 277 | + nil -> acc | |
| 278 | + key -> Map.put(acc, key, typed_value(data_type, data, raw, pool)) | |
| 279 | + end | |
| 280 | + | |
| 281 | + read_attrs(chunk, offset + size, size, count - 1, pool, acc) | |
| 282 | + end | |
| 283 | + end | |
| 284 | + | |
| 285 | + defp read_attrs(_chunk, _offset, _size, _count, _pool, acc), do: acc | |
| 286 | + | |
| 287 | + defp typed_value(@type_string, data, raw, pool), | |
| 288 | + do: pool_at(pool, data) || pool_at(pool, raw) || "" | |
| 289 | + | |
| 290 | + defp typed_value(@type_int_boolean, data, _raw, _pool), do: data != 0 | |
| 291 | + defp typed_value(type, data, _raw, _pool) when type in [@type_int_dec, @type_int_hex], do: data | |
| 292 | + | |
| 293 | + # Anything else — references, dimensions, floats — is reported raw. | |
| 294 | + # No field this module promises is one of those, so a caller asking | |
| 295 | + # for `versionCode` never sees it; a caller poking at `label` does, | |
| 296 | + # and gets an integer resource id, which is the truth. | |
| 297 | + defp typed_value(_type, data, _raw, _pool), do: data | |
| 298 | + | |
| 299 | + # ── assembly ───────────────────────────────────────────────────────── | |
| 300 | + | |
| 301 | + defp assemble(elements) do | |
| 302 | + manifest = first_attrs(elements, "manifest") | |
| 303 | + uses_sdk = first_attrs(elements, "uses-sdk") | |
| 304 | + | |
| 305 | + %{ | |
| 306 | + package: string(manifest["package"]), | |
| 307 | + version_code: integer(manifest["versionCode"]), | |
| 308 | + version_name: string(manifest["versionName"]), | |
| 309 | + min_sdk: integer(uses_sdk["minSdkVersion"]), | |
| 310 | + target_sdk: integer(uses_sdk["targetSdkVersion"]), | |
| 311 | + permissions: permissions(elements) | |
| 312 | + } | |
| 313 | + end | |
| 314 | + | |
| 315 | + defp first_attrs(elements, tag) do | |
| 316 | + case Enum.find(elements, fn {name, _} -> name == tag end) do | |
| 317 | + {_, attrs} -> attrs | |
| 318 | + nil -> %{} | |
| 319 | + end | |
| 320 | + end | |
| 321 | + | |
| 322 | + defp permissions(elements) do | |
| 323 | + elements | |
| 324 | + |> Enum.filter(fn {name, _} -> name == "uses-permission" end) | |
| 325 | + |> Enum.map(fn {_, attrs} -> string(attrs["name"]) end) | |
| 326 | + |> Enum.reject(&is_nil/1) | |
| 327 | + |> Enum.uniq() | |
| 328 | + |> Enum.sort() | |
| 329 | + end | |
| 330 | + | |
| 331 | + defp string(value) when is_binary(value), do: if(value == "", do: nil, else: value) | |
| 332 | + defp string(_), do: nil | |
| 333 | + | |
| 334 | + defp integer(value) when is_integer(value), do: value | |
| 335 | + | |
| 336 | + # `versionCode` is an integer attribute, but a hand-built manifest can | |
| 337 | + # carry it as a string and aapt will have kept it as one. | |
| 338 | + defp integer(value) when is_binary(value) do | |
| 339 | + case Integer.parse(value) do | |
| 340 | + {n, ""} -> n | |
| 341 | + _ -> nil | |
| 342 | + end | |
| 343 | + end | |
| 344 | + | |
| 345 | + defp integer(_), do: nil | |
| 346 | +end |
added
test/git_gud/packages/fdroid_manifest_test.exs
+108
−0
@@ -0,0 +1,108 @@
| 1 | +defmodule GitGud.Packages.Fdroid.ManifestTest do | |
| 2 | + @moduledoc """ | |
| 3 | + The binary `AndroidManifest.xml` reader, checked against a real one: | |
| 4 | + `test/support/fixtures/apk/gitgud-AndroidManifest.xml` is lifted | |
| 5 | + verbatim out of this repo's own `app-debug.apk`, so the parser is | |
| 6 | + tested against what AAPT2 actually emits rather than against a | |
| 7 | + fixture written to match the parser. | |
| 8 | + | |
| 9 | + The malformed cases matter as much as the happy path. An APK is | |
| 10 | + attacker-supplied input from the registry's point of view, so every | |
| 11 | + truncation has to come back as an error rather than as a raise or a | |
| 12 | + hang. | |
| 13 | + """ | |
| 14 | + use ExUnit.Case, async: true | |
| 15 | + | |
| 16 | + alias GitGud.Packages.Formats.Fdroid.Manifest | |
| 17 | + | |
| 18 | + @fixture "test/support/fixtures/apk/gitgud-AndroidManifest.xml" | |
| 19 | + | |
| 20 | + defp fixture, do: File.read!(@fixture) | |
| 21 | + | |
| 22 | + describe "a real AAPT2 manifest" do | |
| 23 | + test "reads the package and version" do | |
| 24 | + assert {:ok, info} = Manifest.parse(fixture()) | |
| 25 | + | |
| 26 | + assert info.package == "org.neiam.gitgud.app" | |
| 27 | + assert info.version_code == 1 | |
| 28 | + assert info.version_name == "0.1.0" | |
| 29 | + end | |
| 30 | + | |
| 31 | + test "reads the SDK levels declared in build.gradle.kts" do | |
| 32 | + assert {:ok, info} = Manifest.parse(fixture()) | |
| 33 | + | |
| 34 | + assert info.min_sdk == 26 | |
| 35 | + assert info.target_sdk == 34 | |
| 36 | + end | |
| 37 | + | |
| 38 | + test "reads the permissions the app asks for" do | |
| 39 | + assert {:ok, info} = Manifest.parse(fixture()) | |
| 40 | + | |
| 41 | + assert "android.permission.INTERNET" in info.permissions | |
| 42 | + assert "android.permission.CAMERA" in info.permissions | |
| 43 | + end | |
| 44 | + | |
| 45 | + test "returns permissions sorted and deduplicated" do | |
| 46 | + assert {:ok, info} = Manifest.parse(fixture()) | |
| 47 | + | |
| 48 | + assert info.permissions == Enum.sort(info.permissions) | |
| 49 | + assert info.permissions == Enum.uniq(info.permissions) | |
| 50 | + end | |
| 51 | + end | |
| 52 | + | |
| 53 | + describe "rejecting input" do | |
| 54 | + test "refuses something that isn't a binary manifest" do | |
| 55 | + assert {:error, {422, _}} = Manifest.parse("<?xml version=\"1.0\"?><manifest/>") | |
| 56 | + assert {:error, {422, _}} = Manifest.parse(<<>>) | |
| 57 | + assert {:error, {422, _}} = Manifest.parse(<<0, 1, 2, 3>>) | |
| 58 | + end | |
| 59 | + | |
| 60 | + test "refuses a file whose declared size overruns the data" do | |
| 61 | + <<head::binary-size(4), _size::little-32, rest::binary>> = fixture() | |
| 62 | + lying = <<head::binary, 999_999::little-32, rest::binary>> | |
| 63 | + | |
| 64 | + assert {:error, {422, message}} = Manifest.parse(lying) | |
| 65 | + assert message =~ "truncated" | |
| 66 | + end | |
| 67 | + | |
| 68 | + test "refuses an implausibly large file without parsing it" do | |
| 69 | + assert {:error, {422, message}} = Manifest.parse(:binary.copy(<<0>>, 5 * 1024 * 1024)) | |
| 70 | + assert message =~ "implausibly large" | |
| 71 | + end | |
| 72 | + | |
| 73 | + @tag timeout: 10_000 | |
| 74 | + test "terminates on every truncation of a real manifest" do | |
| 75 | + data = fixture() | |
| 76 | + | |
| 77 | + # A zero-length chunk or one whose size runs past the end are the | |
| 78 | + # two shapes that could loop or read out of bounds; walking every | |
| 79 | + # prefix is the cheap way to be sure neither does. | |
| 80 | + for len <- Enum.take_every(1..byte_size(data), 37) do | |
| 81 | + prefix = binary_part(data, 0, len) | |
| 82 | + | |
| 83 | + case Manifest.parse(prefix) do | |
| 84 | + {:ok, info} -> assert is_map(info) | |
| 85 | + {:error, {422, _}} -> :ok | |
| 86 | + end | |
| 87 | + end | |
| 88 | + end | |
| 89 | + end | |
| 90 | + | |
| 91 | + describe "a manifest with nothing in it" do | |
| 92 | + test "reports nil fields rather than failing" do | |
| 93 | + # A well-formed file chunk with an empty string pool and no | |
| 94 | + # elements: legal, useless, and it must not crash the reader. | |
| 95 | + pool = | |
| 96 | + <<0x0001::little-16, 28::little-16, 28::little-32, 0::little-32, 0::little-32, | |
| 97 | + 0::little-32, 28::little-32, 0::little-32>> | |
| 98 | + | |
| 99 | + size = 8 + byte_size(pool) | |
| 100 | + data = <<0x0003::little-16, 8::little-16, size::little-32, pool::binary>> | |
| 101 | + | |
| 102 | + assert {:ok, info} = Manifest.parse(data) | |
| 103 | + assert info.package == nil | |
| 104 | + assert info.version_code == nil | |
| 105 | + assert info.permissions == [] | |
| 106 | + end | |
| 107 | + end | |
| 108 | +end |
modified
?
+0
−0
Binary file — not rendered.
Parents: ea1f5be