sylabs / singularity

SingularityCE is the Community Edition of Singularity, an open source container platform designed to be simple, fast, and secure.
https://sylabs.io/docs/
Other
779 stars 98 forks source link

Try unauthenticated OCI registry interactions if expired/invalid credentials are set #212

Open myoung3 opened 3 years ago

myoung3 commented 3 years ago

Version of Singularity 3.8.1

Describe the bug If I associated a username/token pair for an oras repository with singularity using singularity remote login, then cancel that token (ie via github), I can't pull from public repositories.

To Reproduce

ingularity shell oras://ghcr.io/singularityhub/github-ci:latest
INFO[0000] trying next host                              error="failed to authorize: failed to fetch oauth token: unexpected status: 403 Forbidden" host=ghcr.io
FATAL:   Unable to handle oras://ghcr.io/singularityhub/github-ci:latest uri: failed to get checksum for oras://ghcr.io/singularityhub/github-ci:latest: while resolving reference: failed to authorize: failed to fetch oauth token: unexpected status: 403 Forbidden

Expected behavior print a warning that authentication failed, retry anonymously.

OS / Linux Distribution Which Linux distribution are you using?

ingularity shell oras://ghcr.io/singularityhub/github-ci:latest
INFO[0000] trying next host                              error="failed to authorize: failed to fetch oauth token: unexpected status: 403 Forbidden" host=ghcr.io
FATAL:   Unable to handle oras://ghcr.io/singularityhub/github-ci:latest uri: failed to get checksum for oras://ghcr.io/singularityhub/github-ci:latest: while resolving reference: failed to authorize: failed to fetch oauth token: unexpected status: 403 Forbidden

Installation Method source

Additional context it would also be good if there was a way to remove authenticated logins

dtrudg commented 3 years ago

Expected behavior print a warning that authentication failed, retry anonymously.

Not sure that we want to do this. I am of the opinion that if credentials have been set they should always be used, and an auth failure should fail hard. Any thoughts @EmmEff ?

Additional context it would also be good if there was a way to remove authenticated logins

This should definitely be provided.

vsoch commented 3 years ago

I would take an approach that most APIs taken with respect to auth - first try without it, and then get back a 403 to indicate that auth is required, and respond to that. For this case it would mean not using the expired token by default and the public image would pull! And then if auth is required, that would make sense to try the token and report failure to the user.

dtrudg commented 3 years ago

@vsoch - this is not the case for docker, podman etc. though - which I believe are the best targets to emulate here w.r.t. interaction with OCI registries.

With docker, pulling a public image from Docker Hub, if I attempt to pull with an invalid token set it will not fall back and allow a public pull:

$ docker pull ubuntu
Using default tag: latest
Error response from daemon: Head https://registry-1.docker.io/v2/library/ubuntu/manifests/latest: unauthorized: please use personal access token to login

Ditto podman:

$ podman pull ubuntu
Resolved "ubuntu" as an alias (/etc/containers/registries.conf.d/000-shortnames.conf)
Trying to pull docker.io/library/ubuntu:latest...
Error: Error initializing source docker://ubuntu:latest: unable to retrieve auth token: invalid username/password: unauthorized: please use personal access token to login

One reason for this is that we now have things like rate limiting on OCI registries such as Docker Hub. I always want to pull authenticated, even public images. This is so that pulls count against my account limits, and do not eat up the low non-authenticated limits that apply for my IP address for anyone who is not logged in.

vsoch commented 3 years ago

Sorry it's the other way around - you try without any token first, and only add the token to try on that failure.

dtrudg commented 3 years ago

Sorry it's the other way around - you try without any token first, and only add the token to try on that failure.

Right - that's what we don't want to do because...

... we now have things like rate limiting on OCI registries such as Docker Hub. I always want to pull authenticated, even public images. This is so that pulls count against my account limits, and do not eat up the low non-authenticated limits that apply for my IP address for anyone who is not logged in.

vsoch commented 3 years ago

