From 82e35190c898fc68f4f931efb0d86cfe9d801a07 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 4 Oct 2026 04:58:52 +0000 Subject: [PATCH] README child-mnemonic vector and host-key note; ssh install refuses a ~/.ssh it cannot enter (closes #51) The README gains the child mnemonic that `keyfunc mnemonic` prints for the abandon ... about mnemonic at index 0, asserted by the README vectors test, and says that `ssh install` needs the host key already known, with the two ways round it. `ssh install` now also lists `.ssh/.` before fetching. A `.ssh` that can be read but not entered lists as empty, so the fetch read it as holding no file and tried an upload that could only fail. The listing of `.ssh/.` fails instead, and the tool refuses before any upload, saying the directory cannot be entered. The test that failed the put with a file where `.ssh` belongs now uses a `.ssh` of mode 500. Model: opus-5-5 --- README.md | 53 +++++++++++++++--------- internal/cli/cli_test.go | 8 ++++ internal/cli/ssh/install.go | 58 ++++++++++++++++---------- internal/cli/ssh/install_test.go | 16 ++++++-- internal/cli/ssh_test.go | 70 ++++++++++++++++++++++++-------- 5 files changed, 142 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index fda0e0a..5d56d1b 100644 --- a/README.md +++ b/README.md @@ -162,21 +162,23 @@ Adds the `pub` line to `~/.ssh/authorized_keys` on the host. No command is run on the host: the file is fetched, changed here, and written back with the system `sftp` client in batch mode. -The first connection lists `~/.ssh` and then fetches `~/.ssh/authorized_keys` -from it. The file reads as empty in two cases only: `sftp` reported `~/.ssh` -itself as not being there, or the listing came up and the file was not in it. -Any other outcome of that connection fails the run — a `~/.ssh` that is there -but cannot be entered, an `authorized_keys` that is there but cannot be read, or -a connection that did not come up — and the tool prints what `sftp` said and -exits with status 1 without writing anything, rather than put a file back -holding the new key alone. The listing is what tells a missing directory from -one shut to the user, which `sftp` reports on a fetch the same way; the wording -of a missing file elsewhere does not count either, since `ssh` writes -`No such file or directory` about an `-i` it cannot find on a session that then -authenticates through the agent. If an identical line is already in the file, -the tool prints `already present` and connects no further. Otherwise the line is -added (after a newline, if the file did not end with one) and a second -connection: +The first connection lists `~/.ssh`, then `~/.ssh/.`, and then fetches +`~/.ssh/authorized_keys`. The file reads as empty in two cases only: `sftp` +reported `~/.ssh` itself as not being there, or both listings came up and the +file was not found. Any other outcome of that connection fails the run — a +`~/.ssh` that is there but cannot be read or entered, an `authorized_keys` that +is there but cannot be read, or a connection that did not come up — and the tool +prints what `sftp` said and exits with status 1 without writing anything, rather +than put a file back holding the new key alone. The listings are what tell a +missing directory from one shut to the user, which `sftp` reports on a fetch the +same way: one that cannot be read fails the first listing, and one that can be +read but not entered fails the second, after which the tool says that `~/.ssh` +cannot be entered. The wording of a missing file elsewhere does not count +either, since `ssh` writes `No such file or directory` about an `-i` it cannot +find on a session that then authenticates through the agent. If an identical +line is already in the file, the tool prints `already present` and connects no +further. Otherwise the line is added (after a newline, if the file did not end +with one) and a second connection: - makes `~/.ssh` and sets it to mode `0700`, but only when the first connection found none; a `~/.ssh` that was already there keeps the mode it had; @@ -199,7 +201,10 @@ error, so the tool's own standard output is only `added` or `already present`. Anything after `--` is passed to `sftp` unchanged, which is where the port goes (`-P 2222`, not `-p`). How the connection authenticates is up to the user's normal `ssh` setup, except that batch mode does not prompt: a key or an agent -has to do it, not a typed password. +has to do it, not a typed password. Nor does it ask whether to trust a host key +it has not seen, so the host has to be in `known_hosts` already, or the run +fails with `Host key verification failed`. Connect to the host once with `ssh` +first, or pass `-o StrictHostKeyChecking=accept-new` after `--`. ### `keyfunc ssh to [ssh arguments...]` @@ -260,10 +265,18 @@ A child mnemonic is a full mnemonic in its own right: it can seed another `keyfunc`, another wallet, or `secret`, and it never has to be written down, since it can be derived again. -Test vector: the child-mnemonic step is checked against BIP-85's own published -vectors, which derive from the specification's master key -`xprv9s21ZrQH143K2LBWUUQRFXhucrQqBpKdRRxNVq2zBqsx8HVqFk2uYo8kmbaLLHRdqtQpUm98uKfu3vca1LqdGhUtyoFnCNkfmXRyPXLjbKb`. -At key index 0 the 12-word English child mnemonic is: +Test vector, mnemonic +`abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about`: + +``` +index 0: prosper short ramp prepare exchange stove life snack client enough purpose fold +``` + +The child-mnemonic step is also checked against BIP-85's own published vectors. +Those start from the specification's master key +`xprv9s21ZrQH143K2LBWUUQRFXhucrQqBpKdRRxNVq2zBqsx8HVqFk2uYo8kmbaLLHRdqtQpUm98uKfu3vca1LqdGhUtyoFnCNkfmXRyPXLjbKb` +rather than from a mnemonic, so they cannot be given to `keyfunc`; at key index +0 the 12-word English child mnemonic of that key is: ``` girl mad pet galaxy egg matter matrix prison refuse sense ordinary nose diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 2f46688..6c652e6 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -22,6 +22,11 @@ const ( "0I4FKs+eVUulTPHfk9VtXw1tMF" ) +// The child mnemonic the README says the example mnemonic gives at +// index 0. +const childZero = "prosper short ramp prepare exchange stove life " + + "snack client enough purpose fold" + // The two child mnemonic lengths the tests ask for. const ( twelve = 12 @@ -45,6 +50,9 @@ func TestTheReadmeTestVectors(t *testing.T) { vectorOne+" keyfunc/ssh/1", strings.TrimSpace(run(t, "ssh", "pub", "-n", "1")), ) + require.Equal(t, + childZero, strings.TrimSpace(run(t, "mnemonic", "-n", "0")), + ) } func TestTheCommentCanBeChosen(t *testing.T) { diff --git a/internal/cli/ssh/install.go b/internal/cli/ssh/install.go index 761fe3c..162bd23 100644 --- a/internal/cli/ssh/install.go +++ b/internal/cli/ssh/install.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/rand" "encoding/hex" + "errors" "fmt" "os" "os/exec" @@ -32,6 +33,12 @@ const ( localMode = 0o600 ) +// ErrCannotEnter is the refusal of a host whose .ssh is there but +// cannot be entered, so that nothing in it can be read or written. +var ErrCannotEnter = errors.New( + "~/.ssh is there on the host but cannot be entered", +) + // install returns the command that adds the public key to a host. func install() *cobra.Command { cmd := &cobra.Command{ @@ -199,30 +206,39 @@ func merge(content, line string) (string, bool) { // fetch brings the host's authorized_keys into the given path and // returns what is in it, and whether the .ssh directory was already -// there. The one session lists .ssh and then gets the file, so the -// listing settles the state of the directory before the get is read. +// there. The one session lists .ssh, then .ssh/., and then gets the +// file, so the listings settle the state of the directory before the +// get is read. // // The file reads as empty in just two cases: sftp reported .ssh itself -// as not there, or the listing succeeded and the get then reported the -// file as not there. Anything else — the listing refused, the file +// as not there, or both listings succeeded and the get then reported +// the file as not there. Anything else — a listing refused, the file // there but unreadable, the connection down — fails the run and writes // nothing, because writing back over what was not read would leave the // host with the new key and nothing else. sftp cannot tell a missing -// file from one in a directory it cannot enter, so the listing does: -// a directory that is there but cannot be read is a failure, not an -// empty file. +// file from one in a directory it cannot enter, so the listings do: a +// directory that is there but cannot be read fails the first, and one +// that can be read but not entered fails the second, because nothing in +// it can be looked up, not even ".". The first listing of such a +// directory comes up empty, as the server leaves out every name it +// cannot look up. func fetch( cmd *cobra.Command, host string, options []string, into string, ) (string, bool, error) { said, err := session(cmd, host, options, []string{ "ls -1 " + directory, + "ls -1 " + directory + "/.", "get " + authorized + " " + quoted(into), }) if err != nil { - if directoryAbsent(said) { + if listingNotFound(said, directory) { return "", false, nil } + if listingNotFound(said, directory+"/.") { + return "", false, ErrCannotEnter + } + if absent(said) { return "", true, nil } @@ -239,18 +255,18 @@ func fetch( return string(content), true, nil } -// directoryAbsent says whether sftp reported .ssh itself as not being -// there, which is the one listing failure read as a host that has no -// authorized_keys yet. The reading is taken only from the line in which -// sftp reports on that directory: any other failure of the listing, in -// particular a directory that is there but cannot be entered, is left -// as a failure, so that no key is written to a host whose keys were -// never read. -func directoryAbsent(said string) bool { +// listingNotFound says whether sftp reported the path it was asked to +// list as not being there. For .ssh that is the one listing failure +// read as a host that has no authorized_keys yet; for .ssh/., once .ssh +// itself has been listed, it is a .ssh that is there but cannot be +// entered. The reading is taken only from the line in which sftp +// reports on that path: any other failure of a listing, in particular a +// directory that is there but cannot be read, is left as a failure, so +// that no key is written to a host whose keys were never read. +func listingNotFound(said, path string) bool { for line := range strings.Lines(said) { named, is := reportedCannotList(strings.TrimSpace(line)) - if is && (named == directory || - strings.HasSuffix(named, "/"+directory)) { + if is && (named == path || strings.HasSuffix(named, "/"+path)) { return true } } @@ -259,9 +275,9 @@ func directoryAbsent(said string) bool { } // reportedCannotList returns the path an sftp line reports it cannot -// list for want of the directory, and whether the line is such a -// report. The client writes this one wording when the directory a -// listing names is not there, giving the path the server expanded. +// list for want of it, and whether the line is such a report. The +// client writes this one wording when it cannot look up the path a +// listing names, giving the path the server expanded. func reportedCannotList(line string) (string, bool) { const ( before = `Can't ls: "` diff --git a/internal/cli/ssh/install_test.go b/internal/cli/ssh/install_test.go index 09d53de..510404a 100644 --- a/internal/cli/ssh/install_test.go +++ b/internal/cli/ssh/install_test.go @@ -80,8 +80,11 @@ func TestAbsenceIsReadOnlyFromWhatSFTPSaidAboutAuthorizedKeys(t *testing.T) { // TestTheDirectoryIsReadAsAbsentOnlyFromTheListingSayingSo holds the // wordings the OpenSSH client was seen to use when a listing fails: a -// directory it cannot find is reported one way, and one it cannot enter -// another, and only the first is read as a host with no .ssh yet. +// directory it cannot find is reported one way, and one it cannot read +// another, and only the first is read as a host with no .ssh yet. A +// .ssh that can be read but not entered lists as empty, and the +// listing of .ssh/. that follows reports that path, not .ssh, as not +// found. func TestTheDirectoryIsReadAsAbsentOnlyFromTheListingSayingSo(t *testing.T) { t.Parallel() @@ -102,11 +105,16 @@ func TestTheDirectoryIsReadAsAbsentOnlyFromTheListingSayingSo(t *testing.T) { `Can't ls: "/home/someone/.ssh" not found` + "\n", want: true, }, - "the directory is there and cannot be entered": { + "the directory is there and cannot be read": { said: listed + `remote readdir("/home/someone/.ssh/"): Permission denied` + "\n", want: false, }, + "the directory is there and cannot be entered": { + said: listed + "sftp> ls -1 .ssh/.\n" + + `Can't ls: "/home/someone/.ssh/." not found` + "\n", + want: false, + }, "some other directory is not there": { said: listed + `Can't ls: "/home/someone/.config" not found` + "\n", want: false, @@ -122,7 +130,7 @@ func TestTheDirectoryIsReadAsAbsentOnlyFromTheListingSayingSo(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - if directoryAbsent(listing.said) != listing.want { + if listingNotFound(listing.said, directory) != listing.want { t.Errorf( "read as absent: %t, wanted %t, from:\n%s", !listing.want, listing.want, listing.said, diff --git a/internal/cli/ssh_test.go b/internal/cli/ssh_test.go index 7ab7128..04749f4 100644 --- a/internal/cli/ssh_test.go +++ b/internal/cli/ssh_test.go @@ -52,7 +52,7 @@ const ( ) // notADirectory is what a test puts where the .ssh directory belongs -// to make a step of the write session fail. +// to make a .ssh that is listed but cannot be entered. const notADirectory = "a file where the directory belongs\n" // missingIdentity is a path with no file at it, handed to sftp after @@ -106,9 +106,11 @@ const marker = "KEYFUNC_TEST_MARKER" // another for a file that is there and cannot be read, which is a // failure. A directory shut to the user is stood in for by mode 000, // which the listing reads off the mode itself so that the test does not -// turn on the user it runs as. An -i naming a file that is not here -// draws the warning ssh writes for it, which carries the wording of a -// missing file into a session that goes on to authenticate. +// turn on the user it runs as, and one the user can enter but not write +// to by mode 500, which the put reads off the same way. An -i naming a +// file that is not here draws the warning ssh writes for it, which +// carries the wording of a missing file into a session that goes on to +// authenticate. const installer = ` [ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT" previous= @@ -160,7 +162,13 @@ while IFS= read -r line; do printf 'remote open "%s": Permission denied\n' "$home/$2" >&2 fi ;; - put) cp "$2" "$home/$3" 2>/dev/null || worked=no ;; + put) + if [ "$(stat -c '%a' "$(dirname "$home/$3")")" = 500 ]; then + worked=no + else + cp "$2" "$home/$3" 2>/dev/null || worked=no + fi + ;; mkdir) mkdir "$home/$2" 2>/dev/null || worked=no ;; chmod) chmod "$2" "$home/$3" 2>/dev/null || worked=no ;; rename) mv "$home/$2" "$home/$3" 2>/dev/null || worked=no ;; @@ -329,6 +337,29 @@ func TestAnUnreadableDirectoryIsNotWrittenInto(t *testing.T) { require.Equal(t, 1, connections(t, pretend)) } +func TestADirectoryThatCannotBeEnteredIsRefusedBeforeAnyUpload(t *testing.T) { + t.Setenv(mnemonic.Variable, example()) + + pretend := pretendHost(t) + + // A file where .ssh belongs is listed and cannot be entered, which is + // how sftp sees a directory that can be read but not entered: the + // listing of .ssh comes up and the listing of .ssh/. finds nothing. + inTheWay := filepath.Join(pretend.home, keptUnder) + require.NoError(t, + os.WriteFile(inTheWay, []byte(notADirectory), fileMode), + ) + + printed, _, err := attempt(t, host) + require.ErrorIs(t, err, ssh.ErrCannotEnter) + require.Empty(t, printed) + + // The read and nothing after it: no upload was tried, and what was + // on the host is still what is on the host. + require.Equal(t, 1, connections(t, pretend)) + require.Equal(t, notADirectory, read(t, inTheWay)) +} + func TestAnExistingDirectoryKeepsItsModeAndIsNotRemade(t *testing.T) { t.Setenv(mnemonic.Variable, example()) @@ -392,27 +423,30 @@ func TestAFailedStepNamesTheUploadedFileAndChangesNothing(t *testing.T) { pretend := pretendHost(t) - // A file where the .ssh directory belongs: the listing shows it and - // so the directory reads as already there, but then the put has - // nowhere to put anything, so the write session ends at the put. - inTheWay := filepath.Join(pretend.home, keptUnder) - require.NoError(t, - os.WriteFile(inTheWay, []byte(notADirectory), fileMode), - ) + // A .ssh that can be listed and entered but not written to: the + // fetch finds no file in it, and then the put has nowhere to put + // anything, so the write session ends at the put. + const unwritable = 0o500 + + directory := filepath.Join(pretend.home, keptUnder) + require.NoError(t, os.Mkdir(directory, unwritable)) printed, said, err := attempt(t, host) require.Error(t, err) require.Empty(t, printed) require.Contains(t, said, "put failed") - // The put is the first and last command the write session got to, - // and the file it was uploading is the one the message names. + // The put, after the three commands of the fetch, is the first and + // last command the write session got to, and the file it was + // uploading is the one the message names. sent := recorded(t, pretend.batch) - require.Len(t, sent, 3) - require.Equal(t, "put", strings.Fields(sent[2])[0]) - require.Contains(t, err.Error(), strings.Fields(sent[2])[2]) + require.Len(t, sent, 4) + require.Equal(t, "put", strings.Fields(sent[3])[0]) + require.Contains(t, err.Error(), strings.Fields(sent[3])[2]) - require.Equal(t, notADirectory, read(t, inTheWay)) + left, err := os.ReadDir(directory) + require.NoError(t, err) + require.Empty(t, left) // The same run again, this way for the status it ends with. given := os.Args