From 5ef9496e0af46bb14dee81e0587b92467a977372 Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Wed, 15 Jun 2022 09:30:45 -0700 Subject: [PATCH] feat(cli): Support credential brokering with sshpass (#2191) --- CHANGELOG.md | 6 + internal/cmd/commands/connect/connect.go | 17 +- internal/cmd/commands/connect/credentials.go | 61 +++++ .../cmd/commands/connect/credentials_test.go | 231 ++++++++++++++++++ internal/cmd/commands/connect/postgres.go | 43 ++-- internal/cmd/commands/connect/ssh.go | 51 +++- 6 files changed, 371 insertions(+), 38 deletions(-) create mode 100644 internal/cmd/commands/connect/credentials.go create mode 100644 internal/cmd/commands/connect/credentials_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a7b62adf7..411fba65e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. credential sources for targets, similar to credential libraries from the `vault` credential store, and thus can be brokered to users at session authorization time. [PR](https://github.com/hashicorp/boundary/pull/2174) +* `boundary connect` Credential Brokering Integration: we have extended integration + into the `boundary connect` helpers. A new `sshpass` style has been added to the + `ssh` helper, when used, if the credential contains a username/password and `sshpass` + is installed, the command will automatically pass the credentials to the `ssh` process. + Additionally, the default `ssh` helper will now use the `username` of the brokered credential. + [PR](https://github.com/hashicorp/boundary/pull/2191). * controller: Improve response time for listing sessions. This also creates a new periodic job that will delete terminated sessions after 1 hour. diff --git a/internal/cmd/commands/connect/connect.go b/internal/cmd/commands/connect/connect.go index 4c6617499..90f99977c 100644 --- a/internal/cmd/commands/connect/connect.go +++ b/internal/cmd/commands/connect/connect.go @@ -492,6 +492,15 @@ func (c *Command) Run(args []string) (retCode int) { // Credentials are brokered when connecting to the postgres db. // TODO: Figure out how to handle cases where we don't automatically know how to // broker the credentials like unrecognized or multiple credentials. + case "ssh": + if c.flagExec == "sshpass" { + // If we can broker ssh credentials using sshpass, skip displaying creds on client side. + // TODO: Figure out how to handle multiple credentials + break + } + + // We cannot broker credentials print to user + fallthrough case "connect": // "connect" indicates there is no subcommand to the connect function. // The only way a user will be able to connect to the session is by @@ -838,7 +847,13 @@ func (c *Command) handleExec(passthroughArgs []string) { args = append(args, c.rdpFlags.buildArgs(c, port, ip, addr)...) case "ssh": - args = append(args, c.sshFlags.buildArgs(c, port, ip, addr)...) + sshArgs, sshEnvs, sshErr := c.sshFlags.buildArgs(c, port, ip, addr) + if sshErr != nil { + argsErr = sshErr + break + } + args = append(args, sshArgs...) + envs = append(envs, sshEnvs...) case "kube": kubeArgs, err := c.kubeFlags.buildArgs(c, port, ip, addr) diff --git a/internal/cmd/commands/connect/credentials.go b/internal/cmd/commands/connect/credentials.go new file mode 100644 index 000000000..7ca65cf30 --- /dev/null +++ b/internal/cmd/commands/connect/credentials.go @@ -0,0 +1,61 @@ +package connect + +import ( + "errors" + + "github.com/hashicorp/boundary/api/targets" + "github.com/hashicorp/boundary/internal/credential" + "github.com/mitchellh/mapstructure" +) + +type usernamePasswordCredentials struct { + Username string `mapstructure:"username"` + Password string `mapstructure:"password"` +} + +// TODO: this should support multiple types of credentials and return an interface instead +// of only usernamepassword. Each subcommand can use the returned credentials as it needs. +func parseCredentials(creds []*targets.SessionCredential) ([]usernamePasswordCredentials, error) { + if creds == nil { + return nil, nil + } + var out []usernamePasswordCredentials + for _, cred := range creds { + if cred.CredentialSource == nil { + return nil, errors.New("missing credential source") + } + + var c usernamePasswordCredentials + if cred.CredentialSource.CredentialType == string(credential.UsernamePasswordType) { + // Decode attributes from credential struct + if err := mapstructure.Decode(cred.Credential, &c); err != nil { + return nil, err + } + + if c.Username != "" && c.Password != "" { + out = append(out, c) + continue + } + } + + // Credential type is unspecified, make a best effort attempt to parse both username + // and password from Decoded field if it exists + if cred.Secret == nil || cred.Secret.Decoded == nil { + // No secret data continue to next credential + continue + } + + switch cred.CredentialSource.Type { + case "vault", "static": + // Attempt unmarshaling into creds + if err := mapstructure.Decode(cred.Secret.Decoded, &c); err != nil { + return nil, err + } + } + + if c.Username != "" && c.Password != "" { + out = append(out, c) + } + } + return out, nil +} diff --git a/internal/cmd/commands/connect/credentials_test.go b/internal/cmd/commands/connect/credentials_test.go new file mode 100644 index 000000000..5d1f8615f --- /dev/null +++ b/internal/cmd/commands/connect/credentials_test.go @@ -0,0 +1,231 @@ +package connect + +import ( + "testing" + + "github.com/hashicorp/boundary/api/targets" + "github.com/hashicorp/boundary/internal/credential" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_parseUsernamePasswordCredentials(t *testing.T) { + tests := []struct { + name string + creds []*targets.SessionCredential + wantCreds []usernamePasswordCredentials + wantErr bool + }{ + { + name: "no-creds", + wantCreds: nil, + wantErr: false, + }, + { + name: "no-credential-source", + creds: []*targets.SessionCredential{ + { + Credential: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + }, + wantCreds: nil, + wantErr: true, + }, + { + name: "valid-typed", + creds: []*targets.SessionCredential{ + { + CredentialSource: &targets.CredentialSource{ + CredentialType: string(credential.UsernamePasswordType), + }, + Credential: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + }, + wantCreds: []usernamePasswordCredentials{ + { + Username: "user", + Password: "pass", + }, + }, + wantErr: false, + }, + { + name: "valid-typed-instead-of-decoded", + creds: []*targets.SessionCredential{ + { + CredentialSource: &targets.CredentialSource{ + CredentialType: string(credential.UsernamePasswordType), + }, + Credential: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + Secret: &targets.SessionSecret{ + Decoded: map[string]interface{}{ + "username": "secret-user", + "password": "secret-pass", + }, + }, + }, + }, + wantCreds: []usernamePasswordCredentials{ + { + Username: "user", + Password: "pass", + }, + }, + wantErr: false, + }, + { + name: "valid-vault-not-typed", + creds: []*targets.SessionCredential{ + { + CredentialSource: &targets.CredentialSource{ + Type: "vault", + }, + Secret: &targets.SessionSecret{ + Decoded: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + }, + }, + wantCreds: []usernamePasswordCredentials{ + { + Username: "user", + Password: "pass", + }, + }, + wantErr: false, + }, + { + name: "valid-static-not-typed", + creds: []*targets.SessionCredential{ + { + CredentialSource: &targets.CredentialSource{ + Type: "static", + }, + Secret: &targets.SessionSecret{ + Decoded: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + }, + }, + wantCreds: []usernamePasswordCredentials{ + { + Username: "user", + Password: "pass", + }, + }, + wantErr: false, + }, + { + name: "valid-multiple", + creds: []*targets.SessionCredential{ + { + CredentialSource: &targets.CredentialSource{ + CredentialType: string(credential.UsernamePasswordType), + }, + Credential: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + { + CredentialSource: &targets.CredentialSource{ + CredentialType: string(credential.UsernamePasswordType), + }, + Credential: map[string]interface{}{ + "username": "user1", + "password": "pass1", + }, + }, + }, + wantCreds: []usernamePasswordCredentials{ + { + Username: "user", + Password: "pass", + }, + { + Username: "user1", + Password: "pass1", + }, + }, + wantErr: false, + }, + { + name: "valid-multiple-mixed", + creds: []*targets.SessionCredential{ + { + CredentialSource: &targets.CredentialSource{ + CredentialType: string(credential.UsernamePasswordType), + }, + Credential: map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + { + CredentialSource: &targets.CredentialSource{ + Type: "vault", + }, + Secret: &targets.SessionSecret{ + Decoded: map[string]interface{}{ + "username": "user1", + "password": "pass1", + }, + }, + }, + { + CredentialSource: &targets.CredentialSource{ + Type: "static", + }, + Secret: &targets.SessionSecret{ + Decoded: map[string]interface{}{ + "username": "user2", + "password": "pass2", + }, + }, + }, + }, + wantCreds: []usernamePasswordCredentials{ + { + Username: "user", + Password: "pass", + }, + { + Username: "user1", + Password: "pass1", + }, + { + Username: "user2", + Password: "pass2", + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + creds, err := parseCredentials(tt.creds) + if tt.wantErr { + require.Error(err) + assert.Nil(creds) + return + } + require.NoError(err) + assert.ElementsMatch(tt.wantCreds, creds) + }) + } +} diff --git a/internal/cmd/commands/connect/postgres.go b/internal/cmd/commands/connect/postgres.go index 8ef73ffee..9ab0abe2a 100644 --- a/internal/cmd/commands/connect/postgres.go +++ b/internal/cmd/commands/connect/postgres.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/boundary/internal/cmd/base" - "github.com/mitchellh/mapstructure" "github.com/posener/complete" ) @@ -52,32 +51,18 @@ func (p *postgresFlags) defaultExec() string { return strings.ToLower(p.flagPostgresStyle) } -type postgresCredentials struct { - Username string `mapstructure:"username"` - Password string `mapstructure:"password"` -} - func (p *postgresFlags) buildArgs(c *Command, port, ip, addr string) (args, envs []string, retErr error) { - var creds postgresCredentials - if c.sessionAuthz != nil && len(c.sessionAuthz.Credentials) > 0 { - for _, cred := range c.sessionAuthz.Credentials { - if cred.Secret == nil || cred.Secret.Decoded == nil { - continue - } - switch cred.CredentialSource.Type { - case "vault": - // Attempt unmarshaling into creds - if err := mapstructure.Decode(cred.Secret.Decoded, &creds); err != nil { - return nil, nil, fmt.Errorf("Error interpreting Vault secret: %w", err) - } - } - - if creds.Username != "" && creds.Password != "" { - // In the future we can look for other types if we support other - // authentication mechanisms - break - } + var cred usernamePasswordCredentials + if c.sessionAuthz != nil { + creds, err := parseCredentials(c.sessionAuthz.Credentials) + if err != nil { + return nil, nil, fmt.Errorf("Error interpreting secret: %w", err) } + if len(creds) > 0 { + // Just use first credentials returned + cred = creds[0] + } + } switch p.flagPostgresStyle { @@ -89,13 +74,13 @@ func (p *postgresFlags) buildArgs(c *Command, port, ip, addr string) (args, envs } switch { - case creds.Username != "": - args = append(args, "-U", creds.Username) + case cred.Username != "": + args = append(args, "-U", cred.Username) case c.flagUsername != "": args = append(args, "-U", c.flagUsername) } - if creds.Password != "" { + if cred.Password != "" { passfile, err := ioutil.TempFile("", "*") if err != nil { return nil, nil, fmt.Errorf("Error saving postgres password to tmp file: %w", err) @@ -106,7 +91,7 @@ func (p *postgresFlags) buildArgs(c *Command, port, ip, addr string) (args, envs } return nil }) - _, err = passfile.WriteString(fmt.Sprintf("*:*:*:*:%s", creds.Password)) + _, err = passfile.WriteString(fmt.Sprintf("*:*:*:*:%s", cred.Password)) if err != nil { return nil, nil, fmt.Errorf("Error writing password file to %s: %w", passfile.Name(), err) } diff --git a/internal/cmd/commands/connect/ssh.go b/internal/cmd/commands/connect/ssh.go index df446e88c..38ae46496 100644 --- a/internal/cmd/commands/connect/ssh.go +++ b/internal/cmd/commands/connect/ssh.go @@ -1,6 +1,7 @@ package connect import ( + "errors" "fmt" "strings" @@ -19,7 +20,7 @@ func sshOptions(c *Command, set *base.FlagSets) { Name: "style", Target: &c.flagSshStyle, EnvVar: "BOUNDARY_CONNECT_SSH_STYLE", - Completion: complete.PredictSet("ssh", "putty"), + Completion: complete.PredictSet("ssh", "putty", "sshpass"), Default: "ssh", Usage: `Specifies how the CLI will attempt to invoke an SSH client. This will also set a suitable default for -exec if a value was not specified. Currently-understood values are "ssh" and "putty".`, }) @@ -41,18 +42,52 @@ func (s *sshFlags) defaultExec() string { return strings.ToLower(s.flagSshStyle) } -func (s *sshFlags) buildArgs(c *Command, port, ip, addr string) []string { - // Might want -t for ssh or -tt but seems fine without it for now... - var args []string - switch s.flagSshStyle { +func (s *sshFlags) buildArgs(c *Command, port, ip, addr string) (args, envs []string, retErr error) { + var cred usernamePasswordCredentials + if c.sessionAuthz != nil { + creds, err := parseCredentials(c.sessionAuthz.Credentials) + if err != nil { + return nil, nil, fmt.Errorf("Error interpreting secret: %w", err) + } + if len(creds) > 0 { + // Just use first credentials returned + cred = creds[0] + } + } + + switch strings.ToLower(s.flagSshStyle) { case "ssh": + // Might want -t for ssh or -tt but seems fine without it for now... args = append(args, "-p", port, ip) - args = append(args, "-o", fmt.Sprintf("HostKeyAlias=%s", c.sessionAuthzData.HostId)) + + // SSH detects a host key change when localhost proxy port changes, disable localhost + // host key verification to avoid 'WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED...' + args = append(args, "-o", "NoHostAuthenticationForLocalhost=yes") + + case "sshpass": + if cred.Password == "" { + return nil, nil, errors.New("Password is required when using sshpass") + } + // sshpass requires that the password is passed as env-var "SSHPASS" + // when the using env-vars. + envs = append(envs, fmt.Sprintf("SSHPASS=%s", cred.Password)) + args = append(args, "-e", "ssh") + args = append(args, "-p", port, ip) + + // sshpass cannot handle host key checking, disable localhost key verification + // to avoid error: 'SSHPASS detected host authentication prompt. Exiting.' + args = append(args, "-o", "NoHostAuthenticationForLocalhost=yes") + case "putty": args = append(args, "-P", port, ip) } - if c.flagUsername != "" { + + switch { + case cred.Username != "": + args = append(args, "-l", cred.Username) + case c.flagUsername != "": args = append(args, "-l", c.flagUsername) } - return args + + return args, envs, nil }