CyberCoveLLC/threshold
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bug(credential): Fix panic in credential Issue

Browse Source 
Vault KV backend returns a nil secret and no error if the secret does not
exist. This caused a panic in during Issue:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x1388279]

goroutine 2639 [running]:
github.com/hashicorp/boundary/internal/credential/vault.(*Repository).Issue(0xc001222200, {0x3b932d0, 0xc001203350}, {0xc000dc6e90, 0xc}, {0xc0010e5e40, 0x0, 0x0})
        /home/louis/boundary/internal/credential/vault/repository_credentials.go:67 +0x2d9
github.com/hashicorp/boundary/internal/servers/controller/handlers/targets.Service.AuthorizeSession({{}, 0xc000cd4a38, 0xc000cd4840, 0xc000cd48d0, 0xc000cd4a50, 0xc000cd4870, 0xc000cd4858, 0xc000cd48b8, 0xc0001a8880}, {0x3b932d0, ...}, ...)
        /home/louis/boundary/internal/servers/controller/handlers/targets/target_service.go:1059 +0x1525
github.com/hashicorp/boundary/internal/gen/controller/api/services._TargetService_AuthorizeSession_Handler.func1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880})
        /home/louis/boundary/internal/gen/controller/api/services/target_service_grpc.pb.go:631 +0x78
github.com/hashicorp/boundary/internal/servers/controller.auditResponseInterceptor.func1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880}, 0x461279, 0x40d234)
        /home/louis/boundary/internal/servers/controller/interceptor.go:262 +0x4e
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880})
        /home/louis/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x3a
github.com/hashicorp/boundary/internal/servers/controller.statusCodeInterceptor.func1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880}, 0x203000, 0x203000)
        /home/louis/boundary/internal/servers/controller/interceptor.go:207 +0x34
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880})
        /home/louis/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x3a
github.com/hashicorp/boundary/internal/servers/controller.errorInterceptor.func1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880}, 0x18e2b40, 0x0)
        /home/louis/boundary/internal/servers/controller/interceptor.go:156 +0x4e
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880})
        /home/louis/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x3a
github.com/hashicorp/boundary/internal/servers/controller.auditRequestInterceptor.func1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880}, 0x1a1c860, 0xc0010e5280)
        /home/louis/boundary/internal/servers/controller/interceptor.go:248 +0x198
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x3b932d0, 0xc001203350}, {0x1a0dd40, 0xc000dc8880})
        /home/louis/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x3a
github.com/hashicorp/boundary/internal/servers/controller.requestCtxInterceptor.func1({0x3b932d0, 0xc001203260}, {0x1a0dd40, 0xc000dc8880}, 0x18, 0xc0010e52a0)
        /home/louis/boundary/internal/servers/controller/interceptor.go:140 +0x5de
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1({0x3b932d0, 0xc001203260}, {0x1a0dd40, 0xc000dc8880})
        /home/louis/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:25 +0x3a
github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1({0x3b932d0, 0xc001203260}, {0x1a0dd40, 0xc000dc8880}, 0xc00331abb8, 0x18675e0)
        /home/louis/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.3.0/chain.go:34 +0xbf
github.com/hashicorp/boundary/internal/gen/controller/api/services._TargetService_AuthorizeSession_Handler({0x1ada420, 0xc00093a180}, {0x3b932d0, 0xc001203260}, 0xc001200840, 0xc000465260)
        /home/louis/boundary/internal/gen/controller/api/services/target_service_grpc.pb.go:633 +0x138
google.golang.org/grpc.(*Server).processUnaryRPC(0xc000cca540, {0x3bb4c58, 0xc0005a9c80}, 0xc001208480, 0xc0004a7440, 0x499e2b8, 0x0)
        /home/louis/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:1297 +0xccf
google.golang.org/grpc.(*Server).handleStream(0xc000cca540, {0x3bb4c58, 0xc0005a9c80}, 0xc001208480, 0x0)
        /home/louis/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:1626 +0xa2a
google.golang.org/grpc.(*Server).serveStreams.func1.2()
        /home/louis/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:941 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /home/louis/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:939 +0x294
This commit is contained in:
Louis Ruch committed 2021-12-15 16:13:30 -08:00
1 parent 155d8e1cbd
commit 9b39345c7f
7 files changed
+161 -1
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/credential/vault/repository_credentials.go
+3
View File
@@ -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
+48 -1
View File
@@ -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
+33
View File
@@ -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
+67
View File
@@ -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
+1
View File
@@ -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
+5
View File
@@ -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
+4
View File
@@ -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,