morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #7205] [MERGED] fix(apk): match signer serials by signed integer identity #7715

New issue

Closed

opened 2026-11-03 13:39:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:51 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7205
Author: @rudycelekli
Created: 10/26/2026
Status: ✅ Merged
Merged: 10/27/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix-apk-v1-serial-integer-identity

📝 Commits (10+)

8ced814 fix(browser): unify script and context attribution facts
1f5c5c3 refactor(managed): centralize method implementation flag facts
f6478c0 fix(providers): keep each inspection on one environment snapshot
b4b5c37 fix(web): retain admitted provenance through incomplete traces
741f2de test(ci): align source preservation and catalog assertions
416f16e fix(managed): unify metadata row and coded-index ownership
afc79ad fix(providers): bind signal and capture configuration per operation
df6db78 refactor(pyinstaller): centralize CArchive member roles
99ff434 fix(browser): centralize CDP network event facts
bab49aa refactor(artifacts): unify stable read failure ownership

📊 Changes

1661 files changed (+91988 additions, -18813 deletions)

View changed files

📝 .agents/skills/rea-analysis-optimization/SKILL.md (+5 -0)
📝 .agents/skills/reverse-engineer-anything/SKILL.md (+6 -1)
📝 .agents/skills/reverse-engineer-anything/references/environments.md (+72 -26)
📝 .agents/skills/reverse-engineer-anything/references/javascript-applications.md (+9 -0)
📝 .agents/skills/reverse-engineer-anything/references/native-and-artifacts.md (+14 -0)
📝 .github/workflows/ci.yml (+4 -0)
📝 AGENTS.md (+6 -3)
📝 README.md (+6 -0)
📝 SECURITY.md (+1 -1)
📝 bridge/ghidra/ReaGhidraBridge.java (+51 -34)
📝 docs/agent-environments.md (+23 -8)
📝 docs/browser-scenario-contract.md (+182 -3)
📝 docs/cli.md (+42 -9)
📝 docs/client-configuration.md (+40 -10)
📝 docs/desktop-applications.md (+1 -1)
📝 docs/error-contract.schema.json (+2 -6)
📝 docs/firmware-analysis.md (+1 -1)
📝 docs/ghidra-dos.md (+1 -1)
📝 docs/ida-provider.md (+65 -0)
📝 docs/installation.md (+88 -10)

...and 80 more files

📄 Description

Summary

A v1 APK signer with a redundantly sign-padded serial was left unidentified even when Android apksigner successfully verified the APK. Compare certificate and SignerInfo serials by signed INTEGER value while preserving sign-significant octets (00 80 differs from 80; ff 7f differs from 7f). No exact open issue is linked; this was reported directly as a PR.

Normalize serial comparison views once at each identity-reading boundary, reusing existing request-scoped certificate identities. Ordinary serials return the original view; padded serials use a subview. Raw certificate encoding, hashes and issuer/SKI ownership remain intact. This replaces the proposed normalization inside every certificate×signer comparison. Existing CLI/MCP selection, paging and Evidence contracts share the same implementation; this change does not perform signature verification.

Validation

Started from latest main 1cd82597da9711454415f2702c7e44ea153c4438 after #7204. Original regression on unchanged production: 1 failed / 18 passed.

