Update ssh command

command/ssh.go

@@ -9,43 +9,200 @@ import (
 	"os/exec"
 	"os/user"
 	"strings"
+	"syscall"
 
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/builtin/logical/ssh"
-	"github.com/hashicorp/vault/meta"
+	"github.com/mitchellh/cli"
-	homedir "github.com/mitchellh/go-homedir"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
+	"github.com/posener/complete"
 )
 
+// Ensure we are implementing the right interfaces.
+var _ cli.Command = (*SSHCommand)(nil)
+var _ cli.CommandAutocomplete = (*SSHCommand)(nil)
+
 // SSHCommand is a Command that establishes a SSH connection with target by
 // generating a dynamic key
 type SSHCommand struct {
-	meta.Meta
+	*BaseCommand
 
-	// API
+	// Common SSH options
-	client    *api.Client
+	flagMode                  string
-	sshClient *api.SSH
+	flagRole                  string
+	flagNoExec                bool
+	flagMountPoint            string
+	flagStrictHostKeyChecking string
+	flagUserKnownHostsFile    string
 
-	// Common options
+	// SSH CA Mode options
-	mode       string
+	flagPublicKeyPath     string
-	noExec     bool
+	flagPrivateKeyPath    string
-	format     string
+	flagHostKeyMountPoint string
-	mountPoint string
+	flagHostKeyHostnames  string
-	role       string
+}
-	username   string
-	ip         string
-	sshArgs    []string
 
-	// Key options
+func (c *SSHCommand) Synopsis() string {
-	strictHostKeyChecking string
+	return "Initiate an SSH session"
-	userKnownHostsFile    string
+}
 
-	// SSH CA backend specific options
+func (c *SSHCommand) Help() string {
-	publicKeyPath     string
+	helpText := `
-	privateKeyPath    string
+Usage: vault ssh [options] username@ip [ssh options]
-	hostKeyMountPoint string
+
-	hostKeyHostnames  string
+  Establishes an SSH connection with the target machine.
+
+  This command uses one of the SSH authentication backends to authenticate and
+  automatically establish an SSH connection to a host. This operation requires
+  that the SSH backend is mounted and configured.
+
+  SSH using the OTP mode (requires sshpass for full automation):
+
+      $ vault ssh -mode=otp -role=my-role user@1.2.3.4
+
+  SSH using the CA mode:
+
+      $ vault ssh -mode=ca -role=my-role user@1.2.3.4
+
+  SSH using CA mode with host key verification:
+
+      $ vault ssh \
+          -mode=ca \
+          -role=my-role \
+          -host-key-mount-point=host-signer \
+          -host-key-hostnames=example.com \
+          user@example.com
+
+  For the full list of options and arguments, please see the documentation.
+
+` + c.Flags().Help()
+
+	return strings.TrimSpace(helpText)
+}
+
+func (c *SSHCommand) Flags() *FlagSets {
+	set := c.flagSet(FlagSetHTTP | FlagSetOutputField | FlagSetOutputFormat)
+
+	f := set.NewFlagSet("SSH Options")
+
+	// TODO: doc field?
+
+	// General
+	f.StringVar(&StringVar{
+		Name:       "mode",
+		Target:     &c.flagMode,
+		Default:    "",
+		EnvVar:     "",
+		Completion: complete.PredictSet("ca", "dynamic", "otp"),
+		Usage:      "Name of the role to use to generate the key.",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "role",
+		Target:     &c.flagRole,
+		Default:    "",
+		EnvVar:     "",
+		Completion: complete.PredictAnything,
+		Usage:      "Name of the role to use to generate the key.",
+	})
+
+	f.BoolVar(&BoolVar{
+		Name:       "no-exec",
+		Target:     &c.flagNoExec,
+		Default:    false,
+		EnvVar:     "",
+		Completion: complete.PredictNothing,
+		Usage: "Print the generated credentials, but do not establish a " +
+			"connection.",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "mount-point",
+		Target:     &c.flagMountPoint,
+		Default:    "ssh/",
+		EnvVar:     "",
+		Completion: complete.PredictAnything,
+		Usage:      "Mount point to the SSH backend.",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "strict-host-key-checking",
+		Target:     &c.flagStrictHostKeyChecking,
+		Default:    "ask",
+		EnvVar:     "VAULT_SSH_STRICT_HOST_KEY_CHECKING",
+		Completion: complete.PredictSet("ask", "no", "yes"),
+		Usage: "Value to use for the SSH configuration option " +
+			"\"StrictHostKeyChecking\".",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "user-known-hosts-file",
+		Target:     &c.flagUserKnownHostsFile,
+		Default:    "~/.ssh/known_hosts",
+		EnvVar:     "VAULT_SSH_USER_KNOWN_HOSTS_FILE",
+		Completion: complete.PredictFiles("*"),
+		Usage: "Value to use for the SSH configuration option " +
+			"\"UserKnownHostsFile\".",
+	})
+
+	// SSH CA
+	f = set.NewFlagSet("CA Mode Options")
+
+	f.StringVar(&StringVar{
+		Name:       "public-key-path",
+		Target:     &c.flagPublicKeyPath,
+		Default:    "~/.ssh/id_rsa.pub",
+		EnvVar:     "g",
+		Completion: complete.PredictFiles("*"),
+		Usage:      "Path to the SSH public key to send to Vault for signing.",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "private-key-path",
+		Target:     &c.flagPrivateKeyPath,
+		Default:    "~/.ssh/id_rsa",
+		EnvVar:     "",
+		Completion: complete.PredictFiles("*"),
+		Usage: "Path to the SSH private key to use for authentication. This must " +
+			"be the corresponding private key to -public-key-path.",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "host-key-mount-point",
+		Target:     &c.flagHostKeyMountPoint,
+		Default:    "~/.ssh/id_rsa",
+		EnvVar:     "VAULT_SSH_HOST_KEY_MOUNT_POINT",
+		Completion: complete.PredictAnything,
+		Usage: "Mount point to the SSH backend where host keys are signed. " +
+			"When given a value, Vault will generate a custom \"known_hosts\" file " +
+			"with delegation to the CA at the provided mount point to verify the " +
+			"SSH connection's host keys against the provided CA. By default, host " +
+			"keys are validated against the user's local \"known_hosts\" file. " +
+			"This flag forces strict key host checking and ignores a custom user " +
+			"known hosts file.",
+	})
+
+	f.StringVar(&StringVar{
+		Name:       "host-key-hostnames",
+		Target:     &c.flagHostKeyHostnames,
+		Default:    "*",
+		EnvVar:     "VAULT_SSH_HOST_KEY_HOSTNAMES",
+		Completion: complete.PredictAnything,
+		Usage: "List of hostnames to delegate for the CA. The default value " +
+			"allows all domains and IPs. This is specified as a comma-separated " +
+			"list of values.",
+	})
+
+	return set
+}
+
+func (c *SSHCommand) AutocompleteArgs() complete.Predictor {
+	return nil
+}
+
+func (c *SSHCommand) AutocompleteFlags() complete.Flags {
+	return c.Flags().Completions()
 }
 
 // Structure to hold the fields returned when asked for a credential from SSHh backend.
