diff --git a/CHANGELOG.md b/CHANGELOG.md index d310c9b..fd79ed8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,35 @@ ### Fixed +- **Plugin signature verification now runs before `Code.eval_file`** on the + manifest (MOB-74). The v1 signature covered the eval'd manifest map, so + verifiers needed the eval to run first to rebuild the payload — letting + a malicious `priv/mob_plugin.exs` execute arbitrary code on the + consumer's machine during plugin activation. + + Envelope v2 puts the signed `file_hashes` list on disk alongside the + signature; verification runs entirely off the envelope and the file + bytes, no eval required. `priv/mob_plugin.exs` is one of those + file_hashes, so tampering with the manifest bytes shifts its hash and + fails verification. `MobDev.Plugin.Verify.load_verified/1` is the new + safe consumer path: verify → then eval. `MobDev.Plugin.activated/0`, + `mix mob.plugins`, `mix mob.audit_plugins`, and `MobDev.Plugin.Report` + all migrated. + + **v1 envelopes are refused** — accepting them silently reopens the + bug. The error is a distinguished `:envelope_v1_unsupported` reason + and the `SignatureGate` message tells the author to re-sign with a + mob_dev that produces envelope v2 (`mix mob.plugin.sign`). Every + plugin signed with a mob_dev that predates this change must be + re-signed on the consumer's next build. + + Follow-up tickets on file for the walk to data-only manifests + (safe-by-construction): **MOB-185** (`mix mob.plugin.lint`), + **MOB-186** (migrate first-party plugins to the pure-data subset), + **MOB-187** (static JSON/TOML manifest at 1.0). + + See `decisions/2026-09-11-plugin-envelope-v2-verify-before-eval.md`. + - **Play upload keystore no longer bakes a trailing newline into the stored password** (MOB-71). `Mix.shell().prompt/1` returns the whole input line including the trailing `\n`, and `MobDev.GooglePlay.SetupWizard.generate_keystore/2` diff --git a/decisions/2026-09-11-plugin-envelope-v2-verify-before-eval.md b/decisions/2026-09-11-plugin-envelope-v2-verify-before-eval.md new file mode 100644 index 0000000..f07812b --- /dev/null +++ b/decisions/2026-09-11-plugin-envelope-v2-verify-before-eval.md @@ -0,0 +1,110 @@ +# Plugin envelope v2 — verify signature before eval + +- Date: 2026-09-11 +- Status: accepted + +## Context + +Before this change, `MobDev.Plugin.Manifest.load/1` (which is called from +`MobDev.Plugin.activated/0` on every `mix mob.deploy`, and from +`mix mob.plugins` / `mix mob.audit_plugins` / `MobDev.Plugin.Report`) +called `Code.eval_file("priv/mob_plugin.exs")` **before** any signature +check. The signature check that ran afterwards (`SignatureGate.check_plugin/4` +→ `Verify.verify_plugin(dir, manifest)`) covered the *eval'd manifest map* +plus a list of referenced sources — but NOT the manifest bytes themselves. +So a malicious plugin could: + +```elixir +# priv/mob_plugin.exs +File.write!("/tmp/pwned", "evil ran") # side effect +%{name: :innocent, mob_version: "~> 0.7", ...} +``` + +`Code.eval_file/1` would execute the side effect on the consumer's +machine, then return the innocent-looking map. The signature covered the +map (which was untouched) so `Verify` reported clean, and the CVE was +never surfaced. + +Filed as MOB-74 in the July 2026 audit. + +## Decision + +Move to envelope v2, which flips the trust chain end-to-end: + +1. **The manifest bytes join the signed file_hashes list.** `Sign.referenced_files/2` + now always prepends `priv/mob_plugin.exs` — the file that used to be + trusted implicitly is now cryptographically covered like every other + source. +2. **The v2 envelope on disk carries the file_hashes list alongside the + signature.** Payload signed = `%{file_hashes: [...], envelope_version: 2}`. + The eval'd manifest map is no longer part of the payload — its + authoritative representation is the on-disk bytes, whose hash lives + in file_hashes. +3. **`Verify.verify_plugin/1` no longer takes a manifest arg.** It reads + the envelope, rebuilds the payload from the envelope's own file_hashes + (proving the author signed *this* list of files), then re-hashes each + listed file on disk (proving the disk hasn't been tampered with since + signing). Neither step calls `Code.eval_file/1`. +4. **`Verify.load_verified/1`** is the new consumer entry point. Verifies + first; only calls `Manifest.load/1` (which does the eval) after + verification passes. This is the actual MOB-74 close: a plugin that + fails verification never has its `.exs` bytes evaluated on the + consumer's machine. +5. **v1 envelopes are refused.** They can't verify without the eval'd + manifest to rebuild their payload, so accepting them silently reopens + the CVE. The refusal is a distinguished `:envelope_v1_unsupported` + error with an actionable re-sign hint. + +## Consequences + +- **Breaking change for signed plugins.** Every plugin published with a + mob_dev ≤ 0.7.1 signature must be re-signed with mob_dev 0.7.2+. + There is no fallback path; the whole point of the fix is that the + fallback path was the bug. First-party plugins get re-signed as part + of the 0.7.2 release; third-party authors will see the + `:envelope_v1_unsupported` error and follow the re-sign hint in the + message. See [`MOB_PLUGIN_SECURITY.md`](../MOB_PLUGIN_SECURITY.md). +- **Consumer paths that eval a manifest** now go through + `MobDev.Plugin.Verify.load_verified/1` in the four places that + matter to build- or activation-time: `MobDev.Plugin.activated/0`, + `mix mob.plugins`, `mix mob.audit_plugins`, `MobDev.Plugin.Report`. +- **Author-side paths still eval before verify** on purpose — + `mix mob.plugin.sign` and `mix mob.plugin.keygen` operate on plugins + the author owns and is about to sign; there is no attack vector. + `mix mob.plugin.trust` is the first-trust decision by definition; + the user is explicitly reviewing a plugin they don't trust yet. + `mix mob.validate_plugin` runs as an author-side lint pass + (`priv/mob_plugin.exs` shape checks against `Manifest.validate/1`); + no attack vector for the author's own plugin. All four are noted as + separate follow-ups (MOB-185/186/187 lay out the longer-term path + to data-only manifests, which closes the class entirely). +- **`SignatureGate.check_activated/1` tier-0 detection** switched from + "`is_map(manifest)`" (the old proxy) to + "`Manifest.manifest_present?(dir)`". This is necessary because a + failed-verification tier-1 plugin now also has `manifest == nil` — + distinguishing the two by manifest-file presence surfaces the + friendly error for the failed case instead of silently skipping. +- **`Verify.load_verified/2` accepts `acknowledged_unsafe: true`** as + the second-arg option, and `MobDev.Plugin.activated_with_verify/0` + passes it for plugins in `:acknowledge_unsafe_plugins`. Without + this, an acknowledged unsigned plugin's `{:error, :missing_signature}` + would strip its manifest map from the build even though + `SignatureGate.check_plugin/4` still lets the plugin through + (`if name in acknowledged, do: :ok`). Downstream `Merge`, + `AndroidBootstrap`, `RuntimeManifest` all filter on + `is_map(manifest)` — the acknowledged plugin would appear activated + but contribute nothing: no NIFs, no gradle deps, no permissions. The + opt-in escape hatch is scoped to `:missing_signature` only; every + other verify failure (`:invalid_signature`, `:missing_pubkey`, + `:envelope_v1_unsupported`) still refuses the eval because those + are tamper / mis-key / attack signals, not "unsigned during dev". +- **UX regression when the manifest name isn't derivable from a nil + manifest** — the gate falls back to `Path.basename(dir)` for the error + label. In real deployments this equals the Hex package name; in tests + it's a temp-dir basename. Acceptable — the alternative is a bigger + return-shape refactor to thread the dep name through every layer. +- **Follow-up tickets on file:** MOB-185 (`mix mob.plugin.lint`), + MOB-186 (migrate first-party plugins to the pure-data subset the + linter accepts), MOB-187 (static JSON/TOML manifest at 1.0). Together + they walk the manifest format from "safe if verified" (this ADR) + toward "safe by construction" (MOB-187 endstate). diff --git a/lib/mix/tasks/mob.audit_plugins.ex b/lib/mix/tasks/mob.audit_plugins.ex index 1228744..560d268 100644 --- a/lib/mix/tasks/mob.audit_plugins.ex +++ b/lib/mix/tasks/mob.audit_plugins.ex @@ -39,7 +39,7 @@ defmodule Mix.Tasks.Mob.AuditPlugins do """ alias MobDev.Plugin - alias MobDev.Plugin.{Audit, Manifest, Report} + alias MobDev.Plugin.{Audit, Report} @switches [plugin: :string, accept_medium: :boolean] @@ -88,7 +88,7 @@ defmodule Mix.Tasks.Mob.AuditPlugins do for name <- Plugin.activated_names(), dir = deps[name], not is_nil(dir) do manifest = - case Manifest.load(dir) do + case MobDev.Plugin.Verify.load_verified(dir) do {:ok, m} -> m {:error, _} -> nil end diff --git a/lib/mix/tasks/mob.plugin.sign.ex b/lib/mix/tasks/mob.plugin.sign.ex index af90b37..2299a23 100644 --- a/lib/mix/tasks/mob.plugin.sign.ex +++ b/lib/mix/tasks/mob.plugin.sign.ex @@ -5,8 +5,13 @@ defmodule Mix.Tasks.Mob.Plugin.Sign do @moduledoc """ Signs the plugin in `` (default: cwd) and writes - `priv/mob_plugin.sig`. The signature covers the loaded manifest - plus SHA-256 hashes of every file the manifest references. + `priv/mob_plugin.sig`. The signature covers the SHA-256 hashes of + `priv/mob_plugin.exs` and every source file the manifest references — + the manifest bytes are one of those files, so tampering with the + manifest fails verification. The v2 envelope on disk carries the + signed `file_hashes` list so `MobDev.Plugin.Verify.verify_plugin/1` + can check integrity without ever `Code.eval_file`-ing the manifest + (see MOB-74). mix mob.plugin.sign [--plugin ] diff --git a/lib/mix/tasks/mob.plugins.ex b/lib/mix/tasks/mob.plugins.ex index 5d3abe0..5a853ac 100644 --- a/lib/mix/tasks/mob.plugins.ex +++ b/lib/mix/tasks/mob.plugins.ex @@ -20,7 +20,7 @@ defmodule Mix.Tasks.Mob.Plugins do *activated* — only then are its contributions merged into the build. """ - alias MobDev.Plugin.{Manifest, Report, Validator} + alias MobDev.Plugin.{Report, Validator} @impl Mix.Task def run(_args) do @@ -56,7 +56,7 @@ defmodule Mix.Tasks.Mob.Plugins do defp load_all_manifests do Mix.Project.deps_paths() |> Enum.map(fn {app, path} -> - case Manifest.load(path) do + case MobDev.Plugin.Verify.load_verified(path) do {:ok, manifest} -> {app, manifest} diff --git a/lib/mob_dev/plugin.ex b/lib/mob_dev/plugin.ex index e4a9cef..1609baf 100644 --- a/lib/mob_dev/plugin.ex +++ b/lib/mob_dev/plugin.ex @@ -104,17 +104,72 @@ defmodule MobDev.Plugin do `MobDev.Plugin.Merge`. Resolves each activated name to its dependency directory and loads its - manifest (nil for a tier-0 plugin). Activated names that don't resolve to a - dep are skipped — `mix mob.plugins` is where that mismatch surfaces to users. + manifest via `MobDev.Plugin.Verify.load_verified/1` — signature and + file-integrity checks run **before** `Code.eval_file` (MOB-74), so a + plugin that fails verification never has its `priv/mob_plugin.exs` + executed. Failed plugins come back as `{dir, nil}` here; callers that + need to distinguish "tier-0 plugin (no manifest)" from "verify failed" + should use `activated_with_verify/0` — that's the shape + `SignatureGate.check_activated/1` consumes to produce friendly + build-blocking errors. Activated names that don't resolve to a dep + are skipped — `mix mob.plugins` is where that mismatch surfaces + to users. """ @spec activated() :: [{Path.t(), map() | nil}] def activated do + for {dir, manifest, _status} <- activated_with_verify(), do: {dir, manifest} + end + + @typedoc """ + Verification status returned from `activated_with_verify/0`. `:ok` means the + manifest was loaded after a passing signature + tamper check; `:unsigned` + means there was no manifest at all (tier-0 plugin — no signature required); + `{:error, reason}` means the plugin failed verification and its manifest + bytes were never eval'd. Reasons come from `MobDev.Plugin.Verify.verify_error/0`. + """ + @type verify_status :: + :ok + | :unsigned + | {:error, MobDev.Plugin.Verify.verify_error()} + + @typedoc "One entry in `activated_with_verify/0`'s return list." + @type activated_entry :: {Path.t(), map() | nil, verify_status()} + + @doc """ + Same as `activated/0` but also returns the verification status per plugin. + + `SignatureGate.check_activated/1` consumes this shape so it can produce a + clear build-blocking error naming the failed plugin — without needing to + re-load or re-verify anything. A plugin that failed verification appears as + `{dir, nil, {:error, reason}}`; a tier-0 plugin (no `priv/mob_plugin.exs` + and no signature required) appears as `{dir, nil, :unsigned}`. + """ + @spec activated_with_verify() :: [activated_entry()] + def activated_with_verify do deps = Mix.Project.deps_paths() + acknowledged = MobDev.Plugin.SignatureGate.acknowledged_unsafe() for name <- activated_names(), dir = deps[name], not is_nil(dir) do - case MobDev.Plugin.Manifest.load(dir) do - {:ok, manifest} -> {dir, manifest} - {:error, _reason} -> {dir, nil} + # Threading acknowledged-unsafe here matters. Without it, an unsigned + # plugin the user has explicitly opted into via + # :acknowledge_unsafe_plugins would get :missing_signature from + # load_verified/2 and its manifest map would be dropped — silently + # stripping the plugin's NIFs, gradle deps, swift files, and + # permissions from the build. `SignatureGate.check_plugin/4` would + # still let it through as :ok (that's what "acknowledged" means), + # but downstream Merge/AndroidBootstrap/RuntimeManifest filter on + # is_map(manifest) and skip the nil. Result: a build that ships + # nothing from the plugin, no error message. See MOB-74 pre-merge + # review. Every OTHER verify failure (:invalid_signature, + # :missing_pubkey, :envelope_v1_unsupported) still refuses the eval + # — those are tamper/mis-key signals, not the "unsigned during dev" + # case. + opts = if name in acknowledged, do: [acknowledged_unsafe: true], else: [] + + case MobDev.Plugin.Verify.load_verified(dir, opts) do + {:ok, nil} -> {dir, nil, :unsigned} + {:ok, manifest} -> {dir, manifest, :ok} + {:error, reason} -> {dir, nil, {:error, reason}} end end end diff --git a/lib/mob_dev/plugin/manifest.ex b/lib/mob_dev/plugin/manifest.ex index aadb023..67ae56a 100644 --- a/lib/mob_dev/plugin/manifest.ex +++ b/lib/mob_dev/plugin/manifest.ex @@ -67,6 +67,17 @@ defmodule MobDev.Plugin.Manifest do end end + @doc """ + Returns `true` when `plugin_dir` has a `priv/mob_plugin.exs`, `false` + otherwise. Distinguishes tier-0 plugins (no manifest, no signature + needed) from tier-1+ plugins (manifest present, signature required) + without evaluating the manifest itself. See `Verify.load_verified/1`. + """ + @spec manifest_present?(Path.t()) :: boolean() + def manifest_present?(plugin_dir) do + File.exists?(Path.join(plugin_dir, @manifest_path)) + end + defp eval(path) do case Code.eval_file(path) do {map, _bindings} when is_map(map) -> diff --git a/lib/mob_dev/plugin/report.ex b/lib/mob_dev/plugin/report.ex index 39708f8..504706c 100644 --- a/lib/mob_dev/plugin/report.ex +++ b/lib/mob_dev/plugin/report.ex @@ -134,8 +134,12 @@ defmodule MobDev.Plugin.Report do defp find_row_name(rows, name), do: Enum.find_value(rows, name, &(&1.name == name && name)) defp load_manifest(dir) do - case MobDev.Plugin.Manifest.load(dir) do - {:ok, manifest} -> manifest + # Verified load per MOB-74 — refuse to eval a manifest whose signature or + # file-hashes don't check out. Report-only path; a plugin that fails + # verification simply shows up with no capability data instead of running + # its code inside our process. + case MobDev.Plugin.Verify.load_verified(dir) do + {:ok, manifest} when is_map(manifest) -> manifest _ -> %{} end end diff --git a/lib/mob_dev/plugin/sign.ex b/lib/mob_dev/plugin/sign.ex index ece9030..fb44cec 100644 --- a/lib/mob_dev/plugin/sign.ex +++ b/lib/mob_dev/plugin/sign.ex @@ -6,18 +6,39 @@ defmodule MobDev.Plugin.Sign do 1. Loading the manifest (`priv/mob_plugin.exs`). 2. Computing SHA-256 hashes for every file the manifest references - (Swift sources, Android bridge/JNI sources, NIF native_dir contents). - 3. Building the canonical payload (manifest + sorted file hashes). + (Swift sources, Android bridge/JNI sources, NIF native_dir contents, + **and the manifest bytes themselves**). + 3. Building the canonical payload (sorted file hashes + envelope + version). 4. Signing the canonical encoding of the payload via `Crypto.sign/2`. - 5. Writing a binary `priv/mob_plugin.sig` containing the signature. + 5. Writing a binary `priv/mob_plugin.sig` containing the signature + **and the signed file_hashes list**, so verifiers can check + integrity without needing to `Code.eval_file` the manifest first + (see MOB-74). Pure helpers are exposed for tests: `compute_file_hashes/2` and - `build_payload/2` are deterministic given their inputs. + `build_payload/1` are deterministic given their inputs. + + ## Envelope versions + + - **v1** (deprecated, MOB-74) — payload was `%{manifest: , + file_hashes: [...]}` and the envelope on disk carried only the + signature. Verifiers needed the eval'd manifest map to rebuild the + payload, so the eval had to run *before* verification could — + letting a malicious `priv/mob_plugin.exs` execute arbitrary code + at build time. Refused by `MobDev.Plugin.Verify` since mob_dev + 0.7.2. + - **v2** (current) — payload is `%{file_hashes: [...], + envelope_version: 2}`; `file_hashes` includes + `priv/mob_plugin.exs`; envelope on disk embeds `file_hashes` + alongside the signature. Verifiers can check every on-disk file + against the signed hashes without touching the manifest map, so + verification is safe to run before eval. """ alias MobDev.Plugin.{Crypto, Manifest} - @envelope_version 1 + @envelope_version 2 @signature_file "priv/mob_plugin.sig" @manifest_file "priv/mob_plugin.exs" @@ -77,18 +98,22 @@ defmodule MobDev.Plugin.Sign do Shape: %{ - manifest: , file_hashes: [{rel_path, sha256}, ...], - envelope_version: 1 + envelope_version: 2 } Authoritative for what's inside the signature — any new field added here needs both author and host updates. + + Note that the payload no longer includes the manifest term itself + (see MOB-74). The manifest is one of the files hashed in + `file_hashes`, so its bytes are covered — and dropping the map from + the payload lets `Verify` recompute the payload without eval'ing + the manifest first. """ - @spec build_payload(map() | nil, file_hashes()) :: map() - def build_payload(manifest, file_hashes) do + @spec build_payload(file_hashes()) :: map() + def build_payload(file_hashes) do %{ - manifest: manifest, file_hashes: file_hashes, envelope_version: @envelope_version } @@ -105,20 +130,31 @@ defmodule MobDev.Plugin.Sign do @doc """ Signs `plugin_dir` and writes `priv/mob_plugin.sig`. - Orchestrates the full author workflow: loads the manifest, computes - file hashes, builds the payload, signs it, wraps the signature in the - envelope binary, and writes the file. Returns `:ok` on success or - `{:error, reason}` if the manifest is missing/invalid. + Orchestrates the full author workflow: loads the manifest (to know + which files it references), computes file hashes (including the + manifest bytes themselves), builds the v2 payload, signs it, wraps + the signature **and the file_hashes list** in the envelope binary, + and writes the file. Returns `:ok` on success or `{:error, reason}` + if the manifest is missing/invalid. + + The envelope carries `file_hashes` on disk so `Verify.verify_plugin/1` + can check tampering without needing to `Code.eval_file` the manifest + first — see MOB-74. """ @spec sign_plugin(Path.t(), Crypto.priv_key()) :: :ok | {:error, term()} def sign_plugin(plugin_dir, priv_key) when is_binary(priv_key) do with {:ok, manifest} <- Manifest.load(plugin_dir), :ok <- refuse_if_no_manifest(manifest, plugin_dir) do file_hashes = compute_file_hashes(plugin_dir, manifest) - payload = build_payload(manifest, file_hashes) + payload = build_payload(file_hashes) signature = Crypto.sign(payload, priv_key) - envelope = %{signature: signature, envelope_version: @envelope_version} + envelope = %{ + signature: signature, + file_hashes: file_hashes, + envelope_version: @envelope_version + } + sig_path = Path.join(plugin_dir, @signature_file) File.mkdir_p!(Path.dirname(sig_path)) File.write!(sig_path, Crypto.canonical_encode(envelope)) @@ -161,7 +197,11 @@ defmodule MobDev.Plugin.Sign do nifs = nif_files(manifest, plugin_dir) - swift ++ android ++ res ++ nifs + # The manifest itself MUST be signed — otherwise `Verify` has no way to + # detect a tampered `priv/mob_plugin.exs` without eval'ing it first, + # which is the whole class of bug MOB-74 closes. Always included, always + # at a stable relative path so `Verify` can look it up by name. + [@manifest_file | swift ++ android ++ res ++ nifs] end defp nif_files(manifest, plugin_dir) do @@ -193,7 +233,13 @@ defmodule MobDev.Plugin.Sign do for s <- List.wrap(value), is_binary(s), do: s end - defp sha256!(path) do + @doc false + # Exposed so `Verify` can re-hash files on disk without duplicating the + # missing-file convention (missing file hashes as the SHA-256 of empty + # bytes so a tamper-check comparing declared vs actual notices the + # difference). + @spec sha256!(Path.t()) :: file_hash() + def sha256!(path) do case File.read(path) do {:ok, bytes} -> :crypto.hash(:sha256, bytes) {:error, _} -> :crypto.hash(:sha256, <<>>) diff --git a/lib/mob_dev/plugin/signature_gate.ex b/lib/mob_dev/plugin/signature_gate.ex index fe4bacd..b49e4dc 100644 --- a/lib/mob_dev/plugin/signature_gate.ex +++ b/lib/mob_dev/plugin/signature_gate.ex @@ -24,13 +24,14 @@ defmodule MobDev.Plugin.SignatureGate do user must run `mix mob.plugin.trust `. """ - alias MobDev.Plugin.{Crypto, TrustStore, Verify} + alias MobDev.Plugin.{Crypto, Manifest, TrustStore, Verify} @typedoc "Errors `check_plugin/2` can return." @type gate_error :: {:missing_signature, atom()} | {:missing_pubkey, atom()} | {:invalid_signature, atom()} + | {:envelope_v1_unsupported, atom()} | {:untrusted, atom(), Crypto.fingerprint(), Crypto.fingerprint() | nil} @doc """ @@ -56,9 +57,21 @@ defmodule MobDev.Plugin.SignatureGate do @spec check_activated([{Path.t(), map() | nil}], TrustStore.trust_map(), [atom()]) :: :ok | {:error, [gate_error()]} def check_activated(plugins, trust_map, acknowledged) do + # A `nil` manifest can mean two things after MOB-74: + # + # * tier-0 plugin (no `priv/mob_plugin.exs` at all — nothing to sign, + # nothing to verify); skip. + # * tier-1+ plugin whose signature verification failed, so + # `Verify.load_verified/1` refused to eval the manifest; the gate + # must still surface the failure with a friendly name-and-reason + # error, otherwise a tampered plugin silently gets treated like a + # tier-0 one. + # + # `Manifest.manifest_present?/1` distinguishes the two without eval'ing + # anything. errors = for {dir, manifest} <- plugins, - is_map(manifest), + Manifest.manifest_present?(dir), err = check_plugin(dir, manifest, trust_map, acknowledged), err != :ok do err @@ -130,11 +143,16 @@ defmodule MobDev.Plugin.SignatureGate do @doc false # Public for tests: checks a single plugin against the trust map and # acknowledgement list. Returns `:ok` on pass, a gate_error otherwise. - @spec check_plugin(Path.t(), map(), TrustStore.trust_map(), [atom()]) :: :ok | gate_error() + # `manifest` may be `nil` when `Verify.load_verified/1` refused to eval a + # plugin whose signature check failed; the error surface still needs a + # name, so we fall back to the dep-directory basename (which matches the + # published plugin name by convention). + @spec check_plugin(Path.t(), map() | nil, TrustStore.trust_map(), [atom()]) :: + :ok | gate_error() def check_plugin(dir, manifest, trust_map, acknowledged) do - name = manifest[:name] + name = manifest_name(dir, manifest) - case Verify.verify_plugin(dir, manifest) do + case Verify.verify_plugin(dir) do :ok -> check_trust(dir, name, trust_map) @@ -146,9 +164,23 @@ defmodule MobDev.Plugin.SignatureGate do {:error, :invalid_signature} -> {:invalid_signature, name} + + {:error, :envelope_v1_unsupported} -> + {:envelope_v1_unsupported, name} end end + defp manifest_name(_dir, manifest) when is_map(manifest), do: manifest[:name] + + # `String.to_atom` on unbounded input can exhaust the atom table, but the + # domain here is the deps-directory basename — one entry per Hex dep in + # `Mix.Project.deps_paths()`, a small set the consumer controls at + # dependency-declaration time. No attacker-controlled path reaches this + # helper. + defp manifest_name(dir, nil) do + dir |> Path.basename() |> String.to_atom() + end + defp check_trust(dir, name, trust_map) do case Verify.load_pubkey(dir) do {:ok, pub} -> @@ -166,7 +198,19 @@ defmodule MobDev.Plugin.SignatureGate do end end - defp acknowledged_unsafe do + @doc """ + The list of plugin names the consumer has opted into loading unsigned via + `:acknowledge_unsafe_plugins` (in `Application` env or `mob.exs`). + + Exposed so `MobDev.Plugin.activated/0` can pass + `acknowledged_unsafe: true` into `Verify.load_verified/2` for these + plugins — otherwise a missing signature would silently strip the plugin + from the build (its manifest fields would never merge into the app), + producing "acknowledged" plugins that actually contribute nothing. + See MOB-74's pre-merge review. + """ + @spec acknowledged_unsafe() :: [atom()] + def acknowledged_unsafe do Application.get_env(:mob, :acknowledge_unsafe_plugins, []) ++ read_acknowledged_from_mob_exs() end @@ -215,6 +259,14 @@ defmodule MobDev.Plugin.SignatureGate do " tampering with the plugin's manifest or source files." end + defp format_error({:envelope_v1_unsupported, name}) do + " - plugin #{inspect(name)} ships a v1 signature envelope (MOB-74).\n" <> + " v1 required evaluating the manifest before verifying it, which\n" <> + " let a malicious priv/mob_plugin.exs run arbitrary code at build\n" <> + " time. Refused by this mob_dev. Ask the plugin author to re-sign\n" <> + " with a mob_dev that produces envelope v2 (`mix mob.plugin.sign`)." + end + defp format_error({:untrusted, name, actual_fp, nil}) do " - plugin #{inspect(name)} is signed with key\n" <> " #{actual_fp}\n" <> diff --git a/lib/mob_dev/plugin/verify.ex b/lib/mob_dev/plugin/verify.ex index 989ab5e..0b3c683 100644 --- a/lib/mob_dev/plugin/verify.ex +++ b/lib/mob_dev/plugin/verify.ex @@ -2,88 +2,126 @@ defmodule MobDev.Plugin.Verify do @moduledoc """ Host-side signature verification for activated mob plugins. - Given a plugin directory + its loaded manifest, this module: + Given a plugin directory, this module: 1. Loads `priv/mob_plugin.sig` (the signed envelope). 2. Loads `priv/mob_plugin.pub` (the plugin author's public key). - 3. Recomputes the file-hash list via `Sign.compute_file_hashes/2`. - 4. Reconstructs the canonical payload and runs `Crypto.verify/3`. + 3. Rebuilds the canonical payload from the envelope's embedded + `file_hashes` list and runs `Crypto.verify/3` — proving the + envelope on disk came from the author. + 4. Re-hashes each file the envelope declares and compares against the + signed hash — proving the on-disk state has not been tampered with + since the author signed it. + + Because the envelope carries `file_hashes` on disk (v2, see + `Sign` moduledoc for the version history), verification does not + require the eval'd manifest map — so the safe order is + **verify → then eval**. This closes the MOB-74 class of bug where a + malicious `priv/mob_plugin.exs` could execute arbitrary code during + plugin activation because the eval ran before the signature check. Failure modes are distinguished: - `:missing_signature` — no `priv/mob_plugin.sig`. - `:missing_pubkey` — no `priv/mob_plugin.pub`. - - `:invalid_signature` — sig file present but the signature doesn't - verify against the canonical payload reconstructed from disk. This - is the failure mode for both manifest tampering and source-file - tampering: the recomputed `file_hashes` no longer match what was - signed, so the payload differs and the signature check fails. + - `:invalid_signature` — sig present but doesn't verify, or on-disk + files no longer match the signed hashes (tamper detected), or the + envelope is malformed. + - `:envelope_v1_unsupported` — a legacy v1 envelope was found. v1 + verification required the eval'd manifest to rebuild the payload, + which is the very bug we are closing. Author must re-sign with + `mix mob.plugin.sign` on mob_dev 0.7.2 or later. Trust (mapping a verified public key to "the host operator approved it") lives in `TrustStore` and is layered on top of this module. """ - alias MobDev.Plugin.{Crypto, Sign} + alias MobDev.Plugin.{Crypto, Manifest, Sign} @signature_file "priv/mob_plugin.sig" @pubkey_file "priv/mob_plugin.pub" - # Atom keys that appear in the signed envelope term (see `Sign.sign_plugin/2`). - # `load_signature/1` decodes the envelope with `binary_to_term(_, [:safe])`, - # which refuses to *create* atoms — every atom in the encoded term must - # already exist in the runtime atom table or the decode raises `badarg` and a - # valid signature is misreported as `:corrupt`. `Verify` matches `:signature` - # directly, but nothing here references `:envelope_version`; only `Sign` did. - # Because `verify_plugin/2` calls `load_signature/1` *before* it ever touches - # `Sign`, decoding succeeded or failed depending on whether `Sign` happened to - # be loaded earlier in the BEAM — a load-order-dependent intermittent - # "invalid signature" across builds. Naming the atoms in this module-level - # literal interns them at `Verify`-load (guaranteed before any decode), making - # the decode deterministic while keeping `:safe` (sig files are - # attacker-controlled). See decisions/2026-05-31-verify-safe-atom-intern.md. - @envelope_atoms [:signature, :envelope_version] - - @typedoc "Errors `load_signature/1` can return." - @type sig_error :: :missing | :corrupt + # Atom keys that appear in the v2 signed envelope term (see + # `Sign.sign_plugin/2`). `load_envelope/1` decodes the envelope with + # `binary_to_term(_, [:safe])`, which refuses to *create* atoms — every atom + # in the encoded term must already exist in the runtime atom table or the + # decode raises `badarg` and a valid signature is misreported as `:corrupt`. + # Naming the atoms in this module-level literal interns them at `Verify`-load + # (guaranteed before any decode), making the decode deterministic while + # keeping `:safe` (sig files are attacker-controlled). See + # decisions/2026-05-31-verify-safe-atom-intern.md. + @envelope_atoms [:signature, :envelope_version, :file_hashes] + + @typedoc "Errors `load_envelope/1` can return." + @type envelope_error :: :missing | :corrupt | :envelope_v1_unsupported @typedoc "Errors `load_pubkey/1` can return." @type pubkey_error :: :missing | :malformed - @typedoc "Errors `verify_plugin/2` can return." - @type verify_error :: :missing_signature | :missing_pubkey | :invalid_signature + @typedoc "Errors `verify_plugin/1` can return." + @type verify_error :: + :missing_signature + | :missing_pubkey + | :invalid_signature + | :envelope_v1_unsupported + + @typedoc "A decoded v2 envelope." + @type envelope :: %{ + signature: Crypto.signature(), + file_hashes: Sign.file_hashes(), + envelope_version: 2 + } @doc """ - Loads the raw 64-byte signature from `priv/mob_plugin.sig`. + Loads and decodes the signature envelope from `priv/mob_plugin.sig`. - The file is the `Crypto.canonical_encode/1` of an envelope map - (`%{signature: <64-byte sig>, envelope_version: 1}`); this function - decodes the envelope and returns the inner signature binary. + Returns the full envelope map (v2 shape) on success, or a distinguished + error. A v1 envelope on disk is reported as `:envelope_v1_unsupported` so + the caller can print a re-sign hint — v1 required the eval'd manifest to + verify, which is the bug MOB-74 closes. """ - @spec load_signature(Path.t()) :: {:ok, Crypto.signature()} | {:error, sig_error()} - def load_signature(plugin_dir) do + @spec load_envelope(Path.t()) :: {:ok, envelope()} | {:error, envelope_error()} + def load_envelope(plugin_dir) do path = Path.join(plugin_dir, @signature_file) case File.read(path) do - {:ok, bytes} -> decode_signature_envelope(bytes) + {:ok, bytes} -> decode_envelope(bytes) {:error, :enoent} -> {:error, :missing} {:error, _} -> {:error, :corrupt} end end - defp decode_signature_envelope(bytes) do - {:ok, decode_envelope_term!(bytes)} - rescue - _ -> {:error, :corrupt} - end - - defp decode_envelope_term!(bytes) do + defp decode_envelope(bytes) do # Touch the literal so the envelope atoms are guaranteed interned before the # :safe decode runs (see @envelope_atoms above). _ = @envelope_atoms case :erlang.binary_to_term(bytes, [:safe]) do - %{signature: sig} when is_binary(sig) and byte_size(sig) == 64 -> sig - _ -> raise "corrupt" + %{signature: sig, file_hashes: fh, envelope_version: 2} + when is_binary(sig) and byte_size(sig) == 64 and is_list(fh) -> + {:ok, %{signature: sig, file_hashes: fh, envelope_version: 2}} + + %{signature: sig, envelope_version: 1} when is_binary(sig) and byte_size(sig) == 64 -> + {:error, :envelope_v1_unsupported} + + _ -> + {:error, :corrupt} + end + rescue + _ -> {:error, :corrupt} + end + + @doc """ + Back-compat shim for `mob 0.8.x` and the `SignatureGate.maybe_print_unsafe_banner/1` + path, both of which used the v1 helper that returned just the raw + signature. Now returns the same 64-byte signature but extracted from a v2 + envelope. Callers that need the full envelope should use `load_envelope/1`. + """ + @spec load_signature(Path.t()) :: {:ok, Crypto.signature()} | {:error, envelope_error()} + def load_signature(plugin_dir) do + case load_envelope(plugin_dir) do + {:ok, %{signature: sig}} -> {:ok, sig} + {:error, r} -> {:error, r} end end @@ -121,32 +159,114 @@ defmodule MobDev.Plugin.Verify do end @doc """ - Verifies that the plugin in `plugin_dir` has a valid signature for the - given `manifest` + the current file contents on disk. + Verifies that the plugin in `plugin_dir` has a valid v2 signature and + that the on-disk files match what the author signed. + + The check runs entirely off the envelope's embedded `file_hashes` list + — the manifest is one of those files, and rehashing its bytes on disk + detects tampering without any `Code.eval_file` call. That is the + MOB-74 fix: verification is now safe to run *before* eval, closing the + RCE window where a malicious `priv/mob_plugin.exs` could execute + arbitrary code during plugin activation. Returns `:ok` on success or one of the distinguished error reasons (see `t:verify_error/0`). The caller is responsible for any trust decision; this function only proves that the bytes on disk match what the plugin author signed. """ - @spec verify_plugin(Path.t(), map() | nil) :: :ok | {:error, verify_error()} - def verify_plugin(plugin_dir, manifest) do - with {:ok, signature} <- need(load_signature(plugin_dir), :missing_signature), - {:ok, pub} <- need(load_pubkey(plugin_dir), :missing_pubkey), - file_hashes = Sign.compute_file_hashes(plugin_dir, manifest), - payload = Sign.build_payload(manifest, file_hashes), - :ok <- normalise_verify(Crypto.verify(payload, signature, pub)) do + @spec verify_plugin(Path.t()) :: :ok | {:error, verify_error()} + def verify_plugin(plugin_dir) do + with {:ok, envelope} <- normalise_envelope_error(load_envelope(plugin_dir)), + {:ok, pub} <- normalise_pubkey_error(load_pubkey(plugin_dir)), + :ok <- check_files_match(plugin_dir, envelope.file_hashes), + payload = Sign.build_payload(envelope.file_hashes), + :ok <- normalise_verify(Crypto.verify(payload, envelope.signature, pub)) do :ok end end - # Both load_signature and load_pubkey return :missing for a missing file; - # other errors (:corrupt, :malformed) collapse into :invalid_signature - # because they all mean "the bytes that should certify this plugin are - # not usable". - defp need({:ok, value}, _missing_reason), do: {:ok, value} - defp need({:error, :missing}, missing_reason), do: {:error, missing_reason} - defp need({:error, _}, _missing_reason), do: {:error, :invalid_signature} + @doc """ + Verifies the plugin, then loads and evaluates the manifest. + + This is the safe consumer-side path: if `verify_plugin/1` refuses, + `Manifest.load/1` is never called and `Code.eval_file/1` on the + potentially-malicious `priv/mob_plugin.exs` never runs. + + ## Options + + - `:acknowledged_unsafe` (default `false`) — when `true`, an + unsigned plugin (`{:error, :missing_signature}`) is loaded anyway. + This is the documented escape hatch for + `:acknowledge_unsafe_plugins` (see `SignatureGate.check_plugin/4`) + — the user has explicitly opted into running an unsigned plugin's + manifest, and the `SignatureGate` banner already warns them. Every + OTHER failure (`:invalid_signature`, `:missing_pubkey`, + `:envelope_v1_unsupported`) still refuses the eval — those are + the tamper / mis-key cases, not the "unsigned during dev" case. + + For plugins with no `priv/mob_plugin.exs` at all (tier-0 plugins), + returns `{:ok, nil}` without requiring a signature — the + `acknowledged_unsafe` flag has no effect here (no manifest to load). + """ + @spec load_verified(Path.t(), keyword()) :: + {:ok, map() | nil} | {:error, verify_error() | String.t()} + def load_verified(plugin_dir, opts \\ []) do + acknowledged_unsafe? = Keyword.get(opts, :acknowledged_unsafe, false) + + case Manifest.manifest_present?(plugin_dir) do + false -> + {:ok, nil} + + true -> + case verify_plugin(plugin_dir) do + :ok -> + Manifest.load(plugin_dir) + + {:error, :missing_signature} when acknowledged_unsafe? -> + # Documented escape hatch: user opted into an unsigned plugin + # via :acknowledge_unsafe_plugins. Signed but tampered / wrong + # pubkey / v1 envelope still refuse — they're the actual + # attack signals, not the "haven't signed yet during dev" one. + Manifest.load(plugin_dir) + + {:error, _} = err -> + err + end + end + end + + # Rehash each declared file on disk and compare against the signed hash. + # A single mismatch (or missing file) collapses to :invalid_signature — + # the tamper check and the signature check share the same error surface + # because either failure means "the bytes that should certify this plugin + # are not what's on disk right now". + defp check_files_match(plugin_dir, expected_hashes) do + mismatch = + Enum.find(expected_hashes, fn {rel, expected} -> + actual = Sign.sha256!(Path.join(plugin_dir, rel)) + actual != expected + end) + + if mismatch, do: {:error, :invalid_signature}, else: :ok + end + + # Envelope errors that mean "no sig file at all" map to :missing_signature. + # A v1 envelope propagates as its own distinct error so the caller can print + # an actionable re-sign message. Everything else (corrupt, malformed) + # collapses to :invalid_signature. + defp normalise_envelope_error({:ok, envelope}), do: {:ok, envelope} + defp normalise_envelope_error({:error, :missing}), do: {:error, :missing_signature} + + defp normalise_envelope_error({:error, :envelope_v1_unsupported}), + do: {:error, :envelope_v1_unsupported} + + defp normalise_envelope_error({:error, _}), do: {:error, :invalid_signature} + + # Pubkey errors: :missing bubbles as :missing_pubkey; malformed → invalid_signature + # because a garbage .pub file is functionally as bad as a bad signature. + defp normalise_pubkey_error({:ok, pub}), do: {:ok, pub} + defp normalise_pubkey_error({:error, :missing}), do: {:error, :missing_pubkey} + defp normalise_pubkey_error({:error, _}), do: {:error, :invalid_signature} defp normalise_verify(:ok), do: :ok defp normalise_verify({:error, :invalid_signature}), do: {:error, :invalid_signature} diff --git a/test/mix/tasks/mob_plugin_sign_test.exs b/test/mix/tasks/mob_plugin_sign_test.exs index 1ef8815..fb35caf 100644 --- a/test/mix/tasks/mob_plugin_sign_test.exs +++ b/test/mix/tasks/mob_plugin_sign_test.exs @@ -1,7 +1,7 @@ defmodule Mix.Tasks.Mob.Plugin.SignTest do use ExUnit.Case, async: false - alias MobDev.Plugin.{Manifest, Sign, Verify} + alias MobDev.Plugin.{Sign, Verify} setup do tmp_home = @@ -37,8 +37,9 @@ defmodule Mix.Tasks.Mob.Plugin.SignTest do assert File.exists?(Sign.signature_path(dir)) - {:ok, manifest} = Manifest.load(dir) - assert :ok = Verify.verify_plugin(dir, manifest) + # verify_plugin/1 (MOB-74): no manifest arg needed — the envelope carries + # the signed file_hashes list on disk so verification runs off the bytes. + assert :ok = Verify.verify_plugin(dir) end test "errors when no keygen has been run for the plugin", %{plugin_dir: dir} do diff --git a/test/mob_dev/plugin/sign_test.exs b/test/mob_dev/plugin/sign_test.exs index a2c519c..cce508d 100644 --- a/test/mob_dev/plugin/sign_test.exs +++ b/test/mob_dev/plugin/sign_test.exs @@ -1,7 +1,7 @@ defmodule MobDev.Plugin.SignTest do use ExUnit.Case, async: true - alias MobDev.Plugin.{Crypto, Manifest, Sign, Verify} + alias MobDev.Plugin.{Crypto, Sign, Verify} setup do dir = @@ -28,12 +28,20 @@ defmodule MobDev.Plugin.SignTest do assert Sign.compute_file_hashes(dir, nil) == [] end - test "returns [] for a manifest with no referenced files", %{dir: dir} do + test "returns [priv/mob_plugin.exs] for a manifest with no other referenced files", + %{dir: dir} do + # After MOB-74 the manifest bytes themselves are always in the signed + # file_hashes list — even a manifest that declares no sources still + # has ONE hashed file (itself). Revert-verify: remove the + # @manifest_file prefix in `Sign.referenced_files/2` and this fails. manifest = %{name: :mob_x, mob_version: "~> 0.6", plugin_spec_version: 1} - assert Sign.compute_file_hashes(dir, manifest) == [] + write_manifest(dir, manifest) + hashes = Sign.compute_file_hashes(dir, manifest) + assert Enum.map(hashes, &elem(&1, 0)) == ["priv/mob_plugin.exs"] end - test "hashes ios.swift_files and android paths, sorted by path", %{dir: dir} do + test "hashes ios.swift_files and android paths, sorted by path, plus the manifest", + %{dir: dir} do write_file(dir, "ios/A.swift", "a contents") write_file(dir, "ios/B.swift", "b contents") write_file(dir, "android/Bridge.kt", "kt contents") @@ -47,6 +55,7 @@ defmodule MobDev.Plugin.SignTest do android: %{bridge_kt: "android/Bridge.kt", jni_source: "android/jni/Plugin.cpp"} } + write_manifest(dir, manifest) hashes = Sign.compute_file_hashes(dir, manifest) paths = Enum.map(hashes, &elem(&1, 0)) assert paths == Enum.sort(paths) @@ -55,7 +64,8 @@ defmodule MobDev.Plugin.SignTest do "android/Bridge.kt", "android/jni/Plugin.cpp", "ios/A.swift", - "ios/B.swift" + "ios/B.swift", + "priv/mob_plugin.exs" ] end @@ -72,6 +82,7 @@ defmodule MobDev.Plugin.SignTest do } } + write_manifest(dir, manifest) paths = Sign.compute_file_hashes(dir, manifest) |> Enum.map(&elem(&1, 0)) assert "android/res/xml/svc.xml" in paths assert "android/res/values/strings.xml" in paths @@ -90,6 +101,9 @@ defmodule MobDev.Plugin.SignTest do m2 = put_in(m1, [:ios, :swift_files], ["ios/B.swift", "ios/A.swift"]) + # Same manifest bytes on disk (only the map arg differs), so the manifest + # hash contribution is identical for both calls. + write_manifest(dir, m1) assert Sign.compute_file_hashes(dir, m1) == Sign.compute_file_hashes(dir, m2) end @@ -106,6 +120,7 @@ defmodule MobDev.Plugin.SignTest do nifs: [%{module: :mob_x_nif, native_dir: "priv/native"}] } + write_manifest(dir, manifest) paths = manifest |> (&Sign.compute_file_hashes(dir, &1)).() |> Enum.map(&elem(&1, 0)) assert "priv/native/n.c" in paths assert "priv/native/nested/n.h" in paths @@ -123,26 +138,52 @@ defmodule MobDev.Plugin.SignTest do ios: %{swift_files: ["ios/A.swift"]} } - [{_, h1}] = Sign.compute_file_hashes(dir, manifest) + write_manifest(dir, manifest) + hashes_v1 = Sign.compute_file_hashes(dir, manifest) + [{_, h1}] = Enum.filter(hashes_v1, &(elem(&1, 0) == "ios/A.swift")) write_file(dir, "ios/A.swift", "version 2") - [{_, h2}] = Sign.compute_file_hashes(dir, manifest) + hashes_v2 = Sign.compute_file_hashes(dir, manifest) + [{_, h2}] = Enum.filter(hashes_v2, &(elem(&1, 0) == "ios/A.swift")) assert h1 != h2 end + + test "different manifest bytes produce different hashes for priv/mob_plugin.exs", + %{dir: dir} do + # The MOB-74 fix: the manifest bytes are covered by the signed + # file_hashes, so changing a comment or reordering keys in + # `priv/mob_plugin.exs` shifts the hash and breaks verification + # (that's the tamper-detection guarantee). + manifest = %{name: :mob_x, mob_version: "~> 0.6", plugin_spec_version: 1} + + File.write!(Path.join(dir, "priv/mob_plugin.exs"), "# original\n" <> inspect(manifest)) + [{_, hash_a}] = Sign.compute_file_hashes(dir, manifest) + + File.write!(Path.join(dir, "priv/mob_plugin.exs"), "# tampered\n" <> inspect(manifest)) + [{_, hash_b}] = Sign.compute_file_hashes(dir, manifest) + + assert hash_a != hash_b + end end - describe "build_payload/2" do - test "wraps manifest + file_hashes in envelope_version: 1" do - payload = Sign.build_payload(%{name: :mob_x}, [{"a", <<1, 2, 3>>}]) - assert payload.manifest == %{name: :mob_x} + describe "build_payload/1" do + test "wraps file_hashes in envelope_version: 2 (no manifest field)" do + payload = Sign.build_payload([{"a", <<1, 2, 3>>}]) assert payload.file_hashes == [{"a", <<1, 2, 3>>}] - assert payload.envelope_version == 1 + assert payload.envelope_version == 2 + + # MOB-74: v2 payload no longer includes the eval'd manifest map. + # Its bytes are covered indirectly via the file_hashes entry for + # `priv/mob_plugin.exs`, and dropping the map lets `Verify` rebuild + # the payload without eval'ing the manifest first. + refute Map.has_key?(payload, :manifest) end end describe "sign_plugin/2" do - test "writes priv/mob_plugin.sig that Verify.verify_plugin accepts", %{dir: dir} do + test "writes a v2 priv/mob_plugin.sig that Verify.verify_plugin/1 accepts", + %{dir: dir} do manifest = %{name: :mob_demo, mob_version: "~> 0.6", plugin_spec_version: 1} write_manifest(dir, manifest) @@ -152,8 +193,34 @@ defmodule MobDev.Plugin.SignTest do assert :ok = Sign.sign_plugin(dir, priv) assert File.exists?(Sign.signature_path(dir)) - {:ok, loaded_manifest} = Manifest.load(dir) - assert :ok = Verify.verify_plugin(dir, loaded_manifest) + # Verify never touches the manifest map — it works purely off the + # envelope's embedded file_hashes list and the on-disk file bytes. + # See MOB-74. + assert :ok = Verify.verify_plugin(dir) + end + + test "the on-disk envelope carries signature, file_hashes, and envelope_version: 2", + %{dir: dir} do + # Structural regression guard: earlier the envelope carried only the + # signature, so verify needed the eval'd manifest to rebuild the + # payload. If someone tries to shrink the envelope back, verify goes + # back to eval-before-verify and MOB-74 reopens. + manifest = %{name: :mob_demo, mob_version: "~> 0.6", plugin_spec_version: 1} + write_manifest(dir, manifest) + + {priv, pub} = Crypto.generate_keypair() + File.write!(Path.join(dir, "priv/mob_plugin.pub"), Base.encode64(pub) <> "\n") + + :ok = Sign.sign_plugin(dir, priv) + + raw = File.read!(Sign.signature_path(dir)) + envelope = :erlang.binary_to_term(raw, [:safe]) + + assert envelope.envelope_version == 2 + assert is_binary(envelope.signature) and byte_size(envelope.signature) == 64 + + paths = Enum.map(envelope.file_hashes, &elem(&1, 0)) + assert "priv/mob_plugin.exs" in paths end test "errors when no manifest is present", %{dir: dir} do diff --git a/test/mob_dev/plugin/signature_gate_test.exs b/test/mob_dev/plugin/signature_gate_test.exs index 2c43ea6..c574f05 100644 --- a/test/mob_dev/plugin/signature_gate_test.exs +++ b/test/mob_dev/plugin/signature_gate_test.exs @@ -103,8 +103,49 @@ defmodule MobDev.Plugin.SignatureGateTest do SignatureGate.check_activated([{dir, manifest}], %{}, []) end - test "skips tier-0 (nil-manifest) plugins", %{dir: dir} do - assert SignatureGate.check_activated([{dir, nil}], %{}, []) == :ok + test "skips tier-0 plugins — no priv/mob_plugin.exs on disk means no signature required" do + # A tier-0 plugin is one that never publishes a manifest file. After + # MOB-74 the gate distinguishes tier-0 (nothing to verify) from a + # tier-1 plugin whose sig failed (manifest present but eval refused) + # by testing the presence of priv/mob_plugin.exs — not by + # nil-manifest, which now also happens for failed-verification + # plugins whose manifest bytes the gate must NOT skip. + tier0_dir = + Path.join( + System.tmp_dir!(), + "mob_sig_gate_tier0_#{System.unique_integer([:positive])}" + ) + + File.mkdir_p!(tier0_dir) + on_exit(fn -> File.rm_rf!(tier0_dir) end) + + assert SignatureGate.check_activated([{tier0_dir, nil}], %{}, []) == :ok + end + + test "surfaces a friendly error for a tier-1 plugin whose signature failed to verify", + %{dir: dir} do + # Regression guard for the MOB-74 tightening: after `activated/0` moved + # to `Verify.load_verified/1`, a plugin that fails verification comes + # back as `{dir, nil}`. If the gate were still using the old + # `is_map(manifest)` filter, it would silently skip the plugin and + # the build would proceed as if it were tier-0. Instead the gate + # must detect that priv/mob_plugin.exs is on disk and produce a + # named error. + # + # We simulate the failed-verification case by breaking the signature + # file (any tamper works; we just need `Verify.verify_plugin/1` to + # refuse). + File.write!(Sign.signature_path(dir), "garbage") + + # With a nil manifest the gate can't read `manifest[:name]`, so it falls + # back to the deps-directory basename — which matches the Hex package + # name in real deployments (`deps/mob_demo`) but is the temp-dir + # basename here. Assert the shape and reason; the name atom must exist + # (i.e. it was derived from the dir, not left as nil). + assert {:error, [{:invalid_signature, name}]} = + SignatureGate.check_activated([{dir, nil}], %{}, []) + + assert is_atom(name) and not is_nil(name) end end end diff --git a/test/mob_dev/plugin/verify_test.exs b/test/mob_dev/plugin/verify_test.exs index 42daaf4..b48cf54 100644 --- a/test/mob_dev/plugin/verify_test.exs +++ b/test/mob_dev/plugin/verify_test.exs @@ -30,40 +30,61 @@ defmodule MobDev.Plugin.VerifyTest do {:ok, dir: dir, manifest: manifest, pub: pub, priv: priv} end - describe "load_signature/1" do - test "loads the raw 64-byte signature", %{dir: dir} do - assert {:ok, sig} = Verify.load_signature(dir) - assert byte_size(sig) == 64 + describe "load_envelope/1" do + test "loads the full v2 envelope with signature + file_hashes", %{dir: dir} do + assert {:ok, envelope} = Verify.load_envelope(dir) + assert byte_size(envelope.signature) == 64 + assert envelope.envelope_version == 2 + + paths = Enum.map(envelope.file_hashes, &elem(&1, 0)) + assert "priv/mob_plugin.exs" in paths + assert "ios/Demo.swift" in paths end test "returns :missing when the sig file is absent", %{dir: dir} do File.rm!(Sign.signature_path(dir)) - assert {:error, :missing} = Verify.load_signature(dir) + assert {:error, :missing} = Verify.load_envelope(dir) end test "returns :corrupt when the sig file has garbage", %{dir: dir} do File.write!(Sign.signature_path(dir), "garbage") - assert {:error, :corrupt} = Verify.load_signature(dir) + assert {:error, :corrupt} = Verify.load_envelope(dir) + end + + test "refuses a v1 envelope with :envelope_v1_unsupported so the caller can print a re-sign hint", + %{dir: dir} do + # A v1 envelope on disk carried only {signature, envelope_version: 1}. + # v1 verification required the eval'd manifest to rebuild the payload, + # which is the MOB-74 bug. Accepting v1 would silently reopen it. + # Revert-verify: change the guard to also accept envelope_version: 1 + # and this fails. + v1_bytes = + Crypto.canonical_encode(%{ + signature: :binary.copy(<<0>>, 64), + envelope_version: 1 + }) + + File.write!(Sign.signature_path(dir), v1_bytes) + assert {:error, :envelope_v1_unsupported} = Verify.load_envelope(dir) end # Regression: the envelope is decoded with binary_to_term(_, [:safe]), which - # will not *create* atoms. The envelope contains :envelope_version, an atom - # Verify must intern at load time (via @envelope_atoms) — otherwise, in any - # BEAM where Sign (the only other interner) hadn't loaded yet, the :safe - # decode raised and a *valid* signature was misreported as :corrupt. That - # load-order dependence made the build signature gate intermittently reject - # good plugins. The true cold-VM repro is cross-process (atoms can't be - # un-interned in a live VM); these guard the fix's mechanism in-process. - # See decisions/2026-05-31-verify-safe-atom-intern.md. + # will not *create* atoms. The envelope contains :envelope_version and + # :file_hashes, atoms Verify must intern at load time (via @envelope_atoms) + # — otherwise, in any BEAM where Sign (the only other interner) hadn't + # loaded yet, the :safe decode raised and a *valid* signature was + # misreported as :corrupt. That load-order dependence made the build + # signature gate intermittently reject good plugins. See + # decisions/2026-05-31-verify-safe-atom-intern.md. test "interns the envelope atoms at module load (safe-decode guard)" do assert :signature in Verify.envelope_atoms() assert :envelope_version in Verify.envelope_atoms() + assert :file_hashes in Verify.envelope_atoms() end + end - test "decodes an envelope whose term includes the :envelope_version key", - %{dir: dir} do - raw = File.read!(Sign.signature_path(dir)) - assert %{signature: _, envelope_version: 1} = :erlang.binary_to_term(raw, [:safe]) + describe "load_signature/1 (back-compat shim)" do + test "still returns the 64-byte signature from a v2 envelope", %{dir: dir} do assert {:ok, sig} = Verify.load_signature(dir) assert byte_size(sig) == 64 end @@ -91,40 +112,163 @@ defmodule MobDev.Plugin.VerifyTest do end end - describe "verify_plugin/2" do - test "accepts a freshly-signed plugin", %{dir: dir, manifest: manifest} do - assert :ok = Verify.verify_plugin(dir, manifest) + describe "verify_plugin/1" do + test "accepts a freshly-signed plugin without needing the eval'd manifest", %{dir: dir} do + assert :ok = Verify.verify_plugin(dir) end - test "rejects when a referenced source file is tampered", %{dir: dir, manifest: manifest} do + test "rejects when a referenced source file is tampered", %{dir: dir} do File.write!(Path.join(dir, "ios/Demo.swift"), "import SwiftUI // EVIL\n") - assert {:error, :invalid_signature} = Verify.verify_plugin(dir, manifest) + assert {:error, :invalid_signature} = Verify.verify_plugin(dir) end - test "rejects when the manifest is altered after signing", %{dir: dir, manifest: manifest} do - tampered = put_in(manifest, [:ios, :swift_files], ["ios/Other.swift"]) - assert {:error, :invalid_signature} = Verify.verify_plugin(dir, tampered) + test "rejects when the manifest BYTES are tampered — the MOB-74 CVE class", %{dir: dir} do + # Before MOB-74: the signature covered the eval'd manifest map, so a + # tampered priv/mob_plugin.exs that eval'd to the same map (e.g. added + # a side-effecting expression before the returning map literal) would + # verify clean. Now the file bytes are in file_hashes; any change to + # the .exs bytes shifts the hash and fails verification. + # + # Revert-verify: remove the @manifest_file prefix from + # `Sign.referenced_files/2` and this fails (verification would pass + # because the tampered map, if it eval'd to the same shape, wouldn't + # be caught). + original = File.read!(Path.join(dir, "priv/mob_plugin.exs")) + File.write!(Path.join(dir, "priv/mob_plugin.exs"), original <> "\n# tampered\n") + assert {:error, :invalid_signature} = Verify.verify_plugin(dir) end - test "rejects when the signature is missing", %{dir: dir, manifest: manifest} do + test "rejects when the signature is missing", %{dir: dir} do File.rm!(Sign.signature_path(dir)) - assert {:error, :missing_signature} = Verify.verify_plugin(dir, manifest) + assert {:error, :missing_signature} = Verify.verify_plugin(dir) end - test "rejects when the pubkey is missing", %{dir: dir, manifest: manifest} do + test "rejects when the pubkey is missing", %{dir: dir} do File.rm!(Path.join(dir, "priv/mob_plugin.pub")) - assert {:error, :missing_pubkey} = Verify.verify_plugin(dir, manifest) + assert {:error, :missing_pubkey} = Verify.verify_plugin(dir) end - test "rejects when the pubkey doesn't match the signing key", %{dir: dir, manifest: manifest} do + test "rejects when the pubkey doesn't match the signing key", %{dir: dir} do {_other_priv, other_pub} = Crypto.generate_keypair() File.write!(Path.join(dir, "priv/mob_plugin.pub"), Base.encode64(other_pub) <> "\n") - assert {:error, :invalid_signature} = Verify.verify_plugin(dir, manifest) + assert {:error, :invalid_signature} = Verify.verify_plugin(dir) + end + + test "rejects a v1 envelope with a distinguished error", %{dir: dir} do + v1_bytes = + Crypto.canonical_encode(%{ + signature: :binary.copy(<<0>>, 64), + envelope_version: 1 + }) + + File.write!(Sign.signature_path(dir), v1_bytes) + assert {:error, :envelope_v1_unsupported} = Verify.verify_plugin(dir) + end + end + + describe "load_verified/1 — the safe consumer path (MOB-74)" do + test "verifies the plugin first, then evals the manifest", %{dir: dir, manifest: manifest} do + assert {:ok, loaded} = Verify.load_verified(dir) + assert loaded[:name] == manifest.name + end + + test "returns {:ok, nil} for a tier-0 plugin (no priv/mob_plugin.exs, no signature)" do + dir = Path.join(System.tmp_dir!(), "mob_verify_tier0_#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + + assert {:ok, nil} = Verify.load_verified(dir) + end + + test "refuses to eval a tampered manifest even if the side effect would run first", + %{dir: dir} do + # Stronger version: put a side-effect BEFORE the returning map. If + # load_verified accidentally eval'd, the marker file would appear. + # This directly guards the CVE class MOB-74 closes. + marker = Path.join(dir, "SIDE_EFFECT_RAN") + + original_manifest = %{ + name: :mob_demo, + mob_version: "~> 0.6", + plugin_spec_version: 1, + ios: %{swift_files: ["ios/Demo.swift"]} + } + + # The tampered manifest still evaluates to a valid-looking map, so if + # verification had been re-ordered wrong (post-eval) or omitted, the + # side effect would land. + tampered = """ + File.write!(#{inspect(marker)}, "pwned") + #{inspect(original_manifest, limit: :infinity)} + """ + + File.write!(Path.join(dir, "priv/mob_plugin.exs"), tampered) + + assert {:error, :invalid_signature} = Verify.load_verified(dir) + refute File.exists?(marker), "Code.eval_file/1 ran despite invalid signature — MOB-74 open" end test "round-trips against the manifest loaded back from disk", %{dir: dir} do - {:ok, loaded} = Manifest.load(dir) - assert :ok = Verify.verify_plugin(dir, loaded) + {:ok, loaded_direct} = Manifest.load(dir) + {:ok, loaded_verified} = Verify.load_verified(dir) + assert loaded_direct == loaded_verified + end + + test "acknowledged_unsafe: true loads an unsigned manifest (the escape hatch)", + %{dir: dir} do + # The documented :acknowledge_unsafe_plugins path. Without this, + # every acknowledged unsigned plugin's manifest map gets stripped + # out of the build silently (SignatureGate says :ok for the plugin + # but downstream Merge/AndroidBootstrap/RuntimeManifest filter on + # is_map(manifest) and skip the nil). See MOB-74 pre-merge review. + # + # Revert-verify: remove the `when acknowledged_unsafe?` clause in + # `Verify.load_verified/2` and this fails — an unsigned plugin + # would return `{:error, :missing_signature}` even with the opt. + File.rm!(Sign.signature_path(dir)) + + assert {:ok, manifest} = Verify.load_verified(dir, acknowledged_unsafe: true) + assert manifest[:name] == :mob_demo + end + + test "acknowledged_unsafe: true does NOT bypass tamper detection", %{dir: dir} do + # The escape hatch is specifically for :missing_signature ("I know + # this isn't signed"). Invalid signatures — tampered files, wrong + # pubkey — must still refuse. The user opted into unsigned code, + # not into arbitrary attacker code passing itself off as the + # unsigned plugin. + original = File.read!(Path.join(dir, "priv/mob_plugin.exs")) + File.write!(Path.join(dir, "priv/mob_plugin.exs"), original <> "\n# tampered\n") + + assert {:error, :invalid_signature} = + Verify.load_verified(dir, acknowledged_unsafe: true) + end + + test "acknowledged_unsafe: true does NOT bypass a missing pubkey", %{dir: dir} do + # A signed plugin whose pubkey file vanished shouldn't fall into the + # "unsigned during dev" bucket — something is off with the plugin's + # own artefacts (author error or partial upload). Refuse and surface + # the real reason. + File.rm!(Path.join(dir, "priv/mob_plugin.pub")) + + assert {:error, :missing_pubkey} = + Verify.load_verified(dir, acknowledged_unsafe: true) + end + + test "acknowledged_unsafe: true does NOT bypass an envelope_v1 refusal", %{dir: dir} do + # A plugin author who moved from unsigned → v1-signed shouldn't + # auto-upgrade to "loaded via escape hatch" — they need to re-sign + # with v2 (that's the whole point of the v1 refusal). + v1_bytes = + Crypto.canonical_encode(%{ + signature: :binary.copy(<<0>>, 64), + envelope_version: 1 + }) + + File.write!(Sign.signature_path(dir), v1_bytes) + + assert {:error, :envelope_v1_unsupported} = + Verify.load_verified(dir, acknowledged_unsafe: true) end end end