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

[WIP] PKI (two way TLS) auth support #1181

Closed
wants to merge 1 commit into from
Closed

Conversation

cstrahan
Copy link

@cstrahan cstrahan commented Feb 1, 2018

NOTE: This PR is meant to be more for discussion than for merging in this code as it stands.


At @DecipherNow, we have many clients that have a sizable investment in two way SSL/TLS. Rather than having every web app authenticate via their PKI, there's an interest in moving to a setup where they can auth via PKI once to receive an OAuth/OIDC token, and then use that token to authenticate with each web app, using an user_dn claim to pass along the user's Distinguished Name (taken from the cert their browser presented).

The code presented in this PR represents the simplest approach I could find to make this possible, so there are some obvious changes (in the works) e.g. to make corresponding changes to the config options, etc. I'm currently abusing the PasswordConnector, as it was easier to implement this while making minimal changes to the existing code, and allowing the (existing) types to guide the implementation.

With those warts aside, there's still the open question: what to do about custom claims (e.g. user_dn)?
Adding the user_dn claim required touching a number of structs and tweaking the tables/queries In the SQL backend (storage/sql/crud.go).

TL;DR: Would there be interest in an architectural change that would allow for each of the backends supporting any arbitrary set of (potentially user defined) claims? If so, @DecipherNow would like to allocate resources to make the required changes.

If that's something that we could upstream, I'd love to chat about how to proceed design-wise - whether here, IRC, Slack, or whatever works best for you.

@ericchiang
Copy link
Contributor

This would require dex to hold a database of public keys, do TLS termination, etc. In addition, PKI means totally different things depending on the organization or project.

"Authentication via TLS client auth" is sufficiently vague that I don't see dex adding this. If there are specific protocols like token binding that you'd like dex to support, that'd be a more reasonable feature request, but we're not looking to invent authentication schemes.

@ericchiang ericchiang closed this Feb 1, 2018
@cstrahan
Copy link
Author

cstrahan commented Feb 2, 2018

If there are specific protocols like token binding that you'd like dex to support, that'd be a more reasonable feature request, but we're not looking to invent authentication schemes.

@ericchiang That's fair enough.

However, my main point was "Dex might benefit from supporting a list of (potentially user definable) claims rather than just some predefined, hard coded list", not that Dex might want to add a new authentication scheme. It probably doesn't help that I abused the PR system to provide an illustrative diff (if github provides something more appropriate for this purpose, I'm unaware of it); I've opened this issue to better discuss the change I'm interested in making in Dex: #1182

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

Successfully merging this pull request may close these issues.

None yet

2 participants