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

fix(vault): Only persist revokable credentials

Browse Source 
When issuing credentials from vault credential libraries, only persist
the vault credential if it is revokable. A credential that is revokable
has a valid lease id that can be used to revoke the lease in vault. If
the credential is not revokable it is also not renewable since renewing
also requires a lease id.

See: https://developer.hashicorp.com/vault/docs/concepts/lease
This commit is contained in:
Timothy Messier committed 2023-02-02 17:43:17 +00:00
1 parent 12a53bbfa9
commit 32dd7f07d0
3 files changed
+89 -51
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/credential/vault/private_library.go
+3
View File
@@ -20,6 +20,7 @@ import (
"github.com/hashicorp/boundary/internal/credential"
"github.com/hashicorp/boundary/internal/credential/vault/internal/sshprivatekey"
"github.com/hashicorp/boundary/internal/credential/vault/internal/usernamepassword"
"github.com/hashicorp/boundary/internal/db/sentinel"
"github.com/hashicorp/boundary/internal/db/timestamp"
"github.com/hashicorp/boundary/internal/errors"
"github.com/hashicorp/boundary/internal/kms"
@@ -46,6 +47,7 @@ func (bc *baseCred) Library() credential.Library { return bc.lib }
func (bc *baseCred) Purpose() credential.Purpose { return bc.lib.GetPurpose() }
func (bc *baseCred) getExpiration() time.Duration { return bc.expiration }
func (bc *baseCred) getCredential() *Credential { return bc.Credential }
func (bc *baseCred) isRevokable() bool { return bc.ExternalId != sentinel.ExternalIdNone }
// convert converts bc to a specific credential type if bc is not
// UnspecifiedType.
@@ -289,6 +291,7 @@ type dynamicCred interface {
credential.Dynamic
getExpiration() time.Duration
getCredential() *Credential
isRevokable() bool
}
// retrieveCredential retrieves a dynamic credential from Vault for the
internal/credential/vault/repository_credentials.go
+6 -2
View File
@@ -75,6 +75,12 @@ func (r *Repository) Issue(ctx context.Context, sessionId string, requests []cre
return nil, err
}
creds = append(creds, cred)
if !cred.isRevokable() {
// No need to persist since the credential cannot be revoked nor renewed
continue
}
if cred.getExpiration() < runJobsInterval {
event.WriteError(ctx, op,
fmt.Errorf("WARNING: credential will expire before job scheduler can run"),
@@ -123,8 +129,6 @@ func (r *Repository) Issue(ctx context.Context, sessionId string, requests []cre
); err != nil {
return nil, errors.Wrap(ctx, err, op)
}
creds = append(creds, cred)
}
// Best effort update next run time of credential renewal job, but an error should not
internal/credential/vault/repository_credentials_test.go
+80 -49
View File
@@ -23,7 +23,7 @@ import (
)
// since we're not using gorm tags to retrieve this, it's faster and easier to just make a new struct than import usrPassCred from the vault package
type userPassCred struct {
type revokableCred struct {
PublicId string
LibraryId string
SessionId string
@@ -35,7 +35,7 @@ type userPassCred struct {
ExpirationTime *timestamp.Timestamp
}
func lookupDbCred(t *testing.T, ctx context.Context, rw *db.Db, upc credential.UsernamePassword) userPassCred {
func lookupDbCred(t *testing.T, ctx context.Context, rw *db.Db, dc credential.Dynamic) *revokableCred {
rows, err := rw.Query(ctx, `
select
public_id,
@@ -49,11 +49,11 @@ func lookupDbCred(t *testing.T, ctx context.Context, rw *db.Db, upc credential.U
expiration_time
from credential_vault_credential
where public_id = ?;
`, []any{upc.GetPublicId()})
`, []any{dc.GetPublicId()})
require.NoError(t, err)
rowCount := 0
got := userPassCred{}
got := revokableCred{}
for rows.Next() {
rowCount++
@@ -69,9 +69,46 @@ func lookupDbCred(t *testing.T, ctx context.Context, rw *db.Db, upc credential.U
&got.ExpirationTime,
))
}
assert.Equal(t, 1, rowCount)
// Should never get more than one that matches, but can get 0
assert.LessOrEqual(t, rowCount, 1)
return got
if rowCount == 0 {
return nil
}
return &got
}
type libT int
const (
libDB libT = iota
libUsrPassDB
libErrUsrPassDB
libPKI
libErrPKI
libKV
libErrKV
libUsrPassKV
libSshPkKV
libExpiredToken
)
type testLib struct {
PublicId string
HasLease bool
IsRenewable bool
}
type testLibMap map[libT]testLib
func (m testLibMap) GetByPublicId(id string) (testLib, bool) {
for _, v := range m {
if v.PublicId == id {
return v, true
}
}
return testLib{}, false
}
func TestRepository_IssueCredentials(t *testing.T) {
@@ -133,21 +170,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
assert.Equal(t, 1, num)
v.RevokeToken(t, expToken)
type libT int
const (
libDB libT = iota
libUsrPassDB
libErrUsrPassDB
libPKI
libErrPKI
libKV
libErrKV
libUsrPassKV
libSshPkKV
libExpiredToken
)
libs := make(map[libT]string)
libs := make(testLibMap)
{
libPath := path.Join("database", "creds", "opened")
libIn, err := vault.NewCredentialLibrary(origStore.GetPublicId(), libPath)
@@ -156,7 +179,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libDB] = lib.GetPublicId()
libs[libDB] = testLib{PublicId: lib.GetPublicId(), HasLease: true, IsRenewable: true}
}
{
libPath := path.Join("pki", "issue", "boundary")
@@ -166,7 +189,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libPKI] = lib.GetPublicId()
libs[libPKI] = testLib{PublicId: lib.GetPublicId(), HasLease: false, IsRenewable: false}
}
{
@@ -177,7 +200,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libErrPKI] = lib.GetPublicId()
libs[libErrPKI] = testLib{PublicId: lib.GetPublicId(), HasLease: true, IsRenewable: true}
}
{
libPath := path.Join("secret", "data", "my-up-secret")
@@ -187,7 +210,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libKV] = lib.GetPublicId()
libs[libKV] = testLib{PublicId: lib.GetPublicId(), HasLease: false, IsRenewable: true}
}
{
libPath := path.Join("secret", "data", "fake-secret")
@@ -197,7 +220,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libErrKV] = lib.GetPublicId()
libs[libErrKV] = testLib{PublicId: lib.GetPublicId(), HasLease: false, IsRenewable: true}
}
{
libPath := path.Join("database", "creds", "opened")
@@ -210,7 +233,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libUsrPassDB] = lib.GetPublicId()
libs[libUsrPassDB] = testLib{PublicId: lib.GetPublicId(), HasLease: true, IsRenewable: true}
}
{
libPath := path.Join("database", "creds", "opened")
@@ -227,7 +250,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libErrUsrPassDB] = lib.GetPublicId()
libs[libErrUsrPassDB] = testLib{PublicId: lib.GetPublicId(), HasLease: true, IsRenewable: true}
}
{
libPath := path.Join("secret", "data", "my-up-secret")
@@ -240,7 +263,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libUsrPassKV] = lib.GetPublicId()
libs[libUsrPassKV] = testLib{PublicId: lib.GetPublicId(), HasLease: false}
}
{
libPath := path.Join("secret", "data", "my-sshpk-secret")
@@ -253,7 +276,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libSshPkKV] = lib.GetPublicId()
libs[libSshPkKV] = testLib{PublicId: lib.GetPublicId(), HasLease: false}
}
{
libPath := path.Join("secret", "data", "my-up-secret")
@@ -266,7 +289,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
lib, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), libIn)
assert.NoError(t, err)
require.NotNil(t, lib)
libs[libExpiredToken] = lib.GetPublicId()
libs[libExpiredToken] = testLib{PublicId: lib.GetPublicId(), HasLease: false}
}
at := authtoken.TestAuthToken(t, conn, kms, org.GetPublicId())
@@ -303,7 +326,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libDB],
SourceId: libs[libDB].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -313,11 +336,11 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libDB],
SourceId: libs[libDB].PublicId,
Purpose: credential.BrokeredPurpose,
},
{
SourceId: libs[libPKI],
SourceId: libs[libPKI].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -327,7 +350,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libErrPKI],
SourceId: libs[libErrPKI].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -338,11 +361,11 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2nil,
requests: []credential.Request{
{
SourceId: libs[libDB],
SourceId: libs[libDB].PublicId,
Purpose: credential.BrokeredPurpose,
},
{
SourceId: libs[libPKI],
SourceId: libs[libPKI].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -353,7 +376,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libKV],
SourceId: libs[libKV].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -363,7 +386,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libUsrPassDB],
SourceId: libs[libUsrPassDB].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -373,7 +396,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libErrKV],
SourceId: libs[libErrKV].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -384,7 +407,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libErrUsrPassDB],
SourceId: libs[libErrUsrPassDB].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -395,7 +418,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libUsrPassKV],
SourceId: libs[libUsrPassKV].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -405,7 +428,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libSshPkKV],
SourceId: libs[libSshPkKV].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -415,7 +438,7 @@ func TestRepository_IssueCredentials(t *testing.T) {
convertFn: rc2dc,
requests: []credential.Request{
{
SourceId: libs[libExpiredToken],
SourceId: libs[libExpiredToken].PublicId,
Purpose: credential.BrokeredPurpose,
},
},
@@ -451,11 +474,6 @@ func TestRepository_IssueCredentials(t *testing.T) {
if upc, ok := dc.(credential.UsernamePassword); ok {
assert.NotEmpty(upc.Username())
assert.NotEmpty(upc.Password())
// we also want to retrieve the cred from the db and make sure it's the same as the one returned by Issue
retrieved := lookupDbCred(t, ctx, rw, upc)
require.Equal(retrieved.SessionId, sess.GetPublicId())
require.Equal(retrieved.PublicId, upc.GetPublicId())
// these asserts are super basic because the main check we want is that lookupDbCred succeeded
break
}
assert.Fail("want UsernamePassword credential from library with credential type UsernamePassword")
@@ -464,6 +482,19 @@ func TestRepository_IssueCredentials(t *testing.T) {
assert.Fail("do not want UsernamePassword credential from library with credential type Unspecified")
}
}
if lib, ok := libs.GetByPublicId(dc.Library().GetPublicId()); ok {
retrieved := lookupDbCred(t, ctx, rw, dc)
if lib.HasLease {
// we also want to retrieve the cred from the db and make sure it's the same as the one returned by Issue
require.NotNil(retrieved)
require.Equal(retrieved.SessionId, sess.GetPublicId())
require.Equal(retrieved.PublicId, dc.GetPublicId())
assert.NotEmpty(retrieved.ExternalId)
assert.Equal(retrieved.IsRenewable, lib.IsRenewable)
} else {
assert.Nil(retrieved)
}
}
}
})
}