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

Extend Docker authorization with TLS user information #21556

Merged
merged 1 commit into from Mar 28, 2016

Conversation

liron-l
Copy link
Contributor

@liron-l liron-l commented Mar 27, 2016

- What I did
For TLS connections, add client subject name to authorization request
- How to verify it
Add integration test, validated manually

Currently Docker authorization framework does not use any user
information, which already available in the Docker context for TLS
connection.
The purpose of this CR is to complete the existing authz work by adding
the basic client certificate details (SUBJECT_NAME) and authentication
method (TLS) to the authz request.

We think this should be the default behavior when no extended
authorization module is specified (currently WIP under #20883).

Signed-off-by: Liron Levin liron@twistlock.com

Currently Docker authorization framework does not use any user
information, which already available in the Docker context for TLS
connection.
The purpose of this CR is to complete the existing authz work by adding
the basic client certificate details (SUBJECT_NAME) and authentication
method (TLS) to the authz request.

We think this should be the default behavior when no extended
authorization module is specified (currently WIP under moby#20883).

Signed-off-by: Liron Levin <liron@twistlock.com>
@liron-l
Copy link
Contributor Author

liron-l commented Mar 27, 2016

@jfrazelle , @thaJeztah , @NathanMcCauley , @diogomonica, @nalind
I will appreciate your input on this change.

@jessfraz
Copy link
Contributor

LGTM

@calavera
Copy link
Contributor

LGTM

@calavera calavera merged commit dd757de into moby:master Mar 28, 2016
@thaJeztah thaJeztah added this to the 1.12.0 milestone Mar 28, 2016
@thaJeztah
Copy link
Member

Does this need changes in the authorization-plugin docs? https://github.com/docker/docker/blob/master/docs/extend/plugins_authorization.md

@diogomonica
Copy link
Contributor

@liron-l what about adding the whole SubjectDN from the cert? CN feels unnecessarily restrictive.

@liron-l
Copy link
Contributor Author

liron-l commented Mar 29, 2016

Thanks all, @thaJeztah I will update the authorization documentation in another PR.
@diogomonica do you mean taking all the values under DNSNames?

@liron-l liron-l deleted the basic_authn_client_cert branch March 29, 2016 17:56
@diogomonica
Copy link
Contributor

@liron-l I mean taking the whole Subject pkix.Name

@liron-l
Copy link
Contributor Author

liron-l commented Mar 30, 2016

@diogomonica you mean running asn1.Marshal(name)?
What are you expecting to be the value of the user in the request?

liron-l pushed a commit to twistlock/docker that referenced this pull request Mar 30, 2016
…ation docs

Following the discussion in moby#21556, adding a short description of the
default user authentication mechanism (without requiring authentication
plugins)
Signed-off-by: Liron Levin <liron@twistlock.com>
@diogomonica
Copy link
Contributor

@liron-l I guess I would like to have a user that is just more than a string.

@liron-l
Copy link
Contributor Author

liron-l commented Apr 6, 2016

I agree @diogomonica, I think it is covered by the authentication plugin suggested in #20883 (which passes the full client certificate to the plugin).

@eungjun-yi
Copy link
Contributor

Is it possible to make this feature works with swarm? Currently it seems that CN of swarm manager, instead of CN of docker client, goes to the each swarm node.

Should I wait for docker-archive/classicswarm#1366?

@doronp
Copy link
Contributor

doronp commented Apr 14, 2016

@npcode Yes, And in general I plan to align Swarm support in AtuhZ+AuthN with Dockers. When and what is merged is not solely depends on me though.

@eungjun-yi
Copy link
Contributor

Thank you for your effort, @doronp. I seriously need swarm authz+authn.

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

Successfully merging this pull request may close these issues.

None yet

8 participants