-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
aws ecr get-login should use --password-stdin if available #2875
Comments
So I realize getting the warning is not ideal, but the CLI is already using
which defeats the purpose because the password is still getting printed out. Furthermore, the CLI would have to handle the proper redirection expressions based on the operating system it is being ran from. Is the main reason for this request to just ignore the warning? |
When I referred to the I imagine the warning is really about leaking the token in two places: process lists and shell history. I agree that echoing the bare token doesn't resolve either issue any more than passing it via |
|
This is more friendly for Windows (git bash) and related clients.
Its also similar to @dsdenes but doesn't hardcode the repo |
@closedLoop Command works but make sure to include |
Late to the party, but I was tired of seeing the login warning too 🙂 aws ecr get-login --no-include-email --region ${REGION} --profile ${PROFILE} | awk '{printf $6}' | docker login -u AWS ${REPOSITORY} --password-stdin |
Docker seems to have made this a warning that now requires explicit input, meaning that aws-cli ecr get-login can no longer work in a script.... |
@rlees85 - did you try any of the suggestions above? |
Workaround: Actually I think this is unrelated to the issue/warning that this thread is about... my bad |
For the record, this is less of an immediate issue because docker has reverted the prompt. See docker/cli#1008. It sounds like their long time plan is to re-introduce this prompt, so it still would probably be good for |
Any idea about timeline for the fix of this issue? |
What if we introduce the |
Generating an |
@wichert We would evaluate the command and run it instead of copy and paste it. For example: I think that is the use case anyway, user want to generate that token and to be run at the same machine. I don't think this method is less secure than existing one? |
The whole point of the change in |
Presuming pipes are supported in the terminal, it seems to me there needn't be a need to use echo — just have a different # NOTE: --password-stdin IS HYPOTHETICAL! DO NOT EXPECT IT TO WORK
$ aws ecr get-login --no-include-email --region us-east-1 --password-stdin
aws ecr get-login --region us-east-1 --print-password-only | docker login -u AWS --password-stdin https://12345.dkr.ecr.us-east-1.amazonaws.com
$ aws ecr get-login --region us-east-1 --print-password-only
pCwnxiDQWSN1nx9uaphdzRK7dQHxZUSlzMKi4IViZOCvnAE9K1P0qJkH5UMISmbkJlrjut96yzPOb82Jpoc0BEvKeZUlXWtSX4JA
$ aws ecr get-login --region us-east-1 --print-password-only | docker login -u AWS --password-stdin https://12345.dkr.ecr.us-east-1.amazonaws.com
Login Succeeded
$ $(aws ecr get-login --no-include-email --region us-east-1 --password-stdin)
Login Succeeded |
I have been trying your fix but I am not sure I am proceeding correctly...? Have I missed steps before? Thanks! |
this should be fine
|
Using # one-liner
aws ecr get-login --no-include-email --region us-east-1 | awk '{print $6}' | docker login -u AWS --password-stdin $(aws ecr get-login --no-include-email --region us-east-1 | awk '{print $7}')
# long-form
cmd="aws ecr get-login --no-include-email --region us-east-1"
url=$($cmd | awk '{ print $7 }')
$cmd | awk '{ print $6 }' | docker login -u AWS --password-stdin $url |
By the way, there is no flag in latest aws cli installed via pip
|
This is completely broken by the way. How is this still not fixed? |
To be clear, |
Okay so I've had this problem. The CD tool we are using logged the warning as an error so no choice but to avoid it. Here is what I have used with Powershell to push to AWS ECR: Hopefully this works. You will need the credentials (on aws configure) as well. It is however a very long process (I literally crashed through PS with this so there might be faster ways). |
Would be nice to get rid of this warning. Sure there are work arounds but they are larger than my entire build script so it adds a lot of clutter. |
If all you want to do is get rid of the warning, just do this: |
Lots of misunderstanding here. Point 1: (#2875 (comment)) If you are worried about seeing the warning,it's because you should be worried about what aws-cli is doingPutting passwords into shell commands is BAD, BAD, BAD, BAD, BAD. By invoking You have just now LEAKED the password in plaintext to every single user on the system.(Unless you are are using exotic configurations like mounting /proc with hidepid) Developers suggesting hiding stderr or suggesting passing it as shell args to other commands like echo are sticking their collective head in the sand, caring more for untroubled logs than for security. See Passing Passwords. Point 2: It's currently possible to pass the password from AWS CLI to docker in a secure way (#2875 (comment)) though it's ugly and requires other tools (cut, awk). I'm in no position to be casting stones, but it's surprising AWS has left this security issue unaddressed for almost two years. |
Does this official helper address the concerns voiced in this issue? https://github.com/awslabs/amazon-ecr-credential-helper |
I was unaware of that. It is indeed a suitable method. It uses docker's credStore/credHelpers interface. Docker invokes the {
"Username": "george",
"Secret": "passw0rd"
} In fact, it would be convenient if the AWS CLI itself offered this interface, so there wasn't separate install. amazon-ecr-credential-helper has several install methods, but none of them are the |
If you take a look at the README, it even mentions (as it is with other helpers) that Docker will automatically pick up any credentials required for ECR repositories without having to store or pass any usernames or passwords in the future. I’d call that a win-win :) @pauldraper I reckon if it’s easy enough to install it doesn’t matter to most users (which, in this case, at least for me, it is!) |
Indeed using credHelpers is the best way. |
Apparently amazon-ecr-credential-helper only has Ubuntu packages for non-LTS Ubuntu versions :/ So I did the same thing with a four-line script. /usr/local/bin/docker-credential-ecr-login #!/bin/sh
grep -q 'dkr.ecr.[^.]\+.amazonaws.com' - || exit
aws --output text ecr get-authorization-token --query authorizationData[0].authorizationToken \
| base64 --decode \
| sed -e 's/:/", "Secret":"/' -e 's/^/{"Username":"/' -e 's/$/"}/' ~/.docker/config
Now https://gist.github.com/pauldraper/b74a24f869b0acbc3bd488d67f9a53b4 |
as proposed in aws#4867, and previously discussed in aws#2875 (comment) aws#3687 (comment) this commit adds a new customization command for ECR that only returns the password to login to a registry. this approach is composable, is compatible with other container clients, allows use of functionality like Docker's --password-stdin flag, and is more resilient to changes in client APIs
as proposed in aws#4867, and previously discussed in aws#2875 (comment) aws#3687 (comment) this commit adds a new customization command for ECR that only returns the password to login to a registry. this approach is composable, is compatible with other container clients, allows use of functionality like Docker's --password-stdin flag, and is more resilient to changes in client APIs
as proposed in aws#4867, and previously discussed in aws#2875 (comment) aws#3687 (comment) this commit adds a new customization command for ECR that only returns the password to login to a registry. this approach is composable, is compatible with other container clients, allows use of functionality like Docker's --password-stdin flag, and is more resilient to changes in client APIs
as proposed in aws#4867, and previously discussed in aws#2875 (comment) aws#3687 (comment) this commit adds a new customization command for ECR that only returns the password to login to a registry. this approach is composable, is compatible with other container clients, allows use of functionality like Docker's --password-stdin flag, and is more resilient to changes in client APIs
as proposed in aws#4867, and previously discussed in aws#2875 (comment) aws#3687 (comment) this commit adds a new customization command for ECR that only returns the password to login to a registry. this approach is composable, is compatible with other container clients, allows use of functionality like Docker's --password-stdin flag, and is more resilient to changes in client APIs
Yesterday's release (v1.17.10) included a new command |
We also removed |
The following works for me:
Ref.: #4962 (comment), credit to @matthew-russo! |
Or an even more automatic version where your account id is already detected using credentials (Replace $ aws ecr get-login-password --region us-east-1 \
| docker login \
--password-stdin \
--username AWS \
"$(aws sts get-caller-identity --query Account --output text).dkr.ecr.us-east-1.amazonaws.com" |
None of the above solutions are working for me, so far I've tried various iterations of the For ex:
Results in:
Vast majority of the time I'm encountering this error:
|
Currently, on Docker 17.07+, when evaling the output of
aws ecr get-login
, the following error message appears:It would be nice if
aws ecr get-login
could use--password-stdin
if it's available. The workaround is to useget-authorization-token
, but that involves everyone writing code, whereas--password-stdin
is a good idea for everyone who'd ever useaws ecr get-login
.The text was updated successfully, but these errors were encountered: