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

Absolute Session Timeout #18566

Closed
kobelb opened this issue Apr 25, 2018 · 14 comments · Fixed by #49855
Closed

Absolute Session Timeout #18566

kobelb opened this issue Apr 25, 2018 · 14 comments · Fixed by #49855
Assignees
Labels
enhancement New value added to drive a business result Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@kobelb
Copy link
Contributor

kobelb commented Apr 25, 2018

Currently, we have the sessionTimeout that is used as the idle timeout for Kibana sessions, but we don't have a mechanism to enforce an absolute expiration.

Enforcing an absolute expiration minimizes the amount of time an attacker could use a hijacked session.

https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Session_Management_Cheat_Sheet.md#absolute-timeout

@kobelb kobelb added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! enhancement New value added to drive a business result labels Apr 25, 2018
@kobelb kobelb changed the title Absolute Session Expiration Absolute Session Timeout Apr 25, 2018
@jportner
Copy link
Contributor

I'm thinking it would be less effort to tackle this and #36528 in a single enhancement.
Anyone else have thoughts on the matter?

@legrego
Copy link
Member

legrego commented Oct 16, 2019

I'm thinking it would be less effort to tackle this and #36528 in a single enhancement.
Anyone else have thoughts on the matter?

I haven't done any real analysis on these issues yet, but my gut feeling says that it'd be easier to tackle absolute session timeouts via a kibana.yml setting first, before exploring what role-based session timeouts might look like. We don't have a precedent for role-based settings like this in Kibana yet (nor a UI for such a setting), so to me, #36528 would be better tackled as a separate initiative.

Role-based session timeouts also means that any user with the manage_security cluster privilege would be able to influence the session timeouts for Kibana users. This probably isn't a bad thing necessarily, but we currently offer "protection" against the timeout value changing by requiring it to be set within kibana.yml, which requires physical write access to the configuration file.

I'm curious to hear your thoughts though, as I'm likely missing something!

@jportner
Copy link
Contributor

Yeah, I mentioned that before looking at the code or putting too much thought into what a solution for per-role session timeouts would look like.
And, starting to think through the design for that, we would still need a site-wide config for absolute session timeout (session "lifespan") to fall back on anyway.

@jportner jportner self-assigned this Oct 17, 2019
@jportner
Copy link
Contributor

OK, I propose the following:

  1. Add a new config option, xpack.security.sessionLifespan -- this will denote the total amount of time (in milliseconds) that a session may live
  2. If lifespan is null, and timeout is null, sessions last forever
  3. If lifespan is null, and timeout is not null, sessions may be renewed indefinitely
  4. If lifespan is not null, sessions will end at the prescribed time, regardless of timeout

So, if lifespan is not set, sessions will continue to function as they do today.
This control will basically just supersede the existing session timeout.
When a session is first created, the lifespan will be set and it should never change.

Design considerations:

  • The existing pending session timeout notification will need to be reworked -- we don't want to ask a user to renew his session and then kick him out, that would make for a poor UX
  • Will need a different logout message as well to indicate when a session lifespan has been exceeded

Anything to add @kobelb @legrego ?

@kobelb
Copy link
Contributor Author

kobelb commented Oct 18, 2019

Looks great to me!

@legrego
Copy link
Member

legrego commented Oct 18, 2019

++ I think this makes sense too. I had to read the proposal a couple of times, as I kept conflating the existing timeout property with the new lifespan property. In case there are others like me, what do you think about a slight rename?

Currently, we have:

  • xpack.security.sessionTimeout
  • xpack.security.sessionLifespan

What do you two think about something like:

  • xpack.security.session.idleTimeout
  • xpack.security.session.lifespan

The legacy platform has config deprecations that can be leveraged to automatically perform the rename from xpack.security.sessionTimeout => xpack.security.session.idleTimeout for existing Kibana users who upgrade. Here is what security does currently to warn about an unused property:

deprecations: function ({ unused }) {
return [
unused('authorization.legacyFallback.enabled'),
];
},

Adding the following would perform the rename:

rename('sessionTimeout', 'session.idleTimeout')

@jportner
Copy link
Contributor

Agree the naming is currently less than ideal. I was under the assumption it would be difficult / we would lean away from renaming existing config properties. I think renaming them would be better.

@jportner
Copy link
Contributor

So, as this functions today -- when the user receives the session expiring notification, this accomplishes two things:

  1. Provides advance notice before interrupting his work
  2. Facilitates an easy way for him to extend his session (and avoid an interrupted workflow) with a button click

