Merge pull request #1798 from hashicorp/louis-panic

bug(credential): Fix panic in credential Issue

internal/credential/vault/repository_credentials.go

@@ -63,6 +63,9 @@ func (r *Repository) Issue(ctx context.Context, sessionId string, requests []cre
 			// expired or invalid token
 			return nil, errors.Wrap(ctx, err, op)
 		}
+		if secret == nil {
+			return nil, errors.E(ctx, errors.WithCode(errors.VaultEmptySecret), errors.WithOp(op))
+		}
 
 		leaseDuration := time.Duration(secret.LeaseDuration) * time.Second
 		if minLease > leaseDuration {

internal/credential/vault/repository_credentials_test.go

@@ -28,6 +28,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
 	v := vault.NewTestVaultServer(t, vault.WithDockerNetwork(true), vault.WithTestVaultTLS(vault.TestClientTLS))
 	v.MountDatabase(t)
 	v.MountPKI(t)
+	v.AddKVPolicy(t)
 
 	conn, _ := db.TestSetup(t, "postgres")
 	rw := db.New(conn)
@@ -44,7 +45,10 @@ func TestRepository_IssueCredentials(t *testing.T) {
 	err = vault.RegisterJobs(ctx, sche, rw, rw, kms)
 	require.NoError(err)
 
-	_, token := v.CreateToken(t, vault.WithPolicies([]string{"default", "boundary-controller", "database", "pki"}))
+	_, token := v.CreateToken(t, vault.WithPolicies([]string{"default", "boundary-controller", "database", "pki", "secret"}))
+
+	// Create valid KV secret
+	v.CreateKVSecret(t, "my-secret", []byte(`{"data":{"username":"user","password":"pass"}}`))
 
 	var opts []vault.Option
 	opts = append(opts, vault.WithCACert(v.CaCert))
@@ -64,6 +68,8 @@ func TestRepository_IssueCredentials(t *testing.T) {
 		libDB libT = iota
 		libPKI
 		libErrPKI
+		libKV
+		libErrKV
 	)
 
 	libs := make(map[libT]string)
@@ -98,6 +104,26 @@ func TestRepository_IssueCredentials(t *testing.T) {
 		require.NotNil(lib)
 		libs[libErrPKI] = lib.GetPublicId()
 	}
+	{
+		libPath := path.Join("secret", "data", "my-secret")
+		libIn, err := vault.NewCredentialLibrary(origStore.GetPublicId(), libPath, opts...)
+		assert.NoError(err)
+		require.NotNil(libIn)
+		lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
+		assert.NoError(err)
+		require.NotNil(lib)
+		libs[libKV] = lib.GetPublicId()
+	}
+	{
+		libPath := path.Join("secret", "data", "fake-secret")
+		libIn, err := vault.NewCredentialLibrary(origStore.GetPublicId(), libPath, opts...)
+		assert.NoError(err)
+		require.NotNil(libIn)
+		lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
+		assert.NoError(err)
+		require.NotNil(lib)
+		libs[libErrKV] = lib.GetPublicId()
+	}
 
 	at := authtoken.TestAuthToken(t, conn, kms, org.GetPublicId())
 	uId := at.GetIamUserId()
@@ -178,6 +204,27 @@ func TestRepository_IssueCredentials(t *testing.T) {
 			},
 			wantErr: errors.InvalidDynamicCredential,
 		},
+		{
+			name:      "valid-kv-secret",
+			convertFn: rc2dc,
+			requests: []credential.Request{
+				{
+					SourceId: libs[libKV],
+					Purpose:  credential.IngressPurpose,
+				},
+			},
+		},
+		{
+			name:      "invalid-kv-does-not-exist",
+			convertFn: rc2dc,
+			requests: []credential.Request{
+				{
+					SourceId: libs[libErrKV],
+					Purpose:  credential.ApplicationPurpose,
+				},
+			},
+			wantErr: errors.VaultEmptySecret,
+		},
 	}
 	for _, tt := range tests {
 		tt := tt

internal/credential/vault/testing.go

@@ -789,6 +789,39 @@ func (v *TestVaultServer) MountPKI(t *testing.T, opt ...TestOption) *vault.Secre
 	return s
 }
 
