Keep the mnemonic out of the ssh and sftp children (closes #16)
check / check (push) Failing after 1s
check / check (push) Failing after 1s
ssh to and ssh install started the system ssh and sftp with the tool's whole environment, so a mnemonic taken from KEYFUNC_MNEMONIC stayed in the child's environment for as long as it ran, readable by the same user and forwardable to the host through a SendEnv line. ssh to keeps the private key inside the tool; the mnemonic must not leave it either. A shared helper in the ssh cli package hands both children the tool's environment with KEYFUNC_MNEMONIC and KEYFUNC_MNEMONIC_COMMAND removed. The mnemonic command itself still runs with the full environment. The stand-in ssh and sftp in the tests now record their environment, and two tests show neither variable reaches them while another one does. Model: opus-4-8
This commit is contained in:
@@ -54,6 +54,11 @@ If none of these is available and standard input is not a terminal, the tool
|
|||||||
refuses and exits with status 1. A mnemonic that fails the BIP-39 checksum is
|
refuses and exits with status 1. A mnemonic that fails the BIP-39 checksum is
|
||||||
refused with a message saying so.
|
refused with a message saying so.
|
||||||
|
|
||||||
|
`KEYFUNC_MNEMONIC` and `KEYFUNC_MNEMONIC_COMMAND` are removed from the
|
||||||
|
environment before the system `ssh` (`keyfunc ssh to`) and `sftp`
|
||||||
|
(`keyfunc ssh install`) are started, so the mnemonic is never handed on to
|
||||||
|
them.
|
||||||
|
|
||||||
Every command takes `--index` / `-n` and `--mnemonic-command`, and has `--help`.
|
Every command takes `--index` / `-n` and `--mnemonic-command`, and has `--help`.
|
||||||
`keyfunc --version` prints the version. `make build` stamps it; a binary
|
`keyfunc --version` prints the version. `make build` stamps it; a binary
|
||||||
installed with `go install` reports the module version instead.
|
installed with `go install` reports the module version instead.
|
||||||
|
|||||||
@@ -166,6 +166,7 @@ func session(
|
|||||||
|
|
||||||
//nolint:gosec // the options are the user's own, meant for sftp
|
//nolint:gosec // the options are the user's own, meant for sftp
|
||||||
command := exec.CommandContext(cmd.Context(), "sftp", argv...)
|
command := exec.CommandContext(cmd.Context(), "sftp", argv...)
|
||||||
|
command.Env = childEnv()
|
||||||
command.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n")
|
command.Stdin = strings.NewReader(strings.Join(batch, "\n") + "\n")
|
||||||
command.Stdout = &said
|
command.Stdout = &said
|
||||||
command.Stderr = &said
|
command.Stderr = &said
|
||||||
|
|||||||
@@ -3,9 +3,12 @@ package ssh
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"git.eeqj.de/sneak/keyfunc/internal/cli/options"
|
"git.eeqj.de/sneak/keyfunc/internal/cli/options"
|
||||||
"git.eeqj.de/sneak/keyfunc/internal/derive"
|
"git.eeqj.de/sneak/keyfunc/internal/derive"
|
||||||
|
"git.eeqj.de/sneak/keyfunc/internal/mnemonic"
|
||||||
"git.eeqj.de/sneak/keyfunc/internal/sshkey"
|
"git.eeqj.de/sneak/keyfunc/internal/sshkey"
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
)
|
)
|
||||||
@@ -84,6 +87,26 @@ func write(cmd *cobra.Command, text string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// childEnv is the tool's environment with the mnemonic variables taken
|
||||||
|
// out, for the ssh and sftp children it starts. "ssh to" exists so the
|
||||||
|
// private key never leaves the tool; the mnemonic, from either variable,
|
||||||
|
// must not leave it either.
|
||||||
|
func childEnv() []string {
|
||||||
|
environ := os.Environ()
|
||||||
|
kept := make([]string, 0, len(environ))
|
||||||
|
|
||||||
|
for _, entry := range environ {
|
||||||
|
name, _, _ := strings.Cut(entry, "=")
|
||||||
|
if name == mnemonic.Variable || name == mnemonic.CommandVariable {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
kept = append(kept, entry)
|
||||||
|
}
|
||||||
|
|
||||||
|
return kept
|
||||||
|
}
|
||||||
|
|
||||||
// addComment gives a command its comment flag.
|
// addComment gives a command its comment flag.
|
||||||
func addComment(cmd *cobra.Command) {
|
func addComment(cmd *cobra.Command) {
|
||||||
cmd.Flags().String(
|
cmd.Flags().String(
|
||||||
|
|||||||
@@ -70,6 +70,7 @@ func to() *cobra.Command {
|
|||||||
func connect(ctx context.Context, argv []string) error {
|
func connect(ctx context.Context, argv []string) error {
|
||||||
//nolint:gosec // the arguments are the user's own, meant for ssh
|
//nolint:gosec // the arguments are the user's own, meant for ssh
|
||||||
command := exec.CommandContext(ctx, "ssh", argv...)
|
command := exec.CommandContext(ctx, "ssh", argv...)
|
||||||
|
command.Env = childEnv()
|
||||||
command.Stdin = os.Stdin
|
command.Stdin = os.Stdin
|
||||||
command.Stdout = os.Stdout
|
command.Stdout = os.Stdout
|
||||||
command.Stderr = os.Stderr
|
command.Stderr = os.Stderr
|
||||||
|
|||||||
@@ -60,13 +60,19 @@ const (
|
|||||||
// an authorized_keys file.
|
// an authorized_keys file.
|
||||||
const keyLine = vectorZero + " keyfunc/ssh/0\n"
|
const keyLine = vectorZero + " keyfunc/ssh/0\n"
|
||||||
|
|
||||||
|
// marker is a variable set beside the mnemonic ones and expected to
|
||||||
|
// reach the stand-in, so a scrubbed environment is told apart from an
|
||||||
|
// empty one.
|
||||||
|
const marker = "KEYFUNC_TEST_MARKER"
|
||||||
|
|
||||||
// installer is a stand-in for the system sftp for the install
|
// installer is a stand-in for the system sftp for the install
|
||||||
// command. It writes down the arguments and every command of the
|
// command. It writes down the arguments and every command of the
|
||||||
// batch it is given, echoes each command as sftp does, and carries
|
// batch it is given, echoes each command as sftp does, writes down its
|
||||||
// the commands out against a directory standing in for the host's
|
// own environment when a test asks for it, and carries the commands out
|
||||||
// home directory, so that what keyfunc sends can be watched doing its
|
// against a directory standing in for the host's home directory, so that
|
||||||
// work. A command that begins with a dash may fail; any other failure
|
// what keyfunc sends can be watched doing its work. A command that
|
||||||
// ends the session, as it does in sftp's own batch mode.
|
// begins with a dash may fail; any other failure ends the session, as it
|
||||||
|
// does in sftp's own batch mode.
|
||||||
//
|
//
|
||||||
// The listing and the two ways a get can fail are worded as the
|
// The listing and the two ways a get can fail are worded as the
|
||||||
// OpenSSH client words them, each naming the path the server expanded.
|
// OpenSSH client words them, each naming the path the server expanded.
|
||||||
@@ -81,6 +87,7 @@ const keyLine = vectorZero + " keyfunc/ssh/0\n"
|
|||||||
// draws the warning ssh writes for it, which carries the wording of a
|
// draws the warning ssh writes for it, which carries the wording of a
|
||||||
// missing file into a session that goes on to authenticate.
|
// missing file into a session that goes on to authenticate.
|
||||||
const installer = `
|
const installer = `
|
||||||
|
[ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT"
|
||||||
previous=
|
previous=
|
||||||
for argument in "$@"; do
|
for argument in "$@"; do
|
||||||
printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
|
printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
|
||||||
@@ -144,9 +151,11 @@ done
|
|||||||
|
|
||||||
// caller is a stand-in for the system ssh for the to command. It
|
// caller is a stand-in for the system ssh for the to command. It
|
||||||
// writes down the arguments it was given, notes the agent socket if
|
// writes down the arguments it was given, notes the agent socket if
|
||||||
// there really is one at the path it was handed, and ends with the
|
// there really is one at the path it was handed, writes down its own
|
||||||
// status the test asked for.
|
// environment when a test asks for it, and ends with the status the
|
||||||
|
// test asked for.
|
||||||
const caller = `
|
const caller = `
|
||||||
|
[ -n "$KEYFUNC_TEST_ENVIRONMENT" ] && env > "$KEYFUNC_TEST_ENVIRONMENT"
|
||||||
for argument in "$@"; do
|
for argument in "$@"; do
|
||||||
printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
|
printf '%s\n' "$argument" >> "$KEYFUNC_TEST_ARGUMENTS"
|
||||||
done
|
done
|
||||||
@@ -444,6 +453,36 @@ func TestTheToolEndsWithTheStatusSSHEndedWith(t *testing.T) {
|
|||||||
require.Equal(t, failingStatus, cli.Main())
|
require.Equal(t, failingStatus, cli.Main())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestTheMnemonicIsNotHandedToSFTP(t *testing.T) {
|
||||||
|
t.Setenv(mnemonic.CommandVariable, "echo "+example())
|
||||||
|
t.Setenv(mnemonic.Variable, example())
|
||||||
|
t.Setenv(marker, "reaches the stand-in")
|
||||||
|
|
||||||
|
pretendHost(t)
|
||||||
|
environment := recordEnvironment(t)
|
||||||
|
|
||||||
|
install(t, host)
|
||||||
|
|
||||||
|
mnemonicWithheld(t, read(t, environment))
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestTheMnemonicIsNotHandedToSSH(t *testing.T) {
|
||||||
|
t.Setenv(mnemonic.CommandVariable, "echo "+example())
|
||||||
|
t.Setenv(mnemonic.Variable, example())
|
||||||
|
t.Setenv(marker, "reaches the stand-in")
|
||||||
|
|
||||||
|
pretendCall(t)
|
||||||
|
environment := recordEnvironment(t)
|
||||||
|
|
||||||
|
_, err := execute(t, subcommand, "to", host, "uptime")
|
||||||
|
|
||||||
|
var passed ssh.StatusError
|
||||||
|
|
||||||
|
require.ErrorAs(t, err, &passed)
|
||||||
|
|
||||||
|
mnemonicWithheld(t, read(t, environment))
|
||||||
|
}
|
||||||
|
|
||||||
// pretendHost puts the install stand-in on the path and gives back the
|
// pretendHost puts the install stand-in on the path and gives back the
|
||||||
// places it writes to.
|
// places it writes to.
|
||||||
func pretendHost(t *testing.T) pretended {
|
func pretendHost(t *testing.T) pretended {
|
||||||
@@ -592,6 +631,28 @@ func standIn(t *testing.T, name, body string) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// recordEnvironment asks the stand-in to write its environment down and
|
||||||
|
// gives back the file it writes it to.
|
||||||
|
func recordEnvironment(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
path := filepath.Join(t.TempDir(), "environment")
|
||||||
|
t.Setenv("KEYFUNC_TEST_ENVIRONMENT", path)
|
||||||
|
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
|
// mnemonicWithheld requires that neither mnemonic variable reached the
|
||||||
|
// stand-in and that the marker set beside them did, so an empty
|
||||||
|
// environment does not pass for a scrubbed one.
|
||||||
|
func mnemonicWithheld(t *testing.T, environment string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
require.NotContains(t, environment, mnemonic.Variable+"=")
|
||||||
|
require.NotContains(t, environment, mnemonic.CommandVariable+"=")
|
||||||
|
require.Contains(t, environment, marker+"=")
|
||||||
|
}
|
||||||
|
|
||||||
// read returns what is in a file.
|
// read returns what is in a file.
|
||||||
func read(t *testing.T, path string) string {
|
func read(t *testing.T, path string) string {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|||||||
Reference in New Issue
Block a user