@@ -58,74 +215,35 @@ type SSHCredentialResp struct {
 }
 
 func (c *SSHCommand) Run(args []string) int {
+	f := c.Flags()
 
-	flags := c.Meta.FlagSet("ssh", meta.FlagSetDefault)
+	if err := f.Parse(args); err != nil {
-
+		c.UI.Error(err.Error())
-	envOrDefault := func(key string, def string) string {
-		if k := os.Getenv(key); k != "" {
-			return k
-		}
-		return def
-	}
-
-	expandPath := func(p string) string {
-		e, err := homedir.Expand(p)
-		if err != nil {
-			return p
-		}
-		return e
-	}
-
-	// Common options
-	flags.StringVar(&c.mode, "mode", "", "")
-	flags.BoolVar(&c.noExec, "no-exec", false, "")
-	flags.StringVar(&c.format, "format", "table", "")
-	flags.StringVar(&c.mountPoint, "mount-point", "ssh", "")
-	flags.StringVar(&c.role, "role", "", "")
-
-	// Key options
-	flags.StringVar(&c.strictHostKeyChecking, "strict-host-key-checking",
-		envOrDefault("VAULT_SSH_STRICT_HOST_KEY_CHECKING", "ask"), "")
-	flags.StringVar(&c.userKnownHostsFile, "user-known-hosts-file",
-		envOrDefault("VAULT_SSH_USER_KNOWN_HOSTS_FILE", expandPath("~/.ssh/known_hosts")), "")
-
-	// CA-specific options
-	flags.StringVar(&c.publicKeyPath, "public-key-path",
-		expandPath("~/.ssh/id_rsa.pub"), "")
-	flags.StringVar(&c.privateKeyPath, "private-key-path",
-		expandPath("~/.ssh/id_rsa"), "")
-	flags.StringVar(&c.hostKeyMountPoint, "host-key-mount-point", "", "")
-	flags.StringVar(&c.hostKeyHostnames, "host-key-hostnames", "*", "")
-
-	flags.Usage = func() { c.Ui.Error(c.Help()) }
-	if err := flags.Parse(args); err != nil {
 		return 1
 	}
 
-	args = flags.Args()
+	// Use homedir to expand any relative paths such as ~/.ssh
+	c.flagUserKnownHostsFile = expandPath(c.flagUserKnownHostsFile)
+	c.flagPublicKeyPath = expandPath(c.flagPublicKeyPath)
+	c.flagPrivateKeyPath = expandPath(c.flagPrivateKeyPath)
+
+	args = f.Args()
 	if len(args) < 1 {
-		c.Ui.Error("ssh expects at least one argument")
+		c.UI.Error(fmt.Sprintf("Not enough arguments, (expected 1-n, got %d)", len(args)))
 		return 1
 	}
 
-	client, err := c.Client()
-	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error initializing client: %v", err))
-		return 1
-	}
-	c.client = client
-	c.sshClient = client.SSHWithMountPoint(c.mountPoint)
-
 	// Extract the username and IP.
-	c.username, c.ip, err = c.userAndIP(args[0])
+	username, ip, err := c.userAndIP(args[0])
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error parsing user and IP: %s", err))
+		c.UI.Error(fmt.Sprintf("Error parsing user and IP: %s", err))
 		return 1
 	}
 
 	// The rest of the args are ssh args
+	sshArgs := []string{}
 	if len(args) > 1 {
-		c.sshArgs = args[1:]
+		sshArgs = args[1:]
 	}
 
 	// Credentials are generated only against a registered role. If user
