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

Example about how to securely login through Docker CLI #96

Closed
eine opened this issue Sep 3, 2019 · 10 comments
Closed

Example about how to securely login through Docker CLI #96

eine opened this issue Sep 3, 2019 · 10 comments

Comments

@eine
Copy link

eine commented Sep 3, 2019

Currently, using echo "$DOCKER_PASS" | docker login -u "$DOCKER_USER" --password-stdin produces a warning:

WARNING! Your password will be stored unencrypted in /home/runner/.docker/config.json.
Login Succeeded
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credentials-store

Please, provide an example workflow, step or action that allows to securely login through Docker CLI.

@chrispat
Copy link
Member

chrispat commented Sep 4, 2019

Azure has published an action that does a secure docker login https://github.com/Azure/k8s-actions/tree/master/docker-login

@eine
Copy link
Author

eine commented Sep 5, 2019

@chrispat, thanks for the reference! Unfortunately, the action provided by Azure is at least as bad as using 'docker login' directly, if not worse. The point is that docker login explicitly warns the user about the configuration being stored unencrypted. Azure, however, does not show any warning, despite the underlying storage mechanism being the almost the same. Please, bear with me:

        let authenticationToken = new Buffer(`${username}:${password}`).toString('base64');
        let config = { "auths": { [loginServer]: { auth: authenticationToken } } };
...
        fs.writeFileSync(dockerConfigPath, JSON.stringify(config));
        command_1.issueCommand('set-env', { name: 'DOCKER_CONFIG' }, dirPath);

Therefore, the only practical difference is that Azure stores the config file in path.join(runnerTempDirectory, ``docker_login_${Date.now()}``), instead of /home/runner/.docker/config.json.