Hmm, well I think there are different opinions on this, because I'd prefer the first - I don't pull frequently enough either authenticated or not to worry about that (but I see the point that this could go badly in some automated CI). What if you default to what would be safest for CI, but then allow users to customize a config to say "please try without authenticated first?" There is definitely something to say for trying the simplest approaches before just blindly throwing credentials at something.

dtrudg commented 3 years ago

We are calling out to other 3rd party packages to actually do our operations against OCI registries, and that might be where an always auth first if configured vs noauth -> auth if needed configurable behavior might be better placed, but I'm not sure they would consider it.

Implementation would need some careful work in multiple places in Singularity which can interact with OCI registries, and I think this is something where I'd be happy to see it contributed as a PR from the community, but in the reverse direction - we should match docker and podman by default. I think this is vital as with shared IPs etc. it's easy to block someone else by consuming the unauthenticated Docker hub limits when you intend to be auth'd. Also, I don't think this would be a high Sylabs priority for development.

Adam is out at the moment, but I'll ask for his input too when he is back, and I'll talk to @EmmEff as well.

EmmEff commented 3 years ago

I agree with both @vsoch and @dtrudg :)

It makes sense not to burn through unauthorized Docker Hub limits, though I know at least some modules do default to an unauth-then-auth flow if a 403 is returned from the unauthorized request. I would have to investigate what others are doing, but as @dtrudg mentions, aren't Docker and podman doing auth always?

dtrudg commented 3 years ago

@myoung3

Additional context it would also be good if there was a way to remove authenticated logins

Try singularity remote logout .... - I forgout about that one for a minute :-)

myoung3 commented 3 years ago

@myoung3

Additional context it would also be good if there was a way to remove authenticated logins

Try singularity remote logout .... - I forgout about that one for a minute :-)

Don't know how I missed that! But after doing singularity remote logout oras://ghcr.io the URI "oras://ghcr.io" seems to persist in the output of singularity remote. Perhaps the issue is that I have multiple tokens associated with oras://ghcr.io?

dtrudg commented 3 years ago

Don't know how I missed that! But after doing singularity remote logout oras://ghcr.io the URI "oras://ghcr.io" seems to persist in the output of singularity remote. Perhaps the issue is that I have multiple tokens associated with oras://ghcr.io?

It appears there is a bug here where consecutive logins create multiple entries in remote.yaml. This is only happening in the remote.yaml, and not the ~/.singularity/docker-config.json where the actual auth token is kept. It is replaced there, rather than duplicated.

Will open this as a separate bug.

Edit - opened #214 for this bug.

adelavega commented 3 years ago

I also experienced this issue trying to pull my public package from github. I tried logging out and clearing my PAK, and then invalidated said PAK. I was then unable to pull public packages.

At least I think #214 should help since logging out should work now, but I think not being able to pull public packages due to an invalid PAK is pretty unintuitive.

dtrudg commented 3 years ago

At least I think #214 should help since logging out should work now, but I think not being able to pull public packages due to an invalid PAK is pretty unintuitive.

As above (https://github.com/sylabs/singularity/issues/212#issuecomment-892878858), this is the same behavior as docker and podman, so we will keep it as our default too and be consistent... but if anyone wants to contribute a PR for different non-default behavior via flag/env var/config setting, we'd consider it.

vsoch commented 3 years ago

Maybe that's something we can chat about - what should said flag/env var/config look like?

dtrudg commented 3 years ago

Currently we have the env vars DOCKER_LOGIN DOCKER_USERNAME DOCKER_PASSWORD, so I'd suggest probably something fairly explanatory e.g DOCKER_TRY_NOAUTH or DOCKER_NOAUTH_FIRST ... and probably exposed as a flag too --docker-try-noauth? Open to other suggestions though.

It would need to be implemented for a number of commands:

The best place to start exploring is cmd/internal/cli/singularity.go where the existing env vars / flags are defined. From there a bunch of 'find references' in an IDE should lead to the places in the code where they are used, and docker/OCI registry auth is performed.

Edit - note that the actual handling of authentication using tokens stored in a config file (the docker default location, or via a remote login) is through the libraries we use for OCI interactions. Search for syfs.DockerConf() to see where these are being passed the path to a file.