-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Implement Server-Side sessions #68117
Conversation
d6c4454
to
def5820
Compare
3ab2b0e
to
3f155d4
Compare
@legrego @jportner I believe PR is at the stage when we can play with it and discuss the implementation. In #68117 (comment) I tried to explain all the decisions I've made so far and I'd really appreciate if you can take a look at the questions I left for you whenever you have time. I'll try to approach you next week for the quick sync to handle any concerns you may have 🙂 P.S. PR doesn't include any tests yet, I haven't performed performance/concurrency tests as well, but it includes the fix for #53478 (to have at least one real benefit from this work 🙂) Thanks! |
Is my understanding correct that this doesn't use saved-objects to access the documents in the |
Yes, that's correct, but not set in stone yet. My initial reasoning behind having a dedicated index was to have a full control over index (e.g. to not accidentally expose session objects to other parts of Kibana that use SO client heavily even if it's marked as It's all theoretical though, happy to discuss alternatives, pros and cons (here or over Zoom)! |
For at least the remainder of 7.x, and potentially longer, we'll probably want to take the current tenant into account ( |
That's a good point! I'll make sure we take this into account. |
I think we should take the time to regenerate these for at least the username change. Otherwise, it's possible to hijack existing sessions. For example, if Now, the attacker is authenticated as whoever they just authenticated as, but more importantly, Alice's existing session cookie is no longer associated with |
Well, I'd say it's a risk, but it doesn't seem like a new one. I believe if currently you can access someone's else cookie you most likely can replace it with your own too, in one way or another. That would lead to a similar state. But irrespective to whether it's new or existing risk we have a chance to improve the state of things and it's a good practice anyway. I'll see how that would look like. |
Have you considered storing the encryption key for the server-side session in the client-side cookie? This way, if an attacker was to compromise the |
We mitigate this risk via storing AAD (random 256 bit string) only in the client cookie (see |
Nope! That seems completely reasonable. I forgot the AAD would prevent the decryption entirely, and initially thought it was acting more like a checksum, but that's wrong... |
40182e2
to
eddc712
Compare
@@ -78,8 +78,13 @@ const loginViaEnvironmentCredentials = () => { | |||
// programmatically authenticate without interacting with the Kibana login page | |||
cy.request({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @elastic/siem, can anyone please let me know what this code does? Is it only for tests or you use this for anything else? We're changing our internal login endpoint and I want to make sure I properly adjusted your code that depends on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct @azasypkin, this is just for our e2e cypress tests. These tests run as part of CI, so we should see a failure then if there are any issues. Thanks for the ping! 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct @azasypkin, this is just for our e2e cypress tests. These tests run as part of CI, so we should see a failure then if there are any issues. Thanks for the ping! slightly_smiling_face
Thanks for confirming! Security solution Cypress Tests
suite is green, so we should be good.
77c68c1
to
799ce7c
Compare
@@ -59,8 +59,6 @@ For more information, refer to <<authentication-security-settings, authenticatio | |||
|
|||
To successfully log in to {kib}, basic authentication requires a username and password. Basic authentication is enabled by default, and is based on the Native, LDAP, or Active Directory security realm that is provided by {es}. The basic authentication provider uses a {kib} provided login form, and supports authentication using the `Authorization` request header `Basic` scheme. | |||
|
|||
The session cookies that are issued by the basic authentication provider are stateless. Therefore, logging out of {kib} when using the basic authentication provider clears the session cookies from the browser, but does not invalidate the session cookie for reuse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: removed this cookie-related sections, I think they don't make any sense now - it's should be assumed by default that if user logs out session cannot be reused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
===== Login user interface settings | ||
|
||
[cols="2*<"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
===== Login user interface settings | |
[cols="2*<"] | |
===== Login user interface settings | |
You can configure the following settings in the `kibana.yml` file. | |
[cols="2*<"] |
[TIP] | ||
============ | ||
The format is a string of `<count>[ms|s|m|h|d|w|M|Y]` | ||
(e.g. '70ms', '5s', '3d', '1Y'). | ||
The format is a string of `<count>[ms\|s\|m\|h\|d\|w\|M\|Y]` (e.g. '70ms', '5s', '3d', '1Y'). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how you modified this tip for cleanupInterval
to provide more realistic values. We should do the same here 😁
The format is a string of `<count>[ms\|s\|m\|h\|d\|w\|M\|Y]` (e.g. '70ms', '5s', '3d', '1Y'). | |
The format is a string of `<count>[ms\|s\|m\|h\|d\|w\|M\|Y]` (e.g. '20m', '24h', '7d', '1w'). |
[TIP] | ||
============ | ||
The format is a | ||
string of `<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70ms', '5s', '3d', '1Y'). | ||
The format is a string of `<count>[ms\|s\|m\|h\|d\|w\|M\|Y]` (e.g. '70ms', '5s', '3d', '1Y'). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This too
The format is a string of `<count>[ms\|s\|m\|h\|d\|w\|M\|Y]` (e.g. '70ms', '5s', '3d', '1Y'). | |
The format is a string of `<count>[ms\|s\|m\|h\|d\|w\|M\|Y]` (e.g. '20m', '24h', '7d', '1w'). |
You can expire sessions after a period of inactivity. This and `xpack.security.session.lifespan` are both | ||
highly recommended. By default, session doesn't expire because of inactivity. To define a sliding session expiration, set | ||
the `xpack.security.session.idleTimeout` property in the `kibana.yml` | ||
configuration file. The idle timeout is formatted as a duration of | ||
`<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70ms', '5s', '3d', '1Y'). For example, set | ||
the idle timeout to expire sessions after 1 hour of inactivity: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels odd to see lifespan
before idleTimeout
, even though this is the idle timeout section.
How about rewording it like this?
You can expire sessions after a period of inactivity. This and `xpack.security.session.lifespan` are both | |
highly recommended. By default, session doesn't expire because of inactivity. To define a sliding session expiration, set | |
the `xpack.security.session.idleTimeout` property in the `kibana.yml` | |
configuration file. The idle timeout is formatted as a duration of | |
`<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70ms', '5s', '3d', '1Y'). For example, set | |
the idle timeout to expire sessions after 1 hour of inactivity: | |
You can use `xpack.security.session.idleTimeout` to expire sessions after a period of inactivity. This and `xpack.security.session.lifespan` | |
are both highly recommended. | |
By default, sessions don't expire because of inactivity. To define a sliding session expiration, set the property in the `kibana.yml` | |
configuration file. The idle timeout is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]` (e.g. '20m', '24h', '7d', '1w'). For example, | |
set the idle timeout to expire sessions after 1 hour of inactivity: |
Note 1: changed wording "session doesn't expire" -> "sessions don't expire"
Note 2: changed duration example to match that in prior comments
You can configure the maximum session duration or "lifespan" -- also known as | ||
the "absolute timeout". This and `xpack.security.session.idleTimeout` are both highly | ||
recommended. By default, session doesn't have a fixed lifespan, and if an idle timeout is defined, a session can still be extended | ||
indefinitely. To define a maximum session lifespan, set the | ||
`xpack.security.session.lifespan` property in the `kibana.yml` configuration | ||
file. The lifespan is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]` | ||
(e.g. '70ms', '5s', '3d', '1Y'). For example, set the lifespan to expire | ||
sessions after 30 days: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above -- it looked weird to see idleTimeout
in this section before lifespan
. How about this?
You can configure the maximum session duration or "lifespan" -- also known as | |
the "absolute timeout". This and `xpack.security.session.idleTimeout` are both highly | |
recommended. By default, session doesn't have a fixed lifespan, and if an idle timeout is defined, a session can still be extended | |
indefinitely. To define a maximum session lifespan, set the | |
`xpack.security.session.lifespan` property in the `kibana.yml` configuration | |
file. The lifespan is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]` | |
(e.g. '70ms', '5s', '3d', '1Y'). For example, set the lifespan to expire | |
sessions after 30 days: | |
You can use `xpack.security.session.lifespan` to configure the maximum session duration or "lifespan" -- also known as the "absolute | |
timeout". This and `xpack.security.session.idleTimeout` are both highly recommended. | |
By default, sessions don't have a fixed lifespan, and if an idle timeout is defined, a session can still be extended indefinitely. To define | |
a maximum session lifespan, set the property in the `kibana.yml` configuration file. The lifespan is formatted as a duration of | |
`<count>[ms|s|m|h|d|w|M|Y]` (e.g. '20m', '24h', '7d', '1w'). For example, set the lifespan to expire sessions after 30 days: |
Note 1: changed wording "session doesn't have" -> "sessions don't have"
Note 2: changed duration example to match that in prior comments
If you specify neither session idle timeout nor lifespan, then {kib} will not automatically remove session information from the index unless you explicitly log out. This might lead to an infinitely growing session index. Configure the idle timeout and lifespan settings for the {kib} sessions so that they can be cleaned up even if you don't explicitly log out. | ||
============================================================================ | ||
|
||
You can configure the interval at which {kib} tries to remove expired and invalid sessions from the session index. By default, this value is 1 hour and cannot be less than 10 seconds. To define another interval, set the `xpack.security.session.cleanupInterval` property in the `kibana.yml` configuration file. The interval is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70000ms', '50s', '3d', '1Y'). For example, schedule the session index cleanup to perform once a day: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another duration example for consistency 🙂
You can configure the interval at which {kib} tries to remove expired and invalid sessions from the session index. By default, this value is 1 hour and cannot be less than 10 seconds. To define another interval, set the `xpack.security.session.cleanupInterval` property in the `kibana.yml` configuration file. The interval is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70000ms', '50s', '3d', '1Y'). For example, schedule the session index cleanup to perform once a day: | |
You can configure the interval at which {kib} tries to remove expired and invalid sessions from the session index. By default, this value is 1 hour and cannot be less than 10 seconds. To define another interval, set the `xpack.security.session.cleanupInterval` property in the `kibana.yml` configuration file. The interval is formatted as a duration of `<count>[ms|s|m|h|d|w|M|Y]` (e.g. '20m', '24h', '7d', '1w'). For example, schedule the session index cleanup to perform once a day: |
Handled, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments, but otherwise LGTM.
`saml.<provider-name>.maxRedirectURLSize` | ||
| The maximum size of the URL that {kib} is allowed to store during the authentication SAML handshake. For more information, refer to <<security-saml-and-long-urls>>. | ||
`saml.<provider-name>.useRelayStateDeepLink` | ||
| Determines if the provider should treat the `RelayState` parameter as a deep link in {kib} during Identity Provider initiated log in. By default, this setting is set to `false`. The link specified in `RelayState` should be a relative, URL-encoded {kib} URL. For example, the `/app/dashboards#/list` link in `RelayState` parameter would look like this `RelayState=%2Fapp%2Fdashboards%23%2Flist`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Determines if the provider should treat the `RelayState` parameter as a deep link in {kib} during Identity Provider initiated log in. By default, this setting is set to `false`. The link specified in `RelayState` should be a relative, URL-encoded {kib} URL. For example, the `/app/dashboards#/list` link in `RelayState` parameter would look like this `RelayState=%2Fapp%2Fdashboards%23%2Flist`. | |
| Determines if the provider should treat the `RelayState` parameter as a deep link in {kib} during Identity Provider initiated log in. By default, this setting is set to `false`. The link specified in `RelayState` should be a relative, URL-encoded {kib} URL. For example, the `/app/dashboards#/list` link in `RelayState` parameter would look like this: `RelayState=%2Fapp%2Fdashboards%23%2Flist`. |
|
||
[cols="2*<"] | ||
|=== | ||
| `xpack.security.loginAssistanceMessage` | ||
| Adds a message to the login UI. Useful for displaying information about maintenance windows, links to corporate sign up pages etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Adds a message to the login UI. Useful for displaying information about maintenance windows, links to corporate sign up pages etc. | |
| Adds a message to the login UI. Useful for displaying information about maintenance windows, links to corporate sign up pages, and so on. |
browser still requires the user to log back in to {kib}. | ||
|
||
|=== | ||
| This setting ensures that user sessions will expire after a period of inactivity. This and `xpack.security.session.lifespan` are both |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This setting ensures that user sessions will expire after a period of inactivity. This and `xpack.security.session.lifespan` are both | |
| Ensures that user sessions will expire after a period of inactivity. This and `xpack.security.session.lifespan` are both |
is not set, this setting will still cause sessions to expire. | ||
|
||
|=== | ||
| This setting ensures that user sessions will expire after the defined time period. This behavior also known as an "absolute timeout". If |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This setting ensures that user sessions will expire after the defined time period. This behavior also known as an "absolute timeout". If | |
| Ensures that user sessions will expire after the defined time period. This behavior also known as an "absolute timeout". If |
@@ -263,11 +239,11 @@ The following sections apply both to <<saml>> and <<oidc>> | |||
===== Access and refresh tokens | |||
|
|||
Once the user logs in to {kib} Single Sign-On, either using SAML or OpenID Connect, {es} issues access and refresh tokens | |||
that {kib} encrypts and stores them in its own session cookie. This way, the user isn't redirected to the Identity Provider | |||
for every request that requires authentication. It also means that the {kib} session depends on the <<security-ui-settings, | |||
that {kib} encrypts and stores them as a part of its own session. This way, the user isn't redirected to the Identity Provider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that {kib} encrypts and stores them as a part of its own session. This way, the user isn't redirected to the Identity Provider | |
that {kib} encrypts and stores as a part of its own session. This way, the user isn't redirected to the Identity Provider |
configuration file. The idle timeout is formatted as a duration of | ||
`<count>[ms|s|m|h|d|w|M|Y]` (e.g. '70ms', '5s', '3d', '1Y'). For example, set | ||
the idle timeout to expire idle sessions after 10 minutes: | ||
. Configure {kib}'s session expiration settings. We strongly recommend setting both idle timeout and lifespan settings: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
. Configure {kib}'s session expiration settings. We strongly recommend setting both idle timeout and lifespan settings: | |
. Configure {kib}'s session expiration settings. Set both the idle timeout and lifespan settings: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs changes LGTM!
7.x/7.10.0: 2bf5a2c |
Summary
In this PR we're changing the approach we use to handle users sessions. Previously all session information was stored in the client-side cookie, but client-side cookies cannot cover all the session management requirements we have these days and we'd like to store some of the session information in the Elasticsearch index (see details and discussion in #17870).
Below I'm trying to outline the major changes that this PR makes.
Changes
Storage
The session value is now split between two sources: session cookie and session index. Fields in bold are NOT available to consumers of the
Session
APIs and considered as an internal implementation detail of theSession
.Session cookie value fields:
sid
- unique identifier of the session used to match session cookie with session info stored in Elasticsearch index;aad
- additional authenticated data (AAD) used to encrypt\decrypt sensitive part of the session value stored in the index. SeeSecurity
section below for details;path
- base path of the Kibana instance that created the session. Knowing this allows us to detect old cookies that may belong to the session that was created when/if Kibana was hosted under another base path or without it (it may happen during initial configuration or development). Having this in the cookie allows us to avoid querying the index.idleTimeoutExpiration
- as name implies, this field tells us when the session should be considered expired. It's the most frequently updated field of the session value, having this in the cookie allows us to significantly reduce number of costly index writes. SeeOptimizations
section for details.lifespanExpiration
- this field tells us the max total lifespan of the session. This value never changes unlessxpack.security.session.lifespan
is reconfigured. Again, having this in the cookie allows us to avoid querying the index just to figure out that session is expired.Session index value fields:
sid
- unique identifier of the session used to match session cookie with session info stored in Elasticsearch index;username_hash
- hash digest of the username associated with the session. When combined with theprovider
field can be used to uniquely identify session of a particular user within a particular realm. This field is only present in authenticated sessions, unauthenticated sessions (e.g. intermediate sessions used during SSO handshake), for obvious reasons, don't have this field.tenant
- it's a hash of the Kibana index name, we rely on this to only cleanup session values that belong to a specific tenantprovider
- type and name of the authentication provider used to create session. When combined with theusername_hash
field can be used to uniquely identify session of a particular user within a particular realm;idleTimeoutExpiration
- same asidleTimeoutExpiration
described in the session cookie section above. We need to store this in index to know which sessions are still active so that we can automatically clean up expired ones and e.g. figure out how many active session particular user has at the moment (Limit the number of concurrent user sessions #18162). Unlike session cookie, here this value doesn't reflect exact expiration time, seeOptimizations
section for details.lifespanExpiration
- same aslifespanExpiration
described in the session cookie section above. And as well asidleTimeoutExpiration
this field allow us know which sessions are still active so that we can automatically clean up expired ones and e.g. figure out how many active session particular user has at the moment (Limit the number of concurrent user sessions #18162);accessAgreementAcknowledged
- indicates whether user acknowledged access agreement or not. There is no need to store this field in the cookie.content
- it's an encrypted JSON string that includes a portion of the session value that is sensitive and should be hidden. Currently this JSON includesusername
and sessionstate
returned by the authentication provider (e.g. username/password forbasic
provider, access/refresh token pair forsaml
, client certificate fingerprint forpki
etc.). The decrypted fields are available to the consumers ofSession
APIs.metadata
- currently it contains onlyprimaryTerm
andsequenceNumber
we need to deal with concurrent update/extend requests.Note: I don't include
roles
andip
in this iteration as these can easily be added separately when we think we need them.For now the plan is to use new
.kibana_security_session_1
index under.kibana_security_session
alias (we may want to use another numbering scheme, e.g. ES uses7
to indicate ES version in which particular index template was introduced/updated) to store session info. Since it's the first time we add session index I didn't add the code that would handle versions and possible migrations.Currently I'm using index template, to be a little bit more resilient to the cases when index is deleted (so that the first session
create
operation would properly resurrect it), but I'm not completely sure we need this. I'm copying index settings from ES'ssecurity-tokens
index template plus marking index as hidden:Security
As discussed in #17870 we're implementing a number of security-related measures, namely:
sid
andaad
are generated using a secure PRNG (256 bits)aad
is only stored in the session cookie, so one need to have access to both the session cookie and session index to decrypt sensitive session informationusername
is encrypted, and only exposed to the querying viausername_hash
field that issha3-256
digest of theusername
Optimizations
Since querying index on every user request will inevitably affect performance I'm trying to implement a number of optimizations to reduce the impact:
path
,lifespanExpiration
andidleTimeoutExpiration
in the session cookie so that we can do basic session validity checks without querying indexxpack.security.session.lifespan
orxpack.security.session.idleTimeout
is reconfiguredxpack.security.session.idleTimeout
is specified we regularly updateidleTimeoutExpiration
only in the session cookie and eventually update session index only when a difference betweenidleTimeoutExpiration
in a session cookie and in the index exceeds2 * idleTimeout
(we can play with the factor). Automatic cleanup routine will remove sessions from the index only if difference between current time andidleTimeoutExpiration
exceeds3 * idleTimeout
. This way we reduce a number of index updates, but session may be considered valid for a longer than it's in reality. It should not make us less secure since we rely onidleTimeoutExpiration
from the cookie to decide whether session is still valid or not before we perform any action that requires authentication. But this is relevant for automatic cleanup routine and any future checks when we would like to know how many active sessions particular user has at the moment. That sounds like a reasonable trade-off to me, but I'd like to know what you think?xpack.security.session.cleanupInterval
(see all covered cases inSessionIndex.cleanUp
)I did a quick performance test as well:
Input
I created ~115000 of sessions in the
.kibana_security_session
index and querying/internal/security/me
endpoint. The basicautocannon
snippet I used looks like this (100000 requests in total with 100 simultaneous connections):Results
Before the change (session is extracted from the cookie)
After the change (session is extracted from the cookie and index)
The average difference is ~2.5-3 times, same result for almost empty session index. I'll need to investigate where we spend most of the time, it's either network (loopback) or decryption.
Limitations and unresolved questions
* Currently we don't regeneratesid
andaad
if username changes or if user changes their own password. IIRC the recommendation is to re-generate these things when such an event happens, but it increases implementation complexity a bit (still need to evaluate). I'd keep it as is at the initial stage, but don't have a strong opinion. What do you think?xpack.security.session.lifespan
norxpack.security.session.idleTimeout
is configured and encryption key changes, old sessions may stuck in the index indefinitely. I'm not sure how I feel about that yet, but should we store some sort of hash of the encryption key in the index so that we can detect that change and remove old sessions?Should we use_bulk
query for session creation to run cleanup routine together with the_create
query? Theoreticallylogin
action should be more frequent thanlogout
.Checklist
Overwritten Session
view if an active session is implicitly overriden by a different provider #61217/internal/security/login_with
endpoint into/internal/security/login
#61222xpack.security.session.cleanupInterval
in the docsFixes: #17870
Release Note: from now on Kibana will be storing users session information in a dedicated Elasticsearch index. By default expired and invalid sessions are cleaned up from the index every hour. The cleanup interval can be configured with
xpack.security.session.cleanupInterval
setting. Note that after upgrade all existing sessions will become invalid and users will have re-log in to Kibana. Another notable side effect of this change is that from now on it will no longer be possible to host different Kibana tenants on the different ports of the same host. Even though such setup worked well in the past it was always discouraged since browsers share cookie across all applications hosted using the same host name ignoring ports. With this change cookies will be strictly tied to a particular tenant.