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

Enable logging in via environment variables #198

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

c-w
Copy link

@c-w c-w commented Jan 3, 2022

- What I did

As discussed in #176, this pull request adds the ability to log in using environment variables.

The capability added in this pull request is useful when using the CLI in non-interactive scripts so that one doesn't have to use a work-around using expect (which is what I'm currently doing and feels pretty hacky) or self-generating the docker token file (which is non-trivial from a basic bash script).

- How I did it

In RunLogin first check if the environment variables DOCKER_USERNAME or DOCKER_PASSWORD are set before asking the user to provide the values on the command line.

- How to verify it

I ran the following snippet:

 export DOCKER_PASSWORD="<redacted>"
export DOCKER_USERNAME="cwolff"
./bin/hub-tool login

And I verified that the output states that the login succeeded.

- Description for the changelog

Enable logging in by setting the DOCKER_USERNAME and DOCKER_PASSWORD environment variables.

- A picture of a cute animal (not mandatory)

Field mouse

@converge
Copy link
Contributor

hi, not sure if this is a good idea, since with login/password, an attacker could have full access to the user account. Using tokens with specific permissions, would be a better ideia when it comes to security.

1 similar comment
@converge
Copy link
Contributor

hi, not sure if this is a good idea, since with login/password, an attacker could have full access to the user account. Using tokens with specific permissions, would be a better ideia when it comes to security.

@c-w
Copy link
Author

c-w commented Apr 14, 2022

@converge Could you elaborate your concern? There’s several use-cases for this functionality linked on the description and to my understanding some APIs don’t work with tokens currently. In any case, this pull request isn’t adding new functionality to the tool, but only enabling a non-interactive use of existing functionality. As such I don’t see how the attack surface is increased: users of the tool are responsible for keeping their environment variables safe anyways and many other tools offer authentication via environment variables, e.g. PGPASSWORD for the Postgres CLI.

@converge
Copy link
Contributor

Hi @c-w , I'll split it into parts.

  1. I wasn't aware of API endpoints that don't accept tokens. Do you know what endpoints don't accept tokens? I ask it only out of curiosity. And wondering why such endpoints wouldn't accept the token?

  2. I got your point, however, an attacker having access to PG_PASSWORD for example, would have access to a specific database(could delete all database data of that specific database), and an attacker having access to Docker Hub platform (login/passwd) could delete all Docker Hub repositories.

  3. Ideally access to Docker Hub image from the pipeline would use a token that is only able to read/download OCI images, if the token is leaked, the attacker has limited access and will only be able to download the images.

p.s: feel free to correct me if I'm wrong, I'm new to the project.

@c-w
Copy link
Author

c-w commented Apr 20, 2022

I wasn't aware of API endpoints that don't accept tokens. Do you know what endpoints don't accept tokens? I ask it only out of curiosity. And wondering why such endpoints wouldn't accept the token?

Looks like this changed since the last time I checked the docs. IIRC deletions for example didn't use to work via a token but this is now documented differently. In any case, the hub-tool documentation also still states that access tokens only offer a limited subset of the functionality of the full API (see README).

I got your point, however, an attacker having access to PG_PASSWORD for example, would have access to a specific database(could delete all database data of that specific database), and an attacker having access to Docker Hub platform (login/passwd) could delete all Docker Hub repositories.

As per these docs the flow to log in via an access token is the same as the flow to log in via a password, only that the token value replaces the password value. As such, wouldn't this pull request also enable non-interactive login via an access token?

@woky
Copy link

woky commented Apr 25, 2022

hi, not sure if this is a good idea, since with login/password, an attacker could have full access to the user account. Using tokens with specific permissions, would be a better ideia when it comes to security.

If the token has Read/Write/Delete permissions it's not much better than username/password security-wise.

I got your point, however, an attacker having access to PG_PASSWORD for example, would have access to a specific database(could delete all database data of that specific database), and an attacker having access to Docker Hub platform (login/passwd) could delete all Docker Hub repositories.

If using password is so dangerous then why does hub-tool store it in ~/.docker/config.json?:

{
	"auths": {
		"hub-tool": {
			"auth": "REDACTED_BASE64_CREDENTIALS"
		},
	}
}

To script hub-tool you can do the following dance:

echo -n $DOCKER_USERNAME:$DOCKER_PASSWORD | base64 | jo -d. auths.hub-tool.auth=@- | jq -s '.[1] * .[0]' - ~/.docker/config.json | sponge ~/.docker/config.json

Voila:

$ hub-tool account info
Name:		woky
Full name:	
...

@MarcoSaba
Copy link

Is this feature available? I'd like to use hub-tool on a CI :)

@monfardineL
Copy link

I think this project is dead, after all. More than a year and nobody even bought a cake to celebrate all this time waiting for a review.

