diff --git a/docs/reaching-a-host-you-cannot-dial.md b/docs/reaching-a-host-you-cannot-dial.md new file mode 100644 index 0000000..30d3ea3 --- /dev/null +++ b/docs/reaching-a-host-you-cannot-dial.md @@ -0,0 +1,197 @@ +# Reaching a host you cannot dial + +Some machines do not answer from where the user is sitting. This product has **two** answers to that, and +neither of them works. + +- The **relay** pipes raw TCP through the deployment. The server half is built and shipped; the client + half does not exist, and both heads offer a checkbox that promises it. +- A **jump host** reaches the target through a machine already in the keychain. `HostSecret.JumpHostIds` + stores the chain; nothing writes it and nothing reads it. + +This document is the comparison between them, which is the thing that has to be settled before either is +built, and then the plan. It replaces an earlier draft of `docs/jump-hosts.md` that recommended deleting the +jump chain — see the last section for why that was wrong. + +> **Status: planned, nothing started.** Step 0 is a one-line honesty fix and should not wait for the rest. +> +> | Step | State | Notes | +> | --- | --- | --- | +> | 0. Stop promising the relay | Not started | The checkbox is wired to storage and to nothing else | +> | 1. The loopback bridge | Not started | ADR 0004's "one mechanism, two features" | +> | 2. Jump hosts over it | Not started | No server change at all | +> | 3. The relay over it | Not started | Ticket call, WebSocket, then the same bridge | +> | 4. File transfer parity | Not started | The transfers screen opens its own connection | + +## ◆ The relay's checkbox is a false promise, and that is a defect + +`HostSecret.RelayEnabled` is stored, validated — a relay host may not inherit its port — encoded, merged, +and drawn as a checkbox in the host editor on **both** heads. The desktop's says *"Connect through the +server relay"* and warns underneath that the address will be stored on the server in plain text. The +phone's says the same at more length. + +Nothing on the client reads it. `VaultViewModel` builds `SshConnectionRequest(hostname, port, username, +credential)` and `SshNetConnectionFactory` dials that address directly, whether the box is ticked or not. + +So a user who ticks it **pays the privacy and gets nothing**: the host's address and port leave the +encrypted payload and land in plaintext columns on the server — the one deliberate concession in the whole +design, per ADR 0004 — and the connection is still made from their laptop to the machine they already could +not reach. It then fails exactly as it did before, with no hint that the box did nothing. + +This is worse than the jump chain, which is invisible and harmless. It is a control that spends something +real. Step 0 exists because it should not survive another release in that state, and it is one line: the +checkbox says the relay is not wired up yet, the way this codebase already handles port forwarding on the +phone's More screen. + +## The comparison + +Both answers put something between the user and a machine they cannot dial. What differs is *what* is in +between, what it costs, and who has to own it. + +| | Relay | Jump host | +| --- | --- | --- | +| **Reaches** | Anything the **deployment** can reach | Anything a **machine already in the keychain** can reach | +| **Asks of the deployment** | It must sit where it can dial the target, and have the relay enabled | Nothing. The server is not involved at all | +| **Tells the operator** | The host's address and port, in plaintext columns, for every opted-in host — plus an audit row per session: target, duration, bytes, close reason, client IP | Nothing beyond the sync metadata every item already produces | +| **The intermediate's credentials** | None to manage. The deployment is the intermediate | The bastion is an ordinary host: its own key or password, its own host key to pin, its own group defaults | +| **Where SSH terminates** | On the laptop. The relay sees ciphertext, and ADR 0004 is emphatic that no recording is possible | On the laptop. The bastion forwards a TCP stream inside a session the user opened to it | +| **When it is unavailable** | Deployment down, no connection — including to hosts that were reachable directly | Bastion down, no connection to what is behind it | +| **Fits an estate where** | The DodoSSH server is *inside* the network the targets are on | A bastion is the policy and the server is outside — which is the ordinary enterprise shape | +| **Auditable by the operator** | Yes, coarsely, and that is a feature for a team deployment | No, and that is a feature for a private one | + +**The two are not substitutes, and the deciding question is where the deployment sits.** The relay only +answers "unreachable" when the server has line of sight the laptop lacks — a deployment inside the VPC, on +the office network, on the same Tailnet. Point it at a self-hosted box outside the target's network, which +is what most people running this on a VPS will have, and the relay reaches nothing the laptop could not +already reach. + +**And you do not get to choose other people's topology.** Shipping this to strangers means shipping into +estates whose shape is already decided, and bastion-fronted is the common one. Their `ssh_config` says so: +the importer reads `ProxyJump`, records it as an option and writes a note on the host saying *"DodoSSH does +not route through a jump host yet"* — a first-run experience that names the limitation on the hosts it +matters for. + +**A relay is not a bastion with better manners.** ADR 0004 rejected "server terminates SSH" and kept +zero-knowledge, which is right and is not what a jump host asks for either: forwarding a TCP stream through +a machine the user has authenticated to reveals nothing to the operator, because the operator is not in it. +The privacy ordering is the opposite of what the ADR's framing suggests — the relay is the mechanism that +costs a plaintext address, and the jump host is the one that costs nothing. + +## They are one piece of work, and ADR 0004 says so + +The last consequence in ADR 0004, written before either half was built: + +> On the client, SSH.NET cannot be handed a pre-connected stream, so the relay is reached via a loopback TCP +> bridge. The same bridge provides ProxyJump via a SOCKS5 dynamic forward — **one mechanism, two features**. + +That is the plan, and it holds up against the pinned package. SSH.NET 2025.1.0 offers +`ForwardedPortDynamic`, which is a SOCKS5 proxy served over an established `SshClient`, and +`ConnectionInfo(host, port, username, ProxyTypes, proxyHost, proxyPort, proxyUsername, proxyPassword, +AuthenticationMethod[])` with `ProxyTypes.Socks5` — checked in `Renci.SshNet.xml` rather than remembered. So: + +- **Jump host:** connect to the bastion as an ordinary host, `AddForwardedPort(new ForwardedPortDynamic(0))` + on it, then dial the target with a `ConnectionInfo` pointed at that loopback SOCKS5 port. A chain of two + is the same trick twice. +- **Relay:** the same shape with a different thing on the loopback socket — a listener that pipes bytes into + the `dodossh.relay.v1` WebSocket instead of into a bastion's forward. + +Which means the transport work is shared and the ordering is: bridge, then the cheap feature, then the one +that needs the server. + +## The work, in order + +**0. Stop promising the relay.** The checkbox states that the relay is not wired up yet. One line on each +head, and it is the only step that should ship on its own. + +**1. The bridge.** A loopback `TcpListener` on an ephemeral port that accepts exactly one connection, hands +it to a `Stream` supplied by whoever opened the bridge, and disposes with the session. It belongs in +`Client.Ssh` beside `SshNetConnectionFactory`, and it needs to bind `127.0.0.1` explicitly — a bridge on +`0.0.0.0` is an open SOCKS proxy on the user's network for the life of a shell. + +**2. Jump hosts.** No server change. In order: + +- `SshConnectionRequest` grows a route: the resolved chain, each hop carrying what a connect needs, so the + SSH layer is handed hops rather than ids and never looks anything up. +- `VaultViewModel` resolves `JumpHostIds` to hosts in the same vault, applying group inheritance per hop the + way the target already gets it, and refuses a chain that crosses a vault — the same refusal + `RefusesTheDrop` and the group picker already make, for the same reason. +- Per-hop host keys. Each hop is a separate handshake against a separate endpoint, so the pin, the unknown + key prompt and the changed-key refusal run per hop. **The prompt has to name which hop it is about**, or + somebody approves a bastion's fingerprint believing it is the target's — see `HostKeyCard`, which is built + around one connection and one question. +- Per-hop credentials, including a hop that wants a typed password. `IsAskingForConnectPassword` asks about + one host today. +- Teardown: the hops belong to the outer session and go with it, including when the outer connect fails + half way. A leaked bastion connection is an open session on a machine the user believes they left. +- The schema version. A chain becomes a real field, so it joins the ladder in `HostSecretCodec` — a host + carrying one must not be editable by a client that would drop it. That is the whole point of the rule. +- The editor: a picker over other hosts in the same vault, and the host detail's subtitle finally getting + the `⤷ bastion-eu` the design asked for. + +**3. The relay.** `POST /relay/tickets` with the host id, then the WebSocket with the ticket in +`Sec-WebSocket-Protocol`, piped into the bridge from step 1. The ticket is single-use and expires in 30 +seconds, so it is fetched per connect and never cached. Then the checkbox from step 0 becomes true. + +**4. File transfer.** `ISftpSessionFactory.OpenSftpAsync` opens its own second connection, so a host that +needs a chain or a relay to reach needs it there too, or SFTP silently fails for exactly the hosts this +work exists for. + +## Traps already known + +**A relay host may not inherit its port, and a jump host has no such rule.** `TryValidate` enforces the +first because the server stores the port and a group edit would silently change what the relay dials. The +chain has no plaintext counterpart, so it inherits normally — do not copy the restriction across out of +symmetry. + +**Two hosts can name each other.** A chain is ids, and nothing stops A jumping through B while B jumps +through A. Resolve iteratively with a visited set and refuse a cycle before dialling anything, rather than +discovering it as a stack overflow inside a connect. + +**The bastion's own group defaults matter.** A hop is a host, so it resolves its port, username and binding +through `HostInheritance` exactly as the target does. Skipping that dials 22 as nobody on a bastion that is +on 2222 as `deploy`. + +**`ForwardedPortDynamic(0)` and reading the port back.** Binding an ephemeral port and then asking for the +one that was assigned is the part that varies between SSH.NET versions; pin it with a test that opens one +against the test `sshd` rather than trusting the number. + +**The relay bridge and the jump bridge are the same class and not the same lifetime.** A ticket is +single-use with a 30-second expiry; a bastion's forward lives as long as the session. Sharing the listener +is right, sharing a lifetime policy is not. + +## Tests + +- A two-hop connect against the Testcontainers `sshd`, which `Client.Ssh.Tests` already stands up — one + container as bastion, one as target, with the target refusing connections from anywhere else. +- A cycle in a chain is refused before any socket is opened. +- Each hop's host key is asked about separately, and the question names the hop. +- A chain crossing a vault is refused with a reason, as the group picker's is. +- The bridge binds loopback only — assert the bound address, because the failure is silent and the + consequence is an open proxy. +- SFTP to a host behind a chain, once step 4 lands. +- Mutations that must fail something: bind the bridge on `IPAddress.Any`; drop the visited set; skip group + inheritance for a hop; and tear down the outer session without the hops. + +## Prose that becomes false + +- `docs/design-import-gaps.md` — the host subtitle's `⤷ bastion-eu` row, the SFTP `sftp over bastion-eu` + row, and the status bar's `via bastion-eu` row, all of which say jump hosts are data-only. +- `Client.Import/ImportedHost.cs` — the note written onto every imported host with a `ProxyJump`, and the + remark above it. +- `README.md` and `docs/android-port.md` wherever the relay is described as available. +- ADR 0004 gains a note that its last consequence was built, and how. + +## What the first draft of this document got wrong + +It recommended deleting `JumpHostIds`, on the evidence that nothing writes it, nothing reads it, and it is +missing from the schema-version ladder. The first two facts are true and the conclusion did not follow. + +Two things were missed. **ADR 0004 had already designed the implementation** — the loopback bridge, the +SOCKS5 dynamic forward, "one mechanism, two features" — so the transport was a solved problem sitting in an +accepted ADR, and the fortnight that draft estimated was priced without it. And **the stored shape is +right**: an ordered list of host ids is exactly what a chain is, the merge arm is already correct, and the +missing schema version is a line to add rather than evidence of a bad model. + +The lesson is narrower than "read the ADRs": it is that *nothing reads this field* was taken as evidence the +field was a mistake, when it was evidence of an unfinished feature — and the same reasoning applied one +paragraph further would have found the relay checkbox, which is the same shape and is actively lying to +users. diff --git a/docs/unlocking-without-the-passphrase.md b/docs/unlocking-without-the-passphrase.md new file mode 100644 index 0000000..ac4d846 --- /dev/null +++ b/docs/unlocking-without-the-passphrase.md @@ -0,0 +1,195 @@ +# Unlocking without the passphrase + +Every account here is issued a recovery code at enrollment. It is generated on the client, it wraps the +identity bundle, the server stores that wrap, and both heads go to some trouble to make sure the user writes +it down — the phone raises `FLAG_SECURE` for that screen alone and refuses to let anybody click past it. + +**Nothing can use it.** There is no code path in this product that opens a recovery wrap. This document is +the plan for the change that fixes that, and it is written to be picked up cold. + +> **Status: planned. None of the six steps below is built.** +> +> | Step | State | Notes | +> | --- | --- | --- | +> | 1. The endpoint that serves the wrap | Not started | `GET /api/v1/me/recovery-wrap`, and deliberately not `/me` | +> | 2. `SessionOpener.UnlockWithRecoveryAsync` | Not started | The unwrap, then the path `UnlockAsync` already takes | +> | 3. Setting a new passphrase | Not started | `PUT /api/v1/me/wrap`. Delivers *change passphrase* as well | +> | 4. Both heads | Not started | A way in from the unlock screen, and a box for the code | +> | 5. The prose that becomes false | Not started | Three shipped claims disagree with each other today | +> | 6. `crypto.md`'s status note | Not started | "One of four ways" is one of two, and will be one of three | + +## What is wrong + +Walk the failure through, because it is worse than a missing feature. + +Forget the passphrase and the identity bundle cannot be unwrapped. No bundle means no vault keys, and no +vault keys means every item in every vault is unreadable. Signing out and back in does not help: the server +hands back the same passphrase wrap. The device key would be the other door, and **sign-out withdraws it** — +which is the advice the unlock screen gives for exactly this situation. The recovery wrap sitting on the +server is the only thing left, and nothing opens it. + +So the loss is total and permanent, and the thing built to prevent it is inert. + +Meanwhile the product says three things about this, and they do not agree with one another: + +| What it says | Where | True today | +| --- | --- | --- | +| "The code is the only thing standing between a forgotten passphrase and an unrecoverable vault" | `docs/manual-checks.md` §10.2 | **No.** It stands between nothing | +| "losing it *along with* the passphrase means the vault is unrecoverable" | `docs/android-port.md`, state 4 | **No.** It implies the code alone is enough | +| "Signing out … is the only answer to a forgotten passphrase — nothing can recover one" | `README.md`, Locking | Yes, and it contradicts both of the above | + +The third is the honest one. That is the state this plan changes, and until it does, the first two are the +two sentences in this repository most likely to cost somebody their keychain. + +## What already exists + +More than half of it, which is why this is worth doing now rather than treating as a feature. + +- **The code.** `ClientEnrollment.GenerateRecoveryCode` — 160 bits, base32 over a 32-character alphabet with + `I`, `L`, `O` and `U` left out so a transcription cannot land on a different valid code, printed as + 32 characters in groups of five. +- **The wrap.** The same class derives `KEK_rc` under `Argon2Profile.RandomSecret` (64 MiB, 3 passes) and + sends `RecoveryWrappedPrivateKey` and `RecoveryKdfParameters` with the enrollment. +- **The row.** `EnrollmentService` stores it as `UserKeyWrapKind.Recovery`, beside the passphrase and device + wraps. +- **The specification.** `docs/crypto.md` §2 gives the parameters for `KEK_rc` and §3 puts the wrap in the + key hierarchy beside the passphrase and device ones. Nothing below needs a spec change. +- **The cache.** `LocalCacheKey` derives from the identity bundle under `dsh1/localcache/v2`, not from `MK` + — see the changed-2026-07-30 note in `crypto.md` §3.2. That change was made partly *for* this: a recovery + unlock derives a different `MK` and would otherwise open the identity and then fail to read the cache it + had itself written. It is already paid for and currently untested from this direction. + +## What is missing, exactly + +| | Gap | Where it lands | +| --- | --- | --- | +| A | Nothing serves the wrap. `MeResponse` carries `WrappedPrivateKey` and `KdfParameters` — the passphrase pair, and only that | `DodoSSH.Contracts`, `Api/Features/Identity` | +| B | No unlock path. `SessionOpener` has `UnlockAsync` and `UnlockWithDeviceAsync`, and nothing else | `Client.Session` | +| C | No way to set a new passphrase afterwards. No endpoint, no client path, no UI | server + client | +| D | No way in from either unlock screen | `UnlockCard.axaml`, Android `LockedScreen.axaml` | + +The device wrap is not a precedent for A: it is cached locally in `StoredUnlockMaterial` at the moment it is +registered, so it never has to be fetched. + +## The decisions, and the reasons + +**A separate endpoint, not `/me`.** `GET /api/v1/me/recovery-wrap`, called only when somebody has said they +have forgotten their passphrase. `/me` is fetched by every client at every launch, and putting the recovery +wrap in it would hand that blob to anybody holding a stolen OIDC session, permanently, for no benefit. The +wrap's real defence is Argon2id over 160 bits and it does not stop being safe in the response — this is the +cheaper rule of not serving what nothing needs. It answers 404 for an account with no recovery row, which is +every account enrolled by a client that did not send one. + +**Online only, and the screen says so.** The wrap is deliberately *not* added to `StoredUnlockMaterial`. +Caching it would put a second door on every laptop, protected by a weaker KDF profile than the passphrase +one, for a case that already requires a network — recovery begins with signing in. + +**Setting a new passphrase is part of the flow, not a follow-up.** Without it the account unlocks with a +one-time code forever, and the code is a thing people keep on paper. This is the same re-wrap a *change +passphrase* feature needs, so step 3 delivers both; change-passphrase is then a button, not a project. + +**The identity key is not rotated.** Rotating it would invalidate every vault key grant the account holds +and force a rekey of every vault it can read — see [ADR 0010](adr/0010-vault-key-rotation.md) for what one +of those costs, and it is per vault. Nothing about the bundle is compromised by its owner proving possession +of it, so there is nothing to rotate away from. Identity key rotation is M5 and stays there. + +**The recovery code is not re-issued after use.** Issuing a new one means asking somebody to write down a +new code at the exact moment they have demonstrated they lose things, and the old code is not weakened by +having been typed. "Issue a new recovery code" is a separate feature with its own screen, and it is the +right place for that question. + +**Rate limiting is wanted here and is not a blocker.** This is the first endpoint in the product with a +guessable secret behind it, and `docs/platform-flags.md` records that rate limiting is unimplemented (M2). +Argon2id at 64 MiB is the cost that matters — a guessing attack pays it per attempt — so this ships without +and the endpoint is the reason to do the M2 item next. + +## The work, in order + +Each step compiles with the whole suite green before the next begins. + +1. **The endpoint.** `GET /api/v1/me/recovery-wrap`, returning the wrap and its `KdfParameters`, or 404. A + contract type, an entry in `PublicAPI.Unshipped.txt`, and a `SyncEndpointTests`-style refusal test for an + account without one. +2. **The unlock.** `SessionOpener.UnlockWithRecoveryAsync(string code)`: canonicalise the code, derive + `KEK_rc` from the served parameters, unwrap the bundle, and then join the path `UnlockAsync` already + takes from the moment it holds one. Nothing after the unwrap should be new code — if it is, the two + unlocks have diverged and one of them is wrong. +3. **The re-wrap.** `PUT /api/v1/me/wrap` taking a new passphrase wrap and its parameters, and the client + half that derives the new `KEK_pp` and calls it. Forced as the last step of a recovery: the session is + open, so refusing to go further until a passphrase is set costs nothing and is the only moment the user + is certainly present. +4. **Both heads.** A "forgotten your passphrase?" route from `UnlockCard` and from Android's + `LockedScreen`, a box that accepts the code as printed, and the new-passphrase form after it. The phone's + version has to survive the keyboard covering the box — `docs/manual-checks.md` §10.3 lists the five boxes + that already do, and this is a sixth. +5. **The prose.** Below. +6. **`crypto.md`.** A status note, not a spec change — see the last section. + +## Traps already known + +**◆ The code is derived from the string exactly as displayed, dashes included.** `GenerateRecoveryCode` +says so where it builds the groups: the separators are not decoration to be stripped before hashing, they +are part of the input. So step 2 must *canonicalise to the printed form* rather than normalise it away — +up-case, then re-group in fives — and it must not simply strip dashes and hash what is left. Getting this +wrong produces a code that verifies nowhere and an error message that says the code is wrong. + +**The alphabet has holes in it.** `I`, `L`, `O` and `U` are not in it. A user who writes `O` for `0` should +be met with a code that works, so the canonicaliser should fold the four missing letters onto their +look-alikes before deriving. That is a deliberate leniency and belongs beside the alphabet's own comment. + +**The two KDF profiles are not the same, and the stored parameters are what to use.** The passphrase wrap is +`PassphraseDefault` (256 MiB, 4 passes) and the recovery wrap is `RandomSecret` (64 MiB, 3), which is +correct — 160 random bits do not need the same stretching as a chosen phrase. Derive from the parameters the +server served with the wrap, never from a profile constant, or an account enrolled by a client with +different settings will refuse a valid code. + +**The cache property is untested from this direction.** A recovery unlock derives a different `MK` and must +still read a cache written under a passphrase unlock. That works today because `LocalCacheKey` hangs off the +bundle, and no test asserts it, because nothing could reach the state. Assert it in step 2 rather than +trusting the note in `crypto.md`. + +**Sign-out is the neighbouring path and must stay reachable.** The unlock screen offers signing out as the +answer to a forgotten passphrase. It stays: recovery needs the code, and somebody without it still needs a +way to hand a laptop on. What changes is that it stops being the *only* answer, so the copy on both screens +has to place them beside each other rather than replacing one with the other. + +## Tests + +- Round trip: enrol, recover, unlock, set a new passphrase, unlock again with it. +- The old passphrase stops working after step 3, and the recovery code still does. +- A wrong code is refused without disclosing whether the account has a recovery wrap at all. +- The cache written under a passphrase unlock is readable after a recovery unlock — the `localcache/v2` + property, asserted rather than assumed. +- An account with no recovery row: the endpoint 404s and the screen says the code was never issued rather + than that it is wrong. +- One `SystemTests` pass against the real API, because this is a two-endpoint flow and that suite is the one + that consumes the artefacts that ship. +- Mutations to confirm each test can fail: reverse the canonicalisation so dashes are stripped; derive from + `Argon2Profile.RandomSecret` instead of from the served parameters; and skip the forced re-wrap in step 3. + +## Prose that becomes false + +Not a tidy-up. Each of these currently tells a user something that this change makes wrong in the other +direction, and two of them are wrong right now. + +- `docs/manual-checks.md` §10.2 — "the only thing standing between a forgotten passphrase and an + unrecoverable vault". True once this ships, false today. It also needs a new check: typing a code from + paper, which is the one part of this no test can perform. +- `docs/android-port.md`, state 4 — "losing it along with the passphrase means the vault is unrecoverable". +- `README.md`, Locking — "Signing out … is the only answer to a forgotten passphrase — nothing can recover + one." Correct today and the sentence this change exists to falsify. +- `docs/design-import-gaps.md`, Preferences — "a forgotten passphrase — which is unrecoverable by + construction, so the unlock screen would otherwise be a dead end", which is the stated justification for + signing out being on that screen. The justification survives; the parenthesis does not. +- Both unlock screens, wherever they name signing out as the answer. + +## The spec's "four ways" + +`docs/crypto.md` §3.2 says a passphrase is "only one of four ways to open a vault" — passphrase, device, +recovery and escrow. Two exist. This change makes it three; escrow is M5 by decision and has an enum member +and nothing else. + +**Do not edit that sentence.** It is normative and it is about the DSH1 format, where four ways is exactly +right. What is missing is a statement about *this build*, so add a short table under §3.2 saying which wrap +kinds this client can create and which it can open, and update it here rather than in the frozen prose. Land +it in the same commit as step 2, so the two can never disagree again.