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

SAML logout/session renewal bugs #4595

Closed
2 tasks done
gdiazlo opened this issue Sep 27, 2022 · 9 comments
Closed
2 tasks done

SAML logout/session renewal bugs #4595

gdiazlo opened this issue Sep 27, 2022 · 9 comments
Assignees
Labels

Comments

@gdiazlo
Copy link
Member

gdiazlo commented Sep 27, 2022

Wazuh Elastic Rev Security
4..x - - wazuh-dashboards

Description

The SAML integration is failing in wazuh-dashboards when a user tries to log off or when the SAML IDP sends a logout message.

Preconditions

Have a SAML IDP in place and Wazuh configured to use it as an auth mechanism

Steps to reproduce

Scenario 1: The user wants to log off

  1. Navigate to 'the login page and log in
  2. Click on the logout menu and log out

Scenario 2. The user is logged off by IDP

  1. Login into the application using the SAML IDP
  2. Wait for automatic log out from the IDP

Expected Result

  1. Log out process completes and the login page shows again

Actual Result

  1. Application breaks and log out does not get executed

Additional context

We need to design a workaround until this issue is fixed in the base platform.

Related issues
We have discovered two bugs:

  • The log-out button does not work when a SAML integration is configured because the URL changes and the Wazuh app does not.
  • The async requests done through the app do not handle a session renewal. We need to create a service to manage all the requests made by the plugin so we can check whether we're in the middle of a session renovation or not. If we're in the middle of a session renovation, we must avoid making requests until the renewal is finished, or we can break the process and the user will receive an invalid request id failure.

OpenSearch issues:

@gdiazlo gdiazlo added the type/bug Bug issue label Sep 27, 2022
@Tostti
Copy link
Member

Tostti commented Sep 27, 2022

I've been working on this issue.
After applying the correct configuration, it fails with a 401 error, mentioning an error obtaining the token. After some investigation, I think that that specific problem may be related to the IDP used.
I will test the configuration with different IDPs to see if the same behavior occurs

@Tostti
Copy link
Member

Tostti commented Sep 29, 2022

Behavior seen using OpenSearch 2.3

After applying the proper configurations, this is what can be expected from the SAML integration. The tests were done using Keycloak IDP.

Scenario 1: The user wants to log off

If the user wants to log off, it can click on the Log out button. It will close the Dashboard and the IDP sessions, and will redirect the user to the login page.
To make sure that this works, the IDP should have a Logout Service Redirect Binding URL configured, pointing to the login URL.

Grabacion.de.pantalla.desde.29-09-22.15.18.29.mov

Scenario 2. The user is logged off by IDP

OpenSearch doesn't currently support backend logout, and doesn't verify the validity of the session on every transaction. That means that even if the session is ended by the IDP, the user will still be able to navigate and use the dashboard without any limitations.
In this case, the session will only be closed after 1h of inactivity, due to the default session time of the OpenSearch dashboard

Possible workarounds

The default session time is 1h. However, it can be manually changed using the opensearch_security.session.ttl setting on opensearch_dashboards.yml, setting a value in milliseconds. I.e.: opensearch_security.session.ttl: 300000 will make the session last 5 minutes.
If the user does not interact with the dashboard for more time than the session's, upon interaction the dashboard will redirect to the login page again, and if the IDP session is still valid, will automatically log in and redirect to the dashboard.

The cons of this approach are the next:

  • If the user interacts with the dashboard, the issue will still be the same, as the session time is renewed on every interaction. If the IDP session expires, the user will still be able to interact with the dashboard unless the dashboard's session expires.
  • The user will always be redirected to the home page, regardless of the previous page or the page that it wanted to see on the interaction

@Tostti
Copy link
Member

Tostti commented Oct 5, 2022

Tested OpenDistro Environments, changing the different available settings. Analyzing the behaviour on different situations with different configurations, we got the following conclusions:

  • OpenDistro does not validate the session against the IDP on every petition, and the users are available to interact even if the session is closed on the IDP.
  • Setting the Session Idle Time on the IDP does not affect the functionality of OpenDistro. Just sets internally on the IDP the time to wait until the session is automatically closed if is not invalidated.
  • Setting the Session Max Time on the IDP changes the behaviour of OpenDistro. After that time (counting from the login), OpenDistro will redirect to the login page automatically, regardless if the user was idle or not.
  • Setting the opendistro_security.session.ttl on Opendistro does not do anything if SAML is enabled. Users are still able to navigate even after that time.
  • Setting the opendistro_security.cookie.ttl on Opendistro changes its behaviour. If the user is idle for more than that time (by default is 1h), it will automatically redirect to the login page, even if the user doesn't interact.

@Tostti Tostti linked a pull request Oct 11, 2022 that will close this issue
6 tasks
@Tostti
Copy link
Member

Tostti commented Oct 12, 2022

Tested OpenSearch Environments, changing the different available settings. Analyzing the behavior on different situations with different configurations, we got the following conclusions:

  • OpenSearch does not validate the session against the IDP on every petition, and the users are available to interact even if the session is closed on the IDP.
  • Setting the Session Idle Time on the IDP does not affect the functionality of OpenSearch. Just sets internally on the IDP the time to wait until the session is automatically closed if is not invalidated.
  • Setting the Session Max Time on the IDP changes the behavior of OpenSearch. After that time (counting on the initial login), OpenSearch will show a 401 unauthorized error on any interaction, regardless if the user was idle or not.
  • Setting the opensearch_security.cookie.ttl on OpenSearch does not do anything if SAML is enabled. Users are still able to navigate even after that time.
  • Setting the opensearch_security.session.ttl on OpenSearch changes its behavior. If the user is idle for more than that time (by default is 1h), it will be redirected to the login page on any interaction.

