From 06ae371b30b215cf9a12c32a3bdcbc0ca0a2518c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 24 Feb 2022 11:33:59 -0800 Subject: [PATCH 1/4] Moves shared config and credentials resolution inside conditional --- credentials.go | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/credentials.go b/credentials.go index 6b7419f3..a4bf3b7a 100644 --- a/credentials.go +++ b/credentials.go @@ -38,26 +38,26 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv profile = envConfig.SharedConfigProfile } - sharedCredentialsFiles, err := c.ResolveSharedCredentialsFiles() - if err != nil { - return nil, "", err - } - if len(sharedCredentialsFiles) == 0 { - sharedCredentialsFiles = []string{envConfig.SharedCredentialsFile} - } - - sharedConfigFiles, err := c.ResolveSharedConfigFiles() - if err != nil { - return nil, "", err - } - if len(sharedConfigFiles) == 0 { - sharedConfigFiles = []string{envConfig.SharedConfigFile} - } - // The default AWS SDK authentication flow silently ignores invalid Profiles. Pre-validate that the Profile exists // https://github.com/aws/aws-sdk-go-v2/issues/1591 if profile != "" { - _, err := config.LoadSharedConfigProfile(ctx, profile, func(opts *config.LoadSharedConfigOptions) { + sharedCredentialsFiles, err := c.ResolveSharedCredentialsFiles() + if err != nil { + return nil, "", err + } + if len(sharedCredentialsFiles) == 0 { + sharedCredentialsFiles = []string{envConfig.SharedCredentialsFile} + } + + sharedConfigFiles, err := c.ResolveSharedConfigFiles() + if err != nil { + return nil, "", err + } + if len(sharedConfigFiles) == 0 { + sharedConfigFiles = []string{envConfig.SharedConfigFile} + } + + _, err = config.LoadSharedConfigProfile(ctx, profile, func(opts *config.LoadSharedConfigOptions) { opts.CredentialsFiles = sharedCredentialsFiles opts.ConfigFiles = sharedConfigFiles }) @@ -68,7 +68,6 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv loadOptions, config.WithSharedConfigProfile(c.Profile), ) - } if c.AccessKey != "" || c.SecretKey != "" || c.Token != "" { From 2c734df11b09aebb8829c95ebffd4d0db11e7d6a Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 24 Feb 2022 14:49:51 -0800 Subject: [PATCH 2/4] Properly handles precedence when keys and profile envvars are set --- aws_config_test.go | 42 ++++++++++++++++++++++++++++++++++++ credentials.go | 4 ++++ v2/awsv1shim/session_test.go | 42 ++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/aws_config_test.go b/aws_config_test.go index ec815f0a..073b54b7 100644 --- a/aws_config_test.go +++ b/aws_config_test.go @@ -845,6 +845,48 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey [some-profile] aws_access_key_id = DefaultSharedCredentialsAccessKey aws_secret_access_key = DefaultSharedCredentialsSecretKey +`, + }, + { + Config: &Config{}, + Description: "AWS_ACCESS_KEY_ID overrides AWS_PROFILE", + EnvironmentVariables: map[string]string{ + "AWS_ACCESS_KEY_ID": servicemocks.MockEnvAccessKey, + "AWS_SECRET_ACCESS_KEY": servicemocks.MockEnvSecretKey, + "AWS_PROFILE": "SharedCredentialsProfile", + }, + SharedCredentialsFile: ` +[default] +aws_access_key_id = DefaultSharedCredentialsAccessKey +aws_secret_access_key = DefaultSharedCredentialsSecretKey + +[SharedCredentialsProfile] +aws_access_key_id = ProfileSharedCredentialsAccessKey +aws_secret_access_key = ProfileSharedCredentialsSecretKey +`, + MockStsEndpoints: []*servicemocks.MockEndpoint{ + servicemocks.MockStsGetCallerIdentityValidEndpoint, + }, + ExpectedCredentialsValue: mockdata.MockEnvCredentials, + }, + { + Config: &Config{ + Region: "us-east-1", + }, + Description: "AWS_ACCESS_KEY_ID does not override invalid profile name from envvar", + EnvironmentVariables: map[string]string{ + "AWS_ACCESS_KEY_ID": servicemocks.MockEnvAccessKey, + "AWS_SECRET_ACCESS_KEY": servicemocks.MockEnvSecretKey, + "AWS_PROFILE": "no-such-profile", + }, + ExpectedError: func(err error) bool { + var e config.SharedConfigProfileNotExistError + return errors.As(err, &e) + }, + SharedCredentialsFile: ` +[some-profile] +aws_access_key_id = DefaultSharedCredentialsAccessKey +aws_secret_access_key = DefaultSharedCredentialsSecretKey `, }, } diff --git a/credentials.go b/credentials.go index a4bf3b7a..0e3042ea 100644 --- a/credentials.go +++ b/credentials.go @@ -64,6 +64,10 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv if err != nil { return nil, "", err } + } + // We need to validate both the configured and envvar named profiles for validity, + // but to use proper precedence, we only set the configured named profile + if c.Profile != "" { loadOptions = append( loadOptions, config.WithSharedConfigProfile(c.Profile), diff --git a/v2/awsv1shim/session_test.go b/v2/awsv1shim/session_test.go index e1c5b99a..98ee810e 100644 --- a/v2/awsv1shim/session_test.go +++ b/v2/awsv1shim/session_test.go @@ -890,6 +890,48 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey [some-profile] aws_access_key_id = DefaultSharedCredentialsAccessKey aws_secret_access_key = DefaultSharedCredentialsSecretKey +`, + }, + { + Config: &awsbase.Config{}, + Description: "AWS_ACCESS_KEY_ID overrides AWS_PROFILE", + EnvironmentVariables: map[string]string{ + "AWS_ACCESS_KEY_ID": servicemocks.MockEnvAccessKey, + "AWS_SECRET_ACCESS_KEY": servicemocks.MockEnvSecretKey, + "AWS_PROFILE": "SharedCredentialsProfile", + }, + SharedCredentialsFile: ` +[default] +aws_access_key_id = DefaultSharedCredentialsAccessKey +aws_secret_access_key = DefaultSharedCredentialsSecretKey + +[SharedCredentialsProfile] +aws_access_key_id = ProfileSharedCredentialsAccessKey +aws_secret_access_key = ProfileSharedCredentialsSecretKey +`, + MockStsEndpoints: []*servicemocks.MockEndpoint{ + servicemocks.MockStsGetCallerIdentityValidEndpoint, + }, + ExpectedCredentialsValue: mockdata.MockEnvCredentials, + }, + { + Config: &awsbase.Config{ + Region: "us-east-1", + }, + Description: "AWS_ACCESS_KEY_ID does not override invalid profile name from envvar", + EnvironmentVariables: map[string]string{ + "AWS_ACCESS_KEY_ID": servicemocks.MockEnvAccessKey, + "AWS_SECRET_ACCESS_KEY": servicemocks.MockEnvSecretKey, + "AWS_PROFILE": "no-such-profile", + }, + ExpectedError: func(err error) bool { + var e configv2.SharedConfigProfileNotExistError + return errors.As(err, &e) + }, + SharedCredentialsFile: ` +[some-profile] +aws_access_key_id = DefaultSharedCredentialsAccessKey +aws_secret_access_key = DefaultSharedCredentialsSecretKey `, }, } From c0e623bc301694561c4c596c8f674d6bfd156ea3 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 24 Feb 2022 15:11:56 -0800 Subject: [PATCH 3/4] Adds logging for credentials information --- aws_config.go | 6 ++++-- credentials.go | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/aws_config.go b/aws_config.go index de5d89c9..f63c8622 100644 --- a/aws_config.go +++ b/aws_config.go @@ -35,10 +35,12 @@ func GetAwsConfig(ctx context.Context, c *Config) (aws.Config, error) { } } - credentialsProvider, source, err := getCredentialsProvider(ctx, c) + credentialsProvider, initialSource, err := getCredentialsProvider(ctx, c) if err != nil { return aws.Config{}, err } + creds, _ := credentialsProvider.Retrieve(ctx) + log.Printf("[INFO] Retrieved credentials from %q", creds.Source) var retryer aws.Retryer retryer = retry.NewStandard() @@ -65,7 +67,7 @@ func GetAwsConfig(ctx context.Context, c *Config) (aws.Config, error) { return retryer }), ) - if source == ec2rolecreds.ProviderName { + if initialSource == ec2rolecreds.ProviderName { loadOptions = append( loadOptions, config.WithEC2IMDSRegion(), diff --git a/credentials.go b/credentials.go index 0e3042ea..f7a159f3 100644 --- a/credentials.go +++ b/credentials.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "strings" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" @@ -45,16 +46,34 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv if err != nil { return nil, "", err } - if len(sharedCredentialsFiles) == 0 { - sharedCredentialsFiles = []string{envConfig.SharedCredentialsFile} + if len(sharedCredentialsFiles) != 0 { + f := make([]string, len(sharedCredentialsFiles)) + for i, v := range sharedCredentialsFiles { + f[i] = fmt.Sprintf(`"%s"`, v) + } + log.Printf("[DEBUG] Using shared credentials files from configuration: [%s]", strings.Join(f, ", ")) + } else { + if envConfig.SharedCredentialsFile != "" { + log.Printf("[DEBUG] Using shared credentials file environment variables: %q", envConfig.SharedCredentialsFile) + sharedCredentialsFiles = []string{envConfig.SharedCredentialsFile} + } } sharedConfigFiles, err := c.ResolveSharedConfigFiles() if err != nil { return nil, "", err } - if len(sharedConfigFiles) == 0 { - sharedConfigFiles = []string{envConfig.SharedConfigFile} + if len(sharedConfigFiles) != 0 { + f := make([]string, len(sharedConfigFiles)) + for i, v := range sharedConfigFiles { + f[i] = fmt.Sprintf(`"%s"`, v) + } + log.Printf("[DEBUG] Using shared configuration files from configuration: %v", strings.Join(f, ", ")) + } else { + if envConfig.SharedConfigFile != "" { + log.Printf("[DEBUG] Using shared configuration file environment variables: %s", envConfig.SharedConfigFile) + sharedConfigFiles = []string{envConfig.SharedConfigFile} + } } _, err = config.LoadSharedConfigProfile(ctx, profile, func(opts *config.LoadSharedConfigOptions) { @@ -68,6 +87,7 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv // We need to validate both the configured and envvar named profiles for validity, // but to use proper precedence, we only set the configured named profile if c.Profile != "" { + log.Printf("[DEBUG] Using profile from configuration: %q", c.Profile) loadOptions = append( loadOptions, config.WithSharedConfigProfile(c.Profile), @@ -75,6 +95,17 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv } if c.AccessKey != "" || c.SecretKey != "" || c.Token != "" { + params := make([]string, 0, 3) //nolint:gomnd + if c.AccessKey != "" { + params = append(params, "access key") + } + if c.SecretKey != "" { + params = append(params, "secret key") + } + if c.Token != "" { + params = append(params, "token") + } + log.Printf("[DEBUG] Using %s from configuration", strings.Join(params, ", ")) loadOptions = append( loadOptions, config.WithCredentialsProvider( @@ -101,6 +132,7 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv return cfg.Credentials, creds.Source, nil } + log.Printf("[INFO] Retrieved initial credentials from %q", creds.Source) provider, err := assumeRoleCredentialsProvider(ctx, cfg, c) return provider, creds.Source, err @@ -109,8 +141,7 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv func assumeRoleCredentialsProvider(ctx context.Context, awsConfig aws.Config, c *Config) (aws.CredentialsProvider, error) { ar := c.AssumeRole // When assuming a role, we need to first authenticate the base credentials above, then assume the desired role - log.Printf("[INFO] Attempting to AssumeRole %s (SessionName: %q, ExternalId: %q)", - ar.RoleARN, ar.SessionName, ar.ExternalID) + log.Printf("[INFO] Assuming IAM Role %q (SessionName: %q, ExternalId: %q)", ar.RoleARN, ar.SessionName, ar.ExternalID) client := stsClient(awsConfig, c) From ab977183e99fe1cee4e9c21310e90975997d744c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 24 Feb 2022 15:37:16 -0800 Subject: [PATCH 4/4] Adds warning and error when Profile is set with `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` --- credentials.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/credentials.go b/credentials.go index f7a159f3..83302d93 100644 --- a/credentials.go +++ b/credentials.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "os" "strings" "github.com/aws/aws-sdk-go-v2/aws" @@ -34,13 +35,18 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv return nil, "", err } + if c.Profile != "" && os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "" { + log.Printf(`[WARN] A Profile was specified along with the environment variables "AWS_ACCESS_KEY_ID" and "AWS_SECRET_ACCESS_KEY". ` + + "The Profile is now used instead of the environment variable credentials. This may lead to unexpected behavior.") + } + + // The default AWS SDK authentication flow silently ignores invalid Profiles. Pre-validate that the Profile exists + // https://github.com/aws/aws-sdk-go-v2/issues/1591 profile := c.Profile if profile == "" { profile = envConfig.SharedConfigProfile } - // The default AWS SDK authentication flow silently ignores invalid Profiles. Pre-validate that the Profile exists - // https://github.com/aws/aws-sdk-go-v2/issues/1591 if profile != "" { sharedCredentialsFiles, err := c.ResolveSharedCredentialsFiles() if err != nil { @@ -125,6 +131,11 @@ func getCredentialsProvider(ctx context.Context, c *Config) (aws.CredentialsProv creds, err := cfg.Credentials.Retrieve(ctx) if err != nil { + if c.Profile != "" && os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "" { + err = fmt.Errorf(`A Profile was specified along with the environment variables "AWS_ACCESS_KEY_ID" and "AWS_SECRET_ACCESS_KEY". The Profile is now used instead of the environment variable credentials. + +Error: %w`, err) + } return nil, "", c.NewNoValidCredentialSourcesError(err) }