@@ -134,100 +252,101 @@ func (c *SSHCommand) Run(args []string) int {
 	// only one role associated with it, use it to establish the connection.
 	//
 	// TODO: remove in 0.9.0, convert to validation error
-	if c.role == "" {
+	if c.flagRole == "" {
-		c.Ui.Warn("" +
+		c.UI.Warn(wrapAtLength(
-			"WARNING: No -role specified. Use -role to tell Vault which ssh role\n" +
+			"WARNING: No -role specified. Use -role to tell Vault which ssh role " +
-			"to use for authentication. In the future, you will need to tell Vault\n" +
+				"to use for authentication. In the future, you will need to tell " +
-			"which role to use. For now, Vault will attempt to guess based on a\n" +
+				"Vault which role to use. For now, Vault will attempt to guess based " +
-			"the API response.")
+				"on a the API response. This will be removed in the next major " +
+				"version of Vault."))
 
-		role, err := c.defaultRole(c.mountPoint, c.ip)
+		role, err := c.defaultRole(c.flagMountPoint, ip)
 		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error choosing role: %v", err))
+			c.UI.Error(fmt.Sprintf("Error choosing role: %v", err))
 			return 1
 		}
 		// Print the default role chosen so that user knows the role name
 		// if something doesn't work. If the role chosen is not allowed to
 		// be used by the user (ACL enforcement), then user should see an
 		// error message accordingly.
-		c.Ui.Output(fmt.Sprintf("Vault SSH: Role: %q", role))
+		c.UI.Output(fmt.Sprintf("Vault SSH: Role: %q", role))
-		c.role = role
+		c.flagRole = role
 	}
 
 	// If no mode was given, perform the old-school lookup. Keep this now for
 	// backwards-compatability, but print a warning.
 	//
 	// TODO: remove in 0.9.0, convert to validation error
-	if c.mode == "" {
+	if c.flagMode == "" {
-		c.Ui.Warn("" +
+		c.UI.Warn(wrapAtLength(
-			"WARNING: No -mode specified. Use -mode to tell Vault which ssh\n" +
+			"WARNING: No -mode specified. Use -mode to tell Vault which ssh " +
-			"authentication mode to use. In the future, you will need to tell\n" +
+				"authentication mode to use. In the future, you will need to tell " +
-			"Vault which mode to use. For now, Vault will attempt to guess based\n" +
+				"Vault which mode to use. For now, Vault will attempt to guess based " +
-			"on the API response. This guess involves creating a temporary\n" +
+				"on the API response. This guess involves creating a temporary " +
-			"credential, reading its type, and then revoking it. To reduce the\n" +
+				"credential, reading its type, and then revoking it. To reduce the " +
-			"number of API calls and surface area, specify -mode directly.")
+				"number of API calls and surface area, specify -mode directly. This " +
-		secret, cred, err := c.generateCredential()
+				"will be removed in the next major version of Vault."))
+		secret, cred, err := c.generateCredential(username, ip)
 		if err != nil {
 			// This is _very_ hacky, but is the only sane backwards-compatible way
 			// to do this. If the error is "key type unknown", we just assume the
 			// type is "ca". In the future, mode will be required as an option.
 			if strings.Contains(err.Error(), "key type unknown") {
-				c.mode = ssh.KeyTypeCA
+				c.flagMode = ssh.KeyTypeCA
 			} else {
-				c.Ui.Error(fmt.Sprintf("Error getting credential: %s", err))
+				c.UI.Error(fmt.Sprintf("Error getting credential: %s", err))
 				return 1
 			}
 		} else {
-			c.mode = cred.KeyType
+			c.flagMode = cred.KeyType
 		}
 
 		// Revoke the secret, since the child functions will generate their own
 		// credential. Users wishing to avoid this should specify -mode.
 		if secret != nil {
 			if err := c.client.Sys().Revoke(secret.LeaseID); err != nil {
-				c.Ui.Warn(fmt.Sprintf("Failed to revoke temporary key: %s", err))
+				c.UI.Warn(fmt.Sprintf("Failed to revoke temporary key: %s", err))
 			}
 		}
 	}
 
-	switch strings.ToLower(c.mode) {
+	switch strings.ToLower(c.flagMode) {
 	case ssh.KeyTypeCA:
-		if err := c.handleTypeCA(); err != nil {
+		return c.handleTypeCA(username, ip, sshArgs)
-			c.Ui.Error(err.Error())
-			return 1
-		}
 	case ssh.KeyTypeOTP:
-		if err := c.handleTypeOTP(); err != nil {
+		return c.handleTypeOTP(username, ip, sshArgs)
-			c.Ui.Error(err.Error())
-			return 1
-		}
 	case ssh.KeyTypeDynamic:
-		if err := c.handleTypeDynamic(); err != nil {
+		return c.handleTypeDynamic(username, ip, sshArgs)
-			c.Ui.Error(err.Error())
-			return 1
-		}
 	default:
-		c.Ui.Error(fmt.Sprintf("Unknown SSH mode: %s", c.mode))
+		c.UI.Error(fmt.Sprintf("Unknown SSH mode: %s", c.flagMode))
 		return 1
 	}
-
-	return 0
 }
 
 // handleTypeCA is used to handle SSH logins using the "CA" key type.
-func (c *SSHCommand) handleTypeCA() error {
+func (c *SSHCommand) handleTypeCA(username, ip string, sshArgs []string) int {
 	// Read the key from disk
-	publicKey, err := ioutil.ReadFile(c.publicKeyPath)
+	publicKey, err := ioutil.ReadFile(c.flagPublicKeyPath)
 	if err != nil {
-		return errors.Wrap(err, "failed to read public key")
+		c.UI.Error(fmt.Sprintf("failed to read public key %s: %s",
+			c.flagPublicKeyPath, err))
+		return 1
 	}
 
+	client, err := c.Client()
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
+	sshClient := client.SSHWithMountPoint(c.flagMountPoint)
+
 	// Attempt to sign the public key
-	secret, err := c.sshClient.SignKey(c.role, map[string]interface{}{
+	secret, err := sshClient.SignKey(c.flagRole, map[string]interface{}{
 		// WARNING: publicKey is []byte, which is b64 encoded on JSON upload. We
 		// have to convert it to a string. SV lost many hours to this...
 		"public_key":       string(publicKey),
-		"valid_principals": c.username,
+		"valid_principals": username,
 		"cert_type":        "user",
 
 		// TODO: let the user configure these. In the interim, if users want to
@@ -241,55 +360,62 @@ func (c *SSHCommand) handleTypeCA() error {
 		},
 	})
 	if err != nil {
-		return errors.Wrap(err, "failed to sign public key")
+		c.UI.Error(fmt.Sprintf("failed to sign public key %s: %s",
+			c.flagPublicKeyPath, err))
+		return 2
 	}
 	if secret == nil || secret.Data == nil {
-		return fmt.Errorf("client signing returned empty credentials")
+		c.UI.Error("missing signed key")
+		return 2
 	}
 
 	// Handle no-exec
-	if c.noExec {
+	if c.flagNoExec {
-		// This is hacky, but OutputSecret returns an int, not an error :(
+		if c.flagFormat != "" {
-		if i := OutputSecret(c.Ui, c.format, secret); i != 0 {
+			return PrintRawField(c.UI, secret, c.flagField)
-			return fmt.Errorf("an error occurred outputting the secret")
 		}
-		return nil
+		return OutputSecret(c.UI, c.flagFormat, secret)
 	}
 
 	// Extract public key
 	key, ok := secret.Data["signed_key"].(string)
-	if !ok {
+	if !ok || key == "" {
-		return fmt.Errorf("missing signed key")
+		c.UI.Error("signed key is empty")
+		return 2
 	}
 
 	// Capture the current value - this could be overwritten later if the user
 	// enabled host key signing verification.
-	userKnownHostsFile := c.userKnownHostsFile
+	userKnownHostsFile := c.flagUserKnownHostsFile
-	strictHostKeyChecking := c.strictHostKeyChecking
+	strictHostKeyChecking := c.flagStrictHostKeyChecking
 
 	// Handle host key signing verification. If the user specified a mount point,
 	// download the public key, trust it with the given domains, and use that
 	// instead of the user's regular known_hosts file.
-	if c.hostKeyMountPoint != "" {
+	if c.flagHostKeyMountPoint != "" {
-		secret, err := c.client.Logical().Read(c.hostKeyMountPoint + "/config/ca")
+		secret, err := c.client.Logical().Read(c.flagHostKeyMountPoint + "/config/ca")
 		if err != nil {
-			return errors.Wrap(err, "failed to get host signing key")
+			c.UI.Error(fmt.Sprintf("failed to get host signing key: %s", err))
+			return 2
 		}
 		if secret == nil || secret.Data == nil {
-			return fmt.Errorf("missing host signing key")
+			c.UI.Error("missing host signing key")
+			return 2
 		}
 		publicKey, ok := secret.Data["public_key"].(string)
-		if !ok {
+		if !ok || publicKey == "" {
-			return fmt.Errorf("host signing key is empty")
+			c.UI.Error("host signing key is empty")
+			return 2
 		}
 
 		// Write the known_hosts file
-		name := fmt.Sprintf("vault_ssh_ca_known_hosts_%s_%s", c.username, c.ip)
+		name := fmt.Sprintf("vault_ssh_ca_known_hosts_%s_%s", username, ip)
-		data := fmt.Sprintf("@cert-authority %s %s", c.hostKeyHostnames, publicKey)
+		data := fmt.Sprintf("@cert-authority %s %s", c.flagHostKeyHostnames, publicKey)
 		knownHosts, err, closer := c.writeTemporaryFile(name, []byte(data), 0644)
 		defer closer()
 		if err != nil {
-			return errors.Wrap(err, "failed to write host public key")
+			c.UI.Error(fmt.Sprintf("failed to write host public key: %s", err))
+			return 1
 		}
 
 		// Update the variables
@@ -298,20 +424,21 @@ func (c *SSHCommand) handleTypeCA() error {
 	}
 
 	// Write the signed public key to disk
-	name := fmt.Sprintf("vault_ssh_ca_%s_%s", c.username, c.ip)
+	name := fmt.Sprintf("vault_ssh_ca_%s_%s", username, ip)
 	signedPublicKeyPath, err, closer := c.writeTemporaryKey(name, []byte(key))
 	defer closer()
 	if err != nil {
-		return errors.Wrap(err, "failed to write signed public key")
+		c.UI.Error(fmt.Sprintf("failed to write signed public key: %s", err))
+		return 2
 	}
 
 	args := append([]string{
-		"-i", c.privateKeyPath,
+		"-i", c.flagPrivateKeyPath,
 		"-i", signedPublicKeyPath,
 		"-o UserKnownHostsFile=" + userKnownHostsFile,
 		"-o StrictHostKeyChecking=" + strictHostKeyChecking,
-		c.username + "@" + c.ip,
+		username + "@" + ip,
-	}, c.sshArgs...)
+	}, sshArgs...)
 
 	cmd := exec.Command("ssh", args...)
 	cmd.Stdin = os.Stdin
@@ -319,61 +446,71 @@ func (c *SSHCommand) handleTypeCA() error {
 	cmd.Stderr = os.Stderr
 	err = cmd.Run()
 	if err != nil {
-		return errors.Wrap(err, "failed to run ssh command")
+		exitCode := 2
+
+		if exitError, ok := err.(*exec.ExitError); ok {
+			if exitError.Success() {
+				return 0
+			}
+			if ws, ok := exitError.Sys().(syscall.WaitStatus); ok {
+				exitCode = ws.ExitStatus()
+			}
+		}
+
+		c.UI.Error(fmt.Sprintf("failed to run ssh command: %s", err))
+		return exitCode
 	}
 
 	// There is no secret to revoke, since it's a certificate signing
-
+	return 0
-	return nil
 }
 
 // handleTypeOTP is used to handle SSH logins using the "otp" key type.
-func (c *SSHCommand) handleTypeOTP() error {
+func (c *SSHCommand) handleTypeOTP(username, ip string, sshArgs []string) int {
-	secret, cred, err := c.generateCredential()
+	secret, cred, err := c.generateCredential(username, ip)
 	if err != nil {
-		return errors.Wrap(err, "failed to generate credential")
+		c.UI.Error(fmt.Sprintf("failed to generate credential: %s", err))
+		return 2
 	}
 
 	// Handle no-exec
-	if c.noExec {
+	if c.flagNoExec {
-		// This is hacky, but OutputSecret returns an int, not an error :(
+		if c.flagFormat != "" {
-		if i := OutputSecret(c.Ui, c.format, secret); i != 0 {
+			return PrintRawField(c.UI, secret, c.flagField)
-			return fmt.Errorf("an error occurred outputting the secret")
 		}
-		return nil
+		return OutputSecret(c.UI, c.flagFormat, secret)
 	}
 
 	var cmd *exec.Cmd
 
-	// Check if the application 'sshpass' is installed in the client machine.
+	// Check if the application 'sshpass' is installed in the client machine. If
-	// If it is then, use it to automate typing in OTP to the prompt. Unfortunately,
+	// it is then, use it to automate typing in OTP to the prompt. Unfortunately,
 	// it was not possible to automate it without a third-party application, with
-	// only the Go libraries.
+	// only the Go libraries. Feel free to try and remove this dependency.
-	// Feel free to try and remove this dependency.
 	sshpassPath, err := exec.LookPath("sshpass")
 	if err != nil {
-		c.Ui.Warn("" +
+		c.UI.Warn(wrapAtLength(
-			"Vault could not locate sshpass. The OTP code for the session will be\n" +
+			"Vault could not locate \"sshpass\". The OTP code for the session is " +
-			"displayed below. Enter this code in the SSH password prompt. If you\n" +
+				"displayed below. Enter this code in the SSH password prompt. If you " +
-			"install sshpass, Vault can automatically perform this step for you.")
+				"install sshpass, Vault can automatically perform this step for you."))
-		c.Ui.Output("OTP for the session is " + cred.Key)
+		c.UI.Output("OTP for the session is: " + cred.Key)
 
 		args := append([]string{
-			"-o UserKnownHostsFile=" + c.userKnownHostsFile,
+			"-o UserKnownHostsFile=" + c.flagUserKnownHostsFile,
-			"-o StrictHostKeyChecking=" + c.strictHostKeyChecking,
+			"-o StrictHostKeyChecking=" + c.flagStrictHostKeyChecking,
 			"-p", cred.Port,
-			c.username + "@" + c.ip,
+			username + "@" + ip,
-		}, c.sshArgs...)
+		}, sshArgs...)
 		cmd = exec.Command("ssh", args...)
 	} else {
 		args := append([]string{
 			"-e", // Read password for SSHPASS environment variable
 			"ssh",
-			"-o UserKnownHostsFile=" + c.userKnownHostsFile,
+			"-o UserKnownHostsFile=" + c.flagUserKnownHostsFile,
-			"-o StrictHostKeyChecking=" + c.strictHostKeyChecking,
+			"-o StrictHostKeyChecking=" + c.flagStrictHostKeyChecking,
 			"-p", cred.Port,
-			c.username + "@" + c.ip,
+			username + "@" + ip,
-		}, c.sshArgs...)
+		}, sshArgs...)
 		cmd = exec.Command(sshpassPath, args...)
 		env := os.Environ()
 		env = append(env, fmt.Sprintf("SSHPASS=%s", string(cred.Key)))
@@ -385,49 +522,63 @@ func (c *SSHCommand) handleTypeOTP() error {
 	cmd.Stderr = os.Stderr
 	err = cmd.Run()
 	if err != nil {
-		return errors.Wrap(err, "failed to run ssh command")
+		exitCode := 2
+
+		if exitError, ok := err.(*exec.ExitError); ok {
+			if exitError.Success() {
+				return 0
+			}
+			if ws, ok := exitError.Sys().(syscall.WaitStatus); ok {
+				exitCode = ws.ExitStatus()
+			}
+		}
+
+		c.UI.Error(fmt.Sprintf("failed to run ssh command: %s", err))
+		return exitCode
 	}
 
 	// Revoke the key if it's longer than expected
 	if err := c.client.Sys().Revoke(secret.LeaseID); err != nil {
-		return errors.Wrap(err, "failed to revoke key")
+		c.UI.Error(fmt.Sprintf("failed to revoke key: %s", err))
+		return 2
 	}
 
-	return nil
+	return 0
 }
 
 // handleTypeDynamic is used to handle SSH logins using the "dyanmic" key type.
-func (c *SSHCommand) handleTypeDynamic() error {
+func (c *SSHCommand) handleTypeDynamic(username, ip string, sshArgs []string) int {
 	// Generate the credential
-	secret, cred, err := c.generateCredential()
+	secret, cred, err := c.generateCredential(username, ip)
 	if err != nil {
-		return errors.Wrap(err, "failed to generate credential")
+		c.UI.Error(fmt.Sprintf("failed to generate credential: %s", err))
+		return 2
 	}
 
 	// Handle no-exec
-	if c.noExec {
+	if c.flagNoExec {
-		// This is hacky, but OutputSecret returns an int, not an error :(
+		if c.flagFormat != "" {
-		if i := OutputSecret(c.Ui, c.format, secret); i != 0 {
+			return PrintRawField(c.UI, secret, c.flagField)
-			return fmt.Errorf("an error occurred outputting the secret")
 		}
-		return nil
+		return OutputSecret(c.UI, c.flagFormat, secret)
 	}
 
 	// Write the dynamic key to disk
-	name := fmt.Sprintf("vault_ssh_dynamic_%s_%s", c.username, c.ip)
+	name := fmt.Sprintf("vault_ssh_dynamic_%s_%s", username, ip)
 	keyPath, err, closer := c.writeTemporaryKey(name, []byte(cred.Key))
 	defer closer()
 	if err != nil {
-		return errors.Wrap(err, "failed to save dyanmic key")
+		c.UI.Error(fmt.Sprintf("failed to write dynamic key: %s", err))
+		return 1
 	}
 
 	args := append([]string{
 		"-i", keyPath,
-		"-o UserKnownHostsFile=" + c.userKnownHostsFile,
+		"-o UserKnownHostsFile=" + c.flagUserKnownHostsFile,
-		"-o StrictHostKeyChecking=" + c.strictHostKeyChecking,
+		"-o StrictHostKeyChecking=" + c.flagStrictHostKeyChecking,
 		"-p", cred.Port,
-		c.username + "@" + c.ip,
+		username + "@" + ip,
-	}, c.sshArgs...)
+	}, sshArgs...)
 
 	cmd := exec.Command("ssh", args...)
 	cmd.Stdin = os.Stdin
@@ -435,24 +586,44 @@ func (c *SSHCommand) handleTypeDynamic() error {
 	cmd.Stderr = os.Stderr
 	err = cmd.Run()
 	if err != nil {
-		return errors.Wrap(err, "failed to run ssh command")
+		exitCode := 2
+
+		if exitError, ok := err.(*exec.ExitError); ok {
+			if exitError.Success() {
+				return 0
+			}
+			if ws, ok := exitError.Sys().(syscall.WaitStatus); ok {
+				exitCode = ws.ExitStatus()
+			}
+		}
+
+		c.UI.Error(fmt.Sprintf("failed to run ssh command: %s", err))
+		return exitCode
 	}
 
 	// Revoke the key if it's longer than expected
 	if err := c.client.Sys().Revoke(secret.LeaseID); err != nil {
-		return errors.Wrap(err, "failed to revoke key")
+		c.UI.Error(fmt.Sprintf("failed to revoke key: %s", err))
+		return 2
 	}
 
-	return nil
+	return 0
 }
 
 // generateCredential generates a credential for the given role and returns the
 // decoded secret data.
-func (c *SSHCommand) generateCredential() (*api.Secret, *SSHCredentialResp, error) {
+func (c *SSHCommand) generateCredential(username, ip string) (*api.Secret, *SSHCredentialResp, error) {
+	client, err := c.Client()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	sshClient := client.SSHWithMountPoint(c.flagMountPoint)
+
 	// Attempt to generate the credential.
-	secret, err := c.sshClient.Credential(c.role, map[string]interface{}{
+	secret, err := sshClient.Credential(c.flagRole, map[string]interface{}{
-		"username": c.username,
+		"username": username,
-		"ip":       c.ip,
+		"ip":       ip,
 	})
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "failed to get credentials")
@@ -540,9 +711,9 @@ func (c *SSHCommand) defaultRole(mountPoint, ip string) (string, error) {
 		}
 		roleNames = strings.TrimRight(roleNames, ", ")
 		return "", fmt.Errorf("Roles:%q. "+`
-		Multiple roles are registered for this IP.
+			Multiple roles are registered for this IP.
-		Select a role using '-role' option.
+			Select a role using '-role' option.
-		Note that all roles may not be permitted, based on ACLs.`, roleNames)
+			Note that all roles may not be permitted, based on ACLs.`, roleNames)
 	}
 }
 
@@ -580,102 +751,3 @@ func (c *SSHCommand) userAndIP(s string) (string, string, error) {
 
 	return username, ip, nil
 }
-
-func (c *SSHCommand) Synopsis() string {
-	return "Initiate an SSH session"
-}
-
-func (c *SSHCommand) Help() string {
-	helpText := `
-Usage: vault ssh [options] username@ip [ssh options]
-
-  Establishes an SSH connection with the target machine.
-
-  This command uses one of the SSH authentication backends to authenticate and
-  automatically establish an SSH connection to a host. This operation requires
-  that the SSH backend is mounted and configured.
-
-  SSH using the OTP mode (requires sshpass for full automation):
-
-      $ vault ssh -mode=otp -role=my-role user@1.2.3.4
-
-  SSH using the CA mode:
-
-      $ vault ssh -mode=ca -role=my-role user@1.2.3.4
-
-  SSH using CA mode with host key verification:
-
-      $ vault ssh \
-          -mode=ca \
-          -role=my-role \
-          -host-key-mount-point=host-signer \
-          -host-key-hostnames=example.com \
-          user@example.com
-
-  For the full list of options and arguments, please see the documentation.
-
-General Options:
-` + meta.GeneralOptionsUsage() + `
-SSH Options:
-
-  -role            Role to be used to create the key. Each IP is associated with
-                   a role. To see the associated roles with IP, use "lookup"
-                   endpoint. If you are certain that there is only one role
-                   associated with the IP, you can skip mentioning the role. It
-                   will be chosen by default. If there are no roles associated
-                   with the IP, register the CIDR block of that IP using the
-                   "roles/" endpoint.
-
-  -no-exec         Shows the credentials but does not establish connection.
-
-  -mount-point     Mount point of SSH backend. If the backend is mounted at
-                   "ssh" (default), this parameter can be skipped.
-
-  -format          If the "no-exec" option is enabled, the credentials will be
-                   printed out and SSH connection will not be established. The
-                   format of the output can be "json" or "table" (default).
-
-  -strict-host-key-checking   This option corresponds to "StrictHostKeyChecking"
-                   of SSH configuration. If "sshpass" is employed to enable
-                   automated login, then if host key is not "known" to the
-                   client, "vault ssh" command will fail. Set this option to
-                   "no" to bypass the host key checking. Defaults to "ask".
-                   Can also be specified with the
-                   "VAULT_SSH_STRICT_HOST_KEY_CHECKING" environment variable.
-
-  -user-known-hosts-file   This option corresponds to "UserKnownHostsFile" of
-                   SSH configuration. Assigns the file to use for storing the
-                   host keys. If this option is set to "/dev/null" along with
-                   "-strict-host-key-checking=no", both warnings and host key
-                   checking can be avoided while establishing the connection.
-                   Defaults to "~/.ssh/known_hosts". Can also be specified with
-                   "VAULT_SSH_USER_KNOWN_HOSTS_FILE" environment variable.
-
-CA Mode Options:
-
-  - public-key-path=<path>
-      The path to the public key to send to Vault for signing. The default value
-      is ~/.ssh/id_rsa.pub.
-
-  - private-key-path=<path>
-      The path to the private key to use for authentication. This must be the
-      corresponding private key to -public-key-path. The default value is
-      ~/.ssh/id_rsa.
-
-  - host-key-mount-point=<string>
-      The mount point to the SSH backend where host keys are signed. When given
-      a value, Vault will generate a custom known_hosts file with delegation to
-      the CA at the provided mount point and verify the SSH connection's host
-      keys against the provided CA. By default, this command uses the users's
-      existing known_hosts file. When this flag is set, this command will force
-      strict host key checking and will override any values provided for a
-      custom -user-known-hosts-file.
-
-  - host-key-hostnames=<string>
-      The list of hostnames to delegate for this certificate authority. By
-      default, this is "*", which allows all domains and IPs. To restrict
-      validation to a series of hostnames, specify them as comma-separated
-      values here.
-`
-	return strings.TrimSpace(helpText)
-}

command/ssh_test.go

@@ -1,199 +1,23 @@
 package command
 
 import (
-	"bytes"
-	"fmt"
-	"io"
-	"os"
-	"strings"
 	"testing"
 
-	logicalssh "github.com/hashicorp/vault/builtin/logical/ssh"
-	"github.com/hashicorp/vault/http"
-	"github.com/hashicorp/vault/meta"
-	"github.com/hashicorp/vault/vault"
 	"github.com/mitchellh/cli"
 )
 
-const (
+func testSSHCommand(tb testing.TB) (*cli.MockUi, *SSHCommand) {
-	testCidr             = "127.0.0.1/32"
+	tb.Helper()
-	testRoleName         = "testRoleName"
-	testKey              = "testKey"
-	testSharedPrivateKey = `
------BEGIN RSA PRIVATE KEY-----
-MIIEogIBAAKCAQEAvYvoRcWRxqOim5VZnuM6wHCbLUeiND0yaM1tvOl+Fsrz55DG
-A0OZp4RGAu1Fgr46E1mzxFz1+zY4UbcEExg+u21fpa8YH8sytSWW1FyuD8ICib0A
-/l8slmDMw4BkkGOtSlEqgscpkpv/TWZD1NxJWkPcULk8z6c7TOETn2/H9mL+v2RE
-mbE6NDEwJKfD3MvlpIqCP7idR+86rNBAODjGOGgyUbtFLT+K01XmDRALkV3V/nh+
-GltyjL4c6RU4zG2iRyV5RHlJtkml+UzUMkzr4IQnkCC32CC/wmtoo/IsAprpcHVe
-nkBn3eFQ7uND70p5n6GhN/KOh2j519JFHJyokwIDAQABAoIBAHX7VOvBC3kCN9/x
-+aPdup84OE7Z7MvpX6w+WlUhXVugnmsAAVDczhKoUc/WktLLx2huCGhsmKvyVuH+
-MioUiE+vx75gm3qGx5xbtmOfALVMRLopjCnJYf6EaFA0ZeQ+NwowNW7Lu0PHmAU8
-Z3JiX8IwxTz14DU82buDyewO7v+cEr97AnERe3PUcSTDoUXNaoNxjNpEJkKREY6h
-4hAY676RT/GsRcQ8tqe/rnCqPHNd7JGqL+207FK4tJw7daoBjQyijWuB7K5chSal
-oPInylM6b13ASXuOAOT/2uSUBWmFVCZPDCmnZxy2SdnJGbsJAMl7Ma3MUlaGvVI+
-Tfh1aQkCgYEA4JlNOabTb3z42wz6mz+Nz3JRwbawD+PJXOk5JsSnV7DtPtfgkK9y
-6FTQdhnozGWShAvJvc+C4QAihs9AlHXoaBY5bEU7R/8UK/pSqwzam+MmxmhVDV7G
-IMQPV0FteoXTaJSikhZ88mETTegI2mik+zleBpVxvfdhE5TR+lq8Br0CgYEA2AwJ
-CUD5CYUSj09PluR0HHqamWOrJkKPFPwa+5eiTTCzfBBxImYZh7nXnWuoviXC0sg2
-AuvCW+uZ48ygv/D8gcz3j1JfbErKZJuV+TotK9rRtNIF5Ub7qysP7UjyI7zCssVM
-kuDd9LfRXaB/qGAHNkcDA8NxmHW3gpln4CFdSY8CgYANs4xwfercHEWaJ1qKagAe
-rZyrMpffAEhicJ/Z65lB0jtG4CiE6w8ZeUMWUVJQVcnwYD+4YpZbX4S7sJ0B8Ydy
-AhkSr86D/92dKTIt2STk6aCN7gNyQ1vW198PtaAWH1/cO2UHgHOy3ZUt5X/Uwxl9
-cex4flln+1Viumts2GgsCQKBgCJH7psgSyPekK5auFdKEr5+Gc/jB8I/Z3K9+g4X
-5nH3G1PBTCJYLw7hRzw8W/8oALzvddqKzEFHphiGXK94Lqjt/A4q1OdbCrhiE68D
-My21P/dAKB1UYRSs9Y8CNyHCjuZM9jSMJ8vv6vG/SOJPsnVDWVAckAbQDvlTHC9t
-O98zAoGAcbW6uFDkrv0XMCpB9Su3KaNXOR0wzag+WIFQRXCcoTvxVi9iYfUReQPi
-oOyBJU/HMVvBfv4g+OVFLVgSwwm6owwsouZ0+D/LasbuHqYyqYqdyPJQYzWA2Y+F
-+B6f4RoPdSXj24JHPg/ioRxjaj094UXJxua2yfkcecGNEuBQHSs=
------END RSA PRIVATE KEY-----
-`
-)
 
-var testIP string
+	ui := cli.NewMockUi()
-var testPort string
+	return ui, &SSHCommand{
-var testUserName string
+		BaseCommand: &BaseCommand{
-var testAdminUser string
+			UI: ui,
-
+		},
-// Starts the server and initializes the servers IP address,
-// port and usernames to be used by the test cases.
-func initTest() {
-	addr, err := vault.StartSSHHostTestServer()
-	if err != nil {
-		panic(fmt.Sprintf("Error starting mock server:%s", err))
 	}
-	input := strings.Split(addr, ":")
-	testIP = input[0]
-	testPort = input[1]
-
-	testUserName := os.Getenv("VAULT_SSHTEST_USER")
-	if len(testUserName) == 0 {
-		panic("VAULT_SSHTEST_USER must be set to the desired user")
-	}
-	testAdminUser = testUserName
 }
 
-// This test is broken. Hence temporarily disabling it.
+func TestSSHCommand_Run(t *testing.T) {
-func testSSH(t *testing.T) {
+	t.Parallel()
-	initTest()
+	t.Skip("Need a way to setup target infrastructure")
-	// Add the SSH backend to the unsealed test core.
-	// This should be done before the unsealed core is created.
-	err := vault.AddTestLogicalBackend("ssh", logicalssh.Factory)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	core, _, token := vault.TestCoreUnsealed(t)
-	ln, addr := http.TestServer(t, core)
-	defer ln.Close()
-
-	ui := new(cli.MockUi)
-	mountCmd := &MountCommand{
-		Meta: meta.Meta{
-			ClientToken: token,
-			Ui:          ui,
-		},
-	}
-
-	args := []string{"-address", addr, "ssh"}
-
-	// Mount the SSH backend
-	if code := mountCmd.Run(args); code != 0 {
-		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
-	}
-
-	client, err := mountCmd.Client()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	mounts, err := client.Sys().ListMounts()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Check if SSH backend is mounted or not
-	mount, ok := mounts["ssh/"]
-	if !ok {
-		t.Fatal("should have ssh mount")
-	}
-	if mount.Type != "ssh" {
-		t.Fatal("should have ssh type")
-	}
-
-	writeCmd := &WriteCommand{
-		Meta: meta.Meta{
-			ClientToken: token,
-			Ui:          ui,
-		},
-	}
-
-	// Create a 'named' key in vault
-	args = []string{
-		"-address", addr,
-		"ssh/keys/" + testKey,
-		"key=" + testSharedPrivateKey,
-	}
-	if code := writeCmd.Run(args); code != 0 {
-		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
-	}
-
-	// Create a role using the named key along with cidr, username and port
-	args = []string{
-		"-address", addr,
-		"ssh/roles/" + testRoleName,
-		"key=" + testKey,
-		"admin_user=" + testUserName,
-		"cidr=" + testCidr,
-		"port=" + testPort,
-	}
-	if code := writeCmd.Run(args); code != 0 {
-		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
-	}
-
-	sshCmd := &SSHCommand{
-		Meta: meta.Meta{
-			ClientToken: token,
-			Ui:          ui,
-		},
-	}
-
-	// Get the dynamic key and establish an SSH connection with target.
-	// Inline command when supplied, runs on target and terminates the
-	// connection. Use whoami as the inline command in target and get
-	// the result. Compare the result with the username used to connect
-	// to target. Test succeeds if they match.
-	args = []string{
-		"-address", addr,
-		"-role=" + testRoleName,
-		testUserName + "@" + testIP,
-		"/usr/bin/whoami",
-	}
-
-	// Creating pipe to get the result of the inline command run in target machine.
-	stdout := os.Stdout
-	r, w, err := os.Pipe()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	os.Stdout = w
-	if code := sshCmd.Run(args); code != 0 {
-		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
-	}
-	bufChan := make(chan string)
-	go func() {
-		var buf bytes.Buffer
-		io.Copy(&buf, r)
-		bufChan <- buf.String()
-	}()
-	w.Close()
-	os.Stdout = stdout
-	userName := <-bufChan
-	userName = strings.TrimSpace(userName)
-
-	// Comparing the username used to connect to target and
-	// the username on the target, thereby verifying successful
-	// execution
-	if userName != testUserName {
-		t.Fatalf("err: username mismatch")
-	}
 }