Additional discoveries

  • If the user is inside the Wazuh Plugin when the session closes due to the session.ttl time, it will receive a 400 error that says invalid requestID after he logs in (or the IDP revalidates the session). This issue doesn't happen on OpenDistro. Further investigation is needed on this.

Notes about this
The login / revalidation process is the next:

  • User tries to go to a page, there is a GET request to that site. That request sets the security_authentication cookie to an empty value, expired in 1970.
  • The user is then redirected to /auth/saml/login, which contains a set-cookie parameter in the response with a new valid cookie
  • The user is redirected to the IDP, that after the login makes a POST request to /_opendistro/_security/saml/acs with the SAML response. That petition should contain the previous cookie in the Request header, and also will provide a new one in the response header with a set-cookie

If the first step of this process occurs when the user is inside the Wazuh plugin, the last step will not contain any cookie on the request header. That will cause not having any set-cookie and the aforementioned invalid requestID error.

Analyzing the request sent to /auth/saml/login, the only difference between it happening inside Wazuh plugin or another plugin is a Referer, which is set to https://localhost:5601/app/wazuh in case of Wazuh, or the plugin URL in case of another plugin. However, the error doesn't happen in other plugins.

The code for the routes /auth/saml/login and /_opendistro/_security/saml/acs is on plugins/security-dashboards-plugin/server/auth/types/saml/routes.ts

Further troubleshooting concluded that the reason for the cookie to not be present on the last request is that, for some reason, there are requests in between the login and the ACS that invalidates the cookie. This behavior is only observer on the Wazuh plugin. It isn't always the same request that causes the error, however, there are lots of requests done each time the Wazuh plugin is accessed.

image

@Desvelao
Copy link
Member

Research

I was researching the workflow to authenticate the user and could replicate the problem with the suggestions of @Tostti . Moreover, I was reviewing the source code of the security plugin with @asteriscos and @yenienserrano.

@Tostti
Copy link
Member

Tostti commented Oct 20, 2022

Analysis of the issue

The issue happens in OpenDistro and also in Opensearch.

It is not possible to validate the cookie from the code, as it is http-only. Therefore, we cannot validate explicitly that before making requests.

I have been investigating the origin of the requests when the user navigates to or inside the plugin. There are different origins, based on the required window. These are the ones found at the time:

Analyzing other plugins, usually there are not any request made on the mounting of the component. Additionally, the requests made to access the different pages are usually done inside of the views.

@Desvelao
Copy link
Member

Desvelao commented Oct 21, 2022

Update

I was researching and found that the security plugin adds an interceptor to request service core.http. This is the reason that other plugins don't have the error in the login workflow. The requests with status code 401 force a page refresh that triggers the login process. See https://github.com/opensearch-project/security-dashboards-plugin/blob/2.3.0.0/public/plugin.ts#L151-L168. The Wazuh plugin doesn't use this request service.

For X-Pack could have a similar behavior: https://github.com/elastic/kibana/blob/v7.17.5/x-pack/plugins/security/public/plugin.tsx#L84

Solution

Centralize the requests done to a unique service used by the plugin and uses the request service served by the platform, that contains the interceptor.

List of services:

- GeneriqRequest (react-service)
- WzRequest
- axios 

# Review if these use the `core.http` 
- getSavedObjects().client
- getDataPlugin().indexPatterns.get
- getDataPlugin().indexPatterns
- getDataPlugin().search
- getHttp().addLoadingCountSource review

# Review the requests done by:
- Discover

@Desvelao
Copy link
Member

Desvelao commented Oct 24, 2022

Research

The core.http frontend service uses the Fetch browser API to do the requests, see:

The proposed solution is the plugin uses a request service that wraps the core.http frontend service, of this way, the requests contain the interceptor that triggers the login flow. All the requests done by the plugin should use this service.

There are some things in the current implementation that we should resolve how to do it:

Tasks

  • Check the services used to do the requests if these wrap the core.http service

data.indexpatterns: https://github.com/elastic/kibana/blob/v7.10.2/src/plugins/data/public/index_patterns/index_patterns/index_patterns_api_client.ts#L34-L37

  • Implement the new service
    • Based on core.http
    • Implement the request timeout feature
    • Intercept the error request due to unauthorized in the Wazuh API
    • Replace the services used to do request with the new centralized service
      • WzRequest
      • GenericRequest
      • axios
    • Adapt each request response

@gdiazlo gdiazlo changed the title SAML logout crashes SAML logout/session renewal bugs Oct 31, 2022
@gdiazlo gdiazlo removed a link to a pull request Oct 31, 2022
6 tasks
@Tostti
Copy link
Member

Tostti commented Nov 17, 2022

All related issues were successfully closed

@Tostti Tostti closed this as completed Nov 17, 2022
@havidarou havidarou added this to the Release 4.4.0 milestone Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

No branches or pull requests

6 participants