19 source tests passed, including repeated positive/negative padding on either or both sides, sign-significant negatives and raw certificate byte preservation. Four integration/lifecycle suites passed another 18 tests: CLI/MCP v1 facts, selected views, retained identity and cancellation/cleanup. The first combined run exposed a new test assertion comparing Buffer and Uint8Array representations; corrected to byte comparison, then the 19 source tests passed. Production behavior was unchanged by that assertion fix.
Complete npm run verify:apk-signature passed 23 verifier cases using pinned Android build-tools 36.1.0 / platform 36 and an isolated JDK: actual signing, apksigner/keytool oracles, CLI/MCP parity, schema validation, rotation and tamper controls. 57.746 s elapsed / 82.186 s CPU / 1172.92 MiB sampled process-family RSS, two CPUs; includes build.
Separate real minimal APK with aapt2 binary manifest, SDK v1 EC signature and an owned ephemeral key. Repacked variants change only CMS SignerInfo serial/ancestor lengths: redundant leading 00 still verifies in SDK; changed numerical serial does not. Original certificate DER is unchanged. Before the fix, CLI/MCP report null for the SDK-valid padded signer; afterward both bind SDK certificate 46648d5bb7f21f1395eaa9ca9e5a9556e2b533b7d23ca3657fb321a7b004393e. Canonical input stays matched, negative control stays null, APK SHA-256 remains unchanged by inspection, and outputs retain the NOT-verified limitation. Non-minimal padding is SDK-tolerated input, not a claim of strict DER conformance or Android device execution.
This six-call CLI/MCP task: 9.131 → 9.079 s elapsed, 11.272 → 11.262 s CPU, 682.24 → 678.79 MiB sampled family RSS. One uncontrolled pair establishes no speedup/regression; the padded case has improved results rather than equivalent baseline success. Canonical CLI/MCP replies remain 3532/7447 bytes; extra resolved padded-certificate facts account for its reply growth. Cold CLI / first-MCP phases and later warm MCP calls were measured separately. Model tokens are unmeasured; RSS sums may double-count shared pages and miss short peaks. No new cache, protocol calls, integer-width limit or payload copy.
Serial check:fast: passed in 33.300 s elapsed / 52.198 s CPU, 3600.45 MiB sampled family RSS. Heavy checks use two CPUs and bounded process-family memory.

Hosted CI and review

Final CI 40449672672 tested exact a013de842fcfd7ef164075eeef4d066c192cb92b against main f5edef3c963294245c5334abb9e57cdbea312295: 27 successful, 7 failed, 2 skipped, 7 cancelled jobs. Hosted real-apk-signature, static, build, docs, browser and Linux x64/arm64/Darwin arm64 package baselines passed. Cancelled test shards remain unverified.

Source: 17 failed / 20595 passed / 15 skipped. Exact-main controls reproduce identical 17 Apple inventory/provider-binding/Windows-loader failures (17 failed / 49 passed).
Shard 3: 8 failed / 1919 passed / 17 skipped. Exact-main targeted replay reproduces all 8, together with 6 additional setup/process assertions from the Windows/Apple jobs (14 failed / 1 passed / 197 filtered or platform-skipped). These concern client registration, CI-health fixture, Java empty-bitness detail, native plist and retained process-root reporting; none invokes the changed APK serial decoder.
Apple job: 2 failures; Windows job: 16. The loader/Java and five client-environment assertions also reproduce in Linux controls. Mac-only process CLI and Windows-only Kimi/home-fallback cases were not natively replayed; home fallback passes on Linux. Their unchanged upstream source/fixtures establish scope, not a native-platform baseline pass.
Node 22 SQLite: two unguarded matrix/filter fixtures expect the unavailable native limits API; Node 24/26 lanes pass. Owner and fixtures are byte-identical at starting main, this head and CI main. This is source-derived scope evidence, not an executed Node 22 baseline. Portable/required jobs aggregate those existing-main failures.

Greptile reviewed final a013de842 at 3/5 with one existing-main browser secret-save finding. Producer/consumer inspection and identical-blob scope explanation: resolved outside this three-file APK change, not fixed or runtime-reproduced. No actionable APK finding was raised. Codex could not review because repository quota is exhausted. Original human declarations remain unchecked; requester explicitly authorized ready status and unrelated-main CI/readiness bypass.

Author readiness