+// AddKVPolicy adds a Vault policy named 'secret' to v and adds it to the
+// standard set of polices attached to tokens created with v.CreateToken.
+// The policy is defined as:
+//
+//   path "secret/*" {
+//     capabilities = ["create", "read", "update", "delete", "list"]
+//   }
+//
+// All options are ignored.
+func (v *TestVaultServer) AddKVPolicy(t *testing.T, _ ...TestOption) {
+	t.Helper()
+
+	pc := pathCapabilities{
+		"secret/data/*": createCapability | readCapability | updateCapability | deleteCapability | listCapability,
+	}
+	v.addPolicy(t, "secret", pc)
+}
+
+// CreateKVSecret calls the /secret/data/:p endpoint with the provided
+// data. Please note for KV-v2 the provided data needs to be in JSON format similar to:
+// `{"data": {"key": "value", "key2": "value2"}}`
+// See
+// https://www.vaultproject.io/api-docs/secret/kv/kv-v2#create-update-secret
+func (v *TestVaultServer) CreateKVSecret(t *testing.T, p string, data []byte) *vault.Secret {
+	t.Helper()
+	require := require.New(t)
+
+	vc := v.client(t)
+	cred, err := vc.post(path.Join("secret", "data", p), data)
+	require.NoError(err)
+	return cred
+}
+
 // TestVaultServer is a vault server running in a docker container suitable
 // for testing.
 type TestVaultServer struct {

internal/credential/vault/testing_test.go

@@ -7,11 +7,13 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"encoding/pem"
+	"net/http"
 	"path"
 	"testing"
 	"time"
 
 	"github.com/hashicorp/boundary/internal/db"
+	"github.com/hashicorp/boundary/internal/errors"
 	"github.com/hashicorp/boundary/internal/iam"
 	"github.com/hashicorp/go-secure-stdlib/parseutil"
 	vault "github.com/hashicorp/vault/api"
@@ -540,3 +542,68 @@ func Test_testClientCert(t *testing.T) {
 	assert.NotEqual(cert1.Cert.Cert, cert2.Cert.Cert)
 	assert.Equal(cert1.Cert.Key, cert2.Cert.Key)
 }
+
+func TestTestVaultServer_AddKVPolicy(t *testing.T) {
+	t.Run("defaults", func(t *testing.T) {
+		t.Parallel()
+		assert, require := assert.New(t), require.New(t)
+		v := NewTestVaultServer(t)
+
+		sec := v.CreateKVSecret(t, "my-secret", []byte(`{"data" : {"foo":"bar"}}`))
+		require.NotNil(sec)
+
+		_, token := v.CreateToken(t, WithPolicies([]string{"default", "secret"}))
+		require.NotNil(token)
+		client := v.clientUsingToken(t, token)
+
+		_, err := client.get("/secret/data/my-secret")
+		assert.Error(err)
+
+		// An attempt to get my-secret should now fail with a 403
+		var respErr *vault.ResponseError
+		ok := errors.As(err, &respErr)
+		require.True(ok)
+		assert.Equal(http.StatusForbidden, respErr.StatusCode)
+
+		// Add KV policy and get should work
+		v.AddKVPolicy(t)
+		_, token = v.CreateToken(t, WithPolicies([]string{"default", "secret"}))
+		require.NotNil(token)
+		client = v.clientUsingToken(t, token)
+
+		_, err = client.get("/secret/data/my-secret")
+		assert.NoError(err)
+	})
+}
+
+func TestTestVaultServer_CreateKVSecret(t *testing.T) {
+	t.Run("defaults", func(t *testing.T) {
+		t.Parallel()
+		assert, require := assert.New(t), require.New(t)
+		v := NewTestVaultServer(t)
+		vc := v.client(t)
+
+		// Attempt to read my-secret should return a nil secret
+		got, err := vc.cl.Logical().Read("/secret/data/my-secret")
+		require.NoError(err)
+		require.Nil(got)
+
+		secret := v.CreateKVSecret(t, "my-secret", []byte(`{"data" : {"foo":"bar"}}`))
+		require.NotNil(secret)
+
+		// Now that secret exists try read again
+		got, err = vc.cl.Logical().Read("/secret/data/my-secret")
+		require.NoError(err)
+		require.NotNil(got)
+		require.NotNil(got.Data)
+		require.NotNil(got.Data["data"])
+
+		gotData, ok := got.Data["data"].(map[string]interface{})
+		require.True(ok)
+		require.NotNil(gotData["foo"])
+
+		gotFoo, ok := gotData["foo"].(string)
+		require.True(ok)
+		assert.Equal("bar", gotFoo)
+	})
+}

internal/errors/code.go

@@ -107,6 +107,7 @@ const (
 	VaultTokenNotRenewable        Code = 3012 // VaultTokenNotRenewable represents an error for a Vault token that is not renewable
 	VaultTokenMissingCapabilities Code = 3013 // VaultTokenMissingCapabilities represents an error for a Vault token that is missing capabilities
 	VaultCredentialRequest        Code = 3014 // VaultCredentialRequest represents an error returned from Vault when retrieving a credential
+	VaultEmptySecret              Code = 3015 // VaultEmptySecret represents a empty secret was returned from Vault without error
 
 	// OIDC authentication provided errors
 	OidcProviderCallbackError Code = 4000 // OidcProviderCallbackError represents an error that is passed by the OIDC provider to the callback endpoint

internal/errors/code_test.go

@@ -282,6 +282,11 @@ func TestCode_Both_String_Info(t *testing.T) {
 			c:    VaultCredentialRequest,
 			want: VaultCredentialRequest,
 		},
+		{
+			name: "VaultEmptySecret",
+			c:    VaultEmptySecret,
+			want: VaultEmptySecret,
+		},
 		{
 			name: "OidcProviderCallbackError",
 			c:    OidcProviderCallbackError,

internal/errors/info.go

@@ -228,6 +228,10 @@ var errorCodeInfo = map[Code]Info{
 		Message: "request for a new credential from vault failed",
 		Kind:    External,
 	},
+	VaultEmptySecret: {
+		Message: "vault secret is empty",
+		Kind:    Integrity,
+	},
 	OidcProviderCallbackError: {
 		Message: "oidc provider callback error",
 		Kind:    External,