Furthermore, this was brought by @TingluoHuang a week ago (see Azure/container-actions#3).

@chrispat
Copy link
Member

chrispat commented Sep 5, 2019

Beyond making sure the docker config is cleaned up after the job I am not sure there are any other options that will work in an automated sceanrio. In the end the docker client needs to be able to read the credential in order to pull and push containers. Even if the credential was stored in a credstore that would need to be accessible by the user the docker process is running under so you would have the same potential security issue you have today.

I suppose you could have actions that handle pull and push for you that take the crednetial and then use a cred helper that reads from the environment. However, then simple things like docker build or docker run just pulling the image would be more difficult.

@eine
Copy link
Author

eine commented Sep 5, 2019

Beyond making sure the docker config is cleaned up after the job I am not sure there are any other options that will work in an automated sceanrio.

I think that a safer procedure is not to write any sensible unencrypted data to disk, in first place. It is compulsory to clean up after the job is done, of course. But there are four possible contexts with different places to clean up:

  1. Raw data in disk.
  2. Raw data in memory.
  3. Encrypted data in disk.
  4. Encrypted data in memory.

Currently, only 1 seems to be possible with GitHub/Azure Actions.

In the end the docker client needs to be able to read the credential in order to pull and push containers.

Yes. Precisely, credential stores allow to hold encrypted data in memory and only provide it to the authorized user/service/tool that has the corresponding key. And credential helpers allow the docker client to be able to read from credential stores.

Even if the credential was stored in a credstore that would need to be accessible by the user the docker process is running under so you would have the same potential security issue you have today.

I am not sure I understand this. I'd say the the risk of any unauthorized user to read some raw data that has been written to disk (either while it is being used or after it was expectedly removed from a tmp dir) is higher than the same user accessing the memory space of an application at runtime. I expect the memory to be overwritten/reused more frequently. Of course, if the attacker can gain control of your user, you are lost, no matter how secure the procedure.

I suppose you could have actions that handle pull and push for you that take the crednetial and then use a cred helper that reads from the environment. However, then simple things like docker build or docker run just pulling the image would be more difficult.

docker build and docker run don't need credentials at all. It is irrelevant if a credential helper exists, whether it is running or crashed. The credential helper only needs to be setup between docker login and docker logout to execute docker push (or to docker pull from a private registry). E.g.

- run: docker pull <public_image>
- run: docker build -t <test_image>...
- run: docker run ...<test_image>...
- run: docker build -t <image>...
- uses: azure/container-actions/docker-securelogin@master
  with:
    username: '<username>'
    password: '<password>'
- run: docker push <image>
- uses: azure/container-actions/docker-securelogout@master

or

- uses: azure/container-actions/docker-securelogin@master
  with:
    username: '<username>'
    password: '<password>'
- run: docker pull <private_image>
- uses: azure/container-actions/docker-securelogout@master
- run: docker build -t <test_image>...
- run: docker run ...<test_image>...
- run: docker build -t <image>...
- uses: azure/container-actions/docker-securelogin@master
  with:
    username: '<username>'
    password: '<password>'
- run: docker push <image>
- uses: azure/container-actions/docker-securelogout@master

Ref: https://github.com/docker/docker-credential-helpers/blob/master/ci/before_script_linux.sh

@eine
Copy link
Author

eine commented Sep 20, 2019

Ref docker/cli#2089

@ebbeelsborg-unity
Copy link

Hey, thanks for your answer @chrispat . I think that @eine raises a valid point though. IMHO this is still an unresolved security issue so if you could please revisit it, it'd be appreciated. Thanks.

Your link (https://github.com/Azure/k8s-actions/blob/master/docker-login/src/login.ts) points to an archived repo. Its deprecation notice points to another archived repo (https://github.com/Azure/container-actions), and that repo in turn points to https://github.com/Azure/actions. The last repo seems Azure-specific and cannot readily be used to do a docker login to other container registries such as Google Container Registry, as far as I can tell. The closed issue Azure/container-actions#3 explains why you guys prefer storing the creds on disk instead of in a creds helper, which I read like "because it requires the workflow to do docker logout to not let the secret linger in the cred store on the VM and that's a greater risk". Could that perhaps be solved by an auto-generated “post” action (like for actions/checkout etc.)?

So, something along the lines of the following?

  1. Spin up Actions VMs with a cred store/helper already installed (such as pass for Unix/Linux), or document in the docker login action below that an installed cred store is a prerequisite.
  2. Provide a curated action in the marketplace that takes a GPG key, a container registry URL and a container registry token/key as inputs to set up the cred store and then performs docker logout and then docker login.
  3. Do a docker push and whatever in a custom step/action.
  4. Automatically run the post action to ensure docker logout and finally erase the cred from the store.

@chrispat
Copy link
Member

chrispat commented May 19, 2020

Docker has not released their own build and push action would is the "curated" action for this scenario.

However, given the Actions VMs only live for the length of the job and then are deleted I am still not following how your suggestions make things more secure.

Even using a credstore helper like pass or security all of the code in a CI runs under the same user so any of the code could call those tools and read that info back. Credential stores are designed to protect credentials at rest from outside attacks, however, when credentials are active and the potential attack is coming from inside the same user space I don't believe they help.

I think the issue linked above @eine is interesting as it points at the fact that different registries can be configured with different types of authentication. I think the best option would be if we could find a way to get registries to support something like OIDC with a JWT as Vault does https://www.vaultproject.io/docs/auth/jwt. In that case if we had a JWT that said which repo and branch the job was running on the registry could just use that as the auth and you would have no long standing tokens.

@ebbeelsborg-unity
Copy link

I agree that the official Docker action would be the way to go for now (although it also stores the password unencrypted in /github/home/.docker/config.json by default) and that shorter-lived tokens would be preferable.

@eine
Copy link
Author

eine commented Nov 13, 2020

Ref: docker/build-push-action#227

@github-actions
Copy link

This issue has become stale and will be closed automatically within a period of time. Sorry about that.

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

4 participants
@chrispat @eine @ebbeelsborg-unity and others