@spkane
Copy link

spkane commented Feb 9, 2023

@monfardineL Since you approved this, are you also able to merge it in, or is there someone else who can do this?

@monfardineL
Copy link

@monfardineL Since you approved this, are you also able to merge it in, or is there someone else who can do this?

I can't. I'm still unsure how I could approve it.
What I did was a fork of this repository, made same changes proposed in this PR and built it by myself. Unfortunately my GH-Actions-foo didn't allow me to publish it yet. I'll keep trying though.

@spkane
Copy link

spkane commented Feb 9, 2023

@silvin-lubecki Any chance that this could get merged?

williamdes added a commit to sudo-bot/docker-rustpython that referenced this pull request Feb 25, 2023
@williamdes
Copy link

No need for a forked binary, I did choose to fool this tool. As someone said why bother when the JWT is stored in plain sight in config.json.

⚠️ Do not run this or it will override all of your other docker logins, backup before. I made it for a CI.

# Token commands thank to https://stackoverflow.com/a/59334315/5155484
HUB_TOKEN=$(curl -s -H "Content-Type: application/json" -X POST -d "{\"username\": \"$DOCKER_USERNAME\", \"password\": \"$DOCKER_PASSWORD\"}" https://hub.docker.com/v2/users/login/ | jq -r .token)
USERNAME="$(printf '%s:' "$DOCKER_USERNAME" | base64 -w0)"
USER_PASS="$(printf '%s:%s' "$DOCKER_USERNAME" "$DOCKER_PASSWORD" | base64 -w0)"

printf '{"auths": {"hub-tool": {"auth": "%s"}, "hub-tool-refresh-token": {"auth": "%s"}, "hub-tool-token": { "auth": "%s", "identitytoken": "%s"}}}' \
                    "$USER_PASS" "$USERNAME" \
                    "$USERNAME" "$HUB_TOKEN" \
                    > ~/.docker/config.json

And it works on GitHub actions !
See: https://github.com/sudo-bot/docker-rustpython/actions/runs/4267540987/jobs/7429251312
Code: https://github.com/sudo-bot/docker-rustpython/blob/53f16c3265db556be0248b45281180e319df492c/.github/workflows/publish.yml#L83-L115

@monfardineL
Copy link

@williamdes this works nice, thanks.
However, I don't see any good reason for the hub-tool to not support same authentincation settings as docker itself.
Main point here is, project seems to be inactive for a while now.

@williamdes
Copy link

Main point here is, project seems to be inactive for a while now.

/cc @converge @silvin-lubecki @RomainBelorgey
Dear last commiters, is this project dead?

@converge
Copy link
Contributor

converge commented Feb 28, 2023

Main point here is, project seems to be inactive for a while now.

/cc @converge @silvin-lubecki @RomainBelorgey Dear last commiters, is this project dead?

hey @williamdes!
I haven't worked at the project recently, I had the impression few people were using it, and stop contributing.

Is there some main features you guys are missing? feel free to move the discussion into an project issue.

@williamdes
Copy link

Main point here is, project seems to be inactive for a while now.

/cc @converge @silvin-lubecki @RomainBelorgey Dear last commiters, is this project dead?

hey @williamdes! I haven't worked at the project recently, I had the impression few people were using it, and stop contributing.

Is there some main features you guys are missing? feel free to move the discussion into an project issue.

Hi!
Thank you for the quick reply
I think this feature: non interactive login would be great soo the project can be used in CIs properly.
can you review the code?

@converge
Copy link
Contributor

converge commented Mar 1, 2023

Main point here is, project seems to be inactive for a while now.

/cc @converge @silvin-lubecki @RomainBelorgey Dear last commiters, is this project dead?

hey @williamdes! I haven't worked at the project recently, I had the impression few people were using it, and stop contributing.
Is there some main features you guys are missing? feel free to move the discussion into an project issue.

Hi! Thank you for the quick reply I think this feature: non interactive login would be great soo the project can be used in CIs properly. can you review the code?

I can review, but I have no powers to unblock the merge. I have emailed @silvin-lubecki , maybe he can help us.

@spkane
Copy link

spkane commented Mar 6, 2023

@thaJeztah @tonistiigi Do either of you happen to know of anyone who still has commit rights on this project?

@williamdes
Copy link

A new version was released by @silvin-lubecki
Could we get this PR released too?

@zliang-akamai
Copy link

zliang-akamai commented Apr 28, 2024

Love to see this feature is being developed! Any plan to release it?

@spkane
Copy link

spkane commented Jun 23, 2024

@silvin-lubecki Is there any chance that this can get merged in?

@lucj
Copy link

lucj commented Nov 7, 2024

That would be really helpful.

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

Successfully merging this pull request may close these issues.

10 participants