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

Docker client does not handle 401 Unauthorized response codes from registry correctly #18569

Open
hjacobs opened this issue Dec 10, 2015 · 20 comments

Comments

@hjacobs
Copy link

commented Dec 10, 2015

Docker (1.9.1) only checks for 401 HTTP status code on GET /v2/ but it will not attempt authentication on later 401 status response codes. My use case: our registry needs to allow unauthenticated pulls (therefore will return 200 on GET /v2/), but it requires authentication on "push".

Docker client version: 1.9.1

Used registry: https://github.com/zalando-stups/pierone

Docker client will print "" on the console instead of attempting authentication (as it should).

docker-why

@hjacobs

This comment has been minimized.

Copy link
Author

commented Dec 10, 2015

docker version
Client:
 Version:      1.9.1
 API version:  1.21
 Go version:   go1.4.2
 Git commit:   a34a1d5
 Built:        Fri Nov 20 13:12:04 UTC 2015
 OS/Arch:      linux/amd64

Server:
 Version:      1.9.1
 API version:  1.21
 Go version:   go1.4.2
 Git commit:   a34a1d5
 Built:        Fri Nov 20 13:12:04 UTC 2015
 OS/Arch:      linux/amd64

docker info
Containers: 1701
Images: 4743
Server Version: 1.9.1
Storage Driver: aufs
 Root Dir: /var/lib/docker/aufs
 Backing Filesystem: extfs
 Dirs: 8145
 Dirperm1 Supported: false
Execution Driver: native-0.2
Logging Driver: json-file
Kernel Version: 3.13.0-71-generic
Operating System: Ubuntu 14.04.3 LTS
CPUs: 4
Total Memory: 7.688 GiB
Name: zalando-123
ID: HRFO:LNVW:V6IO:ZCI6:NLKB:DXFI:BOZL:S4XA:VA7O:XAIP:2CFT:EREF
Username: hjacobs
Registry: https://index.docker.io/v1/
WARNING: No swap limit support

uname -a
Linux zalando-123 3.13.0-71-generic #114-Ubuntu SMP Tue Dec 1 02:34:22 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
@thaJeztah

This comment has been minimized.

Copy link
Member

commented Dec 10, 2015

Is this different from #17317 (I see you also commented on that issue). I also found these issues in the "distribution" repository; docker/distribution#1028 and docker/distribution#1230

@hjacobs

This comment has been minimized.

Copy link
Author

commented Dec 10, 2015

@thaJeztah I guess it's the same, in general we need an issue "Docker does not handle HTTP status codes correctly" --- I think it's a broader issue of ignoring REST best practices (the Docker v2 upload procedure is weird: e.g. using a PUT without a body) and only implementing the Docker happy case without a clear registry API spec and ignoring all existing specs (like HTTP).

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Dec 10, 2015

The specs and registry are open source, so if you think there's changes to be made, I think the best location would be the https://github.com/docker/distribution/ repository. The current version of the specs are also located there; https://github.com/docker/distribution/tree/master/docs/spec

@stevvooe

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2015

@hjacobs After reading through some of the descriptions in other issues, it seems like you really need to consider token authentication, even when pulling as an anonymous user. This is what we do with the Hub and it allows for a number of complex workflows that involve authentication. It also provides a much better security model that doesn't rely on fragile method and url matching and conditional issuance of 401 responses.

Docker (1.9.1) only checks for 401 HTTP status code on GET /v2/ but it will not attempt authentication on later 401 status response codes.

This is the registry client in docker/distribution. This sounds like a bug. The transport should detect a 401, attempt to authorize based on the response and then reissue the request. Part of this behavior is due to the poor design of v1 authentication and the need for v2 to be compatible (v1 was ping-based, where v2 is optimistic). No one really hits this, since most people use the same authentication across the entire API.

"Docker does not handle HTTP status codes correctly" --- I think it's a broader issue of ignoring REST best practices (the Docker v2 upload procedure is weird: e.g. using a PUT without a body) and only implementing the Docker happy case without a clear registry API spec and ignoring all existing specs (like HTTP).

Insulting the approach to http and the specification is no way to achieve your goal. I would recommend a less flippant approach. This attitude and tone is unacceptable and unprofessional. If you want help, I'd suggest taking a more constructive approach to your feedback from this point forward.

@hjacobs

This comment has been minimized.

Copy link
Author

commented Dec 10, 2015

@stevvooe thanks for the response, apparently my "flippant" (had to look it up) approach at least gave me a comment 😏 --- I just always wonder why new products pop up in the Docker ecosystem while the core is still quite "brittle" (sorry, maybe this is considered "flippant" again).

We found a dirty workaround with multiple domains (one for push, default for pull) for us (for now), but at least you also consider not handling 401 (or error while doing so) a bug 😄

I will read up on the token authentication flow, we use OAuth 2 Bearer tokens anyway already (which was not supported by older versions of Docker).

@stevvooe

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2015

@hjacobs General statements about "ignoring REST best practices" and unrelated things like PUT not having a body are simply not constructive on an issue tracker. The API is not even a REST API. Keep your comments topical and relevant to the issue at hand.

@mickep76

This comment has been minimized.

Copy link

commented Dec 15, 2015

The constructive way here would be to suggest a way forward to resolve the problem. If it follows REST best practices or not, doesn't seem to solve the problem.

@thaJeztah thaJeztah added the kind/bug label Dec 15, 2015
@thaJeztah

This comment has been minimized.

Copy link
Member

commented Dec 15, 2015

@mickep76 would you be able to test if this is still reproducible on master? There have been quite some changes in this area for the coming 1.10 release. Builds from master are available at https://master.dockerproject.org, please be aware that those should not be used for production, just for testing.

@mickep76

This comment has been minimized.

Copy link

commented Dec 15, 2015

Trying with the latest build 1.10 it only does a GET request, therefore you can't trigger an auth. challenge on PUT for non-auth. Pull and auth. Push.

x.x.x.x - - [15/Dec/2015:13:04:50 +0000] "GET /v2/ HTTP/1.1" 200 2 "-" "docker/1.10.0-dev go/go1.5.1 git-commit/ee3e07d kernel/3.10.0-229.7.2.el7.x86_64 os/linux arch/amd64"
@thaJeztah

This comment has been minimized.

Copy link
Member

commented Dec 15, 2015

Thanks @mickep76!

I'll ping @aaronlehmann to have a look at this issue as well

@aaronlehmann

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2015

We currently only handle authentication on the initial GET. It sounds like it would be desirable to handle it at any point in the protocol, but there will have to be some design discussions to figure out how best do support that.

@stevvooe suggested that the transport could try to authorize and reissue the request:

The transport should detect a 401, attempt to authorize based on the response and then reissue the request.

...but it sounds like this would break the RoundTripper contract:

        // RoundTrip executes a single HTTP transaction, returning
        // the Response for the request req.  RoundTrip should not
        // attempt to interpret the response.  In particular,
        // RoundTrip must return err == nil if it obtained a response,
        // regardless of the response's HTTP status code.  A non-nil
        // err should be reserved for failure to obtain a response.
        // Similarly, RoundTrip should not attempt to handle
        // higher-level protocol details such as redirects,
        // authentication, or cookies.

I'm interested to hear any ideas for how to implement this cleanly in the distribution client.

cc @dmcgowan

@mickep76

This comment has been minimized.

Copy link

commented Dec 15, 2015

We need some way of triggering auth. when proxying auth. through nginx or Apache. For v1.0 we used PUT to trigger an auth. challenge. For v2.0 we can't do that since it only uses GET, then we couldn't have it non auth. for pull requests but it would always issue an auth. challenge.

The new version come's with built-in basic auth but that doesn't support LDAP, therefore we need to proxy the requests.

@stevvooe

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2015

@aaronlehmann We probably can't replay a request sent to http.RoundTripper, but we could side-channel a request to get the authentication parameters before the request. If we can't make this work, we can probably introduce the concept of an http.Doer, which "does" the request, at a higher level. Part of the problem is that the body of the request is not idempotent, so rebuilding that correctly is key to getting this to work. Either way, we should definitely try to authorize 401, since that is the requirement of the response. Arguably, many of these responses should actually be a 403, rather than a 401.

@aaronlehmann Another possibility here is to support proper OPTIONS request that can be used to identify properties of the request. That could be use in conjunction with the roundtripper to see what methods require authentication. We may have to make registry modifications for this work, but it could provide a long term solution.

@mickep76 I'll be honest. We really didn't design the registry v2 API to work in a dual authenticated/unauthenticated use case. Resources are mapped directly and authorization is first-class, in that we authorize "pull on ubuntu", rather than GET, HEAD, OPTIONS on "/v2/ubuntu/*". This approach avoids implicit endpoint leaks from incorrect configuration (forgetting to protect PUT, in addition to POST or incorrectly matching a path, for example). We also wanted to avoid the credential leaks that can happen with basic auth. The default is protection, which does affect usability.

I'd really recommend looking into token auth for your use case.
We require exactly this workflow with the Hub and DTR and it works flawlessly with token auth. This is supported with LDAP via DTR but there are open source options, such as https://github.com/cesanta/docker_auth, which may be sufficient. The actual code required to do this is minimal, if it comes down to it, except for getting the LDAP configuration correct. @mickep76 Feel free to contact me on IRC or via the mailing list if you have questions.

@dmcgowan

This comment has been minimized.

Copy link
Member

commented Dec 15, 2015

I'm interested to hear any ideas for how to implement this cleanly in the distribution client.

@aaronlehmann I don't think this is the responsibility of the distribution client. Now that we have retry mechanisms it is possible to swap out the transport after receiving a 401 and retry with the authentication. Not sure I am a fan of taking the OPTIONS approach since it accomplishes mostly the same thing as the ping but with an even more awkward HTTP type. We already successfully do a "side channel" request for token authentication and have not had any issues with it. We simply did not design for this use case with basic auth for v2 because token authentication handles it much more predictably and directly inside the registry as opposed to hacked into the proxy.

@joda70

This comment has been minimized.

Copy link

commented Mar 16, 2016

Can you provide documentation in order to address this issue?
At the moment I am not able to find the right configuration combining registry V2 with nginx.

@icy

This comment has been minimized.

Copy link

commented May 18, 2016

This problem happens because docker client tries to detect authentication method from the registry server. If there is an option to force docker client to send authentication information to the registry, that would be fine. However, solving a problem in registry is much easier...

My two cents.

@icy

This comment has been minimized.

Copy link

commented May 18, 2016

@joda70 You can create two different nginx set up, let's say dockerhub-ro.example.net and dockerhub-rw.example.net. For the ro version, you need to deny all requests except the GET/HEAD ones; this prevent docker client from pushing (actually, docker client tries to push without any authentication method; but it will fail quickly). The rw version you need to provide basic authentication for all methods (event if with pull requests.)

This is really tedious, because you need to push from some special machines. (It's still good for my case though: I only build and push images from a single devops machine; any other clients are read-only clients.)

Hope this helps until we have a better support from docker (registry).

Update: actually, a docker client can push and push to different dockerhub addresses.

@hjacobs

This comment has been minimized.

Copy link
Author

commented May 18, 2016

@icy we use the same approach ("workaround") in our Pier One Docker registry, i.e. we have two DNS entries pointing to the same app:

  • registry-write.opensource.zalan.do to push (with authentication, will return 401 on GET /v2/)
  • registry.opensource.zalan.do to pull (without authentication, will return 200 on GET /v2/)
@BeOleg

This comment has been minimized.

Copy link

commented Mar 2, 2017

👍

@ingwinlu ingwinlu referenced this issue Sep 2, 2018
18 of 35 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.