Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(vault): Correctly handle credential stores with expired tokens #2399

Merged
merged 5 commits into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.

### Bug Fixes

* vault: Correctly handle Vault credential stores and libraries that are linked to an
expired Vault token. ([Issue](https://github.com/hashicorp/boundary/issues/2179),
[PR](https://github.com/hashicorp/boundary/pull/2399)).
* aws host catalog: Fix an issue where the request to list hosts could timeout
on a large number of hosts
([Issue](https://github.com/hashicorp/boundary/issues/2224),
Expand All @@ -40,8 +43,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
used because different filters return values with different casing
([PR](https://github.com/hashicorp/boundary-plugin-host-azure/pull/8))
* sessions: Fix an issue where sessions could not have more than one connection
([Issue](https://github.com/hashicorp/boundary/issues/2362)),
([PR](https://github.com/hashicorp/boundary/pull/2369))
([Issue](https://github.com/hashicorp/boundary/issues/2362),
[PR](https://github.com/hashicorp/boundary/pull/2369))
* workers: Fix repeating error in logs when connected to HCP Boundary about an
unimplemented HcpbWorkers call
([PR](https://github.com/hashicorp/boundary/pull/2361))
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/cmd/commands/credentialstorescmd/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ var keySubstMap = map[string]string{
"tls_server_name": "TLS Server Name",
"tls_skip_verify": "Skip TLS Verification",
"token_hmac": "Token HMAC",
"token_status": "Token Status",
"client_certificate": "Client Certificate",
"client_certificate_key_hmac": "Client Certificate Key HMAC",
"worker_filter": "Worker Filter",
Expand Down
20 changes: 11 additions & 9 deletions internal/credential/vault/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (r *TokenRenewalJob) Run(ctx context.Context) error {
return errors.Wrap(ctx, err, op)
}

var ps []*privateStore
var ps []*renewRevokeStore
// Fetch all tokens that will reach their renewal point within the renewalWindow.
// This is done to avoid constantly scheduling the token renewal job when there are multiple tokens
// set to renew in sequence.
Expand All @@ -151,21 +151,22 @@ func (r *TokenRenewalJob) Run(ctx context.Context) error {
// Set numProcessed and numTokens for status report
r.numProcessed, r.numTokens = 0, len(ps)

for _, s := range ps {
for _, as := range ps {
s := as.Store
// Verify context is not done before renewing next token
if err := ctx.Err(); err != nil {
return errors.Wrap(ctx, err, op)
}
if err := r.renewToken(ctx, s); err != nil {
event.WriteError(ctx, op, err, event.WithInfoMsg("error renewing token", "credential store id", s.StoreId, "token status", s.TokenStatus))
event.WriteError(ctx, op, err, event.WithInfoMsg("error renewing token", "credential store id", s.PublicId, "token status", s.TokenStatus))
}
r.numProcessed++
}

return nil
}

func (r *TokenRenewalJob) renewToken(ctx context.Context, s *privateStore) error {
func (r *TokenRenewalJob) renewToken(ctx context.Context, s *clientStore) error {
const op = "vault.(TokenRenewalJob).renewToken"
databaseWrapper, err := r.kms.GetWrapper(ctx, s.ProjectId, kms.KeyPurposeDatabase)
if err != nil {
Expand Down Expand Up @@ -201,7 +202,7 @@ func (r *TokenRenewalJob) renewToken(ctx context.Context, s *privateStore) error
return errors.New(ctx, errors.Unknown, op, "token expired but failed to update repo")
}
if s.TokenStatus == string(CurrentToken) {
event.WriteSysEvent(ctx, op, "Vault credential store current token has expired", "credential store id", s.StoreId)
event.WriteSysEvent(ctx, op, "Vault credential store current token has expired", "credential store id", s.PublicId)
}

// Set credentials associated with this token to expired as Vault will already cascade delete them
Expand Down Expand Up @@ -373,29 +374,30 @@ or
))
`

var ps []*privateStore
var ps []*renewRevokeStore
err := r.reader.SearchWhere(ctx, &ps, where, nil, db.WithLimit(r.limit))
if err != nil {
return errors.Wrap(ctx, err, op)
}

// Set numProcessed and numTokens for s report
r.numProcessed, r.numTokens = 0, len(ps)
for _, s := range ps {
for _, as := range ps {
s := as.Store
// Verify context is not done before renewing next token
if err := ctx.Err(); err != nil {
return errors.Wrap(ctx, err, op)
}
if err := r.revokeToken(ctx, s); err != nil {
event.WriteError(ctx, op, err, event.WithInfoMsg("error revoking token", "credential store id", s.StoreId))
event.WriteError(ctx, op, err, event.WithInfoMsg("error revoking token", "credential store id", s.PublicId))
}
r.numProcessed++
}

return nil
}

func (r *TokenRevocationJob) revokeToken(ctx context.Context, s *privateStore) error {
func (r *TokenRevocationJob) revokeToken(ctx context.Context, s *clientStore) error {
const op = "vault.(TokenRevocationJob).revokeToken"
databaseWrapper, err := r.kms.GetWrapper(ctx, s.ProjectId, kms.KeyPurposeDatabase)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/credential/vault/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2031,7 +2031,7 @@ func TestCredentialStoreCleanupJob_Run(t *testing.T) {
assert.Equal(1, r.numStores)

// Lookup of cs1 and its token should fail
agg := allocPublicStore()
agg := allocListLookupStore()
agg.PublicId = cs1.PublicId
err = rw.LookupByPublicId(context.Background(), agg)
require.Error(err)
Expand Down Expand Up @@ -2076,7 +2076,7 @@ func TestCredentialStoreCleanupJob_Run(t *testing.T) {
assert.Equal(1, r.numStores)

// Lookup of cs2 and its token should fail
agg = allocPublicStore()
agg = allocListLookupStore()
agg.PublicId = cs2.PublicId
err = rw.LookupByPublicId(context.Background(), agg)
require.Error(err)
Expand Down
66 changes: 33 additions & 33 deletions internal/credential/vault/private_library.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var _ credential.Dynamic = (*baseCred)(nil)
type baseCred struct {
*Credential

lib *privateLibrary
lib *issueCredentialLibrary
secretData map[string]interface{}
}

Expand Down Expand Up @@ -131,48 +131,48 @@ func baseToSshPriKey(ctx context.Context, bc *baseCred) (*sshPrivateKeyCred, err
}, nil
}

var _ credential.Library = (*privateLibrary)(nil)
var _ credential.Library = (*issueCredentialLibrary)(nil)

// A privateLibrary contains all the values needed to connect to Vault and
// A issueCredentialLibrary contains all the values needed to connect to Vault and
// retrieve credentials.
type privateLibrary struct {
type issueCredentialLibrary struct {
PublicId string `gorm:"primary_key"`
StoreId string
CredType string `gorm:"column:credential_type"`
UsernameAttribute string
PasswordAttribute string
PrivateKeyAttribute string
PrivateKeyPassphraseAttribute string
Name string
Description string
CreateTime *timestamp.Timestamp
UpdateTime *timestamp.Timestamp
Version uint32
ProjectId string
VaultPath string
HttpMethod string
HttpRequestBody []byte
CredType string `gorm:"column:credential_type"`
ProjectId string
VaultAddress string
Namespace string
CaCert []byte
TlsServerName string
TlsSkipVerify bool
WorkerFilter string
TokenHmac []byte
Token TokenSecret
CtToken []byte
TokenHmac []byte
TokenKeyId string
ClientCert []byte
ClientKey KeySecret
CtClientKey []byte
ClientKeyId string
UsernameAttribute string
PasswordAttribute string
PrivateKeyAttribute string
PrivateKeyPassphraseAttribute string
Purpose credential.Purpose `gorm:"-"`
}

func (pl *privateLibrary) clone() *privateLibrary {
func (pl *issueCredentialLibrary) clone() *issueCredentialLibrary {
// The 'append(a[:0:0], a...)' comes from
// https://github.com/go101/go101/wiki/How-to-perfectly-clone-a-slice%3F
return &privateLibrary{
return &issueCredentialLibrary{
PublicId: pl.PublicId,
StoreId: pl.StoreId,
CredType: pl.CredType,
Expand Down Expand Up @@ -207,15 +207,15 @@ func (pl *privateLibrary) clone() *privateLibrary {
}
}

func (pl *privateLibrary) GetPublicId() string { return pl.PublicId }
func (pl *privateLibrary) GetStoreId() string { return pl.StoreId }
func (pl *privateLibrary) GetName() string { return pl.Name }
func (pl *privateLibrary) GetDescription() string { return pl.Description }
func (pl *privateLibrary) GetVersion() uint32 { return pl.Version }
func (pl *privateLibrary) GetCreateTime() *timestamp.Timestamp { return pl.CreateTime }
func (pl *privateLibrary) GetUpdateTime() *timestamp.Timestamp { return pl.UpdateTime }
func (pl *issueCredentialLibrary) GetPublicId() string { return pl.PublicId }
func (pl *issueCredentialLibrary) GetStoreId() string { return pl.StoreId }
func (pl *issueCredentialLibrary) GetName() string { return pl.Name }
func (pl *issueCredentialLibrary) GetDescription() string { return pl.Description }
func (pl *issueCredentialLibrary) GetVersion() uint32 { return pl.Version }
func (pl *issueCredentialLibrary) GetCreateTime() *timestamp.Timestamp { return pl.CreateTime }
func (pl *issueCredentialLibrary) GetUpdateTime() *timestamp.Timestamp { return pl.UpdateTime }

func (pl *privateLibrary) CredentialType() credential.Type {
func (pl *issueCredentialLibrary) CredentialType() credential.Type {
switch ct := pl.CredType; ct {
case "":
return credential.UnspecifiedType
Expand All @@ -224,8 +224,8 @@ func (pl *privateLibrary) CredentialType() credential.Type {
}
}

func (pl *privateLibrary) decrypt(ctx context.Context, cipher wrapping.Wrapper) error {
const op = "vault.(privateLibrary).decrypt"
func (pl *issueCredentialLibrary) decrypt(ctx context.Context, cipher wrapping.Wrapper) error {
const op = "vault.(issueCredentialLibrary).decrypt"

if pl.CtToken != nil {
type ptk struct {
Expand Down Expand Up @@ -257,8 +257,8 @@ func (pl *privateLibrary) decrypt(ctx context.Context, cipher wrapping.Wrapper)
return nil
}

func (pl *privateLibrary) client(ctx context.Context) (vaultClient, error) {
const op = "vault.(privateLibrary).client"
func (pl *issueCredentialLibrary) client(ctx context.Context) (vaultClient, error) {
const op = "vault.(issueCredentialLibrary).client"
clientConfig := &clientConfig{
Addr: pl.VaultAddress,
Token: pl.Token,
Expand Down Expand Up @@ -289,7 +289,7 @@ type dynamicCred interface {

// retrieveCredential retrieves a dynamic credential from Vault for the
// given sessionId.
func (pl *privateLibrary) retrieveCredential(ctx context.Context, op errors.Op, sessionId string) (dynamicCred, error) {
func (pl *issueCredentialLibrary) retrieveCredential(ctx context.Context, op errors.Op, sessionId string) (dynamicCred, error) {
// Get the credential ID early. No need to get a secret from Vault
// if there is no way to save it in the database.
credId, err := newCredentialId()
Expand Down Expand Up @@ -338,12 +338,12 @@ func (pl *privateLibrary) retrieveCredential(ctx context.Context, op errors.Op,
}

// TableName returns the table name for gorm.
func (pl *privateLibrary) TableName() string {
return "credential_vault_library_private"
func (pl *issueCredentialLibrary) TableName() string {
return "credential_vault_library_issue_credentials"
}

func (r *Repository) getPrivateLibraries(ctx context.Context, requests []credential.Request) ([]*privateLibrary, error) {
const op = "vault.(Repository).getPrivateLibraries"
func (r *Repository) getIssueCredLibraries(ctx context.Context, requests []credential.Request) ([]*issueCredentialLibrary, error) {
const op = "vault.(Repository).getIssueCredLibraries"

mapper, err := newMapper(requests)
if err != nil {
Expand All @@ -357,7 +357,7 @@ func (r *Repository) getPrivateLibraries(ctx context.Context, requests []credent
}
inClause := strings.Join(inClauseSpots, ",")

query := fmt.Sprintf(selectPrivateLibrariesQuery, inClause)
query := fmt.Sprintf(selectLibrariesQuery, inClause)

var params []interface{}
for idx, v := range libIds {
Expand All @@ -369,9 +369,9 @@ func (r *Repository) getPrivateLibraries(ctx context.Context, requests []credent
}
defer rows.Close()

var libs []*privateLibrary
var libs []*issueCredentialLibrary
for rows.Next() {
var lib privateLibrary
var lib issueCredentialLibrary
if err := r.reader.ScanRows(ctx, rows, &lib); err != nil {
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("scan row failed"))
}
Expand Down
Loading