diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 33e7ff5..25e1d47 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -279,6 +279,11 @@ jobs: # Packaging rather than only compiling, because the two failures this head is most exposed to # are both link-time: a native library with no android ABI, and a managed assembly that # resolves for net10.0 but has nothing to dex. Neither shows up in a compile. + # + # Debug-signed on purpose, and it has to stay that way: no keystore secret, no AndroidKeyStore=true. + # docs/adr/0010-android-distribution.md puts the release key on a machine that is not a runner, + # because a signing key reachable from a workflow is a key held by everyone who can change one. + # This APK is a build check. It is not something anybody installs. - name: package run: > dotnet build src/DodoSSH.Client.Android/DodoSSH.Client.Android.csproj diff --git a/README.md b/README.md index 3550199..5aaa367 100644 --- a/README.md +++ b/README.md @@ -186,10 +186,15 @@ signing in again brings it all back; the count in the confirmation is the one th else. Your session at the identity provider is *not* ended — DodoSSH has no way to end it — so on a machine that is not yours, sign out there too. -Two of M1's known gaps are visible immediately, so they are worth expecting rather than diagnosing: password -authentication asks for the password every time, because nothing in the interface can create a vault -credential yet (they do sync — there is just no editor for one); and unlock asks for the passphrase on every -launch, because no device key is registered. +**Neither a password nor a passphrase has to be typed twice**, and both ways out of that are opt-in. A +password typed to connect is typed once: tick **Remember this password** under the box and it is saved to +your keychain and bound to that host the moment the remote accepts it — or add one outright with **+ +PASSWORD** on the Vault screen. And unlock can be a Windows confirmation instead of the passphrase: +Preferences → *This machine* → **REGISTER** keeps this machine's device key in the TPM, so a later launch +offers **USE WINDOWS HELLO** on the unlock card. A machine with no TPM — and any desktop that is not +Windows — is offered neither button and keeps asking for the passphrase, which Preferences says out loud +rather than leaving you to notice. The passphrase never stops working either way: a declined confirmation +leaves the box exactly where it was. ### Moving files @@ -297,19 +302,31 @@ rather than only in the ID token or the userinfo response. they become owner and you become an admin, in a single transaction. Not two role changes — promoting first leaves it owned twice, demoting first leaves it owned by nobody, and there is nobody with the authority to finish a transfer that stopped in the middle. You are demoted rather than removed, so you keep your vault -key grants; removing you would revoke them and flag the vault for rekey, and somebody handing a vault over -is usually staying in it. +key grants; removing you would revoke them and rotate the vault, and somebody handing a vault over is +usually staying in it. -Five limits, stated rather than discovered: +Six limits, stated rather than discovered: - **A vault cannot be deleted.** Nothing in this product removes one, and the server refuses to archive the membership list behind a vault that still exists — a shared vault is readable *because* of membership, so archiving it would take the vault away from everybody holding a key, including you, quietly and all at once. The screen says so where you would otherwise go looking for the button. -- **Removing a member is not retroactive.** It revokes their grants and flags the vault for rekey, and - blocks future reads. Everything they already pulled is on their machine. Rotate the SSH credentials that - matter — that is the actual remediation, and it is why there is no button labelled anything stronger. -- **The rekey is flagged, never performed.** See the milestone note above. +- **Removing a member is not retroactive.** It revokes their grants, rotates every vault behind that + membership list your machine can open, and hands each new key to the members who are left — so nothing + written from that point on is readable to them. Everything they already pulled is still on their machine. + Rotate the SSH credentials that matter — that is the actual remediation, and it is why there is no button + labelled anything stronger. +- **A rotation moves the vault's contents too, and says so when it could not.** The key changes first, in + one server transaction; then every item already stored is re-sealed under it, so the key somebody left + with opens nothing that is still here. Existing items keep working throughout — everybody still in the + vault holds the older keys as well as the new one, which is what stops a half-finished rotation making a + vault unreadable, and what makes the pass safe to interrupt and run again. An item somebody else was + editing at that moment is left for the next pass, and the message tells you which of the two you got. + See [ADR 0010](docs/adr/0010-vault-key-rotation.md). +- **Adding somebody shares the vaults you can open, including their history.** Membership is still one act + and a key is still another — nothing changed about that — but the client now performs the second one for + you, wrapping every generation it holds so the new member can read the vault back to its first item. A + vault your machine holds no key to is skipped and says so; somebody who holds it has to share that one. - **Host key trust stays in your personal vault.** A pin approved for a shared vault's host is recorded and used from your own vault, not the shared one, so a colleague cannot pre-approve a fingerprint that your client will then trust silently for a host you defined. The cost is that each member approves a shared @@ -372,8 +389,20 @@ design's blue-tinted `#E3E7F4`, and the type scale is a point larger. File transfer **is** here now, in the shape scoped storage allows: one remote pane and the queue, over either an SFTP host or a bucket. There is no local pane, because there is no browsable local filesystem to -put in one — moving a file *in* from the phone needs the system document picker and is the next piece of -work rather than a thing the screen pretends to do. Hosts and groups are made and corrected here now, from +put in one. So the way in is **ADD FILES**, which is the system document picker: point at a document +wherever it lives and it goes to the directory showing, rather than choosing on the left and pressing an +arrow. What Android hands back is a `content://` URI with no path behind it and no promise of a seek, so +the document is copied into the app's own cache and the copy is what the queue moves — which is what lets +a stopped upload resume from where it stopped. The copy is deleted when the transfer finishes, kept while +it is stopped so RESUME has something to read, and swept at the next launch. + +**SAVE FILE is the way back out**, and it is the system's save picker for the same reason: there is nowhere +this application could put a file that you would then be able to open. You choose where it goes before the +transfer starts, the download runs into the cache, and the finished bytes are copied out to the document you +chose. That order has one visible cost, and the screen says it rather than leaving it to be discovered: the +picker creates the file when you dismiss it, so a download that then fails leaves an empty one there. The +alternative is a picker that appears minutes later over whatever you moved on to — and often while the app +is in the background, where Android will not show one at all. Hosts and groups are made and corrected here now, from a floating + on the Hosts screen, and both editors are cards in the list's own row rather than dialogs, so the form never covers the thing it is about. The keychain has no editor of its own: SSH keys and buckets are created on the desktop and sync down, and the phone will delete an item — behind the same counted @@ -518,10 +547,15 @@ keychain plus a terminal — and the spike that gates all of it. host key approved at that shell's prompt reaching the second machine as well. See [End-to-end verification](#end-to-end-verification). - Known gaps in the client, stated rather than implied by the interface: nothing in the interface can create - a vault credential yet, so password authentication still asks for the password each time — SSH keys *are* - editable, and binding one to a host is the way to connect without typing anything; and no device key is - registered, so the passphrase is needed on every launch until the OS keystore is wired. + The two gaps this milestone shipped with have both closed since. A vault credential can be created — from + the Vault screen, or from the REMEMBER tick beside the connect password, which saves it and binds the host + to it once the remote has accepted — so password authentication asks once rather than every time. And a + device key is registered where the machine can hold one: Windows keeps it in the TPM under a CNG policy + that makes the consent dialog a condition of *using* the key rather than a prompt this application draws, + which is stronger than [ADR 0007](docs/adr/0007-device-key-protection.md) originally described and is why + that ADR was corrected. What is left is the floor rather than a gap: a machine with no TPM, or a desktop + that is not Windows, gets a store that reports itself unavailable and keeps asking for the passphrase — + the honest answer rather than a degraded one. Host key trust *is* in the vault, which is what makes trust-on-first-use worth having: a fingerprint approved on one machine is approved on all of them and survives a restart, and the server cannot drop a @@ -582,9 +616,9 @@ keychain plus a terminal — and the spike that gates all of it. directories, an interrupted **upload** starts again rather than resuming (an object cannot be written from the middle), and a rename is a copy and a delete rather than one atomic operation. Downloads do resume — a ranged GET is part of the protocol, which is the one place a bucket beats SFTP. -- **M3 — shared vaults**, sharing, ACLs. *Done, except rekey.* Membership with roles, a public-key - directory, the append-only key log served for clients to verify against, shared vaults, and vault key - grants wrapped by a client and stored opaquely by the server. `VaultAccessService` now resolves team +- **M3 — shared vaults**, sharing, ACLs. *Done.* Membership with roles, a public-key directory, the + append-only key log served for clients to verify against, shared vaults, and vault key grants + wrapped by a client and stored opaquely by the server. `VaultAccessService` now resolves team membership to permissions, so a viewer may pull and may not push; the desktop client reads and syncs every vault it holds a key for, and a real VAULTS screen replaces the placeholder. The screen is vault-shaped rather than team-shaped: naming a vault makes the membership list that carries it, so the @@ -595,20 +629,30 @@ keychain plus a terminal — and the spike that gates all of it. knowing before you rely on it; the reasoning is in [ADR 0009](docs/adr/0009-team-access-model.md). - **What is deliberately not here: the rekey itself.** Removing a member revokes their grants and flags - every team vault `RekeyRequired`, and nothing acts on that flag. A rekey re-wraps every item's data key - under a fresh vault key and can only be performed by a client that holds the current one; that is M5's - key rotation. Until it lands the flag is what the interface reads to say a rotation is owed, which is - more honest than a button that only appears to do it. + **Membership changes now move the keys, not just the flag.** Adding somebody wraps every team vault the + adding machine can open to them — every generation of each, so they can read the vault's history and not + only what happens next. Removing somebody revokes their grants, advances each vault it can open to a + fresh key generation in one server transaction, and wraps that key to the members who remain. What a + rotation buys is exact: everything written from then on is unreadable to the person who left. The items + already stored are then re-sealed under the new key as well, item by item and resumably — which is safe + to do incrementally precisely because a vault at mixed generations stays readable. A change queued before + the rotation is re-sealed as it is pushed, so nothing reaches the server under a superseded key at all. + See [ADR 0010](docs/adr/0010-vault-key-rotation.md). **Ownership transfer is here, and it is one write rather than two.** The member you name becomes owner and you become an admin, in a single transaction — because ownership is sole, so promoting first leaves the team owned twice and demoting first leaves it owned by nobody, and there is nobody left with the authority to finish a transfer that stopped in the middle. Nothing else is touched: you keep your vault - key grants, because removing the outgoing owner would revoke them and flag every team vault for rekey, - which is a much larger act than the one being asked for. -- **M4 — hardening and ops**, packaging, self-hosting guide. -- **M5 — multi-provider OIDC**, key rotation, per-item content keys. + key grants, because removing the outgoing owner would revoke them and rotate every team vault, which is a + much larger act than the one being asked for. +- **M4 — hardening and ops**, packaging, self-hosting guide. *Decided ahead of the work, because the first + release takes it irreversibly:* who signs the client and where it comes from. + [ADR 0011](docs/adr/0011-android-distribution.md) puts the release key with the project rather than with + a store, and rules out the arrangement a self-hosted product reaches for by default — the deployment + serving the client binary, which hands it to the one party the whole trust model is about. An installed + Android app can only ever be updated by a package signed with the same key, so this is the first + release's decision to make and nobody else's afterwards. +- **M5 — multi-provider OIDC**, identity key rotation, per-item content keys. ## Licence diff --git a/docs/adr/0009-team-access-model.md b/docs/adr/0009-team-access-model.md index 1bb0eb1..3e4b44a 100644 --- a/docs/adr/0009-team-access-model.md +++ b/docs/adr/0009-team-access-model.md @@ -67,6 +67,12 @@ One thing is deliberately **not** built, and it is a refusal rather than an omis - **The rekey itself.** Only a client holding the current vault key can re-wrap every item's data key under a new one. The server records that a rotation is owed and the interface reports it. M5. + > **Superseded 2026-08-03 by [ADR 0010](0010-vault-key-rotation.md).** Rotation now ships, and it + > turned out to divide differently than this paragraph assumed: advancing the generation is one + > server transaction and is not the same act as re-wrapping the items, which is still outstanding. + > Removing a member rotates the vaults the removing client can open and hands the new key to whoever + > is left. + Two smaller choices, recorded because the alternative was written down first and rejected: - **No `v_user_vault_permission` view.** ADR-adjacent notes and the old `VaultAccessService` remark diff --git a/docs/adr/0010-vault-key-rotation.md b/docs/adr/0010-vault-key-rotation.md new file mode 100644 index 0000000..d754577 --- /dev/null +++ b/docs/adr/0010-vault-key-rotation.md @@ -0,0 +1,144 @@ +# ADR 0010 — Rotation advances a generation; the keys before it are kept + +- Status: accepted +- Date: 2026-08-03 +- Builds on: [ADR 0001](0001-e2ee-trust-model.md), [ADR 0009](0009-team-access-model.md) + +## Context + +ADR 0009 shipped removal as "revoke the grants and flag the vault", and named the missing half +plainly: only a client holding the current vault key can produce the next one, so the server could +record that a rotation was owed and nothing more. Nothing acted on the flag. In practice that meant +removing somebody from a team left every vault they could read encrypted under the key they had, for +ever — the interface said a rotation was owed and no button existed to perform one. + +Two things had to be decided before that flag could be acted on, and they are not independent. + +**When does the generation change?** A vault key is per vault *per generation* (`docs/crypto.md` §3), +and a grant names the generation it opens. If two admins rotate at the same time, both wrap a key, +both issue grants, and the vault ends up with two claimed "current" keys and a set of members split +between them — half of whom cannot read what the other half writes, with nothing to point at as the +cause. + +**What happens to everything already stored?** An item carries the generation it was sealed under, in +its own row and in its AAD. A rotation that advanced the generation and left the old grants behind +would make every item written before it unreadable to everybody, including the person who rotated. A +rotation that re-encrypted every item would avoid that — and is a different, much larger operation: +`crypto.md` §3 puts it at N × 32 bytes of re-wrapped data keys, which is cheap in bytes and is still +a write to every row of a vault, in batches, against a server that caps a push at 500 operations and +8 MB, with the connection/activity logs alone reaching five thousand entries per kind. + +## Decision + +**The rotation is the generation bump, and it is one server transaction. Grants for earlier +generations are kept.** + +`POST /api/v1/vaults/{id}/rekey` takes the next generation and the new key sealed to the caller. In +one transaction the vault's `key_generation` advances, the caller's grant for it is inserted, and the +rekey flag is cleared. The request must name exactly `current + 1`, and the vault's `xmin` +concurrency token makes that check binding rather than advisory — the second of two simultaneous +rotations is refused and told to read the vault again. The server contributes the *moment*, which is +the one part of a rotation a client cannot decide for itself; it contributes no cryptography, cannot +tell that the key it is handed differs from the old one, and cannot tell whether the caller held the +old one. That last part is checked the only way it can be: the caller must hold a live grant at the +current generation, which is a row rather than a proof. + +Everything that follows from keeping the old grants: + +- **A member holds one grant per generation, and `VaultSummary` serves all of them.** The current + wrap stays where it was; the rest arrive as `PriorKeyWraps`, oldest first. `VaultKeyring` holds a + key per generation, hands out the newest for writing and the item's own for reading. Every read + path picks its key from the payload's `keyGeneration` rather than from the vault's. +- **Sharing hands over the history.** `ShareVaultAsync` issues a grant for every generation the + sharing client holds, oldest first. Somebody added after a rotation who was given only the newest + key would open the vault to a list of items that will not decrypt — which reads as corruption, not + as a missing grant. The server accordingly accepts a grant for any generation the vault has + reached, and refuses one for a generation ahead of it: nothing is sealed under that, and accepting + it would let a client move the vault forward outside the transaction that is allowed to. +- **Revocation takes every generation.** Removing a member, and `RevokeGrantAsync`, revoke all of a + recipient's grants rather than the current one. Leaving the history would leave them able to read + everything written before the rotation, which is exactly what the rotation was for. +- **A member between the rotation and their re-wrap can read and cannot write.** They hold the + history and no current key, so the vault lists as unreadable and writes refuse. Writing under a + superseded key would produce items nobody else could open, and the author's own keyring — which + still holds that key — would show no sign of it. + +**Removing a member rotates automatically.** The teams screen removes the member, then rotates every +team vault the machine can currently open and wraps each new key to the members who remain. Adding a +member is the mirror image: every team vault this machine can open is wrapped to them as part of the +add. Both report per vault, including what they could not do — a vault whose key this machine does +not hold is skipped and stays flagged, because somebody else has to finish it. + +## The second half: re-sealing what is already stored + +> **Added 2026-08-04.** This was deferred when the decision above was taken, and is now built. The +> reasoning that made it safe to defer is what made it cheap to add, so it is recorded here rather +> than in an ADR of its own. + +A rotation on its own re-keys the vault and not its contents, which leaves one gap: somebody who left +with a copy of the old key could still open old ciphertext they later got hold of. `VaultResealer` +closes it by walking the vault and rewriting each item under the current key, as an ordinary upsert +against the version the server holds. + +Four properties, each of which is a decision: + +- **It never decodes the plaintext.** An item is opened and the *same bytes* are sealed again under a + fresh data key. No codec, no merge, no schema version — so an item written by a newer client + survives untouched, where re-encoding it through this build's codec would silently drop the fields + this build has no concept of. It is also why one pass covers every item type, including types added + after it was written. +- **It is resumable, and needs no transaction.** Each item is one upsert, so a pass that dies half way + leaves a vault at mixed generations — which is a state that reads perfectly well, because that is + precisely what the decision above bought. Running it again picks up what is left. +- **A conflict is counted, not merged.** The pass changes no content, so there is nothing to merge: + an item somebody else wrote meanwhile is left at their version and re-sealed on the next pass. +- **A queued local edit is left alone, and re-sealed on the way out instead.** Rewriting it here would + overwrite the user's unpushed work with the version the server holds. Instead `SyncEngine` re-seals + a queued payload whose generation is stale as it dispatches it, and writes the revision back to the + outbox first so a retry sends the same bytes. That closes the one hole a pass over *stored* items + cannot see: a change made before the rotation and pushed after it would otherwise put a brand-new + item into the vault under the key the departed member holds. + +The pass runs as the last step of a rotation, after a sync — a mirror that is behind produces a batch +of conflicts rather than a re-sealed vault. The interface reports which of the two guarantees was +reached, because they are different: a vault fully re-sealed is closed to the person who left, and one +where items were left behind is closed only to what happens next. + +## What this still does not do + +**It does not reach what they already pulled.** The person who left keeps whatever plaintext is on +their machine — that is the non-retroactive limit ADR 0001 records and no design here changes it. The +honest remediation for a departure is still to rotate the credentials themselves, and the product says +so rather than the reassuring version. + +## Alternatives rejected + +- **Revoke the old grants on rotation.** Tidier, and it makes the grant list say exactly one thing + per member. It also makes every item written before the rotation unreadable to everybody, which is + data loss performed by a security feature. +- **Chain the keys: store each old key sealed under its successor.** One wrap per rotation instead of + one grant per member per generation, and new members get the history for free. It needs a new table, + a new AAD purpose, and a recursive unwrap on the read path — and it makes the vault's whole history + reachable from the current key, which is a strictly larger blast radius than a set of grants that + can be revoked one at a time. +- **Rotate atomically with every item re-sealed, in one request.** The safest shape on paper and the + one `crypto.md` implies. It caps rotation at the push limits — 500 operations and 8 MB — which a + vault with a year of connection log in it exceeds, and the failure mode is a vault that can never + be rotated at all. +- **Let the server generate the new key.** It would make rotation a single call and would end the + product: a server that can produce a vault key can read the vault. + +## Consequences + +`vault_key_grant` grows by one row per member per rotation. The unique index is already per +`(vault, generation, recipient)`, so this needed no migration; the rows are 110-byte seals and a vault +rotated monthly for a decade with ten members holds twelve hundred of them. + +The sharing graph gains a dimension the operator can read: which generation each member holds, and so +which of them have been re-wrapped since the last rotation. That is the same class of metadata ADR +0009 already records as visible, and it is the same fact the sharing screen shows the members +themselves. + +A client that never comes back holds keys to generations that no longer receive writes, which is the +same exposure as any copy of a vault key on a machine that has been lost — bounded by the fact that +the server will not serve them anything, and unbounded in the way every non-retroactive revocation is. diff --git a/docs/adr/0011-android-distribution.md b/docs/adr/0011-android-distribution.md new file mode 100644 index 0000000..3c7cf0c --- /dev/null +++ b/docs/adr/0011-android-distribution.md @@ -0,0 +1,125 @@ +# ADR 0011 — Distributing the Android client, and who holds the release key + +- Status: accepted +- Date: 2026-08-03 +- Builds on: [ADR 0001](0001-e2ee-trust-model.md) +- Settles: the second open question in [`docs/android-port.md`](../android-port.md#still-open) + +## Context + +[ADR 0001](0001-e2ee-trust-model.md) ends on the hole it cannot close with cryptography: **an operator +who wants the secrets attacks the client, not the crypto**, and what that costs is *release signing with +a key not held by the server*, and eventually reproducible builds. Until now that sentence had nothing +to bind to. The desktop head is not packaged at all — packaging is M4 — and the Android head is a debug +artefact: CI runs `-t:SignAndroidPackage` with no keystore, so it is signed with the debug key .NET for +Android falls back to when `AndroidKeyStore` is false, and that APK is a build check rather than +something anyone installs. + +The first release changes that, and it does so **irreversibly**, which is why this is decided here +rather than at upload time. Two Android facts make it a one-way door: + +- **An installed app can only be updated by a package signed with the same key.** The signing key is + the app's identity for its whole life; changing it means every existing user uninstalls first, losing + their local cache and re-enrolling. (v3 signature rotation exists, but the lineage has to be created + *before* it is needed, by the key it is rotating away from.) +- **A new app on Google Play must ship as an App Bundle, which means Play App Signing**, so Google + generates and holds the key that signs what users install; the developer holds an upload key only. + There is no un-enrolling. For an app that already exists outside Play, the only way to keep one + package id across both channels is to *hand Google the existing key*. + +So "publish on Play" and "hold our own key" are not two settings. They are two package identities, and +the first release picks one. + +The third party changes but the shape does not: **whoever can sign an update can ship one person a +build that copies the passphrase.** The vault's encryption is irrelevant to that attack — the client is +where the plaintext is, by construction (ADR 0001, `Connect` cannot be a security boundary). So this ADR +is about *which* parties are in that position, not about removing them, and there are three candidates: +the deployment operator, DodoTech, and Google. + +## Decision + +**DodoTech holds the release key, the deployment never serves the client, and Play is a separate +decision that has not been taken.** + +1. **One release key, held by the project, kept offline, and never in CI.** Release signing is a + deliberate manual step on a machine that is not a runner. CI keeps doing exactly what it does now — + packaging with the debug key to catch link-time failures — and must never gain a keystore secret or + an `AndroidKeyStore=true`. A signing key in CI is a key held by whoever can push a workflow file, + which for a public repository is a wider set than it looks. + +2. **The APK is published on the project's own release page, and a DodoSSH deployment never distributes + it.** This is the refusal that carries the security content, and it is the one a self-hosted product + gets wrong by default: a "download the app" link on your own server is convenient, obvious, and hands + the client binary to the exact party ADR 0001 models as the adversary. The operator may tell people + where to get it. They may not be the place it comes from. + + The same rule reaches the update path. A version check pointed at the deployment lets the operator + pin a chosen user to a known-vulnerable build by withholding the answer — a weaker attack than + signing one, and available without any key at all. If an update check is ever added it points at the + project's domain, and the first release simply has none: the release page is the channel and the + README says so. + +3. **Play App Signing is not entered, and cannot be entered by accident.** Not because Google is a worse + custodian than DodoTech — on the mechanics it is a better one, since the key lives in Google's + infrastructure rather than on a laptop, and the reach and auto-update story is not close. It is + declined because of what it costs *this* product specifically: the buyers named in ADR 0001 are teams + who refuse to put infrastructure credentials in a SaaS, and telling them the client that holds their + plaintext is signed by a key the vendor cannot see is the same answer they already rejected, one + layer down. A targeted signed build compelled by a lawful order or produced from a compromised + console account is the archetype of the attack ADR 0001 calls the largest practical hole. + + Deferring is cheap and reversing is not, so the default falls the deferrable way. Revisiting is a + second ADR, and it has two honest exits: hand Google the existing key and keep one identity, or take + a distinct package id and accept two apps. Both are worse decisions to discover than to take. + +4. **Reproducible builds are the goal that makes all of the above matter less, and they are not + achievable today.** A build a third party can reproduce from source turns the signing key from a + trusted authority into a convenience — anyone can check that the published APK is the published + source. .NET for Android is not there: dex output, AOT images and archive timestamps are not + bit-reproducible across machines in practice. It stays the standing goal ADR 0001 names, recorded + here as the thing that would let point 3 be reconsidered on the merits rather than on custody. + +5. **F-Droid is not a channel.** Its build server compiles from source and signs with its own key, which + would be a genuinely better transparency story — but it has no support for a .NET workload plus an + Android SDK toolchain, and this head needs both. Not refused; unavailable. + +## Consequences + +**The reach cost is real and should not be talked down.** Installing means enabling installation from +the browser or file manager, per source, on Android 8 and later — a permission the platform frames as +dangerous, correctly. There is no discovery, no automatic update, and no Play channel for a corporate +MDM to deploy from, which for a product sold to teams is the sharpest edge of this decision. What +partially answers it is that this client is installed by people who already run their own identity +provider and their own vault server; sideloading is not the strangest thing they will do that week. + +**The key becomes a single point of failure with no recovery.** Losing it means existing installs can +never be updated again — not a bad update, *no* update — and the only way out is a new package id and a +manual migration. It is therefore backed up offline in more than one place, and a v3 rotation lineage is +created at the first release rather than at the first emergency, because a lineage can only be signed by +the key it replaces. + +**The attack ADR 0001 names is narrowed, not removed.** DodoTech can still ship one user a malicious +build. What changes is that the deployment operator — the party the threat model is actually about, and +the one with a motive to read their own team's credentials — cannot, and that a compelled or breached +third-party store is not in the path either. That is the whole of what this decision buys, and it is +worth stating at that size rather than larger. + +**M4's desktop packaging inherits rule 2 and not the rest.** Windows and macOS have no equivalent of +Play App Signing in the mandatory sense: Authenticode and Developer ID both leave the private key with +the developer, and Apple's notarization is a scan rather than a signature over the shipped binary. The +custody question is therefore easy there; the "not served by the deployment" rule is the part that +carries over, and it carries over unchanged. + +## Rejected + +- **Play as the primary channel, sideloading as the fallback.** This is the arrangement most Android + products land on, and it does not survive contact with rule 2's reasoning: it is the same shape — + the binary that holds the plaintext arriving through a party who can be compelled — with a larger and + better-resourced party in the middle. Better mechanics, same class. +- **Shipping the APK from the DodoSSH server it will talk to**, so a new phone gets the client from the + deployment it is enrolling against. Genuinely the nicest onboarding available, and it makes the + operator the distributor of the client that holds their team's credentials. Refused outright, and + named here because it will be proposed again. +- **A release key held by CI so tagging cuts a release.** The convenience is the point of CI and the + key is the point of this ADR; where they collide the key wins. Signing an artefact is one command, run + rarely, by a person. diff --git a/docs/android-port.md b/docs/android-port.md index bb9e5d9..dbebd99 100644 --- a/docs/android-port.md +++ b/docs/android-port.md @@ -28,7 +28,9 @@ verified is that it compiles, links, packages, and carries the right natives. **The shape agreed:** a **phone-first** client that is the keychain plus a **terminal**, with sessions and transfers protected by a **foreground service**. File transfer is not in the first scope; when it arrives it -is **one remote pane** with Android's document picker for moving files in and out. +is **one remote pane** with Android's document picker for moving files in and out. *It has since arrived, +both ways:* the pane, the queue, `ACTION_OPEN_DOCUMENT` going in and `ACTION_CREATE_DOCUMENT` coming out, +with the foreground service now counting transfers as well as shells. **What was actually checked**, so the rest can be read with the right amount of trust: @@ -132,7 +134,9 @@ mean the transfer queue's local half (`LocalDirectory`, the drive list, the brea code. Note what *does* carry: `FileTransferQueue` itself, and `IRemoteFileStore` — Phase 6 already proved that -seam holds two very different remotes, and a `Uri`-backed Android document would be a third. +seam holds two very different remotes, and a `Uri`-backed Android document would be a third. (**⚠️ That last +clause is wrong, and the build is what corrected it**: a document URI cannot resume, so what shipped stages a +copy and hands over a path. See the note under the decision below.) **Decided: one remote pane and the document picker**, and out of the first scope. See [Decisions](#file-transfer-when-it-comes-one-pane-and-the-document-picker). @@ -256,6 +260,24 @@ and `IRemoteFileStore` both carry over unchanged. Phase 6 already put a bucket b an SFTP host, so a picker-granted document is a third implementation of a seam that has been exercised twice. What is desktop-only is the *left* pane — `LocalDirectory`, the drive list, the breadcrumb trail. +> **⚠️ Corrected by the build. Both directions shipped, and neither as a third `IRemoteFileStore`.** A document +> URI cannot sit behind that interface honestly: the queue seeks, because an upload resumes from the byte the +> last attempt reached, and a `content://` stream promises no seek, no stable length, and no grant that +> survives the document being edited underneath it. So `DocumentStaging` copies the chosen document into the +> application's cache and hands the queue an ordinary path — which cost one class in the head and *nothing* +> in the shared layers, rather than a third implementation of a seam and every resume rule rewritten to cope +> with a stream that cannot rewind. The copy is deleted when the transfer completes, kept while it is stopped +> so RESUME can read it, and swept at the next launch. +> +> **Outbound is the mirror image, with one decision the inbound half did not have to take: when to ask.** The +> save picker is raised *before* the transfer, so `QueueDeliveredDownload` runs into the same staging +> directory and hands the finished bytes to a callback the head supplied. Asking afterwards would put the +> picker minutes away from the button that caused it and, on a phone, frequently in the background — where +> Android will not show one. The cost is that `ACTION_CREATE_DOCUMENT` creates its file when it is +> dismissed, so a download that then fails leaves an empty one; that is said on the screen and in the +> README rather than left to be found. A delivery that fails keeps the staged bytes for the sweep instead of +> deleting the one copy of something that was just fetched over somebody's network. + ### Sessions survive backgrounding, via a foreground service A persistent notification for as long as a shell or a transfer is live. @@ -577,9 +599,11 @@ What is left, in the order it matters: - **Running any of it on a device.** Still the one that is not optional, and still true: nothing here has ever been launched on hardware or an emulator. Everything below is reasoning from the code. -- **The document picker.** The files screen browses a remote, downloads from it and runs the queue; putting - a file *on* a host from the phone needs `ACTION_OPEN_DOCUMENT` behind a third `IRemoteFileStore`-shaped - source, which is the decision recorded above and the obvious next piece of work. +- ~~**The save picker — the other half of file transfer.**~~ **Built**, along with the half before it: both + pickers go through Avalonia's storage provider and both stage through the cache. See the correction under + the decision above for the shape and for the one thing about it a person will notice — an empty file where + a failed download was pointed. What is *not* built is a folder picker for several downloads at once: the + save picker names one destination, so SAVE FILE takes the selected row. - **Editors.** There is no host editor and no keychain item editor on the phone, so both are create-on- desktop-and-sync. That is why the v2 design's `+` buttons on HOSTS and on the keychain are not drawn. - **Pins and import**, which v2 does not draw either. Teams *is* drawn, behind MORE — it was the one of @@ -590,7 +614,9 @@ What is left, in the order it matters: ## Still open -Neither of these blocks the spike, and both want answering before there is anything to release. +Neither of these blocked the spike, and both wanted answering before there was anything to release. Both +now have answers, kept here under the questions that produced them — the heading stays because what is +worth reading is which way each went and why, not that the list is empty. - ~~**Which Android versions.**~~ **✅ Settled: minSdk 28, targetSdk 36**, and the reasoning divided in two. @@ -606,13 +632,18 @@ Neither of these blocks the spike, and both want answering before there is anyth devices that mostly cannot hold a hardware-backed key anyway — which is the one thing the store is for. API 28 and 29 still cost one branch each in `BiometricGate`, because allowed-authenticator lists arrived in 30. -- **How it is distributed, and what that does to the supply-chain story.** ADR 0001 says plainly that an - operator who wants the secrets attacks the client rather than the crypto, and that release signing with a - key **not held by the server** is what that costs. Play App Signing means Google holds the release key. - That is not necessarily wrong — it is a different, and in some ways better-audited, trust arrangement — - but it is a change to a documented security property of this product, and it should be reasoned about in - an ADR rather than discovered at upload time. Sideloading a self-signed APK preserves the current story and - costs reach. +- ~~**How it is distributed, and what that does to the supply-chain story.**~~ **✅ Settled in + [ADR 0011](adr/0011-android-distribution.md): the project holds the release key, the deployment never + serves the client, and Play is deferred.** The question was whether Play App Signing — Google generating + and holding the release key — is a change to the security property ADR 0001 documents. It is, and the ADR + takes it as a one-way door rather than a setting: a new Play app must use App Bundles and therefore Play + App Signing, an installed app can only be updated by a package signed with the same key, so the first + release picks one identity for good. + + Two things follow for this head *now*, before there is anything to release. CI keeps signing with the + debug key and must never gain a keystore secret. And no download link for the APK may ever be served by a + DodoSSH deployment — that hands the client binary to the party the threat model is about, which is a + worse arrangement than either of the two the question was originally between. ## Smaller things, decided by default diff --git a/docs/crypto.md b/docs/crypto.md index b514afd..c507ce5 100644 --- a/docs/crypto.md +++ b/docs/crypto.md @@ -216,13 +216,27 @@ This is the load-bearing structural choice. Because every wrap protects the *sam ### Why a per-item DataKey -1. **Cheap rotation.** Rotating a vault key re-wraps N × 32-byte data keys and never touches - content blobs. A 10,000-item vault rotates in a few hundred kilobytes of writes. +1. **Cheap rotation.** Re-keying N items re-wraps N × 32-byte data keys and never touches + content blobs. A 10,000-item vault re-keys in a few hundred kilobytes of writes. 2. **Narrow sharing.** A single item can be re-wrapped to another vault key or user key. 3. **Nonce hygiene.** Each key encrypts about one message. 4. **Versioning.** A new item version gets a new data key, so prior ciphertext stays independently decryptable for history and undo. +> **Added 2026-08-03: what a vault key rotation actually does.** Advancing a vault to a new +> generation does **not** re-wrap the items already in it. Each item keeps the generation it was +> sealed under, in its row and in its AAD, so a rotated vault holds items under two or three keys +> at once and every read chooses the key its item names. That is why a member's grants for earlier +> generations are kept rather than revoked, why `VaultSummary` serves all of them, and why sharing +> issues one grant per generation held: a client holding only the newest key would read the vault's +> whole history as tag failures. +> +> Moving those items onto the new key is a **separate pass** (`VaultResealer`), run last and resumable +> because a vault at mixed generations is readable. It opens each item and seals the same plaintext +> again under a fresh data key — the envelope is re-made rather than only the wrap, because the AAD +> binds `keyGeneration` into both. The plaintext is never decoded, so an item written by a newer client +> crosses a rotation untouched. See [ADR 0010](adr/0010-vault-key-rotation.md). + Per-item keys wrapped *to individual users* — which is what would make per-item ACLs cryptographic rather than server-enforced — are deferred to M5. The `content_key_id` column exists from the first migration so that lands without a migration. Until then, **an item ACL diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index 53b82c3..9cf65d9 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -46,7 +46,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > | Logs: a blinking **FOLLOW** pill over a live `tail` | A REFRESH button. These are the keychain's own synced audit records, written once at close and read when the screen opens — there is nothing to tail. | > | Logs: `ALL 8 / INFO 5 / WARN 2 / ERROR 1` and per-host chips | The real two-way choice: connections, or keychain activity. A connection record is not a syslog line and carries no level, and neither log filters by host. | > | SFTP: the `LOCAL / REMOTE` toggle | One remote pane. Scoped storage means there is no browsable local filesystem, which `docs/android-port.md` decided before any of this was built. | -> | SFTP: the transfer card, and moving files at all | ◆ **Browsing, opening and deleting only, and the screen says so.** Both transfer commands exist and both work — against the local pane. `QueueDownloads` writes to `Path.Combine(LocalPath, name)`, and `LocalPath` starts at `SpecialFolder.UserProfile`, which on Android is the application's own private directory: a download would report success and leave the file somewhere the person who asked for it cannot open. That is worse than not offering it, because a refusal is visible and a file in `/data/user/0/…` is not. The queue is not drawn either, since nothing on this head can put anything in it. Both return with the document picker. | +> | SFTP: the transfer card, and moving files at all | ◆ **Both directions ship, and neither is the button the design drew.** There is no local pane for an UPLOAD arrow to point away from, so the gestures are the platform's two pickers: ADD FILES is `ACTION_OPEN_DOCUMENT` — "point at a document wherever it lives" — and SAVE FILE is `ACTION_CREATE_DOCUMENT`, asked *before* the transfer because a picker raised on completion arrives over whatever the person moved on to, and often while the app is backgrounded and Android will not show one. Both stage through the cache, for the same reason: a `content://` URI has no path, no trustworthy length and no promised seek, and the queue needs all three to resume. `QueueDownloads` — the desktop's path, writing into `LocalPath` — is unused here, because `LocalPath` on Android is the app's own private directory. The queue is drawn now, since this head can fill it. | > | S3: `96 objects · 412 GB` and `lifecycle: IA after 7d` | Omitted. Neither a bucket total nor a lifecycle policy is fetched; the listing is what `IRemoteFileStore` returns. | > | A `+` floating button on **HOSTS** | **Shipped in v3.** It was omitted twice, on the grounds that it would open a host editor this head had not got — and that was true until one existed. It raises a sheet rather than a form, because "add" on this screen is two operations: a host, or a group to file hosts under. | > | A 14px rounded inset around the terminal | Not drawn. The renderer is a native child view composited above everything Avalonia draws, so a rounded frame behind it clips nothing. What the design was after comes from the page's own background, which is the design's `#171A26`. | @@ -429,9 +429,12 @@ answerable by anybody willing to create a team first. It simply gets claimed soo | Shared vaults | server + client | A team owns vaults; each is created with the creator's own grant, because a vault with no grant is a container nobody can open. | | Roles | contracts + server | `TeamMemberRole` on the wire, numerically pinned to `DodoSSH.Domain.TeamRole` by a test. Viewer reads, Member writes, Admin and Owner also share and administer. | | Members table | server | `TeamMemberSummary`, and a directory that resolves an exact email to a public key. | -| Sharing an item | client | `VaultSession.ShareVaultAsync`: verify the recipient's key against the key log, wrap, sign, record. The server stores the wrap and the signature and can check neither. | +| Sharing an item | client | `VaultSession.ShareVaultAsync`: verify the recipient's key against the key log, wrap, sign, record. The server stores the wrap and the signature and can check neither. One grant per generation the sharing client holds, so a recipient can read a rotated vault's history and not only what happens next. | +| Adding a member shares the team's vaults | client | Adding somebody wraps every team vault the adding machine can open to them, as part of the add rather than as a button to remember. Membership and a key are still two acts on two machines; the client just performs both. A vault this machine holds no key to is skipped and named. | +| Removing a member rotates the vaults | server + client | `POST /api/v1/vaults/{id}/rekey` advances the generation and records the caller's new grant in one transaction — the server contributes the moment and no cryptography. The client then wraps the new key to the members who remain. Grants for earlier generations are kept, or the vault's stored items would become unreadable to everybody. See [ADR 0010](adr/0010-vault-key-rotation.md). | +| Re-sealing a rotated vault's stored items | client | `VaultResealer`, the last step of a rotation: every item is opened and the same plaintext sealed again under the new key, in batches, against the version the server holds. The plaintext is never decoded, so an item written by a newer client crosses a rotation untouched. Resumable, because a vault at mixed generations is readable — a pass that stops half way leaves a working vault and is re-run. A change queued before the rotation is re-sealed by the push path instead, so nothing reaches the server under a superseded key at all. | | Pending invites, and withdrawing one | server | A `team_invitation` row per (team, address), listed beside the members it is about and withdrawable until it is taken up. It becomes a membership when an account with that address signs in — **and only if the access token asserts `email_verified`**, because membership is authorisation and an invitation anybody could take by naming somebody else's address is a way in. Fourteen days, because an address that is reassigned would otherwise carry a standing offer to whoever holds the job next. | -| Ownership transfer | server | `POST /api/v1/teams/{id}/owner`, owner only. One transaction: the named member becomes owner and the outgoing owner becomes an admin. Not two role changes — ownership is sole, so promoting first leaves the team owned twice and demoting first leaves it owned by nobody. The outgoing owner is demoted rather than removed, because removing them would revoke their vault key grants and flag every team vault for rekey, which is a far larger act than the one being asked for. | +| Ownership transfer | server | `POST /api/v1/teams/{id}/owner`, owner only. One transaction: the named member becomes owner and the outgoing owner becomes an admin. Not two role changes — ownership is sole, so promoting first leaves the team owned twice and demoting first leaves it owned by nobody. The outgoing owner is demoted rather than removed, because removing them would revoke their vault key grants and rotate every team vault, which is a far larger act than the one being asked for. | | `LAST ACTIVE` | server | Real, and coarse on purpose. `UserAccount.LastSeenAtUtc` is now refreshed on ordinary authenticated requests, at most once per account per hour: writing it per request would put an UPDATE on the hot path of every authenticated call and start losing races on `user_account`'s own concurrency token. So the column answers "this week or not", which is the granularity the question is actually asked at, and is shown coarsely rather than to the minute. | | Renaming and archiving a team | server | `PUT` and `DELETE /api/v1/teams/{id}`. The slug is deliberately not renameable: it is unique only among *live* teams, so a rename could take a slug an archived team still holds and strand it. Archiving soft-deletes the team, every membership and every pending invitation in one transaction — and is refused outright while the team owns any vault. | @@ -443,7 +446,6 @@ answerable by anybody willing to create a team first. It simply gets claimed soo | The invitation mail, and **resend** | server | An outbound mail path: an SMTP configuration, a template, a bounce story and a deliverability problem, none of which this server has. | **Nothing is sent, and the interface says so.** An invitation is a standing instruction rather than a message — the next account to sign in with that address joins the team — so there is no token, no link, and nothing to resend. Telling somebody to sign in is done over a channel this server does not carry. A link nobody can deliver would be worse than no link. | | Archiving a team that owns vaults | — | Nothing that would be safe. A team vault resolves through membership, so archiving would take those vaults away from everybody holding a key, silently, including the caller — and nothing in this product deletes a vault, so there is no sequence of calls that turns the refusal into a success. | Refused, with `team-not-empty` and a count of the vaults in the way. A stated limit rather than a coming feature, for the reason the SFTP layer refuses a recursive delete: a refusal is visible and a quiet removal is not. | | `SSO · OIDC · okta.dodotech.dev` | server | Per-team SSO. Authentication is one global JWT scheme bound to one authority. | Omitted. | -| A rekey after a membership change | client | Re-wrapping every item's data key under a fresh vault key, which only a client holding the current one can do. M5. | The vault is flagged `RekeyRequired` and the row says a rotation is owed. | > **The trap this document warned about is still a trap.** `GET /api/v1/meta` advertises > `features: ["teams"]` *unconditionally* (`MetaEndpoints.cs`). It was meaningless when nothing implemented diff --git a/docs/manual-checks.md b/docs/manual-checks.md index 19a75b7..a33fdd8 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -1158,7 +1158,11 @@ follows is about what happens the first time it does. and Bob is now in the members table. 6. **Pass, and this is the half that is easiest to lose:** the vault is in Bob's list **saying it is waiting for a key**, and nothing in it is readable. Have Alice press SHARE KEY and Bob sync; now it - opens. + opens. This is where an invitation differs from an add and the difference is not a defect: adding an + account the directory already knows wraps the key on the spot, because the adding machine has it and + the recipient has a published key to wrap to. An invitation has neither at the moment it is issued — + there is no account yet — and the claim happens on Bob's machine, which holds nothing. So the key is + still owed, and somebody has to hand it over. **Failure means:** step 5 failing with everything else passing is almost always the `email_verified` claim — go to 12.2 rather than reading the invitation code, because the server is doing exactly what it should. @@ -1342,3 +1346,122 @@ rather than two. There is no confirmation prompt, deliberately. **Failure means:** a phone that still unlocks itself after this is the local half not happening, which is the half that matters when the handset is the thing that was lost. + +--- + +## Phase 14 — Moving files to and from the phone's remote + +Every check here needs a real Android device or emulator, a host with SFTP or a bucket, and at least one +document on the phone to send. What is automated is what can be: `TransferQueueingTests` says a staged +upload obeys the same rules as any other and that a delivered download refuses a directory before the +picker's damage is done. What cannot be automated is everything below — the two system pickers are another +application, and the staging copies, the delivery, the notification and the resume are all things only a +running phone does. + +### 14.1 ADD FILES opens the system picker, and takes more than one + +Connect to a host on SFTP, navigate somewhere writable, press **ADD FILES**, and choose two documents in one +go — long-press to multi-select in Android's picker. + +**Pass:** two rows appear in the queue with the names the picker showed, and both land in the directory the +breadcrumb names. The pane's listing shows them after **↻**. + +**Failure means:** one row from a two-document pick is `PickAsync` losing the rest, and two rows with one +name is the per-file staging directory having gone — that is the overwrite `DocumentStaging` documents, and +it silently uploads the same bytes twice. + +### 14.2 The name that arrives is the name that was picked · **the one most likely to be wrong** + +Pick a document whose display name has a space and a non-Latin character in it, and one from a cloud +provider — Drive, or the Downloads shortcut — rather than local storage. + +**Pass:** the file on the host is called what the picker called it. A cloud document uploads too, or fails +with the provider's own message in the status line rather than a crash. + +**Failure means:** a mangled name is `SafeName` over-reaching. A name that reaches the host with a `/` in it +is `SafeName` under-reaching, and that one writes to a path nobody chose. A cloud document that hangs is +the copy being made on the interface thread — the whole reason `CopyInAsync` leaves it. + +### 14.3 The queue is bounded, and the buttons stay reachable + +Queue five or six files at once, on a small phone if there is one. + +**Pass:** the queue scrolls inside its own region and **ADD FILES**, **DELETE** and **CLOSE** are all still +on screen. Every button is a thumb's size. + +**Failure means:** buttons pushed off the bottom is the `MaxHeight` gone from the queue's scroller, and it +makes the screen unusable exactly when somebody has queued the most work. + +### 14.4 A stopped upload resumes rather than starting again + +Start a large upload, press **STOP** part way, then press **RESUME**. + +**Pass:** it carries on from roughly where it stopped rather than from zero — the progress text is the thing +to read. + +**Failure means:** restarting from zero means the staged copy was deleted at the stop, which is precisely +what `QueueStagedUploads` does not do and why it does not. A failure saying the file cannot be found is the +same bug, one step further along. + +### 14.5 The copies do not accumulate · **the one nothing else would catch** + +Note the app's storage in Android Settings → Apps → DodoSSH → Storage. Upload a large file, let it finish, +and look again. Then stop an upload part way, leave it stopped, force-stop the app and relaunch it. + +**Pass:** storage returns to about what it was after the successful upload — the copy is deleted the moment +the transfer completes. After the stopped one, the cache is bigger while the app stays open (the copy is +being kept for RESUME) and back to its old size after the relaunch, which is `DocumentStaging.Sweep`. + +**Failure means:** growth after a successful upload is `ReleaseStaged` not firing, and every file sent +leaves a second copy on the phone until Android reclaims the cache. Growth that survives a relaunch is the +sweep not running. + +### 14.6 The notification is up while it transfers, and gone afterwards + +Queue several files in each direction, put the phone to sleep with the screen off, and wait. + +**Pass:** the foreground notification is up, the transfers finish while the screen is off, and the +notification goes away when the last one does — with no shell open. With a shell open it stays, because that +is what it was already for. + +**Failure means:** an upload that stalls with the screen off is the count not reaching +`SessionForegroundService`, and Android has stopped the process mid-transfer. A notification left up +afterwards is `ActivityChanged` not being subscribed — the other end of the same wire. + +### 14.7 SAVE FILE writes where you pointed it, and the file opens + +Select a file on the host — something with a viewer, an image or a PDF — press **SAVE FILE**, and put it +somewhere reachable: Downloads, or a folder in Drive. When the transfer finishes, open it from the phone's +own Files app. + +**Pass:** the status line says it was saved, the file is where the picker was pointed under the name shown +there, and it opens with the right contents. The queue row says DONE. + +**Failure means:** a row that says DONE with nothing at the destination is `DeliverAsync` never running — +the delivery is registered per transfer id, and losing it makes the download look like a success while the +bytes sit in a cache nobody can reach. A file that is there but empty or truncated is the copy out, not the +transfer: check the `SetLength(0)` and that the write stream is being disposed before the status is written. + +### 14.8 The button is dead until a file is chosen, and refuses a directory + +With nothing selected, look at **SAVE FILE**. Then select a directory row. + +**Pass:** disabled in both cases — it needs a connected remote and a selected *file*, which is the desktop's +own `CanDownload`. + +**Failure means:** an enabled button over a directory reaches `QueueDeliveredDownload`'s refusal, which is +the right answer arriving too late: the save picker has already created an empty file, so the person is left +with a file they did not want and a message saying nothing happened. + +### 14.9 A download that fails leaves the empty file it warned about + +Point SAVE FILE at a destination for a large file, then break the transfer — turn off Wi-Fi and mobile data +while it runs. + +**Pass:** the row goes to FAILED with the reason, the status line does not claim it was saved, and there is +an empty file at the destination. Reconnect, press **RETRY**, and the same destination fills in — the +delivery survives the failure because it is held against the transfer rather than the attempt. + +**Failure means:** a retry that succeeds but leaves the destination empty is the delivery having been +dropped on the failure. An error saying the staged file is missing is the copy having been deleted at the +stop, which is what `QueueDeliveredDownload` documents it does not do. diff --git a/src/DodoSSH.Api/Features/Identity/IdentityService.cs b/src/DodoSSH.Api/Features/Identity/IdentityService.cs index b6951ea..94dfb6d 100644 --- a/src/DodoSSH.Api/Features/Identity/IdentityService.cs +++ b/src/DodoSSH.Api/Features/Identity/IdentityService.cs @@ -81,15 +81,25 @@ internal sealed class IdentityService(DodoDbContext database, IVaultAccessServic { var vault = access.Vault!; - // The grant must match both the current key generation and the exact identity key it - // was wrapped to. A grant left over from a superseded key is not merely stale — the - // client's current private key cannot open it, so offering it would produce a tag - // failure the user reads as data corruption. - var grant = grants.Find(g => - g.VaultId == vault.Id - && g.KeyGeneration == vault.KeyGeneration - && key is not null - && g.RecipientKeyFingerprint.AsSpan().SequenceEqual(key.FingerprintSha256)); + // The grant must match the exact identity key it was wrapped to. One left over from a + // superseded identity key is not merely stale — the client's current private key cannot + // open it, so offering it would produce a tag failure the user reads as data corruption. + var mine = grants + .Where(g => g.VaultId == vault.Id + && key is not null + && g.RecipientKeyFingerprint.AsSpan().SequenceEqual(key.FingerprintSha256)) + .ToList(); + + var grant = mine.Find(g => g.KeyGeneration == vault.KeyGeneration); + + // Everything older, oldest first. A rotation does not re-encrypt what is already stored — + // each item keeps the generation it was sealed under — so a client holding only the + // current key would read the vault's whole history as corrupt. See RekeyVaultRequest. + var prior = mine + .Where(g => g.KeyGeneration < vault.KeyGeneration) + .OrderBy(g => g.KeyGeneration) + .Select(g => new VaultKeyWrap((uint)g.KeyGeneration, g.WrappedKey)) + .ToArray(); summaries.Add(new VaultSummary( VaultId: vault.Id, @@ -103,7 +113,8 @@ internal sealed class IdentityService(DodoDbContext database, IVaultAccessServic // re-wrap it; the client has to say so rather than showing an empty vault. WrappedVaultKey: grant?.WrappedKey, - RekeyRequired: vault.RekeyRequired)); + RekeyRequired: vault.RekeyRequired, + PriorKeyWraps: prior)); } return summaries; diff --git a/src/DodoSSH.Api/Features/Teams/TeamLog.cs b/src/DodoSSH.Api/Features/Teams/TeamLog.cs index 459c5f2..076dee0 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamLog.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamLog.cs @@ -76,6 +76,21 @@ internal static partial class TeamLog internal static partial void GrantRevoked( ILogger logger, Guid vaultId, Guid recipientId, Guid actorId); + /// + /// Warning, because a rotation is the one operation that changes what every other member's key is + /// worth: until each of them is wrapped the new generation, they hold the vault's history and + /// cannot read anything written since. An operator seeing members report an unreadable vault needs + /// this line and its timestamp to explain it. + /// + [LoggerMessage( + EventId = 2115, + Level = LogLevel.Warning, + Message = "Rotated the key of vault {VaultId} to generation {KeyGeneration}, by {ActorId}. " + + "Earlier grants are kept so stored items stay readable; every other member needs the new " + + "generation wrapped to them before they can read anything written from now on.")] + internal static partial void VaultRekeyed( + ILogger logger, Guid vaultId, int keyGeneration, Guid actorId); + [LoggerMessage( EventId = 2108, Level = LogLevel.Information, diff --git a/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs b/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs index f832e12..b62730c 100644 --- a/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs +++ b/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs @@ -186,6 +186,70 @@ internal sealed class IssueVaultGrantEndpoint( } } +/// Moves this vault to a fresh key. +/// +/// Gated on Share rather than on a rotation permission of its own. Rotating decides who can read what +/// is written next, which is the same question sharing and withdrawing answer, and a fourth permission +/// would be a distinction nobody administering a team would be able to explain. +/// +internal sealed class RekeyVaultEndpoint( + ICurrentUserContext currentUser, + IVaultAccessService vaultAccess, + VaultGrantService grants) + : Endpoint, NotFound, ProblemHttpResult>> +{ + /// + public override void Configure() + { + Post("/api/v1/vaults/{vaultId:guid}/rekey"); + + Policies(Auth.EnrolledPolicy); + + Description(b => b + .WithName("RekeyVault") + .WithSummary("Advances this vault's key generation, wrapped to the caller.") + .WithTags("Vaults")); + } + + /// + public override async Task, NotFound, ProblemHttpResult>> ExecuteAsync( + RekeyVaultRequest req, + CancellationToken ct) + { + var user = await currentUser.GetOrProvisionAsync(ct).ConfigureAwait(false); + var access = await vaultAccess + .ResolveAsync(user.Id, Route("vaultId"), ct) + .ConfigureAwait(false); + + if (!access.Granted || !access.Permissions.HasFlag(PermissionFlags.Read)) + { + return TypedResults.NotFound(); + } + + if (!access.Permissions.HasFlag(PermissionFlags.Share)) + { + return Problems.Coded( + StatusCodes.Status403Forbidden, + ProblemCodes.Forbidden, + "You do not have permission to share this vault, so you cannot rotate its key."); + } + + try + { + var summary = await grants + .RekeyAsync(user, access.Vault!, (int)access.Permissions, req, ct) + .ConfigureAwait(false); + + return TypedResults.Ok(summary); + } + catch (VaultGrantInvalidException exception) + { + return Problems.Coded( + StatusCodes.Status400BadRequest, ProblemCodes.InvalidVaultGrant, exception.Message); + } + } +} + /// Withdraws a member's key to this vault. /// /// 404 for a member who holds no live grant, rather than a bland 204, for the reason device diff --git a/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs b/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs index bf84e41..3f2b67e 100644 --- a/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs +++ b/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs @@ -217,6 +217,13 @@ internal sealed class VaultGrantService( } /// Lists who can open a vault. + /// + /// One row per holder, not one per grant. A rotated vault holds several grants per member — one per + /// generation, which is what lets them read its history — and a listing that showed each of them + /// would answer "who can open this" with the same person three times. The row carries the best key + /// they hold: the live grant at the highest generation, or, for somebody whose access has been + /// withdrawn, the most recent grant they had, so the withdrawal is still visible. + /// internal async Task ListGrantsAsync( Vault vault, CancellationToken cancellationToken) @@ -228,13 +235,25 @@ internal sealed class VaultGrantService( .ToListAsync(cancellationToken) .ConfigureAwait(false); + var holders = grants + .GroupBy(g => g.RecipientUserId!.Value) + .Select(group => group + .OrderByDescending(g => g.RevokedAtUtc is null) + .ThenByDescending(g => g.KeyGeneration) + .First()) + + // The order the first grant of each holder was made in, so the list reads as the vault was + // shared rather than reshuffling itself every time somebody is re-wrapped. + .OrderBy(g => grants.Find(first => first.RecipientUserId == g.RecipientUserId)!.CreatedAtUtc) + .ToList(); + return new VaultGrantsResponse( VaultId: vault.Id, KeyGeneration: (uint)vault.KeyGeneration, RekeyRequired: vault.RekeyRequired, Grants: [ - .. grants.Select(g => new VaultGrantSummary( + .. holders.Select(g => new VaultGrantSummary( g.RecipientUserId!.Value, g.RecipientUser?.Email, g.RecipientUser?.DisplayName, @@ -248,10 +267,18 @@ internal sealed class VaultGrantService( /// Wraps a vault key to another member. /// - /// Re-issuing to a recipient who already holds a live grant replaces it in place rather than - /// inserting a second row, because the unique index permits exactly one live grant per recipient - /// per generation — and because the operation somebody is actually performing when they do this - /// is "wrap it again", after a rotation or a botched first attempt. + /// + /// Re-issuing to a recipient who already holds a live grant for that generation replaces it + /// in place rather than inserting a second row, because the unique index permits exactly one live + /// grant per recipient per generation — and because the operation somebody is actually performing + /// when they do this is "wrap it again", after a botched first attempt. + /// + /// + /// A recipient may hold one grant per generation at once, and after a rotation they need to: an item + /// is sealed under whatever generation was current when it was written, so somebody given only the + /// newest key would find everything older unreadable. Which generations get wrapped is the sharing + /// client's decision — it is the only party that can tell which ones it holds. + /// /// internal async Task IssueGrantAsync( UserAccount actor, @@ -264,10 +291,12 @@ internal sealed class VaultGrantService( var granterKey = await RequireCurrentKeyAsync(actor.Id, cancellationToken) .ConfigureAwait(false); + var generation = (int)request.KeyGeneration; + var existing = await database.VaultKeyGrants .SingleOrDefaultAsync( g => g.VaultId == vault.Id - && g.KeyGeneration == vault.KeyGeneration + && g.KeyGeneration == generation && g.RecipientUserId == request.RecipientUserId && g.RevokedAtUtc == null, cancellationToken) @@ -277,7 +306,7 @@ internal sealed class VaultGrantService( { Id = Guid.CreateVersion7(), VaultId = vault.Id, - KeyGeneration = vault.KeyGeneration, + KeyGeneration = generation, Kind = GrantKind.Member, RecipientUserId = request.RecipientUserId, CreatedAtUtc = clock.GetUtcNow(), @@ -304,7 +333,7 @@ internal sealed class VaultGrantService( await database.SaveChangesAsync(cancellationToken).ConfigureAwait(false); TeamLog.GrantIssued( - logger, vault.Id, vault.KeyGeneration, request.RecipientUserId, actor.Id); + logger, vault.Id, generation, request.RecipientUserId, actor.Id); } /// @@ -335,7 +364,11 @@ internal sealed class VaultGrantService( RequireDigest(request.RecipientKeyFingerprint, "recipient key fingerprint"); RequireDigest(request.KeyLogHead, "key log head"); - if (request.KeyGeneration != (uint)vault.KeyGeneration) + // Any generation the vault has actually reached, not only the current one — sharing a rotated + // vault means handing over its history as well as its present. A generation ahead of the + // current one is refused: nothing is sealed under it, so the grant would open nothing, and + // accepting it would let a client move the vault forward without the transaction that does so. + if (request.KeyGeneration is 0 || request.KeyGeneration > (uint)vault.KeyGeneration) { throw new VaultGrantInvalidException( $"This vault is at key generation {vault.KeyGeneration}. A grant for generation " @@ -374,6 +407,170 @@ internal sealed class VaultGrantService( } } + /// + /// Moves a vault to a fresh key generation, wrapped to the caller. + /// + /// The vault as the caller now sees it, at the generation this call created. + /// + /// + /// What the server contributes is the moment, not the key. It cannot generate a vault key, tell + /// that the one it is handed differs from the old one, or check that the caller held the old one at + /// all. What it can do — and what nothing else can — is advance the generation exactly once, so two + /// admins rotating the same vault at the same time do not both walk away believing they succeeded. + /// The stale one's generation is no longer one past the current, and it is refused. + /// + /// + /// Earlier grants are left standing. They are what the remaining members read the vault's + /// history with: an item carries the generation it was sealed under, and nothing here re-encrypts + /// items — only a client holding both keys could. The departed member is cut off by the revocation + /// that removal already performed, which takes every generation they held. + /// + /// + /// The rekey flag is cleared here rather than when the last member is re-wrapped, because it records + /// that a membership change left the vault owing a rotation, and the rotation is this. Who still + /// needs the new key is a different question, and the grant list answers it by generation. + /// + /// + internal async Task RekeyAsync( + UserAccount actor, + Vault vault, + int permissions, + RekeyVaultRequest request, + CancellationToken cancellationToken) + { + var key = await RequireRotatableAsync(actor, vault, request, cancellationToken) + .ConfigureAwait(false); + + var now = clock.GetUtcNow(); + var generation = (int)request.KeyGeneration; + + AddSelfGrant(actor, vault, key, generation, request, now); + + vault.KeyGeneration = generation; + vault.RekeyRequired = false; + vault.RekeyReason = RekeyReason.None; + vault.UpdatedAtUtc = now; + + try + { + // One SaveChanges, so the row and the grant land together. The vault's xmin concurrency + // token is what makes the generation check above binding rather than advisory: a second + // rotation that read the same generation fails here instead of overwriting this one. + await database.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + } + catch (DbUpdateConcurrencyException) + { + // Reported as the same refusal the pre-check gives, because it is the same situation seen a + // moment later — and a 500 about a concurrency token would tell the user nothing they could + // act on. Retrying is safe: the caller generates a fresh key and reads the generation again. + throw new VaultGrantInvalidException( + "Somebody else rotated this vault while this rotation was being recorded. Read it again " + + "and rotate from the generation they left behind."); + } + + TeamLog.VaultRekeyed(logger, vault.Id, generation, actor.Id); + + var prior = await database.VaultKeyGrants + .Where(g => g.VaultId == vault.Id + && g.RecipientUserId == actor.Id + && g.KeyGeneration < generation + && g.State == GrantState.Active + && g.RevokedAtUtc == null) + .OrderBy(g => g.KeyGeneration) + .Select(g => new VaultKeyWrap((uint)g.KeyGeneration, g.WrappedKey)) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + + return new VaultSummary( + VaultId: vault.Id, + Name: vault.Name, + IsPersonal: false, + TeamId: vault.TeamId, + KeyGeneration: request.KeyGeneration, + Permissions: permissions, + WrappedVaultKey: request.WrappedVaultKey, + RekeyRequired: false, + PriorKeyWraps: prior); + } + + /// Records the rotating client's grant for the generation it has just created. + private void AddSelfGrant( + UserAccount actor, + Vault vault, + UserKey key, + int generation, + RekeyVaultRequest request, + DateTimeOffset now) => + database.VaultKeyGrants.Add(new VaultKeyGrant + { + Id = Guid.CreateVersion7(), + VaultId = vault.Id, + KeyGeneration = generation, + Kind = GrantKind.Member, + RecipientUserId = actor.Id, + RecipientKeyFingerprint = key.FingerprintSha256, + WrappedKey = request.WrappedVaultKey, + GranterUserId = actor.Id, + GranterKeyFingerprint = key.FingerprintSha256, + + // No key log head, as every self-grant carries none: there is no third party whose key + // could have been substituted when you wrap something to yourself. + KeyLogHead = null, + Signature = request.GrantSignature, + State = GrantState.Active, + CreatedAtUtc = now, + }); + + /// + /// Everything that can be checked about a rotation before it is recorded. + /// + /// The caller's current identity key, which the new grant is filed against. + private async Task RequireRotatableAsync( + UserAccount actor, + Vault vault, + RekeyVaultRequest request, + CancellationToken cancellationToken) + { + if (vault.OwnerKind != VaultOwnerKind.Team || vault.TeamId is null) + { + throw new VaultGrantInvalidException( + "Only a team vault can be rotated. A personal vault has one reader, so a rotation " + + "would re-wrap a key to the same person and change nothing about who can read it."); + } + + RequireWrappedKey(request.WrappedVaultKey); + RequireSignature(request.GrantSignature); + + if (request.KeyGeneration != (uint)vault.KeyGeneration + 1) + { + throw new VaultGrantInvalidException( + $"This vault is at key generation {vault.KeyGeneration}, so the next one is " + + $"{vault.KeyGeneration + 1} and not {request.KeyGeneration}. Read the vault again — " + + "somebody else has rotated it since you last looked."); + } + + var key = await RequireCurrentKeyAsync(actor.Id, cancellationToken).ConfigureAwait(false); + + // Held now, not merely permitted. The new key has to be wrapped from the old one, and an + // account that cannot open the current generation cannot have done that — so a request from + // one is either a mistake or an attempt to strand every other member behind a key nobody has. + var holdsCurrent = await database.VaultKeyGrants + .AnyAsync( + g => g.VaultId == vault.Id + && g.KeyGeneration == vault.KeyGeneration + && g.RecipientUserId == actor.Id + && g.State == GrantState.Active + && g.RevokedAtUtc == null, + cancellationToken) + .ConfigureAwait(false); + + return holdsCurrent + ? key + : throw new VaultGrantInvalidException( + "You hold no key to this vault at its current generation, so you cannot rotate it. Ask " + + "a member who does."); + } + /// /// Withdraws a member's key grant. /// diff --git a/src/DodoSSH.Api/Setup/EndpointRegistration.cs b/src/DodoSSH.Api/Setup/EndpointRegistration.cs index c9e266f..7ddbd54 100644 --- a/src/DodoSSH.Api/Setup/EndpointRegistration.cs +++ b/src/DodoSSH.Api/Setup/EndpointRegistration.cs @@ -60,10 +60,11 @@ internal static class EndpointRegistration typeof(ListVaultGrantsEndpoint), typeof(IssueVaultGrantEndpoint), typeof(RevokeVaultGrantEndpoint), + typeof(RekeyVaultEndpoint), // Registered as each feature lands: // Identity — key rotation, passphrase change - // Vaults — rekey, per-item ACLs + // Vaults — per-item ACLs // Relay — tickets and the WebSocket // Audit, Admin }); diff --git a/src/DodoSSH.Client.Android/App.axaml.cs b/src/DodoSSH.Client.Android/App.axaml.cs index 25ca427..c780419 100644 --- a/src/DodoSSH.Client.Android/App.axaml.cs +++ b/src/DodoSSH.Client.Android/App.axaml.cs @@ -74,27 +74,37 @@ public sealed partial class DodoSshApp : Avalonia.Application workspace.Start(); + // Before anything can queue a transfer, which is the only moment at which emptying this is + // provably safe. What it clears is the copy a stopped upload leaves behind on purpose — kept so + // RESUME has something to read — and whatever a process death interrupted. See DocumentStaging. + DocumentStaging.Sweep(); + + var viewModel = ComposeShell(paths, caches, workspace, knownHosts, connections); + // Difference 2: the foreground service, which is what makes TerminalWorkspace's promise — that a // shell outlives a vault lock — true on a platform that stops backgrounded processes. // - // Still zero transfers, and the reason moved rather than went away. v2 built the files screen, so - // this head can now browse a remote — but it cannot start a transfer, because both directions need - // the system document picker that scoped storage forces and that is not built (see FilesScreen). - // So the count is zero because the queue provably cannot have anything in it, not because nothing - // was wired. This is still the seam it arrives through: when the picker lands, this reads the - // queue and Refresh() gets called as transfers start and finish. + // The transfer count is real now that the document picker gives this head a way to start one, and + // it is the half that matters most here: a shell survives backgrounding because somebody is looking + // at it, and an upload has to survive precisely when nobody is — the screen is off and the phone is + // in a pocket. Queued counts as active, so putting five files in the queue and locking the phone + // moves five files. + // // A local rather than a field, matching the desktop head: an Avalonia Application has no disposal // hook, so a field holding a disposable would have nowhere honest to release it. It stays alive // because it is subscribed to the workspace, which lives as long as the process. - // - // Refresh() is called once here. Calling it again when a shell opens is what the terminal screen - // will wire, and there is nothing to wire it to yet — the workspace announces sessions ending on - // its own, which is the half that would otherwise leave a notification up over nothing. - var keepAlive = new SessionKeepAlive(workspace, activeTransfers: () => 0); + var keepAlive = new SessionKeepAlive( + workspace, + activeTransfers: () => viewModel.Transfers.ActiveTransfers); + + // The other end of the same wire: the workspace announces its own sessions ending, and the queue + // announces transfers appearing and finishing. Without this the notification would come up when an + // upload started and stay up after it finished, which is the failure this class exists to prevent. + viewModel.Transfers.ActivityChanged += (_, _) => keepAlive.Refresh(); keepAlive.Refresh(); - return new PhoneShell { DataContext = ComposeShell(paths, caches, workspace, knownHosts, connections) }; + return new PhoneShell { DataContext = viewModel }; } /// diff --git a/src/DodoSSH.Client.Android/Platform/DocumentStaging.cs b/src/DodoSSH.Client.Android/Platform/DocumentStaging.cs new file mode 100644 index 0000000..513001c --- /dev/null +++ b/src/DodoSSH.Client.Android/Platform/DocumentStaging.cs @@ -0,0 +1,220 @@ +using Avalonia.Controls; +using Avalonia.Platform.Storage; + +namespace DodoSSH.Client.Android.Platform; + +/// +/// Turns documents chosen in the system picker into ordinary local files the transfer queue can upload. +/// +/// +/// +/// The way in, and the only one this head has. Android has no browsable local filesystem for a +/// second pane to show — the decision docs/android-port.md took before any of this was built — so a file +/// leaves this phone by being pointed at in the system picker, which hands back a content:// URI +/// belonging to whichever app owns the document. +/// +/// +/// Copied rather than streamed, and that is a requirement rather than a shortcut. A document URI has +/// no path behind it, its stream is not promised to be seekable, and the grant that opens it can be revoked +/// or the document edited while an upload is in flight. FileTransferQueue needs all three of the +/// things that costs: a path, a length it can trust, and a seek so a resumed upload starts from the byte +/// the last attempt reached. So the document is copied into this application's own cache first and the copy +/// is what gets queued — a real file, behaving like every other thing in that queue. +/// +/// +/// One directory per file, named by a UUIDv7. Two documents chosen in one go can have the same +/// display name, and two picks a minute apart certainly can; a shared staging directory would make the +/// second copy overwrite the first, which is a data-loss bug that only shows up when somebody uploads two +/// files called config. The directory is the uniqueness, so the file inside it can keep the name the +/// picker gave it — which is the name the remote end gets, because QueueUploads takes it from the +/// path. +/// +/// +internal static class DocumentStaging +{ + /// Everything staged in either direction, under one directory so a sweep is one call. + private static string Root => + Path.Combine(PhoneEnvironment.CacheDirectory, "staging"); + + /// A path in the staging area for a file of this name, with the directory made. + /// + /// One directory per file, named by a UUIDv7 — see the type's own remarks for why the uniqueness + /// is the directory rather than the name. Used by both directions: a document copied in for upload, and + /// a download on its way out to the document the save picker made. + /// + internal static string NewStagingPath(string? name) + { + var folder = Path.Combine(Root, Guid.CreateVersion7().ToString("n")); + Directory.CreateDirectory(folder); + + return Path.Combine(folder, SafeName(name)); + } + + /// + /// Asks for documents and copies each one into the cache, returning the paths of the copies. + /// + /// + /// Multiple by design: the queue moves one file at a time, but choosing them is a trip out to another + /// application and back, and making somebody take that trip once per file is the kind of thing a phone + /// is judged on. An empty list means the picker was dismissed, which is not an error and is reported as + /// nothing having happened rather than as a failure. + /// + internal static async Task> PickAsync( + TopLevel top, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(top); + + var chosen = await top.StorageProvider + .OpenFilePickerAsync(new FilePickerOpenOptions + { + Title = "Files to upload", + AllowMultiple = true, + }) + .ConfigureAwait(true); + + if (chosen.Count == 0) + { + return []; + } + + var staged = new List(chosen.Count); + + foreach (var document in chosen) + { + staged.Add(await CopyInAsync(document, cancellationToken).ConfigureAwait(true)); + } + + return staged; + } + + /// + /// Asks where a download should end up, and returns the document the picker made. + /// + /// + /// + /// ACTION_CREATE_DOCUMENT, which is the only way a file leaves this application: everything else + /// on this phone is either the app's own private storage or somewhere it has no permission to write. + /// The suggested name is the remote file's, because that is the name the person was looking at when + /// they pressed the button, and they can change it in the picker like any other save. + /// + /// + /// The document exists as soon as this returns — the picker creates it, empty, and a download + /// that then fails leaves that empty file behind. Nothing here can prevent it: the alternative is + /// raising the picker after the transfer, minutes later, over whatever the person moved on to, and on + /// Android often while the application is backgrounded and cannot show one at all. + /// + /// + internal static async Task PickDestinationAsync(TopLevel top, string name) + { + ArgumentNullException.ThrowIfNull(top); + + return await top.StorageProvider + .SaveFilePickerAsync(new FilePickerSaveOptions + { + Title = "Save file", + SuggestedFileName = name, + ShowOverwritePrompt = true, + }) + .ConfigureAwait(true); + } + + /// Copies a finished download out to the document the picker made. + /// + /// The write is truncating rather than appending, which matters on a retry: the picker's document is + /// created when it is dismissed and a second attempt writes over the empty — or partly written — file + /// rather than after it. + /// + internal static async Task DeliverAsync(IStorageFile destination, string localPath) + { + ArgumentNullException.ThrowIfNull(destination); + + var source = new FileStream( + localPath, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 81920, useAsync: true); + + await using (source.ConfigureAwait(false)) + { + var target = await destination.OpenWriteAsync().ConfigureAwait(false); + + await using (target.ConfigureAwait(false)) + { + if (target.CanSeek) + { + target.SetLength(0); + } + + await source.CopyToAsync(target).ConfigureAwait(false); + } + } + } + + /// + /// Deletes everything left in the staging directory. + /// + /// + /// Called once at composition, before anything can have queued a transfer, which is what makes deleting + /// the lot safe: at that moment nothing in there belongs to a transfer that could still want it. What it + /// is for is the residue a stopped upload leaves deliberately — the copy is kept so RESUME has something + /// to read — a download whose delivery failed, and whatever a process death left behind mid-copy. + /// + internal static void Sweep() + { + try + { + if (Directory.Exists(Root)) + { + Directory.Delete(Root, recursive: true); + } + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // A cache that could not be emptied is not a reason to refuse to start. Android reclaims this + // directory itself when the device runs short of storage. + } + } + + private static async Task CopyInAsync(IStorageFile document, CancellationToken cancellationToken) + { + var path = NewStagingPath(document.Name); + + // Off the interface thread from here down — this is a byte copy of something that can be a hundred + // megabytes, and nothing in it touches a control. The caller's own await is what comes back to the + // interface thread to queue the result. Hence the two-step disposal: configuring the await on a + // using declaration would leave the variable a ConfiguredAsyncDisposable rather than a stream. + var source = await document.OpenReadAsync().ConfigureAwait(false); + + await using (source.ConfigureAwait(false)) + { + var target = new FileStream( + path, FileMode.CreateNew, FileAccess.Write, FileShare.None, bufferSize: 81920, useAsync: true); + + await using (target.ConfigureAwait(false)) + { + await source.CopyToAsync(target, cancellationToken).ConfigureAwait(false); + } + } + + return path; + } + + /// + /// The display name reduced to something that can be a file name here and a name on the remote. + /// + /// + /// A picker's display name comes from whichever application owns the document and is not obliged to be + /// a valid file name — it can carry a separator, which without this would write outside the directory + /// staging just made, and would then be joined onto the remote path by QueueUploads. Both are + /// worth refusing at the one point where the name enters this application. + /// + private static string SafeName(string? name) + { + var trimmed = Path.GetFileName(name ?? string.Empty).Trim(); + + if (trimmed.Length == 0 || trimmed is "." or "..") + { + return "file"; + } + + return string.Join('_', trimmed.Split(Path.GetInvalidFileNameChars())); + } +} diff --git a/src/DodoSSH.Client.Android/Platform/PhoneEnvironment.cs b/src/DodoSSH.Client.Android/Platform/PhoneEnvironment.cs index 9b4936f..58b204a 100644 --- a/src/DodoSSH.Client.Android/Platform/PhoneEnvironment.cs +++ b/src/DodoSSH.Client.Android/Platform/PhoneEnvironment.cs @@ -77,6 +77,18 @@ internal static class PhoneEnvironment } } + /// Where this phone keeps copies that only have to survive the thing that made them. + /// + /// cacheDir — per-app like , and unlike it, reclaimable: Android deletes from + /// here when the device runs short of storage. That is the right trade for the upload staging in + /// , whose files are worthless the moment their transfer finishes, and it + /// is why the profile is not kept here. The cost is stated rather than hidden: a file reclaimed under + /// storage pressure while its upload is still running fails that upload. + /// + public static string CacheDirectory => + Require().CacheDir?.AbsolutePath + ?? throw new InvalidOperationException("Android returned no cacheDir for this application."); + /// /// The activity currently on screen, or null while the app is backgrounded. /// diff --git a/src/DodoSSH.Client.Android/Views/FilesScreen.axaml b/src/DodoSSH.Client.Android/Views/FilesScreen.axaml index 2d31c00..514a71d 100644 --- a/src/DodoSSH.Client.Android/Views/FilesScreen.axaml +++ b/src/DodoSSH.Client.Android/Views/FilesScreen.axaml @@ -19,17 +19,28 @@ browsable local filesystem to put in the other half. TransfersViewModel's local pane — LocalPath, LocalRoots, LocalEntries — is desktop-only and is left alone here rather than shown empty. - ◆ **And that is why neither DOWNLOAD nor UPLOAD is on this screen.** Both commands exist and both work; - what they work *against* is the local pane. `QueueDownloads` writes to `Path.Combine(LocalPath, name)`, - and `LocalPath` starts at `LocalDirectory.Home` — `SpecialFolder.UserProfile`, which on Android is the - application's own private directory. A download would report success and put the file somewhere the - person who asked for it cannot open it, which is worse than not offering it: a refusal is visible and a - file in `/data/user/0/…` is not. The way in and out is the system document picker, which is the shape - docs/android-port.md decided on and is the next piece of work. + ◆ **ADD FILES is the way in, and it is the system document picker rather than an UPLOAD button.** There + is nothing local to select from, so the gesture cannot be "choose on the left, press the arrow": it is + "point at a document wherever it lives, and it goes to the directory showing". What Android hands back + is a `content://` URI, so `DocumentStaging` copies it into this application's cache and queues the copy — + the queue needs a path, a length and a seek, and a document URI promises none of the three. See that + class for why the copy is a requirement rather than a shortcut, and `QueueStagedUploads` for when it is + deleted again. - So what ships is browsing a remote, and the two remote-side operations that need nothing local — - opening a directory and deleting. The queue is drawn because a transfer can still be running when this - screen is opened; it is simply not something this head can start yet. + ◆ **SAVE FILE is the way out, and it is the save picker rather than a DOWNLOAD button.** `QueueDownloads` + writes to `Path.Combine(LocalPath, name)`, and `LocalPath` on Android is the application's own private + directory — a download that way would report success and leave the file where the person who asked for + it cannot open it. So this head does not use it: `QueueDeliveredDownload` runs the transfer into the + cache and hands the finished bytes to the document `ACTION_CREATE_DOCUMENT` made. + + The destination is chosen *before* the transfer, which is a decision with a visible cost — the picker + creates the document when it is dismissed, so a download that then fails leaves an empty file where it + was pointed. The alternative is a picker raised minutes later, over whatever the person moved on to and + frequently while this application is backgrounded, where Android will not show one at all. + + So what ships is browsing a remote and moving files both ways, plus the two remote-side operations that + need nothing local — opening a directory and deleting. The queue is drawn under the actions, because + this head can now fill it. ◆ **The host key prompts are here too.** File transfer is a second, separate authenticated connection and it makes its own trust decision — the host records a second login. So this screen carries its own @@ -255,6 +266,70 @@ + + + + + + + + + + + + + + + + +