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

Multiple cookies and infinite redirects #36825

Closed
kobelb opened this issue May 21, 2019 · 5 comments · Fixed by #50452
Closed

Multiple cookies and infinite redirects #36825

kobelb opened this issue May 21, 2019 · 5 comments · Fixed by #50452
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects

Comments

@kobelb
Copy link
Contributor

kobelb commented May 21, 2019

Problem

We currently have an infinite redirect when Kibana goes from being hosted on no base-path to being hosted on a base-path. We end up with multiple sid cookies, one with the path set to /${basePath} and one with the path set to /. When we see this, we'll respond with a 401 and try to clear the cookie at /${basePath} but we leave behind the one at /. The user logs back in, and the same situation repeats itself, it's glorious.

A similar situation occurs when we go from hosting Kibana at https to being hosted at http. We end up with multiple sid cookies being set, one with the secure flag set and one without the secure flag being set. A similar situation to the above then manifests itself.

Potential Solution

What if we were to change the name of the cookie which we use for the session based on whether it was hosted at a basePath and secure? Something like sid-${basePath ? '0' : '1' }-${secure ? '0' : '1'} with a better scheme. That way we wouldn't be trying to use cookies for auth which we can't use.

@kobelb kobelb added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label May 21, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security

@kobelb kobelb added the discuss label May 21, 2019
@legrego
Copy link
Member

legrego commented May 21, 2019

I'm not opposed to a suffix on the cookie name. For the sake of discussion, I'll pose another potential solution:

What if we were to encode these attributes as part of the session data? When we see multiple sessions on a single request, the server should be smart enough to determine which one of them satisfies the server's current configuration.

I know it's not part of our public API of any sort, but I'm worried that users with custom setups will expect cookies to be a specific name that they've already configured via kibana.yml. Upgrading Kibana would in turn break their setups, and they'd have to know what the expected cookie suffix is going to be. For a contrived example, a reverse-proxy might be configured to allow specific cookies through, while denying others.

@kobelb
Copy link
Contributor Author

kobelb commented May 21, 2019

What if we were to encode these attributes as part of the session data? When we see multiple sessions on a single request, the server should be smart enough to determine which one of them satisfies the server's current configuration.

I like it better, I can't think of any downsides to this approach either. We'll likely have to ensure it works properly with all of the various scenarios of http -> https, https -> http, etc.

@azasypkin
Copy link
Member

What if we were to encode these attributes as part of the session data? When we see multiple sessions on a single request, the server should be smart enough to determine which one of them satisfies the server's current configuration.

++, also it's kind of in line with the provider part of the session data (e.g. we already analyze provider and clear cookie if it isn't available anymore for some reason).

@kobelb
Copy link
Contributor Author

kobelb commented May 30, 2019

Removing the discuss tag since it seems we have an approach we should pursue

@kobelb kobelb added bug Fixes for quality problems that affect the customer experience and removed discuss labels May 30, 2019
@kobelb kobelb changed the title [DISCUSS] Multiple cookies and infinite redirects Multiple cookies and infinite redirects May 30, 2019
@kobelb kobelb added this to Scheduled in Security Oct 21, 2019
@jportner jportner self-assigned this Oct 21, 2019
@jportner jportner moved this from Scheduled to In Progress in Security Nov 13, 2019
@jportner jportner moved this from In Progress to Done in Security Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants