From da20a8b434012732d7a484c2e8fdeca845c4cb7c Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Thu, 2 Dec 2021 21:48:22 -0800 Subject: [PATCH] feat(credential): Add support for Vault KV2 --- .../vault/internal/userpassword/doc.go | 3 + .../internal/userpassword/userpassword.go | 96 +++++++++ .../userpassword/userpassword_test.go | 196 ++++++++++++++++++ internal/credential/vault/private_library.go | 15 +- .../credential/vault/private_library_test.go | 189 +++++++++++++++++ .../vault/repository_credentials_test.go | 30 ++- 6 files changed, 513 insertions(+), 16 deletions(-) create mode 100644 internal/credential/vault/internal/userpassword/doc.go create mode 100644 internal/credential/vault/internal/userpassword/userpassword.go create mode 100644 internal/credential/vault/internal/userpassword/userpassword_test.go diff --git a/internal/credential/vault/internal/userpassword/doc.go b/internal/credential/vault/internal/userpassword/doc.go new file mode 100644 index 000000000..13c2b4264 --- /dev/null +++ b/internal/credential/vault/internal/userpassword/doc.go @@ -0,0 +1,3 @@ +// Package userpassword provides access to the username and password +// stored in a Vault secret. +package userpassword diff --git a/internal/credential/vault/internal/userpassword/userpassword.go b/internal/credential/vault/internal/userpassword/userpassword.go new file mode 100644 index 000000000..3f04ab096 --- /dev/null +++ b/internal/credential/vault/internal/userpassword/userpassword.go @@ -0,0 +1,96 @@ +package userpassword + +type ( + data map[string]interface{} + + // extractFunc attempts to extract the username and password + // from sd using the provided attribute names, using a known + // Vault data response format. + extractFunc func(sd data, usernameAttr, passwordAttr string) (string, string) +) + +// Extract attempts to extract the values of the username and password +// stored within the provided data using the given attribute names. +// +// Extract does not return partial results, i.e. if one of the attributes +// were extracted but not the other ("", "") will be returned. +func Extract(d data, usernameAttr, passwordAttr string) (string, string) { + for _, f := range []extractFunc{ + defaultExtract, + kv2Extract, + } { + username, password := f(d, usernameAttr, passwordAttr) + if username != "" && password != "" { + // got valid username and password from secret + return username, password + } + } + + return "", "" +} + +// defaultExtract looks for the usernameAttr and passwordAttr in the data map +func defaultExtract(sd data, usernameAttr, passwordAttr string) (username string, password string) { + if u, ok := sd[usernameAttr]; ok { + if u, ok := u.(string); ok { + username = u + } + } + if p, ok := sd[passwordAttr]; ok { + if p, ok := p.(string); ok { + password = p + } + } + + return username, password +} + +// kv2Extract looks for the the usernameAttr and passwordAttr in the embedded +// 'data' field within the data map. +// +// Additionaly it validates the data is in the expected KV-v2 format: +// { +// "data": {}, +// "metadata: {} +// } +// If the format does not match, it returns ("", ""). See: +// https://www.vaultproject.io/api/secret/kv/kv-v2#sample-response-1 +func kv2Extract(d data, usernameAttr, passwordAttr string) (username string, password string) { + var data, metadata map[string]interface{} + for k, v := range d { + switch k { + case "data": + var ok bool + if data, ok = v.(map[string]interface{}); !ok { + // data field should be of type map[string]interface{} in KV-v2 + return "", "" + } + case "metadata": + var ok bool + if metadata, ok = v.(map[string]interface{}); !ok { + // metadata field should be of type map[string]interface{} in KV-v2 + return "", "" + } + default: + // secretData contains a non valid KV-v2 top level field + return "", "" + } + } + if data == nil || metadata == nil { + // missing required KV-v2 field + return "", "" + } + + if u, ok := data[usernameAttr]; ok { + if u, ok := u.(string); ok { + username = u + } + } + if p, ok := data[passwordAttr]; ok { + if p, ok := p.(string); ok { + password = p + } + } + + return username, password +} diff --git a/internal/credential/vault/internal/userpassword/userpassword_test.go b/internal/credential/vault/internal/userpassword/userpassword_test.go new file mode 100644 index 000000000..3a6fcae7a --- /dev/null +++ b/internal/credential/vault/internal/userpassword/userpassword_test.go @@ -0,0 +1,196 @@ +package userpassword + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBaseToUsrPass(t *testing.T) { + t.Parallel() + + type args struct { + s data + uAttr string + pAttr string + } + type usrPass struct { + user string + pass string + } + tests := []struct { + name string + given args + want usrPass + }{ + { + name: "nil-input", + want: usrPass{user: "", pass: ""}, + }, + { + name: "no-input", + given: args{}, + want: usrPass{user: "", pass: ""}, + }, + { + name: "no-secret", + given: args{ + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "no-match-username-secret", + given: args{ + s: data{ + "username-wrong": "user", + "password": "pass", + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "no-match-username-secret", + given: args{ + s: data{ + "username": "user", + "password-wrong": "pass", + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "valid-default", + given: args{ + s: data{ + "username": "user", + "password": "pass", + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "user", pass: "pass"}, + }, + { + name: "no-match-username-secret-kv2", + given: args{ + s: data{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username-wrong": "user", + "password": "pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "no-match-password-secret-kv2", + given: args{ + s: data{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "user", + "password-wrong": "pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "valid-kv2", + given: args{ + s: data{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "user", pass: "pass"}, + }, + { + name: "no-metadata-kv2", + given: args{ + s: data{ + "data": map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "invalid-metadata-kv2", + given: args{ + s: data{ + "metadata": "string", + "data": map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "invalid-field-kv2", + given: args{ + s: data{ + "invalid": map[string]interface{}{}, + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "user", + "password": "pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "", pass: ""}, + }, + { + name: "valid-order-default-first", + given: args{ + s: data{ + "username": "default-user", + "password": "default-pass", + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "kv2-user", + "password": "kv2-pass", + }, + }, + uAttr: "username", + pAttr: "password", + }, + want: usrPass{user: "default-user", pass: "default-pass"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + user, pass := Extract(tt.given.s, tt.given.uAttr, tt.given.pAttr) + assert.Equal(tt.want.user, user) + assert.Equal(tt.want.pass, pass) + }) + } +} diff --git a/internal/credential/vault/private_library.go b/internal/credential/vault/private_library.go index 393616551..f0392b29e 100644 --- a/internal/credential/vault/private_library.go +++ b/internal/credential/vault/private_library.go @@ -8,6 +8,7 @@ import ( "time" "github.com/hashicorp/boundary/internal/credential" + "github.com/hashicorp/boundary/internal/credential/vault/internal/userpassword" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" @@ -69,19 +70,7 @@ func baseToUsrPass(ctx context.Context, bc *baseCred) (*usrPassCred, error) { if pAttr == "" { pAttr = "password" } - - var username, password string - if u, ok := bc.secretData[uAttr]; ok { - if u, ok := u.(string); ok { - username = u - } - } - if p, ok := bc.secretData[pAttr]; ok { - if p, ok := p.(string); ok { - password = p - } - } - + username, password := userpassword.Extract(bc.secretData, uAttr, pAttr) if username == "" || password == "" { return nil, errors.E(ctx, errors.WithCode(errors.VaultInvalidCredentialMapping)) } diff --git a/internal/credential/vault/private_library_test.go b/internal/credential/vault/private_library_test.go index fcc6f652d..c2f71b62f 100644 --- a/internal/credential/vault/private_library_test.go +++ b/internal/credential/vault/private_library_test.go @@ -462,6 +462,195 @@ func TestBaseToUsrPass(t *testing.T) { }, wantErr: errors.VaultInvalidCredentialMapping, }, + { + name: "invalid-kv2-no-metadata-field", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "data": map[string]interface{}{ + "username": "my-username", + "password": "my-password", + }, + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "invalid-kv2-no-data-field", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "invalid-kv2-no-username-default-password-attribute", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "password": "my-password", + }, + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "invalid-kv2-no-passsword-default-username-attribute", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "my-username", + }, + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "invalid-kv2-invalid-metadata-type", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "metadata": "hello", + "data": map[string]interface{}{ + "username": "my-username", + "password": "my-password", + }, + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "invalid-kv2-invalid-metadata-type", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": "hello", + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "invalid-kv2-additional-field", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "bad-field": "hello", + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "my-username", + "password": "my-password", + }, + }, + }, + wantErr: errors.VaultInvalidCredentialMapping, + }, + { + name: "valid-kv2-default-attributes", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "my-username", + "password": "my-password", + }, + }, + }, + want: &usrPassCred{ + username: "my-username", + password: credential.Password("my-password"), + }, + }, + { + name: "valid-kv2-override-attributes", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + UsernameAttribute: "test-username", + PasswordAttribute: "test-password", + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "default-username", + "password": "default-password", + "test-username": "override-username", + "test-password": "override-password", + }, + }, + }, + want: &usrPassCred{ + username: "override-username", + password: credential.Password("override-password"), + }, + }, + { + name: "valid-kv2-default-username-override-password", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + PasswordAttribute: "test-password", + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "default-username", + "password": "default-password", + "test-username": "override-username", + "test-password": "override-password", + }, + }, + }, + want: &usrPassCred{ + username: "default-username", + password: credential.Password("override-password"), + }, + }, + { + name: "valid-kv2-override-username-default-password", + given: &baseCred{ + lib: &privateLibrary{ + CredType: string(credential.UserPasswordType), + UsernameAttribute: "test-username", + }, + secretData: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "data": map[string]interface{}{ + "username": "default-username", + "password": "default-password", + "test-username": "override-username", + "test-password": "override-password", + }, + }, + }, + want: &usrPassCred{ + username: "override-username", + password: credential.Password("default-password"), + }, + }, } for _, tt := range tests { tt := tt diff --git a/internal/credential/vault/repository_credentials_test.go b/internal/credential/vault/repository_credentials_test.go index 7689f661a..978663109 100644 --- a/internal/credential/vault/repository_credentials_test.go +++ b/internal/credential/vault/repository_credentials_test.go @@ -45,7 +45,7 @@ func TestRepository_IssueCredentials(t *testing.T) { _, token := v.CreateToken(t, vault.WithPolicies([]string{"default", "boundary-controller", "database", "pki", "secret"})) - // Create valid KV secret + // Create valid user password KV secret v.CreateKVSecret(t, "my-secret", []byte(`{"data":{"username":"user","password":"pass"}}`)) var opts []vault.Option @@ -70,6 +70,7 @@ func TestRepository_IssueCredentials(t *testing.T) { libErrPKI libKV libErrKV + libUsrPassKV ) libs := make(map[libT]string) @@ -154,6 +155,19 @@ func TestRepository_IssueCredentials(t *testing.T) { require.NotNil(t, lib) libs[libErrUsrPassDB] = lib.GetPublicId() } + { + libPath := path.Join("secret", "data", "my-secret") + opts := []vault.Option{ + vault.WithCredentialType(credential.UserPasswordType), + } + libIn, err := vault.NewCredentialLibrary(origStore.GetPublicId(), libPath, opts...) + assert.NoError(t, err) + require.NotNil(t, libIn) + lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn) + assert.NoError(t, err) + require.NotNil(t, lib) + libs[libUsrPassKV] = lib.GetPublicId() + } at := authtoken.TestAuthToken(t, conn, kms, org.GetPublicId()) uId := at.GetIamUserId() @@ -245,7 +259,7 @@ func TestRepository_IssueCredentials(t *testing.T) { }, }, { - name: "one-valid-username-password-library", + name: "one-db-valid-username-password-library", convertFn: rc2dc, requests: []credential.Request{ { @@ -266,7 +280,7 @@ func TestRepository_IssueCredentials(t *testing.T) { wantErr: errors.VaultEmptySecret, }, { - name: "one-valid-username-password-library", + name: "invalid-username-password-library", convertFn: rc2dc, requests: []credential.Request{ { @@ -276,6 +290,16 @@ func TestRepository_IssueCredentials(t *testing.T) { }, wantErr: errors.VaultInvalidCredentialMapping, }, + { + name: "valid-kv-username-password-library", + convertFn: rc2dc, + requests: []credential.Request{ + { + SourceId: libs[libUsrPassKV], + Purpose: credential.ApplicationPurpose, + }, + }, + }, } for _, tt := range tests { tt := tt