diff --git a/README.md b/README.md index 715f08c..f0671be 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ src/ DodoSSH.Client.Terminal the loopback data plane and credit-based flow control DodoSSH.Client.Transfer the transfer queue, part files and resume, and the local file listing DodoSSH.Client.ObjectStore S3-compatible buckets, behind the same interface as SFTP - DodoSSH.Client.Import reading ~/.ssh/config, with no I/O of its own + DodoSSH.Client.Import reading ~/.ssh/config, and the keys it names when asked DodoSSH.Client.Shell the view models both heads drive, the renderer's files, the palette DodoSSH.Client.App the desktop head: its views, and its Windows integration DodoSSH.Client.Android the phone head: its views, and its Android integration @@ -231,6 +231,24 @@ Windows — is offered neither button and keeps asking for the passphrase, which rather than leaving you to notice. The passphrase never stops working either way: a declined confirmation leaves the box exactly where it was. +**Importing an `~/.ssh/config` can bring the keys with it, and only if you ask.** Preferences → IMPORT +HOSTS scans the file and shows what it found; nothing is stored until you press the button. Below the list +there is one tick — *Also import the private keys these hosts point at* — and it starts off. + +With it off, an `IdentityFile` becomes a note recording the path, and the host asks for a password until you +bind it to a key yourself. With it on, IMPORT reads each host's first `IdentityFile` out of `~/.ssh`, stores +it in the vault encrypted like any other key, and binds the host to it. One key is stored per **file** +however many entries name it, and a file already in your keychain is bound to rather than stored twice, so +running the import again does not fill the keychain with copies. + +**This is the only place DodoSSH opens a private key you did not point it at file by file**, which is why +the tick is off, why the sentence beside it names the directory, and why nothing is read during SCAN — you +can tick it, read what it says and untick it, and no key will have been opened. A key protected by a +passphrase comes in without one, because nothing on disk says what the passphrase is; the report under the +button names those files, and the keychain's editor is where the passphrase goes. A file that is not on this +machine — the ordinary case for a config carried from another one — leaves its host imported and unbound, +and is named in the same report. + ### Moving files **SFTP** and **S3** in the tab strip are a two-pane browser: this machine on the left, the remote on the diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index 0cae216..b0c0258 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -210,8 +210,10 @@ saved, and there is no frame on the terminal data plane that would carry a chang **Three things the design did not ask for and this build now has.** A key can be generated in the client rather than pasted in (`SshKeyGenerator`, and the `openssh-key-v1` container is written by hand — see `OpenSshKeyWriter` for why there was no alternative and why it is written unencrypted). Hosts can be -imported from `~/.ssh/config` (`DodoSSH.Client.Import`; it reads no key material, and `ProxyJump` is -recorded as intent because the SSH layer still has no jump hosts). And the file-transfer screen takes drag +imported from `~/.ssh/config` (`DodoSSH.Client.Import`; `ProxyJump` is recorded as intent because the SSH +layer still has no jump hosts, and the private keys the config names come in **only behind a tick that +starts off** — the one control in this application that opens key material out of a directory the user did +not point at file by file, which is why nothing is read until IMPORT is pressed). And the file-transfer screen takes drag and drop in four directions — remote to Explorer is the one that does not ship, because it needs a virtual file the platform layer cannot supply; see `docs/manual-checks.md`. diff --git a/docs/manual-checks.md b/docs/manual-checks.md index 4a006c1..c87b45c 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -218,12 +218,57 @@ Scan, then navigate away without importing. ### 2.9 Imported hosts are correct -Import a couple, then open one on the Hosts screen. +Import a couple with the key tick **off**, then open one on the Hosts screen. **Pass:** the address, port and username match the config, and the notes record any `IdentityFile` path and -any `ProxyJump` — with `ProxyJump` clearly stated as not routing. Connecting should ask for a password even -where the config named a key, because **no key material is read**; binding it to a key in the keychain is a -separate act. +any `ProxyJump` — with `ProxyJump` clearly stated as not routing. Connecting asks for a password even where +the config named a key, because with the tick off **no key material is read**; binding it to a key in the +keychain is a separate act. + +### 2.10 The key tick reads `~/.ssh`, and only when it is ticked · **the one worth the most care** + +Scan a config that names at least one `IdentityFile`. + +**Pass:** below the list there is a tick reading *Also import the private keys these hosts point at*, and it +is **off**. On a config that names no `IdentityFile` anywhere there is no tick at all. + +Turn it on without pressing anything else. + +**Pass:** a warning appears naming what IMPORT will read, and the rows whose config named a key change from +*a key on disk* to *a key, imported*. Nothing has been read yet — turn it off again and navigate away, and +the point is that no private key was ever opened. + +Turn it on and press IMPORT. + +**Pass:** the keychain holds one key per **file**, not per host — a config pointing twelve entries at one +`~/.ssh/id_ed25519` produces one key, and all twelve hosts show `key` as their authentication. The key's +notes name the file it came from. A report appears under the button with one line per file. + +**Pass, and this is the half that is easiest to lose:** for a key protected by a passphrase, the report says +so and names the file. The key is still imported; what it does not have is the passphrase, which is nowhere +on disk to be read. Add it on the keychain screen and the host connects. + +Now import the same config again. + +**Pass:** the hosts duplicate — a second bookmark for one machine is allowed and takes a click — and the +**key does not**. The report should be identical and the keychain should still hold one. + +**Failure means:** a tick that is on by default, or one that reads keys during SCAN, is the failure that +matters here — this is the only control in the application that opens private key material out of a +directory the user did not point at file by file, and the whole of what makes it acceptable is that it took +a deliberate press. A second copy of a key on a re-import means the material comparison in +`VaultViewModel.ImportHostsAsync` is not finding what is already there, which turns "import my config" +into a way to fill a keychain with duplicates of one file. + +### 2.11 A key file that is not on this machine leaves the host alone + +Point an `IdentityFile` at a path that does not exist, tick the box and import. + +**Pass:** the host is imported, unbound, exactly as it would have been with the tick off — and the report +names the file and says there is no such file. One bad path must not stop the other entries. + +**Failure means:** an import that stopped, or a host that vanished, on a config carried from another +machine. That is the ordinary case rather than an error. --- @@ -237,27 +282,27 @@ covered by nothing at all. Connect the SFTP page to a host first. All four of these should queue transfers. -### 2.10 Explorer → remote pane +### 2.12 Explorer → remote pane Drag one file, then several, from Explorer onto the right-hand pane. **Pass:** the pane outlines in accent colour while the pointer is over it, and the drop queues one transfer per file into the directory showing. -### 2.11 Local pane → remote pane +### 2.13 Local pane → remote pane **Pass:** as above. This uses the same platform file format as the Explorer drag, so a failure here with -2.10 passing points at the drag *source*, not the drop target. +2.12 passing points at the drag *source*, not the drop target. -### 2.12 Remote pane → local pane +### 2.14 Remote pane → local pane **Pass:** the left pane outlines and the drop queues a download. -### 2.13 Local pane → Explorer +### 2.15 Local pane → Explorer **Pass:** the file copies out. -### 2.14 The highlight clears · **the one most likely to be wrong** +### 2.16 The highlight clears · **the one most likely to be wrong** Drag something over a pane and then out of it again without dropping. @@ -268,13 +313,13 @@ why the layout test cannot catch it — but once visible it swallows the `DragOv the pointer appears to leave immediately, the highlight sticks, and the drop lands nowhere. The fix is `IsHitTestVisible="False"` on the highlight `Border` in `TransfersScreen.axaml`. -### 2.15 Dropping while disconnected +### 2.17 Dropping while disconnected Disconnect, then drag a file over the remote pane. **Pass:** the pane outlines in red and says "Connect to a host first." Nothing is queued on drop. -### 2.16 A click still selects a row +### 2.18 A click still selects a row Click rows in both panes, and drag a row a few pixels without releasing. @@ -794,7 +839,7 @@ around the cards. space around the cards connects to nothing. **Failure means:** the 5-pixel threshold in `HostsScreen.axaml.cs` is not doing its job — the same failure -as 2.16 on the other screen, and here it would make the grid unusable. Empty space that connects means the +as 2.18 on the other screen, and here it would make the grid unusable. Empty space that connects means the double-tap handler has lost its check that the pointer was over a card. **And the same two gestures on the group cards above**, where they mean select and open rather than select diff --git a/src/DodoSSH.Client.App/Views/ImportScreen.axaml b/src/DodoSSH.Client.App/Views/ImportScreen.axaml index d5bd3f5..daff5e7 100644 --- a/src/DodoSSH.Client.App/Views/ImportScreen.axaml +++ b/src/DodoSSH.Client.App/Views/ImportScreen.axaml @@ -15,6 +15,17 @@ Reachable from the preferences screen and not from the nav rail. It is a task rather than a destination — done once, or once a year — and a seventh rail entry would cost every screen a slot for something almost nobody is looking at. + + ── ◆ THE ONE TICK THAT READS PRIVATE KEYS ───────────────────────────────────────────────────────────── + Below the list, off, and drawn only where the scan actually found an IdentityFile. It is the only control + in this application that opens private key material out of a directory the user did not point at file by + file, so three things about how it is drawn are load-bearing rather than tidy: it is a default nobody + arrives at by accident, the sentence beside it names the directory rather than saying "keys", and nothing + is read until IMPORT is pressed — so ticking it, reading the sentence and unticking it costs nothing. + + What comes back afterwards is the report under the list: one line per key file, saying which were stored, + which are protected by a passphrase this cannot know, and which were not there at all. That is reported + rather than previewed for the same reason — previewing would mean reading them. --> @@ -100,17 +111,57 @@ IsVisible="{Binding HasRows}"> + IsVisible="{Binding !ImportsKeys}" + Text="Key files are not read. Where ssh_config names an IdentityFile the path is recorded as a note, and the host asks for a password until you bind it to a key in your keychain." /> + + + + + + + + +