fix(cli): Fallback parsing of un-typed credentials (#2989)

The `boundary connect` subcommands attempt to parse the credentials to use for brokering. This allows the subcommands to insert the credentials automatically into the process called by the subcommand, i.e. `ssh` or `psql`. There is some fallback logic to try a best attempt at parsing the credential in cases where a credential type has not be set on the credential source. The bug was introduced with the recent change to the vault credential library subtype being renamed from `vault` to `vault-generic`. This fallback logic would only run if the credential source type was "vault" or "static", so if the vault credential library was created without specifying a credential-type, the credential would not be parsed and automatically passed to the sub-process. This fix removes the additional check on the credential source's type, instead it will attempt to parse the credential, relying just on the structure of the credential. This also fixes a similar bug in the table output of secrets when using `boundary connect` without a subcommand. The parsing of the credentials for display in the table was only running for `vault` or `static`. Blame: db42eafd7a
3 years ago · b69d1c2e2e
parent 41b1b5b4b7
commit b69d1c2e2e
4 changed files with 171 additions and 52 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -2,6 +2,22 @@

 Canonical reference for changes, improvements, and bugfixes for Boundary.

+## Next
+
+### Bug Fixes
+
+* cli: Fix fallback parsing of un-typed credentials for `boundary connect`.
+  When using a vault credential library with no credential type set, boundary
+  will perform a best effort attempt to parse any brokered credentials. If the
+  credentials are successfully parsed, they can be used by the subprocess
+  spawned when using the connect subcommands, i.e. `ssh` or `psql`. With the
+  change to the vault credential library subtype, this fallback parsing would
+  fail. Note that if the credential library has a credential type, by using
+  `-credential-type` when creating the credential library, the credential was
+  correctly parsed. This fallback parsing is now fixed, but in order to support
+  older clients, credential libraries will need to be recreated with a
+  credential type. [PR](https://github.com/hashicorp/boundary/pull/2989)
+
 ## 0.12.0 (2023/01/24)

 ### Deprecations/Changes
--- a/internal/cmd/commands/connect/credentials.go
+++ b/internal/cmd/commands/connect/credentials.go
@ -92,29 +92,26 @@ func parseCredentials(creds []*targets.SessionCredential) (credentials, error) {
 		}

 		// Credential type is unspecified, make a best effort attempt to parse
-		// a username_password credential from the Decoded field if it exists
+		// a credential from the Decoded field if it exists
 		if cred.Secret != nil && cred.Secret.Decoded != nil {
-			switch cred.CredentialSource.Type {
-			case "vault", "static":
-				// Attempt unmarshaling into username password creds
-				if err := mapstructure.Decode(cred.Secret.Decoded, &upCred); err != nil {
-					return credentials{}, err
-				}
-				if upCred.Username != "" && upCred.Password != "" {
-					upCred.raw = cred
-					out.usernamePassword = append(out.usernamePassword, upCred)
-					continue
-				}
-
-				// Attempt unmarshaling into ssh private key creds
-				if err := mapstructure.Decode(cred.Secret.Decoded, &spkCred); err != nil {
-					return credentials{}, err
-				}
-				if spkCred.Username != "" && spkCred.PrivateKey != "" {
-					spkCred.raw = cred
-					out.sshPrivateKey = append(out.sshPrivateKey, spkCred)
-					continue
-				}
+			// Attempt unmarshaling into username password creds
+			if err := mapstructure.Decode(cred.Secret.Decoded, &upCred); err != nil {
+				return credentials{}, err
+			}
+			if upCred.Username != "" && upCred.Password != "" {
+				upCred.raw = cred
+				out.usernamePassword = append(out.usernamePassword, upCred)
+				continue
+			}
+
+			// Attempt unmarshaling into ssh private key creds
+			if err := mapstructure.Decode(cred.Secret.Decoded, &spkCred); err != nil {
+				return credentials{}, err
+			}
+			if spkCred.Username != "" && spkCred.PrivateKey != "" {
+				spkCred.raw = cred
+				out.sshPrivateKey = append(out.sshPrivateKey, spkCred)
+				continue
 			}
 		}

--- a/internal/cmd/commands/connect/credentials_test.go
+++ b/internal/cmd/commands/connect/credentials_test.go
@ -8,6 +8,8 @@ import (

 	"github.com/hashicorp/boundary/api/targets"
 	"github.com/hashicorp/boundary/internal/credential"
+	"github.com/hashicorp/boundary/internal/credential/static"
+	"github.com/hashicorp/boundary/internal/credential/vault"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@ -33,9 +35,33 @@ var (
 		},
 	}

+	vaultUsernamePasswordDeprecatedSubtype = &targets.SessionCredential{
+		CredentialSource: &targets.CredentialSource{
+			Type: vault.Subtype.String(),
+		},
+		Secret: &targets.SessionSecret{
+			Decoded: map[string]any{
+				"username": "vault-decoded-user",
+				"password": "vault-decoded-pass",
+			},
+		},
+	}
+
+	vaultSshPrivateKeyDeprecatedSubtype = &targets.SessionCredential{
+		CredentialSource: &targets.CredentialSource{
+			Type: vault.Subtype.String(),
+		},
+		Secret: &targets.SessionSecret{
+			Decoded: map[string]any{
+				"username":    "vault-decoded-user",
+				"private_key": "vault-decoded-pk",
+			},
+		},
+	}
+
 	vaultUsernamePassword = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type: "vault",
+			Type: vault.GenericLibrarySubtype.String(),
 		},
 		Secret: &targets.SessionSecret{
 			Decoded: map[string]any{
@ -47,7 +73,7 @@ var (

 	vaultSshPrivateKey = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type: "vault",
+			Type: vault.GenericLibrarySubtype.String(),
 		},
 		Secret: &targets.SessionSecret{
 			Decoded: map[string]any{
@ -57,9 +83,29 @@ var (
 		},
 	}

+	unknownUsernamePassword = &targets.SessionCredential{
+		CredentialSource: &targets.CredentialSource{},
+		Secret: &targets.SessionSecret{
+			Decoded: map[string]any{
+				"username": "unknown-decoded-user",
+				"password": "unknown-decoded-pass",
+			},
+		},
+	}
+
+	unknownSshPrivateKey = &targets.SessionCredential{
+		CredentialSource: &targets.CredentialSource{},
+		Secret: &targets.SessionSecret{
+			Decoded: map[string]any{
+				"username":    "unknown-decoded-user",
+				"private_key": "unknown-decoded-pk",
+			},
+		},
+	}
+
 	staticUsernamePassword = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type: "static",
+			Type: static.Subtype.String(),
 		},
 		Secret: &targets.SessionSecret{
 			Decoded: map[string]any{
@ -71,7 +117,7 @@ var (

 	staticSshPrivateKey = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type: "static",
+			Type: static.Subtype.String(),
 		},
 		Secret: &targets.SessionSecret{
 			Decoded: map[string]any{
@ -83,7 +129,7 @@ var (

 	staticKv = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type:              "static",
+			Type:              static.Subtype.String(),
 			CredentialType:    "json",
 			CredentialStoreId: "csst_id",
 			Description:       "test",
@ -101,7 +147,7 @@ var (

 	unspecifiedCred = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type: "static",
+			Type: static.Subtype.String(),
 		},
 		Secret: &targets.SessionSecret{
 			Decoded: map[string]any{
@ -113,7 +159,7 @@ var (

 	unspecifiedCred1 = &targets.SessionCredential{
 		CredentialSource: &targets.CredentialSource{
-			Type: "static",
+			Type: static.Subtype.String(),
 		},
 		Secret: &targets.SessionSecret{
 			Decoded: map[string]any{
@ -213,6 +259,70 @@ func Test_parseCredentials(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "vault-deprecated-username-password-decoded",
+			creds: []*targets.SessionCredential{
+				vaultUsernamePasswordDeprecatedSubtype,
+			},
+			wantCreds: credentials{
+				usernamePassword: []usernamePassword{
+					{
+						Username: "vault-decoded-user",
+						Password: "vault-decoded-pass",
+						raw:      vaultUsernamePasswordDeprecatedSubtype,
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "vault-deprecated-private-key-decoded",
+			creds: []*targets.SessionCredential{
+				vaultSshPrivateKeyDeprecatedSubtype,
+			},
+			wantCreds: credentials{
+				sshPrivateKey: []sshPrivateKey{
+					{
+						Username:   "vault-decoded-user",
+						PrivateKey: "vault-decoded-pk",
+						raw:        vaultSshPrivateKeyDeprecatedSubtype,
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "unknown-username-password-decoded",
+			creds: []*targets.SessionCredential{
+				unknownUsernamePassword,
+			},
+			wantCreds: credentials{
+				usernamePassword: []usernamePassword{
+					{
+						Username: "unknown-decoded-user",
+						Password: "unknown-decoded-pass",
+						raw:      unknownUsernamePassword,
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "unknown-private-key-decoded",
+			creds: []*targets.SessionCredential{
+				unknownSshPrivateKey,
+			},
+			wantCreds: credentials{
+				sshPrivateKey: []sshPrivateKey{
+					{
+						Username:   "unknown-decoded-user",
+						PrivateKey: "unknown-decoded-pk",
+						raw:        unknownSshPrivateKey,
+					},
+				},
+			},
+			wantErr: false,
+		},
 		{
 			name: "static-username-password-decoded",
 			creds: []*targets.SessionCredential{
--- a/internal/cmd/commands/connect/funcs.go
+++ b/internal/cmd/commands/connect/funcs.go
@ -85,33 +85,29 @@ func generateCredentialTableOutputSlice(prefixIndent int, creds []*targets.Sessi
 func fmtSecretForTable(indent int, sc *targets.SessionCredential) []string {
 	prefixStr := strings.Repeat(" ", indent)
 	origSecret := []string{fmt.Sprintf("%s    %s", prefixStr, sc.Secret.Raw)}
-	switch sc.CredentialSource.Type {
-	case "vault", "static":
-		if sc.Credential != nil {
-			maxLength := 0
-			for k := range sc.Credential {
-				if len(k) > maxLength {
-					maxLength = len(k)
-				}
+	if sc.Credential != nil {
+		maxLength := 0
+		for k := range sc.Credential {
+			if len(k) > maxLength {
+				maxLength = len(k)
 			}
-			return []string{fmt.Sprintf("%s    %s", prefixStr, base.WrapMap(2, maxLength+2, sc.Credential))}
 		}
+		return []string{fmt.Sprintf("%s    %s", prefixStr, base.WrapMap(2, maxLength+2, sc.Credential))}
+	}

-		in, err := base64.StdEncoding.DecodeString(strings.Trim(string(sc.Secret.Raw), `"`))
-		if err != nil {
-			return origSecret
-		}
-		dst := new(bytes.Buffer)
-		if err := json.Indent(dst, in, fmt.Sprintf("%s    ", prefixStr), fmt.Sprintf("%s  ", prefixStr)); err != nil {
-			return origSecret
-		}
-		secretStr := strings.Split(dst.String(), "\n")
-		if len(secretStr) > 0 {
-			secretStr[0] = fmt.Sprintf("%s    %s", prefixStr, secretStr[0])
-		}
-		return secretStr
+	in, err := base64.StdEncoding.DecodeString(strings.Trim(string(sc.Secret.Raw), `"`))
+	if err != nil {
+		return origSecret
+	}
+	dst := new(bytes.Buffer)
+	if err := json.Indent(dst, in, fmt.Sprintf("%s    ", prefixStr), fmt.Sprintf("%s  ", prefixStr)); err != nil {
+		return origSecret
+	}
+	secretStr := strings.Split(dst.String(), "\n")
+	if len(secretStr) > 0 {
+		secretStr[0] = fmt.Sprintf("%s    %s", prefixStr, secretStr[0])
 	}
-	return origSecret
+	return secretStr
 }

 func generateConnectionInfoTableOutput(in ConnectionInfo) string {