Human owner: @rudycelekli

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/7205 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/26/2026 **Status:** ✅ Merged **Merged:** 10/27/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix-apk-v1-serial-integer-identity` --- ### 📝 Commits (10+) - [`8ced814`](https://github.com/morluto/rea/commit/8ced814b96bc78e936905c02ffabf9240f5ad146) fix(browser): unify script and context attribution facts - [`1f5c5c3`](https://github.com/morluto/rea/commit/1f5c5c31c1d1017f7b895bdd8cc38247a50005f6) refactor(managed): centralize method implementation flag facts - [`f6478c0`](https://github.com/morluto/rea/commit/f6478c056f7bd4dcc8ab951352f165a6f4653c9c) fix(providers): keep each inspection on one environment snapshot - [`b4b5c37`](https://github.com/morluto/rea/commit/b4b5c37ab3de1e0f8f4369dcad26fd6ceb18aa85) fix(web): retain admitted provenance through incomplete traces - [`741f2de`](https://github.com/morluto/rea/commit/741f2de7517d97d26a7cd84cf1045532963eda84) test(ci): align source preservation and catalog assertions - [`416f16e`](https://github.com/morluto/rea/commit/416f16e2ede8b31ec3bbb8d02efbf8c62074c8aa) fix(managed): unify metadata row and coded-index ownership - [`afc79ad`](https://github.com/morluto/rea/commit/afc79add7e6e88419b723782a22fddb568064368) fix(providers): bind signal and capture configuration per operation - [`df6db78`](https://github.com/morluto/rea/commit/df6db781ddae6a85a370591b3c4f17e140029d87) refactor(pyinstaller): centralize CArchive member roles - [`99ff434`](https://github.com/morluto/rea/commit/99ff4344c0702afad782cd26178d50b18fa6ee4b) fix(browser): centralize CDP network event facts - [`bab49aa`](https://github.com/morluto/rea/commit/bab49aaf7c1ffb791bc8ca423bffb42f6b0d5560) refactor(artifacts): unify stable read failure ownership ### 📊 Changes **1661 files changed** (+91988 additions, -18813 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/rea-analysis-optimization/SKILL.md` (+5 -0) 📝 `.agents/skills/reverse-engineer-anything/SKILL.md` (+6 -1) 📝 `.agents/skills/reverse-engineer-anything/references/environments.md` (+72 -26) 📝 `.agents/skills/reverse-engineer-anything/references/javascript-applications.md` (+9 -0) 📝 `.agents/skills/reverse-engineer-anything/references/native-and-artifacts.md` (+14 -0) 📝 `.github/workflows/ci.yml` (+4 -0) 📝 `AGENTS.md` (+6 -3) 📝 `README.md` (+6 -0) 📝 `SECURITY.md` (+1 -1) 📝 `bridge/ghidra/ReaGhidraBridge.java` (+51 -34) 📝 `docs/agent-environments.md` (+23 -8) 📝 `docs/browser-scenario-contract.md` (+182 -3) 📝 `docs/cli.md` (+42 -9) 📝 `docs/client-configuration.md` (+40 -10) 📝 `docs/desktop-applications.md` (+1 -1) 📝 `docs/error-contract.schema.json` (+2 -6) 📝 `docs/firmware-analysis.md` (+1 -1) 📝 `docs/ghidra-dos.md` (+1 -1) 📝 `docs/ida-provider.md` (+65 -0) 📝 `docs/installation.md` (+88 -10) _...and 80 more files_ </details> ### 📄 Description ## Summary A v1 APK signer with a redundantly sign-padded serial was left unidentified even when Android apksigner successfully verified the APK. Compare certificate and SignerInfo serials by signed INTEGER value while preserving sign-significant octets (`00 80` differs from `80`; `ff 7f` differs from `7f`). No exact open issue is linked; this was reported directly as a PR. Normalize serial comparison views once at each identity-reading boundary, reusing existing request-scoped certificate identities. Ordinary serials return the original view; padded serials use a subview. Raw certificate encoding, hashes and issuer/SKI ownership remain intact. This replaces the proposed normalization inside every certificate×signer comparison. Existing CLI/MCP selection, paging and Evidence contracts share the same implementation; this change does not perform signature verification. ## Validation Started from latest main `1cd82597da9711454415f2702c7e44ea153c4438` after #7204. Original regression on unchanged production: 1 failed / 18 passed. - 19 source tests passed, including repeated positive/negative padding on either or both sides, sign-significant negatives and raw certificate byte preservation. Four integration/lifecycle suites passed another 18 tests: CLI/MCP v1 facts, selected views, retained identity and cancellation/cleanup. The first combined run exposed a new test assertion comparing Buffer and Uint8Array representations; corrected to byte comparison, then the 19 source tests passed. Production behavior was unchanged by that assertion fix. - Complete `npm run verify:apk-signature` passed **23 verifier cases** using pinned Android build-tools 36.1.0 / platform 36 and an isolated JDK: actual signing, apksigner/keytool oracles, CLI/MCP parity, schema validation, rotation and tamper controls. 57.746 s elapsed / 82.186 s CPU / 1172.92 MiB sampled process-family RSS, two CPUs; includes build. - Separate real minimal APK with aapt2 binary manifest, SDK v1 EC signature and an owned ephemeral key. Repacked variants change only CMS SignerInfo serial/ancestor lengths: redundant leading `00` still verifies in SDK; changed numerical serial does not. Original certificate DER is unchanged. Before the fix, CLI/MCP report null for the SDK-valid padded signer; afterward both bind SDK certificate `46648d5bb7f21f1395eaa9ca9e5a9556e2b533b7d23ca3657fb321a7b004393e`. Canonical input stays matched, negative control stays null, APK SHA-256 remains unchanged by inspection, and outputs retain the NOT-verified limitation. Non-minimal padding is SDK-tolerated input, not a claim of strict DER conformance or Android device execution. - This six-call CLI/MCP task: 9.131 → 9.079 s elapsed, 11.272 → 11.262 s CPU, 682.24 → 678.79 MiB sampled family RSS. One uncontrolled pair establishes no speedup/regression; the padded case has improved results rather than equivalent baseline success. Canonical CLI/MCP replies remain 3532/7447 bytes; extra resolved padded-certificate facts account for its reply growth. Cold CLI / first-MCP phases and later warm MCP calls were measured separately. Model tokens are unmeasured; RSS sums may double-count shared pages and miss short peaks. No new cache, protocol calls, integer-width limit or payload copy. - Serial `check:fast`: passed in 33.300 s elapsed / 52.198 s CPU, 3600.45 MiB sampled family RSS. Heavy checks use two CPUs and bounded process-family memory. ## Hosted CI and review [Final CI 40449672672](https://github.com/morluto/rea/actions/runs/40449672672) tested exact `a013de842fcfd7ef164075eeef4d066c192cb92b` against main `f5edef3c963294245c5334abb9e57cdbea312295`: **27 successful, 7 failed, 2 skipped, 7 cancelled jobs**. Hosted real-apk-signature, static, build, docs, browser and Linux x64/arm64/Darwin arm64 package baselines passed. Cancelled test shards remain unverified. - Source: 17 failed / 20595 passed / 15 skipped. Exact-main controls reproduce identical 17 Apple inventory/provider-binding/Windows-loader failures (17 failed / 49 passed). - Shard 3: 8 failed / 1919 passed / 17 skipped. Exact-main targeted replay reproduces all 8, together with 6 additional setup/process assertions from the Windows/Apple jobs (14 failed / 1 passed / 197 filtered or platform-skipped). These concern client registration, CI-health fixture, Java empty-bitness detail, native plist and retained process-root reporting; none invokes the changed APK serial decoder. - Apple job: 2 failures; Windows job: 16. The loader/Java and five client-environment assertions also reproduce in Linux controls. Mac-only process CLI and Windows-only Kimi/home-fallback cases were not natively replayed; home fallback passes on Linux. Their unchanged upstream source/fixtures establish scope, not a native-platform baseline pass. - Node 22 SQLite: two unguarded matrix/filter fixtures expect the unavailable native limits API; Node 24/26 lanes pass. Owner and fixtures are byte-identical at starting main, this head and CI main. This is source-derived scope evidence, not an executed Node 22 baseline. Portable/required jobs aggregate those existing-main failures. Greptile reviewed final `a013de842` at 3/5 with one existing-main browser secret-save finding. [Producer/consumer inspection and identical-blob scope explanation](https://github.com/morluto/rea/pull/7205#discussion_r4385120256): resolved outside this three-file APK change, **not fixed or runtime-reproduced**. No actionable APK finding was raised. Codex could not review because repository quota is exhausted. Original human declarations remain unchecked; requester explicitly authorized ready status and unrelated-main CI/readiness bypass. ## Author readiness Human owner: @rudycelekli - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-03 13:39:51 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7715

No description provided.

Rows
Columns