diff --git a/docs/03-conventions.md b/docs/03-conventions.md index 3b8dd64..bb812e1 100644 --- a/docs/03-conventions.md +++ b/docs/03-conventions.md @@ -22,9 +22,9 @@ - Tagged tests: `@tag :live` (real providers, opt-in), `@tag :desktop` (needs Tauri), `@tag :slow`. Default `mix test` excludes `:live` and `:desktop`. - Every crash-safety claim has a test that actually kills a process. -- Coverage: reported in PROOF.md each slice and written to `coverage.tsv` in the repo root, one row per slice, +- Coverage: written to `coverage.tsv` in the repository root, one row per increment, so there is a baseline to compare against. The gate reads that file: a drop of more than 3 points against the - previous slice fails until a NOTES.md justification names the reason. The rule as originally written stored no + previous increment fails until a justification names the reason. The rule as originally written stored no baseline, so nothing could check it, which is the pattern the rules of evidence forbid. ## Tools (slice 020) diff --git a/lib/trinity/gateways/identities.ex b/lib/trinity/gateways/identities.ex index bf340ae..1f612a9 100644 --- a/lib/trinity/gateways/identities.ex +++ b/lib/trinity/gateways/identities.ex @@ -94,10 +94,22 @@ defmodule Trinity.Gateways.Identities do ) end - @doc "A fresh pairing code." + @doc """ + A fresh pairing code, from the cryptographically secure generator. + + The code is a credential: it is the whole of the proof that the person holding a channel is the + person at this machine, so it is generated with `:crypto.strong_rand_bytes/1` and not with + `Enum.random/1`, which draws from `:rand` and is predictable from observed output. The alphabet + has 32 characters and a byte has 256 values, so mapping a byte with `rem/2` is uniform: 256 is + exactly eight whole cycles of the alphabet, and no value is more likely than another. + """ @spec generate_code() :: String.t() def generate_code do - for _ <- 1..@code_length, into: "", do: <> + size = length(@alphabet) + + for <>, + into: "", + do: <> end @doc "How long a code lives, in seconds." diff --git a/test/trinity/gateways/identities_test.exs b/test/trinity/gateways/identities_test.exs index 526cb0b..1d28175 100644 --- a/test/trinity/gateways/identities_test.exs +++ b/test/trinity/gateways/identities_test.exs @@ -87,6 +87,44 @@ defmodule Trinity.Gateways.IdentitiesTest do assert {:ok, _, :pending} = Identities.admit("other", "same-id") end + # A pairing code is a credential, so where it comes from is a property worth holding rather than + # a detail. `Enum.random/1` draws from `:rand`, which is predictable from observed output; this + # was the generator until the security audit for the OpenSSF criteria caught it. + test "pairing codes come from the cryptographically secure generator, over the whole alphabet" do + codes = for _ <- 1..200, do: Identities.generate_code() + + assert Enum.all?(codes, &(String.length(&1) == 6)) + assert Enum.all?(codes, &(&1 =~ ~r/^[A-HJ-NP-Z2-9]+$/)) + + # Distinct: 200 draws from 32^6 collide with negligible probability, so a repeat means the + # generator is not drawing from the space it claims to. + assert length(Enum.uniq(codes)) == 200 + + # The alphabet excludes the characters a person reads for one another (I, O, 0, 1), and the + # draw reaches the rest of it: a generator stuck on a subset would fail here. + seen = codes |> Enum.join() |> String.graphemes() |> Enum.uniq() + assert length(seen) > 24, "only #{length(seen)} distinct characters in 1,200 draws" + refute Enum.any?(seen, &(&1 in ["I", "O", "0", "1"])) + end + + test "no security value in the gateway package is drawn from the non-cryptographic generator" do + {out, 0} = System.cmd("git", ["ls-files", "lib/trinity/gateways"]) + + for file <- String.split(out, "\n", trim: true) do + # Comments and docs are stripped first: a moduledoc explaining why the non-cryptographic + # generator is not used names it, and naming it is not calling it. + source = + file + |> File.read!() + |> String.replace(~r/@(module)?doc\s+"""(.|\n)*?"""/, "") + |> String.replace(~r/#[^\n]*/, "") + + refute source =~ ~r/Enum\.random|:rand\./, + "#{file} draws from the non-cryptographic generator; use :crypto.strong_rand_bytes/1 " <> + "for anything that is a credential, a nonce or a key" + end + end + test "the desktop can pair and revoke without a code" do {:ok, identity, :pending} = Identities.admit(@adapter, @user) assert {:ok, allowed} = Identities.allow(identity)