Ideally I'd like to accomplish the same thing when a session's lifespan is exceeded. Point 1 is easy, just a small change to the text of the notification. However, I don't see a clean way to accomplish point 2. It would be great to show a login form in the re-auth prompt, but that wouldn't work for all authentication realms (e.g., SSO).

I don't love this, but I am thinking we can change the color of the notification modal to something more irritating, and warn the user to save his work because he is about to be logged out. Unless we agree on a "re-auth" login modal for a subset of compatible authentication realms?

@kobelb
Copy link
Contributor Author

kobelb commented Oct 18, 2019

I don't love this, but I am thinking we can change the color of the notification modal to something more irritating, and warn the user to save his work because he is about to be logged out.

I like this approach. I think waiting to add a "re-auth" login modal until later is a good call, as we'd have to figure out how to make this work with our SSO integrations (it likely wouldn't because they all do redirects).

@legrego
Copy link
Member

legrego commented Oct 18, 2019

I'm also on board with this -- most Kibana installations won't take advantage of this feature (we aren't enabling a lifespan by default, right?), and those that are will likely be accustomed to other systems behaving in a similar way.

As an aside, we need to confirm whether we want to enable this setting for cloud (ESS) installations. The current sessionTimeout setting is permitted for cloud, so I assume that we'd want lifespan enabled as well. I can work with you on this @jportner when the time comes so you know what to do.

@jportner
Copy link
Contributor

Well, if I had my druthers, it would be enabled by default..! But that's another conversation I suppose.

OK, I'll be in touch regarding ESS installations.

Digging into this a bit more, it looks like the UX changes will be somewhat non-trivial. The current idle timeout notification isn't based on the user's actual session info. I discovered and opened two new bugs, see #48858 and #48859.

I think we need to change this notification so it has actual context of the user's session expiration. With the addition of session lifetimes, we will no longer be able to "estimate" the expiration based on the session.idleTimeout config value; we will need to expose the user's session expiration to the browser. I can see two ways of doing this:

  1. When the user authenticates, in addition to setting the "sid" cookie, set another "sidinfo" cookie that is not HttpOnly, which contains non-sensitive session metadata (current expiration, max lifetime). Then JS in the browser can read this cookie before displaying a timeout notification, and act accordingly. This will also allow JS in the browser to correctly schedule a lifespan expiration notification upon each new page load.
  2. Create a new REST API route which exposes the same non-sensitive session metadata. Then some JS in the browser can call this route and store it (perhaps in the browser's "sessionStorage").

I think approach number 1 is simpler, number 2 is a bit cleaner but probably unnecessary. I'd lean towards number 1. Thoughts?

@legrego
Copy link
Member

legrego commented Oct 22, 2019

I think we need to change this notification so it has actual context of the user's session expiration.

Yeah, I vaguely remember talking about a related issue with Brandon not too long ago (#18751). We've historically counted API calls as "user activity" for the purpose of session expiration, but since Kibana is a collection of massive SPAs, it's possible that a user could be doing meaningful work without making server calls. I think it'd be interesting to explore a /sessionInfo?extend=${boolean} endpoint (or similar) which we could call when we detect client-side activity, and that call could serve two purposes:

  1. Optionally extend the user's session
  2. Return this session metadata to inform the UI about the current user's session, so that we can properly warn about pending an expiration.

Taking a step back though, we can keep this as two separate issues -- a simple /sessionInfo endpoint without the ability to extend would be useful for solving this issue, and we can add the session extension work in a followup PR once we have a better idea of what "client-side activity" really means, if it's even something we want to do.

So I guess that's my long-winded way of saying I prefer your second option, but could be convinced otherwise.


edit
Another reason I like option 2: The current client-side timer approach doesn't account for multiple tabs either. Each tab will have its own session timer, and the oldest tab will effectively sign you out of all tabs.

@kobelb
Copy link
Contributor Author

kobelb commented Oct 22, 2019

To potentially further complicate matters, I have a PR open to move the "idle session timeouts" client-side to the new platform: #39477

@jportner
Copy link
Contributor

Well, after thinking on it some more, the cookie approach is somewhat problematic because of customer security audits. Having a non-HttpOnly cookie, which an end-user could edit to influence the session timeout, could raise some eyebrows in customer security audits. Even though we could make the argument that it's not a real concern, it's probably best to avoid opening that can of worms entirely.

I'll work with the /sessionInfo endpoint approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants