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

OIDC - Add support for "Authentication code" flow #3088

Closed
ssurovich opened this issue Aug 5, 2020 · 12 comments
Closed

OIDC - Add support for "Authentication code" flow #3088

ssurovich opened this issue Aug 5, 2020 · 12 comments
Assignees
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features.

Comments

@ssurovich
Copy link

Is your feature request related to a problem? Please describe.
Its a problem from a security side - assuming I understand how Kiali integrates with OIDC.
I was trying to use our OIDC provider, OpenUnison, and it failed to work. We did some tracing and discovered that Kiali wants the OIDC provider to send back the id_token in the response. The GET response is logged by any system that sees the traffic (ie: the K8s Ingress, the F5, etc...) - So if you can see the log, you have the K8s id_token.

Second - we use short lived id_tokens, and it appears that Kiali's OIDC implementation does not use refresh_tokens? With our timeout on the id_tokens, a user will need to re-authenticate every few minutes.

Describe the solution you'd like
A more secure method to access Kiali using OIDC with support for refresh_tokens.

Describe alternatives you've considered
My original thought is to use the auth_header method that the K8s Dashboard supports - This is an easy and secure method, using a reverse proxy that will send the auth_header from a link on our OIDC page.
OR
Change Kiali code to include a response_type=form_post in the auth request to a form post instead of a 302 redirect.

Additional context
I cant think of anything, if I can offer any information, please let me know.

If my understanding is way off base, let me know.

@ssurovich ssurovich added the enhancement This is the preferred way to describe new end-to-end features. label Aug 5, 2020
@israel-hdez
Copy link
Member

Hi, @ssurovich

AFAIK, the response_type=form_post is not part of the OpenID spec (probably it's an OAuth feature). At least, that's what I understand from section 3 of this document: https://openid.net/specs/openid-connect-core-1_0.html#Authentication. I wouldn't like to introduce features outside of the OpenID spec, so the only alternative I see is to use response_type=code which triggers the "Authorization code" flow, but to support that my understanding is that Kiali will need back-end storage (which does not have). If you have any idea/suggestion/though about how to implement the Authorization code flow without back-end storage (i.e, in a state-less way), please share your ideas!

The current Kiali OIDC implementation is using nonce and state codes to mitigate replay and CSRF attacks (see https://auth0.com/docs/api-auth/tutorials/nonce and https://auth0.com/docs/protocols/oauth2/oauth-state for some details). I know these are just "mitigations", but all systems logging the traffic shouldn't be able to use the logged credentials to gain access to Kiali -- Kiali should reject because of the codes.

About the refresh token support, well... most (if not all) OIDC providers don't provide refresh tokens with the Implicit Flow (which Kiali is using). So, we must first solve the issue about adding support for response_type=code before thinking in refresh tokens. I'll appreciate if you open a new issue for the refresh_token support to keep conversations separated.

@mlbiam
Copy link
Contributor

mlbiam commented Aug 6, 2020

AFAIK, the response_type=form_post is not part of the OpenID spec (probably it's an OAuth feature)

OpenID Connect is a superset of OAuth2. https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html is the specification for response_type=form_post. A note hear that the id_token profile is generally considered a bad security practice because it moves the id_token through the user's browser and is logged by reverse proxies. This mechanism will likely be deprecated in the next version of OIDC.

so the only alternative I see is to use response_type=code which triggers the "Authorization code" flow, but to support that my understanding is that Kiali will need back-end storage (which does not have). If you have any idea/suggestion/though about how to implement the Authorization code flow without back-end storage (i.e, in a state-less way), please share your ideas!

Do you have some way of encrypting cookies? If the backend storage isn't available then an encrypted cookie would work OK as well. A better implementation would be to support a reverse proxy's pass through authentication. similar to how the k8s dashboard works. Take the authorization header and pass it through to the API server. You could also support impersonation headers this way for deployments on managed clusters that don't support openid connect.

The current Kiali OIDC implementation is using nonce and state codes to mitigate replay and CSRF attacks (see https://auth0.com/docs/api-auth/tutorials/nonce and https://auth0.com/docs/protocols/oauth2/oauth-state for some details).

These do not mitigate the risk pointed out by @ssurovich as the API server does not use these mechanisms to validate id_token used in the Authorization header. A long lived token that is pulled from either the ingress log or the logs of another network layer can be used to make API calls to Kubernetes, even if the Kali wouldn't accept the token.

@ssurovich
Copy link
Author

Hi @israel-hdez -

On the id_token, our bigger concern was that id_token could be used to generate an API call to the K8s API - I havent tested it, but with the id_token, someone could issue a curl -H "Authorization: Bearer https:///xxxxxxxxxxxxx
I could be off base on this thought, its one of the main reasons I posted the issue in the first place.?

@israel-hdez
Copy link
Member

israel-hdez commented Aug 6, 2020

@ssurovich Your concern is valid but I don't see how form_post could help to close that issue (see my following comment). So, my suggestion is to somehow check how to support the "Authorization code" flow.

@israel-hdez
Copy link
Member

@mlbiam

OpenID Connect is a superset of OAuth2.

I partially agree. At the end, this depends a lot on the IdP.
What I have found, is that IdPs implement what is mentioned in the OIDC spec, but once you move away from it and start looking at the OAuth spec, support of OAuth spec is not guaranteed. Dex is a good example of an IdP implementing the response types of the OIDC Spec, but not the response types mentioned in the OAuth spec.

Anyway, if the concern is to avoid moving the id_token across the network, then response_type=form_post won't help. It would only move the id_token from the HTTP Location header to the HTTP request body. I'm not really sure if that would reduce the chances for the id_token to be logged, yet I'm sure the token will still travel across the network.

Do you have some way of encrypting cookies?

I think we could look into AES-GCM encryption.

A better implementation would be to support a reverse proxy's pass through authentication.

This has already been requested in #2216. If you think it's better, please up-vote and drop a comment on that issue :-)

The problem with the reverse proxy setup is that it doesn't work well with short-lived tokens (like in @ssurovich case). Once Kiali is loaded in the browser, if the session needs a refresh, Kiali will throw errors everywhere because it makes heavy use of Ajax which does not honor the redirection to the refresh endpoint of the proxy .We haven't found a way to catch the redirection on Ajax calls. There is only the workaround of manually refreshing the page on the browser, but this is a detrimental experience.

The current Kiali OIDC implementation is using nonce and state codes to mitigate replay and CSRF attacks

These do not mitigate the risk pointed out by @ssurovich as the API server does not use these mechanisms to validate id_token used in the Authorization header.

OK, I do agree with this.

@mlbiam
Copy link
Contributor

mlbiam commented Aug 7, 2020

I partially agree. At the end, this depends a lot on the IdP.
What I have found, is that IdPs implement what is mentioned in the OIDC spec, but once you move away from it and start looking at the OAuth spec, support of OAuth spec is not guaranteed. Dex is a good example of an IdP implementing the response types of the OIDC Spec, but not the response types mentioned in the OAuth spec.

That's fair. We stumbled across this issue because OpenUnison doesn't implement the implicit flow due to security issues like the ones pointed out in this issue.

Anyway, if the concern is to avoid moving the id_token across the network, then response_type=form_post won't help. It would only move the id_token from the HTTP Location header to the HTTP request body. I'm not really sure if that would reduce the chances for the id_token to be logged, yet I'm sure the token will still travel across the network.

I don't think the issue is so much that the token will move across the network with response_type=form_post so much as the id_token won't be logged in proxy server logs since it won't be a part of the URL's query string. It's one of the reasons why you submit a username and password via an HTTP POST instead of a GET. Its the same data but the user's password doesn't get logged in web server logs because it's not a part of the URL. It isn't as good as moving to the authorization code flow but it's better then what is there now and is a good short term option.

The problem with the reverse proxy setup is that it doesn't work well with short-lived tokens (like in @ssurovich case). Once Kiali is loaded in the browser, if the session needs a refresh, Kiali will throw errors everywhere because it makes heavy use of Ajax which does not honor the redirection to the refresh endpoint of the proxy .We haven't found a way to catch the redirection on Ajax calls. There is only the workaround of manually refreshing the page on the browser, but this is a detrimental experience.

Can you store an expiration in your session cookie? Then have a timer in the javascript that looks for that expiration to hit and does a refresh. You can also have a background thread that polls the backend to see if the session is still alive and if not, forces a reload. In the case of Oidc this is able to refresh the token on its own without forcing you to do refresh.

There are two big advantages to the reverse proxy model:

  1. token refresh isn't your problem anymore. Let the reverse proxy handle it. For instance OpenUnison manages the refresh process for the dashboard in our deployments. The dashboard doesn't get forced to refresh but we still use short lived tokens (1-2 minutes)
  2. Cookies are generally limited to 4096 bytes. It does not take long for an enterprise user, who's groups are represented by LDAP distinguished names and might have hundreds of groups, to go past that limit. Letting a reverse proxy inject the id_token as a header eliminates that issue.

@israel-hdez
Copy link
Member

I don't think the issue is so much that the token will move across the network with response_type=form_post so much as the id_token won't be logged in proxy server logs since it won't be a part of the URL's query string.

At the end, I think it depends a lot on the network. I mean, you did a mention of reverse proxies, and some proxies have the capabilities to inspect the bodies of the request (thinking of URL rewrite) and even parse POST params to do conditional load balancing. So, I think there is still a chance of the id_token to be recorded in logs even with form_post.

But I understand the concern. It's valid.

Can you store an expiration in your session cookie?

We are already doing that, but that's when the session is managed by Kiali.

The session expiration issue is for the reverse-auth-proxy case. When using the proxy, we don't own the session, so Kiali can't know when it expires. If the auth proxy triggers a redirection for refresh/re-auth, this is an issue with Kiali -- this is what ambassador gateway is doing. However, if the auth proxy refreshes the session in the background, I think that can work fine.

@israel-hdez
Copy link
Member

@ssurovich @mlbiam Personally, I would like to focus my efforts into supporting the OIDC "Authentication code" flow, which is (apparently) the most recommended.

However, I think you are leaning towards the reverse proxy model and, alternatively, the form_post mode. Kiali is open to contributions. @ssurovich In your main comment I read you considered changing the Kiali code. So, I'm wondering if you would like to contribute to add support for these (or one of these) models? I can provide assistance, if needed.

@ssurovich
Copy link
Author

@israel-hdez Thanks for the background and answers. You are correct that a post wouldnt help the in-transit data and it also wouldnt eliminate the logging concerns either - That was just my quick idea to attempt to run it on a development cluster I need to implement Kiali RBAC on.

The reverse proxy was my favorite one from the way Dashboard implements it - It's secure and works well, and while I would love to help the project, I just need to find time before I can look at contributing anything. I may have to use token mode for now, which isnt ideal, but it works for the multi-tenant clusters I have right now. Just has a management overhead that I was hoping to avoid.

@israel-hdez
Copy link
Member

Changing issue title to better reflect the ask about OIDC Code flow.

@israel-hdez israel-hdez changed the title OIDC Integration changes - Response wants the provider to include the id_token? OIDC - Add support for "Authentication code" flow Aug 10, 2020
@israel-hdez israel-hdez added the backlog Triaged Issue added to backlog label Aug 10, 2020
@israel-hdez
Copy link
Member

@ssurovich OK, I'll leave this issue for the "Authentication code" flow.
If you prefer the reverse proxy support, please up-vote and drop a comment on #2216.

@israel-hdez israel-hdez self-assigned this Sep 1, 2020
israel-hdez added a commit to israel-hdez/swscore that referenced this issue Sep 8, 2020
This initial implementation of the "authentication code" flow for OpenID authentication let's Kiali negotiate authentication with an extenal IdP provider. Kiali is acting as an "unauthenticated client" (i.e. no client secret is used).

This is using AES-GCM encryption to encode the cookie which prevents revealing the OpenId id_token to the network and to the browser.

Related to kiali#3088
israel-hdez added a commit to israel-hdez/swscore that referenced this issue Sep 11, 2020
israel-hdez added a commit that referenced this issue Sep 14, 2020
* Initial implementation of OpenId's auth code flow

This initial implementation of the "authentication code" flow for OpenID authentication let's Kiali negotiate authentication with an extenal IdP provider. Kiali is acting as an "unauthenticated client" (i.e. no client secret is used).

This is using AES-GCM encryption to encode the cookie which prevents revealing the OpenId id_token to the network and to the browser.

Related to #3088

* Refactor: cleanup and prepare to re-use some code

Related #3088

* Mazz feedback: Unique error messages

Co-authored-by: John Mazzitelli <mazz@redhat.com>

* Feedback: Jay & Mazz

* Feedback: Fix message

Co-authored-by: John Mazzitelli <mazz@redhat.com>
israel-hdez added a commit to israel-hdez/swscore that referenced this issue Sep 15, 2020
When OIDC client should authenticate, we need the client-secret provided/configured by/in the OIDC server. For this, we re-use our old/reserved "kiali" secret that was used in the past for the already removed login authentication strategy avoiding another set of changes in the operator.

This also:
* Fixes logout
* Fixes broken OIDC "implicit flow" (nonce cookie deleted early)

Related kiali#3088
israel-hdez added a commit to israel-hdez/swscore that referenced this issue Sep 15, 2020
When OIDC client should authenticate, we need the client-secret provided/configured by/in the OIDC server. For this, we re-use our old/reserved "kiali" secret that was used in the past for the already removed login authentication strategy avoiding another set of changes in the operator.

This also:
* Fixes logout
* Fixes broken OIDC "implicit flow" (nonce cookie deleted early)

Related kiali#3088
israel-hdez added a commit to israel-hdez/kiali.io that referenced this issue Sep 17, 2020
israel-hdez added a commit that referenced this issue Sep 17, 2020
…low) (#3215)

* Support for acting as authenticated OIDC client

When OIDC client should authenticate, we need the client-secret provided/configured by/in the OIDC server. For this, we re-use our old/reserved "kiali" secret that was used in the past for the already removed login authentication strategy avoiding another set of changes in the operator.

This also:
* Fixes logout
* Fixes broken OIDC "implicit flow" (nonce cookie deleted early)

Related #3088

* Unify oidc implicit flow code (deduplicate)

This commit reuses the functions added to openid_auth.go created for the authorization code flow. Most of that code is shared. It's just the order of execution that changes. So, this is a small rework of performOpenIdAuthentication function in authentication.go file to remove code duplication.
israel-hdez added a commit to kiali/kiali.io that referenced this issue Sep 18, 2020
…305)

* Add documentation related to the OpenId "autorization code" support

RelatedBack-end PR is kiali/kiali#3215

Related issue is kiali/kiali#3088

* Feedback: grammar
@israel-hdez
Copy link
Member

"Authorization code flow" has been implemented and going to be available in Kiali v1.24.

cfcosta pushed a commit to cfcosta/kiali that referenced this issue Oct 2, 2020
* Initial implementation of OpenId's auth code flow

This initial implementation of the "authentication code" flow for OpenID authentication let's Kiali negotiate authentication with an extenal IdP provider. Kiali is acting as an "unauthenticated client" (i.e. no client secret is used).

This is using AES-GCM encryption to encode the cookie which prevents revealing the OpenId id_token to the network and to the browser.

Related to kiali#3088

* Refactor: cleanup and prepare to re-use some code

Related kiali#3088

* Mazz feedback: Unique error messages

Co-authored-by: John Mazzitelli <mazz@redhat.com>

* Feedback: Jay & Mazz

* Feedback: Fix message

Co-authored-by: John Mazzitelli <mazz@redhat.com>
cfcosta pushed a commit to cfcosta/kiali that referenced this issue Oct 2, 2020
…low) (kiali#3215)

* Support for acting as authenticated OIDC client

When OIDC client should authenticate, we need the client-secret provided/configured by/in the OIDC server. For this, we re-use our old/reserved "kiali" secret that was used in the past for the already removed login authentication strategy avoiding another set of changes in the operator.

This also:
* Fixes logout
* Fixes broken OIDC "implicit flow" (nonce cookie deleted early)

Related kiali#3088

* Unify oidc implicit flow code (deduplicate)

This commit reuses the functions added to openid_auth.go created for the authorization code flow. Most of that code is shared. It's just the order of execution that changes. So, this is a small rework of performOpenIdAuthentication function in authentication.go file to remove code duplication.
mtho11 pushed a commit to mtho11/kiali.io that referenced this issue Nov 18, 2020
…iali#305)

* Add documentation related to the OpenId "autorization code" support

RelatedBack-end PR is kiali/kiali#3215

Related issue is kiali/kiali#3088

* Feedback: grammar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features.
Projects
None yet
Development

No branches or pull requests

3 participants