From e646d57b0a8c8aba960a7d54c5972971aa5faa6f Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 12 Jun 2019 16:22:52 +1000 Subject: [PATCH 1/6] add SSH master connection feature By default, sshcode will now start a master connection with no command so that users only need to authenticate once and multiple connections don't need to be established. This speeds up load times significantly as there are less handshakes required. To disable this behaviour you can use `--no-reuse-connection`. --- main.go | 23 +++++++----- sshcode.go | 108 ++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 104 insertions(+), 27 deletions(-) diff --git a/main.go b/main.go index bc0de4a..ddd8eb3 100644 --- a/main.go +++ b/main.go @@ -34,11 +34,12 @@ var _ interface { } = new(rootCmd) type rootCmd struct { - skipSync bool - syncBack bool - printVersion bool - bindAddr string - sshFlags string + skipSync bool + syncBack bool + printVersion bool + noReuseConnection bool + bindAddr string + sshFlags string } func (c *rootCmd) Spec() cli.CommandSpec { @@ -53,6 +54,7 @@ func (c *rootCmd) RegisterFlags(fl *flag.FlagSet) { fl.BoolVar(&c.skipSync, "skipsync", false, "skip syncing local settings and extensions to remote host") fl.BoolVar(&c.syncBack, "b", false, "sync extensions back on termination") fl.BoolVar(&c.printVersion, "version", false, "print version information and exit") + fl.BoolVar(&c.noReuseConnection, "no-reuse-connection", false, "do not reuse SSH connection via control socket") fl.StringVar(&c.bindAddr, "bind", "", "local bind address for ssh tunnel") fl.StringVar(&c.sshFlags, "ssh-flags", "", "custom SSH flags") } @@ -76,10 +78,11 @@ func (c *rootCmd) Run(fl *flag.FlagSet) { } err := sshCode(host, dir, options{ - skipSync: c.skipSync, - sshFlags: c.sshFlags, - bindAddr: c.bindAddr, - syncBack: c.syncBack, + skipSync: c.skipSync, + sshFlags: c.sshFlags, + bindAddr: c.bindAddr, + syncBack: c.syncBack, + noReuseConnection: c.noReuseConnection, }) if err != nil { @@ -101,7 +104,7 @@ Environment variables: More info: https://github.com/cdr/sshcode Arguments: -%vHOST is passed into the ssh command. Valid formats are '' or 'gcp:'. +%vHOST is passed into the ssh command. Valid formats are '' or 'gcp:'. %vDIR is optional.`, helpTab, vsCodeConfigDirEnv, helpTab, vsCodeExtensionsDirEnv, diff --git a/sshcode.go b/sshcode.go index 97e7502..859651a 100644 --- a/sshcode.go +++ b/sshcode.go @@ -20,14 +20,16 @@ import ( ) const codeServerPath = "~/.cache/sshcode/sshcode-server" +const sshControlPath = "~/.ssh/control-%h-%p-%r" type options struct { - skipSync bool - syncBack bool - noOpen bool - bindAddr string - remotePort string - sshFlags string + skipSync bool + syncBack bool + noOpen bool + noReuseConnection bool + bindAddr string + remotePort string + sshFlags string } func sshCode(host, dir string, o options) error { @@ -53,6 +55,41 @@ func sshCode(host, dir string, o options) error { return xerrors.Errorf("failed to find available remote port: %w", err) } + // Start SSH master connection socket. This prevents multiple password prompts from appearing as authentication + // only happens on the initial connection. + var sshMasterCmd *exec.Cmd + if !o.noReuseConnection { + newSSHFlags := fmt.Sprintf(`%v -o "ControlPath=%v"`, o.sshFlags, sshControlPath) + + // -MN means "start a master socket and don't open a session, just connect". + sshMasterCmdStr := fmt.Sprintf(`ssh %v -MN %v`, newSSHFlags, host) + sshMasterCmd = exec.Command("sh", "-c", sshMasterCmdStr) + sshMasterCmd.Stdin = os.Stdin + sshMasterCmd.Stdout = os.Stdout + sshMasterCmd.Stderr = os.Stderr + err = sshMasterCmd.Start() + if err != nil { + flog.Error("failed to start SSH master connection, disabling connection reuse feature: %v", err) + o.noReuseConnection = true + } else { + // Wait for master to be ready. + err = checkSSHMaster(newSSHFlags, host) + if err != nil { + flog.Error("SSH master failed to start in time, disabling connection reuse feature: %v", err) + o.noReuseConnection = true + if sshMasterCmd.Process != nil { + err = sshMasterCmd.Process.Kill() + if err != nil { + flog.Error("failed to kill SSH master connection, ignoring: %v", err) + } + } + } else { + sshMasterCmd.Stdin = nil + o.sshFlags = newSSHFlags + } + } + } + dlScript := downloadScript(codeServerPath) // Downloads the latest code-server and allows it to be executed. @@ -146,22 +183,39 @@ func sshCode(host, dir string, o options) error { case <-ctx.Done(): case <-c: } + flog.Info("exiting") - if !o.syncBack || o.skipSync { - flog.Info("shutting down") - return nil - } + if o.syncBack && !o.skipSync { + flog.Info("synchronizing VS Code back to local") - flog.Info("synchronizing VS Code back to local") + err = syncExtensions(o.sshFlags, host, true) + if err != nil { + flog.Error("failed to sync extensions back: %v", err) + } - err = syncExtensions(o.sshFlags, host, true) - if err != nil { - return xerrors.Errorf("failed to sync extensions back: %w", err) + err = syncUserSettings(o.sshFlags, host, true) + if err != nil { + flog.Error("failed to sync user settings settings back: %v", err) + } } - err = syncUserSettings(o.sshFlags, host, true) - if err != nil { - return xerrors.Errorf("failed to sync user settings settings back: %w", err) + // Kill the master connection if we made one. + if !o.noReuseConnection { + // Try using the -O exit syntax first before killing the master. + sshCmdStr = fmt.Sprintf(`ssh %v -O exit %v`, o.sshFlags, host) + sshCmd = exec.Command("sh", "-c", sshCmdStr) + sshCmd.Stdout = os.Stdout + sshCmd.Stderr = os.Stderr + err = sshCmd.Run() + if err != nil { + flog.Error("failed to gracefully stop SSH master connection, killing: %v", err) + if sshMasterCmd.Process != nil { + err = sshMasterCmd.Process.Kill() + if err != nil { + flog.Error("failed to kill SSH master connection, ignoring: %v", err) + } + } + } } return nil @@ -263,6 +317,26 @@ func randomPort() (string, error) { return "", xerrors.Errorf("max number of tries exceeded: %d", maxTries) } +// checkSSHMaster polls every second for 30 seconds to check if the SSH master +// is ready. +func checkSSHMaster(sshFlags string, host string) (err error) { + maxTries := 30 + check := func() error { + sshCmdStr := fmt.Sprintf(`ssh %v -O check %v`, sshFlags, host) + sshCmd := exec.Command("sh", "-c", sshCmdStr) + return sshCmd.Run() + } + + for i := 0; i < maxTries; i++ { + err = check() + if err == nil { + return nil + } + time.Sleep(time.Second) + } + return err +} + func syncUserSettings(sshFlags string, host string, back bool) error { localConfDir, err := configDir() if err != nil { From 3141f7f4bb93be3a3614e21b2b5510916c81dbdd Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 13 Jun 2019 18:53:51 +1000 Subject: [PATCH 2/6] move SSH master process tidyup to a deferred func - Replace the `ssh -O exit` tidyup command with just a SIGTERM on the master - Add `exec` to the front of the SSH master cmd so it replaces the `sh` process (so we can send SIGTERM to it easier) --- sshcode.go | 66 ++++++++++++++++++++++-------------------------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/sshcode.go b/sshcode.go index 859651a..51a432e 100644 --- a/sshcode.go +++ b/sshcode.go @@ -11,6 +11,7 @@ import ( "os/signal" "path/filepath" "strconv" + "syscall" "strings" "time" @@ -57,32 +58,36 @@ func sshCode(host, dir string, o options) error { // Start SSH master connection socket. This prevents multiple password prompts from appearing as authentication // only happens on the initial connection. - var sshMasterCmd *exec.Cmd if !o.noReuseConnection { newSSHFlags := fmt.Sprintf(`%v -o "ControlPath=%v"`, o.sshFlags, sshControlPath) // -MN means "start a master socket and don't open a session, just connect". - sshMasterCmdStr := fmt.Sprintf(`ssh %v -MN %v`, newSSHFlags, host) - sshMasterCmd = exec.Command("sh", "-c", sshMasterCmdStr) + sshCmdStr := fmt.Sprintf(`exec ssh %v -MN %v`, newSSHFlags, host) + sshMasterCmd := exec.Command("sh", "-c", sshCmdStr) sshMasterCmd.Stdin = os.Stdin sshMasterCmd.Stdout = os.Stdout sshMasterCmd.Stderr = os.Stderr + stopSSHMaster := func () { + if sshMasterCmd.Process != nil { + err := sshMasterCmd.Process.Signal(syscall.SIGTERM) + if err != nil { + flog.Error("failed to send SIGTERM to SSH master process: %v", err) + } + } + } + defer stopSSHMaster() + err = sshMasterCmd.Start() if err != nil { flog.Error("failed to start SSH master connection, disabling connection reuse feature: %v", err) o.noReuseConnection = true + stopSSHMaster() } else { - // Wait for master to be ready. err = checkSSHMaster(newSSHFlags, host) if err != nil { - flog.Error("SSH master failed to start in time, disabling connection reuse feature: %v", err) + flog.Error("SSH master failed to be ready in time, disabling connection reuse feature: %v", err) o.noReuseConnection = true - if sshMasterCmd.Process != nil { - err = sshMasterCmd.Process.Kill() - if err != nil { - flog.Error("failed to kill SSH master connection, ignoring: %v", err) - } - } + stopSSHMaster() } else { sshMasterCmd.Stdin = nil o.sshFlags = newSSHFlags @@ -183,39 +188,22 @@ func sshCode(host, dir string, o options) error { case <-ctx.Done(): case <-c: } - flog.Info("exiting") - if o.syncBack && !o.skipSync { - flog.Info("synchronizing VS Code back to local") + flog.Info("shutting down") + if !o.syncBack || o.skipSync { + return nil + } - err = syncExtensions(o.sshFlags, host, true) - if err != nil { - flog.Error("failed to sync extensions back: %v", err) - } + flog.Info("synchronizing VS Code back to local") - err = syncUserSettings(o.sshFlags, host, true) - if err != nil { - flog.Error("failed to sync user settings settings back: %v", err) - } + err = syncExtensions(o.sshFlags, host, true) + if err != nil { + return xerrors.Errorf("failed to sync extensions back: %v", err) } - // Kill the master connection if we made one. - if !o.noReuseConnection { - // Try using the -O exit syntax first before killing the master. - sshCmdStr = fmt.Sprintf(`ssh %v -O exit %v`, o.sshFlags, host) - sshCmd = exec.Command("sh", "-c", sshCmdStr) - sshCmd.Stdout = os.Stdout - sshCmd.Stderr = os.Stderr - err = sshCmd.Run() - if err != nil { - flog.Error("failed to gracefully stop SSH master connection, killing: %v", err) - if sshMasterCmd.Process != nil { - err = sshMasterCmd.Process.Kill() - if err != nil { - flog.Error("failed to kill SSH master connection, ignoring: %v", err) - } - } - } + err = syncUserSettings(o.sshFlags, host, true) + if err != nil { + return xerrors.Errorf("failed to sync user settings settings back: %v", err) } return nil From 4d64fc0393636307296b2bd5db7577ecb5ea28a4 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 13 Jun 2019 19:11:31 +1000 Subject: [PATCH 3/6] add "process is running" check to checkSSHMaster() Checks if the master process is running by sending signal 0 to it. According to kill(2), sending a signal of 0 will send no signal but will still perform error checking. To prevent the SSH master from becoming a zombie process, a wait call was added in a goroutine. --- sshcode.go | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/sshcode.go b/sshcode.go index 51a432e..41f6e23 100644 --- a/sshcode.go +++ b/sshcode.go @@ -11,8 +11,8 @@ import ( "os/signal" "path/filepath" "strconv" - "syscall" "strings" + "syscall" "time" "github.com/pkg/browser" @@ -67,9 +67,13 @@ func sshCode(host, dir string, o options) error { sshMasterCmd.Stdin = os.Stdin sshMasterCmd.Stdout = os.Stdout sshMasterCmd.Stderr = os.Stderr - stopSSHMaster := func () { + stopSSHMaster := func() { if sshMasterCmd.Process != nil { - err := sshMasterCmd.Process.Signal(syscall.SIGTERM) + err := sshMasterCmd.Process.Signal(syscall.Signal(0)) + if err != nil { + return + } + err = sshMasterCmd.Process.Signal(syscall.SIGTERM) if err != nil { flog.Error("failed to send SIGTERM to SSH master process: %v", err) } @@ -78,12 +82,13 @@ func sshCode(host, dir string, o options) error { defer stopSSHMaster() err = sshMasterCmd.Start() + go sshMasterCmd.Wait() if err != nil { flog.Error("failed to start SSH master connection, disabling connection reuse feature: %v", err) o.noReuseConnection = true stopSSHMaster() } else { - err = checkSSHMaster(newSSHFlags, host) + err = checkSSHMaster(sshMasterCmd, newSSHFlags, host) if err != nil { flog.Error("SSH master failed to be ready in time, disabling connection reuse feature: %v", err) o.noReuseConnection = true @@ -307,22 +312,32 @@ func randomPort() (string, error) { // checkSSHMaster polls every second for 30 seconds to check if the SSH master // is ready. -func checkSSHMaster(sshFlags string, host string) (err error) { - maxTries := 30 - check := func() error { +func checkSSHMaster(sshMasterCmd *exec.Cmd, sshFlags string, host string) error { + var ( + maxTries = 30 + sleepDur = time.Second + err error + ) + for i := 0; i < maxTries; i++ { + // Check if the master is running + if sshMasterCmd.Process == nil { + return xerrors.Errorf("SSH master process not running") + } + err = sshMasterCmd.Process.Signal(syscall.Signal(0)) + if err != nil { + return xerrors.Errorf("failed to check if SSH master process was alive: %v", err) + } + + // Check if it's ready sshCmdStr := fmt.Sprintf(`ssh %v -O check %v`, sshFlags, host) sshCmd := exec.Command("sh", "-c", sshCmdStr) - return sshCmd.Run() - } - - for i := 0; i < maxTries; i++ { - err = check() + err = sshCmd.Run() if err == nil { return nil } - time.Sleep(time.Second) + time.Sleep(sleepDur) } - return err + return xerrors.Errorf("max number of tries exceeded: %d", maxTries) } func syncUserSettings(sshFlags string, host string, back bool) error { From eee34f58bd4cac785dd6ef58eb8e1a4d80975a4a Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Sun, 16 Jun 2019 03:12:09 +1000 Subject: [PATCH 4/6] add ~/.ssh directory sanity check before starting Checks: - if it exists - if it's a directory (if not warn and disable reuse connection feature) - if it has safe permissions (not writable by anyone except the owner, if not then warn) --- sshcode.go | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/sshcode.go b/sshcode.go index 41f6e23..92b7f3c 100644 --- a/sshcode.go +++ b/sshcode.go @@ -21,7 +21,9 @@ import ( ) const codeServerPath = "~/.cache/sshcode/sshcode-server" -const sshControlPath = "~/.ssh/control-%h-%p-%r" +const sshDirectory = "~/.ssh" +const sshDirectoryUnsafeModeMask = 0022 +const sshControlPath = sshDirectory + "/control-%h-%p-%r" type options struct { skipSync bool @@ -34,8 +36,6 @@ type options struct { } func sshCode(host, dir string, o options) error { - flog.Info("ensuring code-server is updated...") - host, extraSSHFlags, err := parseHost(host) if err != nil { return xerrors.Errorf("failed to parse host IP: %w", err) @@ -56,6 +56,28 @@ func sshCode(host, dir string, o options) error { return xerrors.Errorf("failed to find available remote port: %w", err) } + // Check the SSH directory's permissions and warn the user if it is not safe. + sshDirectoryMode, err := os.Lstat(expandPath(sshDirectory)) + if err != nil { + if !o.noReuseConnection { + flog.Info("failed to stat %v directory, disabling connection reuse feature: %v", sshDirectory, err) + o.noReuseConnection = true + } + } else { + if !sshDirectoryMode.IsDir() { + if !o.noReuseConnection { + flog.Info("%v is not a directory, disabling connection reuse feature", sshDirectory) + o.noReuseConnection = true + } else { + flog.Info("warning: %v is not a directory", sshDirectory) + } + } + if sshDirectoryMode.Mode().Perm()&sshDirectoryUnsafeModeMask != 0 { + flog.Info("warning: the %v directory has unsafe permissions, they should only be writable by "+ + "the owner (and files inside should be set to 0600)", sshDirectory) + } + } + // Start SSH master connection socket. This prevents multiple password prompts from appearing as authentication // only happens on the initial connection. if !o.noReuseConnection { @@ -100,6 +122,7 @@ func sshCode(host, dir string, o options) error { } } + flog.Info("ensuring code-server is updated...") dlScript := downloadScript(codeServerPath) // Downloads the latest code-server and allows it to be executed. @@ -214,6 +237,23 @@ func sshCode(host, dir string, o options) error { return nil } +// expandPath returns an expanded version of path. +func expandPath(path string) string { + path = filepath.Clean(os.ExpandEnv(path)) + + // Replace tilde notation in path with the home directory. + homedir := os.Getenv("HOME") + if homedir != "" { + if path == "~" { + path = homedir + } else if strings.HasPrefix(path, "~/") { + path = filepath.Join(homedir, path[2:]) + } + } + + return filepath.Clean(path) +} + func parseBindAddr(bindAddr string) (string, error) { if bindAddr == "" { bindAddr = ":" From 379475593da8f175df7d40394b8efb22156b9235 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 27 Jun 2019 16:12:48 +1000 Subject: [PATCH 5/6] restructure SSH master code, apply requested fixes --- main.go | 10 ++-- sshcode.go | 172 +++++++++++++++++++++++++++++------------------------ 2 files changed, 100 insertions(+), 82 deletions(-) diff --git a/main.go b/main.go index ddd8eb3..b53b21f 100644 --- a/main.go +++ b/main.go @@ -78,11 +78,11 @@ func (c *rootCmd) Run(fl *flag.FlagSet) { } err := sshCode(host, dir, options{ - skipSync: c.skipSync, - sshFlags: c.sshFlags, - bindAddr: c.bindAddr, - syncBack: c.syncBack, - noReuseConnection: c.noReuseConnection, + skipSync: c.skipSync, + sshFlags: c.sshFlags, + bindAddr: c.bindAddr, + syncBack: c.syncBack, + reuseConnection: !c.noReuseConnection, }) if err != nil { diff --git a/sshcode.go b/sshcode.go index 92b7f3c..9f80fe0 100644 --- a/sshcode.go +++ b/sshcode.go @@ -21,18 +21,21 @@ import ( ) const codeServerPath = "~/.cache/sshcode/sshcode-server" -const sshDirectory = "~/.ssh" -const sshDirectoryUnsafeModeMask = 0022 -const sshControlPath = sshDirectory + "/control-%h-%p-%r" + +const ( + sshDirectory = "~/.ssh" + sshDirectoryUnsafeModeMask = 0022 + sshControlPath = sshDirectory + "/control-%h-%p-%r" +) type options struct { - skipSync bool - syncBack bool - noOpen bool - noReuseConnection bool - bindAddr string - remotePort string - sshFlags string + skipSync bool + syncBack bool + noOpen bool + reuseConnection bool + bindAddr string + remotePort string + sshFlags string } func sshCode(host, dir string, o options) error { @@ -57,68 +60,19 @@ func sshCode(host, dir string, o options) error { } // Check the SSH directory's permissions and warn the user if it is not safe. - sshDirectoryMode, err := os.Lstat(expandPath(sshDirectory)) - if err != nil { - if !o.noReuseConnection { - flog.Info("failed to stat %v directory, disabling connection reuse feature: %v", sshDirectory, err) - o.noReuseConnection = true - } - } else { - if !sshDirectoryMode.IsDir() { - if !o.noReuseConnection { - flog.Info("%v is not a directory, disabling connection reuse feature", sshDirectory) - o.noReuseConnection = true - } else { - flog.Info("warning: %v is not a directory", sshDirectory) - } - } - if sshDirectoryMode.Mode().Perm()&sshDirectoryUnsafeModeMask != 0 { - flog.Info("warning: the %v directory has unsafe permissions, they should only be writable by "+ - "the owner (and files inside should be set to 0600)", sshDirectory) - } - } + o.reuseConnection = checkSSHDirectory(sshDirectory, o.reuseConnection) // Start SSH master connection socket. This prevents multiple password prompts from appearing as authentication // only happens on the initial connection. - if !o.noReuseConnection { - newSSHFlags := fmt.Sprintf(`%v -o "ControlPath=%v"`, o.sshFlags, sshControlPath) - - // -MN means "start a master socket and don't open a session, just connect". - sshCmdStr := fmt.Sprintf(`exec ssh %v -MN %v`, newSSHFlags, host) - sshMasterCmd := exec.Command("sh", "-c", sshCmdStr) - sshMasterCmd.Stdin = os.Stdin - sshMasterCmd.Stdout = os.Stdout - sshMasterCmd.Stderr = os.Stderr - stopSSHMaster := func() { - if sshMasterCmd.Process != nil { - err := sshMasterCmd.Process.Signal(syscall.Signal(0)) - if err != nil { - return - } - err = sshMasterCmd.Process.Signal(syscall.SIGTERM) - if err != nil { - flog.Error("failed to send SIGTERM to SSH master process: %v", err) - } - } - } - defer stopSSHMaster() - - err = sshMasterCmd.Start() - go sshMasterCmd.Wait() + if o.reuseConnection { + flog.Info("starting SSH master connection...") + newSSHFlags, cancel, err := startSSHMaster(o.sshFlags, sshControlPath, host) + defer cancel() if err != nil { - flog.Error("failed to start SSH master connection, disabling connection reuse feature: %v", err) - o.noReuseConnection = true - stopSSHMaster() + flog.Error("failed to start SSH master connection: %v", err) + o.reuseConnection = false } else { - err = checkSSHMaster(sshMasterCmd, newSSHFlags, host) - if err != nil { - flog.Error("SSH master failed to be ready in time, disabling connection reuse feature: %v", err) - o.noReuseConnection = true - stopSSHMaster() - } else { - sshMasterCmd.Stdin = nil - o.sshFlags = newSSHFlags - } + o.sshFlags = newSSHFlags } } @@ -226,12 +180,12 @@ func sshCode(host, dir string, o options) error { err = syncExtensions(o.sshFlags, host, true) if err != nil { - return xerrors.Errorf("failed to sync extensions back: %v", err) + return xerrors.Errorf("failed to sync extensions back: %w", err) } err = syncUserSettings(o.sshFlags, host, true) if err != nil { - return xerrors.Errorf("failed to sync user settings settings back: %v", err) + return xerrors.Errorf("failed to sync user settings settings back: %w", err) } return nil @@ -350,6 +304,74 @@ func randomPort() (string, error) { return "", xerrors.Errorf("max number of tries exceeded: %d", maxTries) } +// checkSSHDirectory performs sanity and safety checks on sshDirectory, and +// returns a new value for o.reuseConnection depending on the checks. +func checkSSHDirectory(sshDirectory string, reuseConnection bool) bool { + sshDirectoryMode, err := os.Lstat(expandPath(sshDirectory)) + if err != nil { + if reuseConnection { + flog.Info("failed to stat %v directory, disabling connection reuse feature: %v", sshDirectory, err) + } + reuseConnection = false + } else { + if !sshDirectoryMode.IsDir() { + if reuseConnection { + flog.Info("%v is not a directory, disabling connection reuse feature", sshDirectory) + } else { + flog.Info("warning: %v is not a directory", sshDirectory) + } + reuseConnection = false + } + if sshDirectoryMode.Mode().Perm()&sshDirectoryUnsafeModeMask != 0 { + flog.Info("warning: the %v directory has unsafe permissions, they should only be writable by "+ + "the owner (and files inside should be set to 0600)", sshDirectory) + } + } + return reuseConnection +} + +// startSSHMaster starts an SSH master connection and waits for it to be ready. +// It returns a new set of SSH flags for child SSH processes to use. +func startSSHMaster(sshFlags string, sshControlPath string, host string) (string, func(), error) { + ctx, cancel := context.WithCancel(context.Background()) + + newSSHFlags := fmt.Sprintf(`%v -o "ControlPath=%v"`, sshFlags, sshControlPath) + + // -MN means "start a master socket and don't open a session, just connect". + sshCmdStr := fmt.Sprintf(`exec ssh %v -MNq %v`, newSSHFlags, host) + sshMasterCmd := exec.CommandContext(ctx, "sh", "-c", sshCmdStr) + sshMasterCmd.Stdin = os.Stdin + sshMasterCmd.Stderr = os.Stderr + + // Gracefully stop the SSH master. + stopSSHMaster := func() { + if sshMasterCmd.Process != nil { + if sshMasterCmd.ProcessState != nil && sshMasterCmd.ProcessState.Exited() { + return + } + err := sshMasterCmd.Process.Signal(syscall.SIGTERM) + if err != nil { + flog.Error("failed to send SIGTERM to SSH master process: %v", err) + } + } + cancel() + } + + // Start ssh master and wait. Waiting prevents the process from becoming a zombie process if it dies before + // sshcode does, and allows sshMasterCmd.ProcessState to be populated. + err := sshMasterCmd.Start() + go sshMasterCmd.Wait() + if err != nil { + return "", stopSSHMaster, err + } + err = checkSSHMaster(sshMasterCmd, newSSHFlags, host) + if err != nil { + stopSSHMaster() + return "", stopSSHMaster, xerrors.Errorf("SSH master wasn't ready on time: %w", err) + } + return newSSHFlags, stopSSHMaster, nil +} + // checkSSHMaster polls every second for 30 seconds to check if the SSH master // is ready. func checkSSHMaster(sshMasterCmd *exec.Cmd, sshFlags string, host string) error { @@ -359,16 +381,12 @@ func checkSSHMaster(sshMasterCmd *exec.Cmd, sshFlags string, host string) error err error ) for i := 0; i < maxTries; i++ { - // Check if the master is running - if sshMasterCmd.Process == nil { - return xerrors.Errorf("SSH master process not running") - } - err = sshMasterCmd.Process.Signal(syscall.Signal(0)) - if err != nil { - return xerrors.Errorf("failed to check if SSH master process was alive: %v", err) + // Check if the master is running. + if sshMasterCmd.Process == nil || (sshMasterCmd.ProcessState != nil && sshMasterCmd.ProcessState.Exited()) { + return xerrors.Errorf("SSH master process is not running") } - // Check if it's ready + // Check if it's ready. sshCmdStr := fmt.Sprintf(`ssh %v -O check %v`, sshFlags, host) sshCmd := exec.Command("sh", "-c", sshCmdStr) err = sshCmd.Run() From dbf7a484881632d2fc1f0129d756632923cb022e Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 27 Jun 2019 16:21:29 +1000 Subject: [PATCH 6/6] add comment about tildes in expandPath --- sshcode.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sshcode.go b/sshcode.go index 9f80fe0..565ac43 100644 --- a/sshcode.go +++ b/sshcode.go @@ -195,7 +195,8 @@ func sshCode(host, dir string, o options) error { func expandPath(path string) string { path = filepath.Clean(os.ExpandEnv(path)) - // Replace tilde notation in path with the home directory. + // Replace tilde notation in path with the home directory. You can't replace the first instance of `~` in the + // string with the homedir as having a tilde in the middle of a filename is valid. homedir := os.Getenv("HOME") if homedir != "" { if path == "~" {