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

Support AssumeRole when credential_source = Environment #181

Open
b-willard opened this issue Sep 13, 2019 · 5 comments
Open

Support AssumeRole when credential_source = Environment #181

b-willard opened this issue Sep 13, 2019 · 5 comments

Comments

@b-willard
Copy link

Use Case

I am storing credentials (AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY) encrypted in a remote vault and do not want to expose them to the docker host in question via ~/.aws/credentials. Instead, credentials should temporarily exist on the docker host via the environment. The environment is populated during a configuration management run or image build provisioning process which interfaces with the remote vault and decrypts the credentials.

Issue

The configuration required to meet this use case results in a catch-22:

~/.aws/config with environment credential source

[profile some_ecr_readonly_profile]
region = us-west-2
role_arn = arn:aws:iam::xxxxxxxxxxxx:role/Some-ECR-ReadOnly-Role
credential_source = Environment

environment vars

AWS_SDK_LOAD_CONFIG = '1'
AWS_PROFILE = 'some_ecr_readonly_profile'
AWS_ACCESS_KEY_ID = 'XXXXXXXXXXXXXXXXXXXX'
AWS_SECRET_ACCESS_KEY = 'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX'

where arn:aws:iam::xxxxxxxxxxxx:role/Some-ECR-ReadOnly-Role uses the AWS ECR Read Only managed policy and the IAM user associated to AWS_ACCESS_KEY_ID has the following policy directly attached:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "1",
            "Effect": "Allow",
            "Action": "sts:AssumeRole",
            "Resource": "arn:aws:iam::xxxxxxxxxxxx:role/Some-ECR-ReadOnly-Role"
        }
    ]
}

With this setup and aws-go-sdk's credential load precedence, AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY are used directly rather than AssumeRole'ing and using the resulting STS credentials instead. As a result, any docker pull or docker push under this configuration results in #164. This is acknowledged with a solution in the sdk's documentation.

Solution

I have a working docker-credential-ecr-login binary meeting my needs based on the diff below. (I did not submit this as an official PR because these are the first lines of Go I've ever written :D so I'm certain there are better methods.) The change essentially asks two questions:

  • Are both AWS_SDK_LOAD_CONFIG and AWS_PROFILE set?
  • Is AWS_SDK_LOAD_CONFIG truthy?

Iff both answers are 'yes' then create a session with the Profile option set to the value of AWS_PROFILE. Otherwise, business as usual. Prior to applying this diff I updated aws-sdk-go to v1.23.17 in order to pick up aws/aws-sdk-go#1901 and aws/aws-sdk-go#2385.

diff --git a/ecr-login/api/factory.go b/ecr-login/api/factory.go
index 941e6ba..1d43b34 100644
--- a/ecr-login/api/factory.go
+++ b/ecr-login/api/factory.go
@@ -36,6 +36,7 @@ type ClientFactory interface {
        NewClientWithOptions(opts Options) Client
        NewClientFromRegion(region string) Client
        NewClientWithFipsEndpoint(region string) (Client, error)
+       NewClientWithExplicitProfile(profile string) (Client, error)
        NewClientWithDefaults() Client
 }

@@ -75,6 +76,21 @@ func (defaultClientFactory DefaultClientFactory) NewClientWithFipsEndpoint(regio
        }), nil
 }

+func (defaultClientFactory DefaultClientFactory) NewClientWithExplicitProfile(profile string) (Client, error) {
+       awsSession, err := session.NewSessionWithOptions(session.Options{
+               Profile: profile,
+       })
+       if err != nil {
+               return nil, err
+       }
+       awsSession.Handlers.Build.PushBackNamed(userAgentHandler)
+       awsConfig := awsSession.Config
+       return defaultClientFactory.NewClientWithOptions(Options{
+               Session: awsSession,
+               Config:  awsConfig,
+       }), nil
+}
+
 // NewClientFromRegion uses the region to create the client
 func (defaultClientFactory DefaultClientFactory) NewClientFromRegion(region string) Client {
        awsSession := session.New()
diff --git a/ecr-login/ecr.go b/ecr-login/ecr.go
index fb91561..2ba3ac1 100644
--- a/ecr-login/ecr.go
+++ b/ecr-login/ecr.go
@@ -16,6 +16,8 @@ package ecr
 import (
        "errors"
        "fmt"
+       "os"
+       "strconv"

        "github.com/sirupsen/logrus"

@@ -43,6 +45,7 @@ func (ECRHelper) Delete(serverURL string) error {
 }

 func (self ECRHelper) Get(serverURL string) (string, string, error) {
+       logrus.Debug("Getting credentials")
        registry, err := api.ExtractRegistry(serverURL)
        if err != nil {
                logrus.
@@ -52,6 +55,9 @@ func (self ECRHelper) Get(serverURL string) (string, string, error) {
                return "", "", credentials.NewErrCredentialsNotFound()
        }

+       profile, profile_exists := os.LookupEnv("AWS_PROFILE")
+       load_sdk_config_str, load_sdk_exists := os.LookupEnv("AWS_SDK_LOAD_CONFIG")
+
        var client api.Client
        if registry.FIPS {
                client, err = self.ClientFactory.NewClientWithFipsEndpoint(registry.Region)
@@ -59,6 +65,17 @@ func (self ECRHelper) Get(serverURL string) (string, string, error) {
                        logrus.WithError(err).Error("Error resolving FIPS endpoint")
                        return "", "", credentials.NewErrCredentialsNotFound()
                }
+       } else if profile_exists && load_sdk_exists {
+               load_sdk_config, err := strconv.ParseBool(load_sdk_config_str)
+               if err != nil || !load_sdk_config {
+                       client = self.ClientFactory.NewClientFromRegion(registry.Region)
+               } else {
+                       client, err = self.ClientFactory.NewClientWithExplicitProfile(profile)
+                       if err != nil {
+                               logrus.WithError(err).Error("Error creating client with explicit profile")
+                               return "", "", credentials.NewErrCredentialsNotFound()
+                       }
+               }
        } else {
                client = self.ClientFactory.NewClientFromRegion(registry.Region)
        }
@samuelkarp
Copy link
Contributor

Hey @b-willard, thanks for opening this! I think it would be good to support credential_source = Environment and would welcome a pull request to this effect. I think I'd ask for some slightly different logic than what you have implemented (specifically: I think it'd be better to introduce a new environment variable to control this behavior rather than relying on AWS_SDK_LOAD_CONFIG and AWS_PROFILE together), but we can hash the details out in the PR.

@benhowes
Copy link

benhowes commented Aug 4, 2020

Hi @samuelkarp I'm planning to take a look at this - do you have any ideas what you wanted in terms of a new environment variable?

I've not looked at this a great deal yet, but I'm even wondering if it would be possible to just use the presence of AWS_PROFILE to trigger the NewClientWithExplicitProfile flow?

benhowes added a commit to cookpad/amazon-ecr-credential-helper that referenced this issue Aug 4, 2020
@StephanePaulus
Copy link

Hi @benhowes, I am currently blocked from using the credential helper as I need use credential_source = Environment in my ~/.aws/config. Any idea what the status is on your PR? Do you know of a workaround without making my own binary?

@benhowes
Copy link

@StephanePaulus the PR works, but does require you to build it. I'm quite surprised to have not heard anything from the AWS team about it by now to be honest! FWIW it is quite easy to build/distribute a binary for go if you need to and it's looking like we're going to have no choice but to do that.

@kzys
Copy link
Contributor

kzys commented Nov 19, 2020

@benhowes Sorry about that. I've just reviewed your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants