Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Bug matching authorization protocol with docker 1.8 #276

Closed
deitch opened this issue Aug 17, 2015 · 25 comments
Closed

Bug matching authorization protocol with docker 1.8 #276

deitch opened this issue Aug 17, 2015 · 25 comments

Comments

@deitch
Copy link

deitch commented Aug 17, 2015

Is it possible to access a public repo anonymously? Sure, if it is public, and I am logged in as a user not of that workspace, I can do a pull, but how about if I am not logged in?

@deitch
Copy link
Author

deitch commented Aug 17, 2015

I take it back. Public repos don't appear to work. I set up a namespace called it, added two accounts avid (part of the it team) and avi2 (not), then set the namespace to public.

Login as avi2 then do docker pull reg.local/it/other gives me 404 not found, while the Portus logs show:

Started GET "/v2/token?account=avi2&scope=repository%3Ait%2Fother%3Apush%2Cpull&service=reg.local" for 107.170.247.94 at 2015-08-17 18:06:07 +0000
Cannot render console from 107.170.247.94! Allowed networks: 127.0.0.1, ::1, 127.0.0.0/127.255.255.255
Processing by Api::V2::TokensController#show as JSON
  Parameters: {"account"=>"avi2", "scope"=>"repository:it/other:push,pull", "service"=>"reg.local"}
  User Load (1.4ms)  SELECT  `users`.* FROM `users` WHERE `users`.`username` = 'avi2'  ORDER BY `users`.`id` ASC LIMIT 1
   (0.3ms)  BEGIN
  SQL (0.6ms)  UPDATE `users` SET `last_sign_in_at` = '2015-08-17 18:06:03', `current_sign_in_at` = '2015-08-17 18:06:07', `sign_in_count` = 18, `updated_at` = '2015-08-17 18:06:07' WHERE `users`.`id` = 3
   (5.6ms)  COMMIT
  Registry Load (0.4ms)  SELECT  `registries`.* FROM `registries` WHERE `registries`.`hostname` = 'reg.local' LIMIT 1
  Namespace Load (0.7ms)  SELECT  `namespaces`.* FROM `namespaces` WHERE `namespaces`.`registry_id` = 1 AND `namespaces`.`name` = 'it' LIMIT 1
  Team Load (0.5ms)  SELECT  `teams`.* FROM `teams` WHERE `teams`.`id` = 4 LIMIT 1
  User Exists (0.9ms)  SELECT  1 AS one FROM `users` INNER JOIN `team_users` ON `users`.`id` = `team_users`.`user_id` WHERE `team_users`.`team_id` = 4 AND `team_users`.`role` = 2 AND `users`.`id` = 3 LIMIT 1
  User Exists (0.8ms)  SELECT  1 AS one FROM `users` INNER JOIN `team_users` ON `users`.`id` = `team_users`.`user_id` WHERE `team_users`.`team_id` = 4 AND `team_users`.`role` = 1 AND `users`.`id` = 3 LIMIT 1
Completed 401 Unauthorized in 138ms (ActiveRecord: 11.4ms)

While if I log in as avid it works fine.

It looks like the registry asking for push&pull, instead of just pull?

@flavio
Copy link
Member

flavio commented Aug 19, 2015

Which version of the docker registry and of the docker client are you using?

@deitch
Copy link
Author

deitch commented Aug 19, 2015

Actually, it isn't the registry at all. It is the docker daemon itself.

I am using docker 1.8.1 with registry 2.0.1.

I had a discussion with some of the docker people on another github issues thread. It turns out that the protocol as they explain it may not be implemented correctly in Portus, or cesanta, or anywhere except for docker's actual auth server.. because the documentation wasn't clear. They asked for a PR to fix it.

Here is how they described it:

  1. client does pull or push or whatever
  2. daemon does ping to registry i.e. https://registry.ip/v2/
  3. registry returns 401 along with realm and service, but not scope
  4. daemon asks for a token from the auth server, with service=<registry>&scope=repository:namespace/image:push,pull

In other words, the daemon always asks for push,pull, even if you are just doing a pull. The auth server is supposed to respond in the following fashion:

  • If unauthenticated access is not allowed, return a 401 requiring user to authentictae
  • If unauthenticated access is allowed to that repo, return a web token

When user tries the token path against the auth server with credentials:

  • If invalid credentials, return 401
  • If valid credentials, always return a 200 with a JWT that has the maximum credentials allowed this user on this repository in this service that is a subset of the scope provided.

The daemon will always ask for push,pull, and - as long as I am validly authenticated - the auth server should always return 200 with a valid Web token. The Web token will list the max I am allowed.

  • If I am not allowed push or pull, then return a token with no access
  • If I am allowed pull but not push, then return a token with pull access only
  • If I am allowed pull and push, then return a token with push and pull access

Yet Portus seems to return a 401 if I am allowed pull but not push.

So there are 2 issues:

  1. Return a JWT with 200 if I am not logged in and the image allows anonymous access
  2. Return a JWT with 200 if I am logged in and the image allows me to pull but not push, even if the request was for push,pull

Is this all new to 1.8.1?

@flavio
Copy link
Member

flavio commented Aug 19, 2015

Wow, thanks for the detailed explanation! This is really helpful.

We are going to address the issue.

Is this all new to 1.8.1?

Yes it is. With docker 1.7.1 I cannot reproduce the issue.

@flavio flavio added the bug label Aug 19, 2015
@flavio flavio added this to the 2nd stable release of Portus milestone Aug 19, 2015
@deitch
Copy link
Author

deitch commented Aug 19, 2015

I just now saw someone referenced my issue as a something they didn't have a problem with in 1.7.1. If you want the real dirty details, right down to my debug output and packet traces, check out moby/moby#15640

Sigh, OK, I will spin up a VM, add 1.7.1 and check it out.

@deitch
Copy link
Author

deitch commented Aug 19, 2015

Oh, yes, that handles the "I only have pull rights, it asks for push,pull." What about the anonymous access?

@flavio
Copy link
Member

flavio commented Aug 19, 2015

Yes you are right, sorry for not being specific.

As for the anonymous access this is something that has never worked in the past. With distribution 2.0.1 anonymous users (aka users not logged into the registry) aren't even forwarded to Portus: the registry immediately replies with a 404 message.

@deitch
Copy link
Author

deitch commented Aug 19, 2015

That doesn't sound right. After all, the first request has to get a 401.

I am running some tests right now...

@deitch
Copy link
Author

deitch commented Aug 19, 2015

I can confirm that this is a change from 1.7 to 1.8. I spun up a clean VM and installed 1.7.1, replicated the environment, and ran docker in debug. Then I tried to pull a VM docker pull reg.local/my/other

Here is the debug output

DEBU[0088] Calling POST /images/create
INFO[0088] POST /v1.19/images/create?fromImage=reg.local%2Fmy%2Fother%3Alatest
DEBU[0088] pulling image from host "reg.local" with remote name "my/other"
DEBU[0088] pinging registry endpoint https://reg.local/v0/
DEBU[0088] attempting v2 ping for registry endpoint https://reg.local/v2/
DEBU[0088] attempting v2 ping for registry endpoint https://reg.local/v2/
DEBU[0088] pulling v2 repository with local name "reg.local/my/other"
DEBU[0088] pinging registry endpoint https://reg.local/v0/
DEBU[0088] attempting v2 ping for registry endpoint https://reg.local/v2/
DEBU[0088] Getting authorization for my/other [pull]

Notice the last line: it tries to get authorization for my/other [pull] whereas 1.8 daemon tries to get the maximal push,pull.

On the other hand, it looks like the registry at least can handle getting a Web token for less than the user requests and rejecting it. So changing to always 200 with a lesser Web token than requested will be handled by the client. So the right thing to do is implement the API that the docker team described for 1.8.x, knowing pre-1.8 will work with it.

@deitch
Copy link
Author

deitch commented Aug 19, 2015

Is this an easy fix?

@deitch
Copy link
Author

deitch commented Aug 19, 2015

Trying to be helpful. It appears to be somewhere in https://github.com/SUSE/Portus/blob/master/app/controllers/api/v2/tokens_controller.rb specifically https://github.com/SUSE/Portus/blob/master/app/controllers/api/v2/tokens_controller.rb#L30

But I couldn't figure out where it actually matches up the scopes to the user?

@flavio
Copy link
Member

flavio commented Aug 19, 2015

Yes, this fix should be easy.

Anonymous pulls are not hard to implement, but would require more evaluations on our side (like a ui control to toggle the feature). For example: I can imagine customers who want to enforce everybody to be logged into the registry even when pulling public images.

@deitch
Copy link
Author

deitch commented Aug 19, 2015

You are right. And whereas now we have 2 levels of access control per namespace - public (owner/member can pull+push) and private (anyone logged in can pull) - now we would need 3 levels - public, private and open (anyone anonymous can pull).

Truth is, that is not a bad thing, especially if we got it to the level of a repository rather than a namespace. Think how docker hub has public/private per repo, not just per namespace.

Here is what I will do. I will rename this issue to "Bug matching authorization protocol with docker 1.8", and open a separate issue for "Anonymous Access", since it is likely to last a lot longer.

Can you tell me where the authorize gets called in that line?

@deitch deitch changed the title Anonymous path access? Bug matching authorization protocol with docker 1.8 Aug 19, 2015
@flavio
Copy link
Member

flavio commented Aug 20, 2015

@deitch sorry for the delay, I'm currently on vacation. I got the Docker 1.8 client work with the code I submitted to #282, however the PR needs more polishing (we have to update the test cases). Currently it's holiday seasons, so please be patient :)

@deitch
Copy link
Author

deitch commented Aug 21, 2015

Vacation? They let those of us in the tech world take those??? :-)

I see the PR. I will keep an eye on the issue. I would offer to beta test, but it is such a small change, you probably already did it yourself, and the rspec probably hits everything anyways.

Thanks.

@flavio
Copy link
Member

flavio commented Aug 21, 2015 via email

@deitch
Copy link
Author

deitch commented Aug 21, 2015

So we need to check registry:2.0.1 and registry:2.1.1 against docker:1.8.1, docker:1.7.1, docker:1.7.0. We probably should do 1.6.2 as well, but who has the time?

I will try to find some time. If it moves the PR through quickly....

I just pull the branch? And your test cases are:

  1. unauthenticated user is rejected
  2. authenticated user can pull in public other user's namespace
  3. authenticated user cannot push in public other user's namespace
  4. authenticated user cannot pull in private other user's namespace
  5. authenticated user cannot push in private other user's namespace
  6. authenticated user can pull in own namespace
  7. authenticated user can push in own namespace
  8. authenticated user can pull in public group's namespace of which he is not a member
  9. authenticated user cannot push in public group's namespace of which he is not a member
  10. authenticated user cannot pull in private group's namespace of which he is not a member
  11. authenticated user cannot push in private group's namespace of which he is not a member
  12. authenticated user can pull in group's namespace of which he is a member
  13. authenticated user can push in private group's namespace of which he is a member

@deitch
Copy link
Author

deitch commented Aug 26, 2015

Hi. Any update?

@mssola
Copy link
Collaborator

mssola commented Aug 26, 2015

Hi @deitch ! Thanks for all the info that you've provided. I was on vacation last week, so I've been catching up this week :) As you probably know, there's the PR #282 dealing with this (which is a WIP). I'll take a look at it asap and provide more feedback.

@deitch
Copy link
Author

deitch commented Aug 27, 2015

Thanks.

@flavio
Copy link
Member

flavio commented Sep 2, 2015

The code has been merged into master, closing the issue.

@flavio flavio closed this as completed Sep 2, 2015
@deitch
Copy link
Author

deitch commented Sep 2, 2015

Excellent!

Has it been pushed to the registry?

@flavio
Copy link
Member

flavio commented Sep 2, 2015

The "issue" was inside of Portus, not "distribution". Hence the fix has been pushed to portus' master branch.

@deitch
Copy link
Author

deitch commented Sep 2, 2015

Isn't there a suse/portus image on the docker hub?

@flavio
Copy link
Member

flavio commented Sep 2, 2015

Not yet. But you can build it from scratch using docker-compose

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

No branches or pull requests

3 participants