-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
client certificate auth via common name #3916
Conversation
refactor http basic auth code to combine basic auth extraction and validation
introduce client certificate authentication using certificate cn.
Also need to consider how this will interact with proxy, which terminates the tls connection with the client and makes its own connection using different certs to the server. |
Can you explain this further? I don't follow. Otherwise this seems good. |
@philips sure. Here I'm using the tls sessions's If, for example, we want to verify a specific key usage to allow client auth by common name to continue, we'd have to verify the certificates again in this block using the new Since etcd is already accepting options at the server level to determine client certificate trust, I thought it made sense to piggy back on that rather than accepting (yet another) set of certificate validation flags. |
@philips What are your thoughts on proxy authentication? right now this will not pass the user's certificate from the proxy back to the etcd cluster, so the proxy will be authenticated, not the user. Maybe that's acceptable if you assume etcd-proxy as a single tenant proxy, but with multiple consumers of a proxy they are all effectively sharing credentials. |
Is there a reason this never got merged? etcd authz is basically broken right now since most applications can't tolerate the 100ms+ latency hit per request. This would very easily solve that issue and enable people to actually use etcd authz. |
@roboll @ghodss @xiang90 the concept seems good to me. I think it can be merged after rebasing. The rebasing would be big but straightforward. And I'm not sure how the problem of proxy is serious. Just making this feature and proxy mutually exclusive functionalities might be enough, I think. IIUC, v2 proxy is only for easy discovery so giving up the proxy would be not so serious for users who want this feature. |
Great to hear, @mitake. Is there any difference in how etcdv2 and etcdv3 handle authentication? |
@ghodss yes, there is a big difference. For example, bcrypt password checking is required only when a client connects to servers. So I believe the 100ms+ latency per request can be avoided. |
Got it. In any case it would be great to avoid it altogether and enable us to not have to distribute passwords either. |
@ghodss hashed password is replicated to every node of the cluster. Is this problem for you? It is required for enabling authentication on all nodes. |
That's fine - I meant more distributing the passwords to the etcd clients, and relying on common name instead. |
@ghodss ah I see. v3 authentication doesn't support the auth based on common name yet. It would be worth to consider. |
@gyuho Can you try to cherry pick this patch set and resolve the conflicts? |
Closing in preference to #5991. |
is there any document on how to use this feature? |
@qiujian16 I'll add docs related to auth features. The issue is here: #7251 |
This is an initial proof of concept for implementing authentication using the common name on client certificates, as opposed to the current scheme which requires client certificates in conjunction with http basic authentication.
This is similar to the way kubernetes implements client authentication, using the common name on client certificates as a username.
There are two commits, the first is a small refactor to centralize the existing basic auth code, and the second is an initial implementation of pulling the client username from the certificate.
The approach looks like this:
Authorization
header exists on the request, do not check the certificate. proceed normally and check credentials.Authorization
header is missing, attempt to extract the common name from the presented certificate and authenticate as the specified user (even if the user account specifies a password, the client certificate will bypass that and authenticate)Wanted to get a bit of review and see if this was something you'd be interested in merging, if so, some considerations before merging..