From 73bd34250a53b0723cb48119809f7935d478dad2 Mon Sep 17 00:00:00 2001 From: Daniel George Holz Date: Wed, 8 Mar 2023 12:10:22 +0000 Subject: [PATCH 1/3] Check if profile has stored credentials before calling credential_process --- vault/credentialkeyring.go | 14 +++++++++++++- vault/vault.go | 3 ++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/vault/credentialkeyring.go b/vault/credentialkeyring.go index d57ee6fcc..0feb0a20a 100644 --- a/vault/credentialkeyring.go +++ b/vault/credentialkeyring.go @@ -18,13 +18,25 @@ func (ck *CredentialKeyring) Keys() (credentialsNames []string, err error) { return credentialsNames, err } for _, keyName := range allKeys { - if !IsSessionKey(keyName) && !IsOIDCTokenKey(keyName) { + if IsStoredCredential(keyName) { credentialsNames = append(credentialsNames, keyName) } } return credentialsNames, nil } +func IsStoredCredential(keyName string) bool { + return !IsSessionKey(keyName) && !IsOIDCTokenKey(keyName) +} + +func (ck *CredentialKeyring) HasStoredCredential(credentialsName string) bool { + _, err := ck.Has(credentialsName) + if err == nil { + return IsStoredCredential(credentialsName) + } + return false +} + func (ck *CredentialKeyring) Has(credentialsName string) (bool, error) { allKeys, err := ck.Keyring.Keys() if err != nil { diff --git a/vault/vault.go b/vault/vault.go index 300f3e711..e30843d5f 100644 --- a/vault/vault.go +++ b/vault/vault.go @@ -271,7 +271,8 @@ func (t *tempCredsCreator) GetProviderForProfile(config *ProfileConfig) (aws.Cre return NewAssumeRoleWithWebIdentityProvider(t.Keyring.Keyring, config, !t.DisableCache) } - if config.HasCredentialProcess() { + storedCredentialForProfile := t.Keyring.HasStoredCredential(config.ProfileName) + if !storedCredentialForProfile && config.HasCredentialProcess() { log.Printf("profile %s: using credential process", config.ProfileName) return NewCredentialProcessProvider(t.Keyring.Keyring, config, !t.DisableCache) } From cda0841b27ad1942bfcd39923b9ff266d5840d2e Mon Sep 17 00:00:00 2001 From: Michael Tibben Date: Thu, 9 Mar 2023 20:28:34 +1100 Subject: [PATCH 2/3] Remove config validtion. Debug logs show which credential source is used --- vault/config.go | 32 ------------------------------ vault/config_test.go | 46 -------------------------------------------- vault/vault.go | 29 +++++++++++++--------------- 3 files changed, 13 insertions(+), 94 deletions(-) diff --git a/vault/config.go b/vault/config.go index f769b772d..a4fdc1e7c 100644 --- a/vault/config.go +++ b/vault/config.go @@ -680,35 +680,3 @@ func (c *ProfileConfig) GetSessionTokenDuration() time.Duration { } return c.NonChainedGetSessionTokenDuration } - -func (c *ProfileConfig) Validate() error { - if c.HasSSOSession() && !c.HasSSOStartURL() { - return fmt.Errorf("profile '%s' has sso_session but no sso_start_url", c.ProfileName) - } - - n := 0 - if c.HasSSOStartURL() { - n++ - } - if c.HasWebIdentity() { - n++ - } - if c.HasCredentialProcess() { - n++ - } - if c.HasSourceProfile() { - n++ - } - if c.HasRole() && - // these cases require the role to be set in addition, so it's part of - // their credential. - !c.HasSourceProfile() && - !c.HasWebIdentity() { - n++ - } - if n > 1 { - return fmt.Errorf("profile '%s' has more than one source of credentials", c.ProfileName) - } - - return nil -} diff --git a/vault/config_test.go b/vault/config_test.go index c69c116db..b9f10375b 100644 --- a/vault/config_test.go +++ b/vault/config_test.go @@ -616,49 +616,3 @@ source_profile = interim t.Fatalf("Expected transitive_session_tags to be empty, got %+v", baseConfig.TransitiveSessionTags) } } - -func TestValidConfigValidation(t *testing.T) { - f := newConfigFile(t, []byte(` -[profile foo] -region = eu-west-1 -mfa_serial = arn:aws:iam::9999999999999:mfa/david - -[profile foo:staging] -role_arn = arn:aws:iam::1111111111111:role/admin -source_profile = foo -region = eu-west-2 -mfa_serial = arn:aws:iam::9999999999999:mfa/david - -[profile foo:production] -role_arn = arn:aws:iam::1111111111111:role/admin -source_profile = foo -region = eu-west-2 -mfa_serial = arn:aws:iam::9999999999999:mfa/david -credential_process = true - -[profile withwebidentity] -role_arn = arn:aws:iam::123457890:role/foo -web_identity_token_process = oidccli -issuer=https://example.com -client-id=aws -client-secret=localonly raw -`)) - defer os.Remove(f) - configFile, _ := vault.LoadConfig(f) - configLoader := &vault.ConfigLoader{File: configFile} - - config, _ := configLoader.GetProfileConfig("foo:staging") - err := config.Validate() - if err != nil { - t.Fatalf("Should have validated: %v", err) - } - - config, _ = configLoader.GetProfileConfig("foo:production") - err = config.Validate() - if err == nil { - t.Fatalf("Should have failed validation: %v", err) - } - - config, _ = configLoader.GetProfileConfig("withwebidentity") - err = config.Validate() - if err != nil { - t.Fatalf("Should have validated withwebidentity: %v", err) - } -} diff --git a/vault/vault.go b/vault/vault.go index e30843d5f..c6f4e7831 100644 --- a/vault/vault.go +++ b/vault/vault.go @@ -257,24 +257,21 @@ func (t *tempCredsCreator) getSourceCreds(config *ProfileConfig) (sourcecredsPro } func (t *tempCredsCreator) GetProviderForProfile(config *ProfileConfig) (aws.CredentialsProvider, error) { - if err := config.Validate(); err != nil { - return nil, err - } - - if config.HasSSOStartURL() { - log.Printf("profile %s: using SSO role credentials", config.ProfileName) - return NewSSORoleCredentialsProvider(t.Keyring.Keyring, config, !t.DisableCache) - } + if !t.Keyring.HasStoredCredential(config.ProfileName) { + if config.HasSSOStartURL() { + log.Printf("profile %s: using SSO role credentials", config.ProfileName) + return NewSSORoleCredentialsProvider(t.Keyring.Keyring, config, !t.DisableCache) + } - if config.HasWebIdentity() { - log.Printf("profile %s: using web identity", config.ProfileName) - return NewAssumeRoleWithWebIdentityProvider(t.Keyring.Keyring, config, !t.DisableCache) - } + if config.HasWebIdentity() { + log.Printf("profile %s: using web identity", config.ProfileName) + return NewAssumeRoleWithWebIdentityProvider(t.Keyring.Keyring, config, !t.DisableCache) + } - storedCredentialForProfile := t.Keyring.HasStoredCredential(config.ProfileName) - if !storedCredentialForProfile && config.HasCredentialProcess() { - log.Printf("profile %s: using credential process", config.ProfileName) - return NewCredentialProcessProvider(t.Keyring.Keyring, config, !t.DisableCache) + if config.HasCredentialProcess() { + log.Printf("profile %s: using credential process", config.ProfileName) + return NewCredentialProcessProvider(t.Keyring.Keyring, config, !t.DisableCache) + } } sourcecredsProvider, err := t.getSourceCreds(config) From ef2b8b9470827d97c8bf3fe0f91ccf1abdd8c9fc Mon Sep 17 00:00:00 2001 From: Michael Tibben Date: Thu, 9 Mar 2023 21:43:35 +1100 Subject: [PATCH 3/3] Elevate stored credentials above source_profile --- vault/credentialkeyring.go | 14 +------------- vault/vault.go | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/vault/credentialkeyring.go b/vault/credentialkeyring.go index 0feb0a20a..d57ee6fcc 100644 --- a/vault/credentialkeyring.go +++ b/vault/credentialkeyring.go @@ -18,25 +18,13 @@ func (ck *CredentialKeyring) Keys() (credentialsNames []string, err error) { return credentialsNames, err } for _, keyName := range allKeys { - if IsStoredCredential(keyName) { + if !IsSessionKey(keyName) && !IsOIDCTokenKey(keyName) { credentialsNames = append(credentialsNames, keyName) } } return credentialsNames, nil } -func IsStoredCredential(keyName string) bool { - return !IsSessionKey(keyName) && !IsOIDCTokenKey(keyName) -} - -func (ck *CredentialKeyring) HasStoredCredential(credentialsName string) bool { - _, err := ck.Has(credentialsName) - if err == nil { - return IsStoredCredential(credentialsName) - } - return false -} - func (ck *CredentialKeyring) Has(credentialsName string) (bool, error) { allKeys, err := ck.Keyring.Keys() if err != nil { diff --git a/vault/vault.go b/vault/vault.go index c6f4e7831..5480dad25 100644 --- a/vault/vault.go +++ b/vault/vault.go @@ -237,27 +237,27 @@ type tempCredsCreator struct { chainedMfa string } -func (t *tempCredsCreator) getSourceCreds(config *ProfileConfig) (sourcecredsProvider aws.CredentialsProvider, err error) { +func (t *tempCredsCreator) getSourceCreds(config *ProfileConfig, hasStoredCredentials bool) (sourcecredsProvider aws.CredentialsProvider, err error) { + if hasStoredCredentials { + log.Printf("profile %s: using stored credentials", config.ProfileName) + return NewMasterCredentialsProvider(t.Keyring, config.ProfileName), nil + } + if config.HasSourceProfile() { log.Printf("profile %s: sourcing credentials from profile %s", config.ProfileName, config.SourceProfile.ProfileName) return t.GetProviderForProfile(config.SourceProfile) } + return nil, fmt.Errorf("profile %s: credentials missing", config.ProfileName) +} + +func (t *tempCredsCreator) GetProviderForProfile(config *ProfileConfig) (aws.CredentialsProvider, error) { hasStoredCredentials, err := t.Keyring.Has(config.ProfileName) if err != nil { return nil, err } - if hasStoredCredentials { - log.Printf("profile %s: using stored credentials", config.ProfileName) - return NewMasterCredentialsProvider(t.Keyring, config.ProfileName), nil - } - - return nil, fmt.Errorf("profile %s: credentials missing", config.ProfileName) -} - -func (t *tempCredsCreator) GetProviderForProfile(config *ProfileConfig) (aws.CredentialsProvider, error) { - if !t.Keyring.HasStoredCredential(config.ProfileName) { + if !hasStoredCredentials { if config.HasSSOStartURL() { log.Printf("profile %s: using SSO role credentials", config.ProfileName) return NewSSORoleCredentialsProvider(t.Keyring.Keyring, config, !t.DisableCache) @@ -274,7 +274,7 @@ func (t *tempCredsCreator) GetProviderForProfile(config *ProfileConfig) (aws.Cre } } - sourcecredsProvider, err := t.getSourceCreds(config) + sourcecredsProvider, err := t.getSourceCreds(config, hasStoredCredentials) if err != nil { return nil, err }