[RFC] Improved session management #1311
Comments
davidlago
added
enhancement
|
My 2 cents. Server side session management This seems to be the right direction for a long term fix for limitations like large assertions, excessive roles and opens up the avenue for a lot of other user-behaviour related insights. In the future different plugins would want to extend this to collect data relevant to their use-case. The design would need to take this into consideration. Time will be a constraint here to solve the pain-point quickly. JWT Attribute filter The subject key and roles form the largest portion of the JWT we generate from a SAML Response. These can be filtered to reduce the cookie size. However, there is a corner case where it can still restrict a user from adopting an approach of having a large number of granular roles for their use-case. This would depend on the existing role hierarchy used by the user. Stitch multiple cookies together This could be a viable option in the short term. The entire storage layer of cookie is currently abstracted using the session_storage interface in Dashboards. A child implementation of the existing cookie storage can be created which handles the breaking and re-creation of one cookie into multiple one's and re-uses the existing logic of cookie management.
|
|
IMHO, we should identify that Cookie shall not carry the entire token. Better options are to store in the session storage instead or backend. Demerits of storing the token in Cookie is that Client has to send this Cookie on each request, and if there are multi-cookie - all the cookies will be sent and impact the throughput of the systems. For the per the best practices we should decouple this into : Front-end : Backend: |
|
Regarding "Filter JWT attributes stored in the cookie": This approach is a bit more complicated as it might sound. The Dashboards plugin would be not able to just filter the JWT attrs, as then the backend would no longer trust these JWTs (due to a wrong signature). To overcome this, some module would need to re-issue JWTs which are trusted by the backend. As this is still just a workaround, I am not sure whether this is worth the effort. Also, as mentioned before, there is the chance that it is actually the roles which blow up the JWT size. So, I don't really think that this approach is suitable. |
I like this approach, as the abstraction layer exists already and we can eventually point to what @prabhat-chaturvedi suggests as an externalized storage for sessions (and at that point stop the cutting/stitching). This is the shortest path to solving the immediate problem while allowing some more time for the larger efforts in Dashboards linked above (some of which might even make things easier to build, like the externalized metadata store). |
|
The externalised storage is the best solution, but we will have similar challenged for the Frontend session management, which is where to store the JWT. If its opaque token which is stored as Key:randomString,Value:JWT, then the frontend can just keep a handle of that randomString which maps to the session identifier. Otherwise, even when the JWT should be stored in the Frontend, Cookie length issue is must to be solved. Dashboard frontend, can extract the JWT from the browser session storage was the session is governed by JSESSIONID(or such application managed session cookie) and then populate the JWT in a POST param, header or Cookie which ever best suits the security plugin to validate. Again emphasising the demerits for using Cookie to store the JWT are:
There were some benefits when the Cookie was relayed directly from Dashboard to Security plugin, but it might just be a big change to make in the exit gate of Dashboard to pull from Session Storage and populate in the ongoing request to plugin. |
Seems really cheap and easy to check. While we want to target the end state, this might be able to mitigate the user experience for a couple of impacted clusters while we get there. Has anyone looked into this? |
|
Unless I'm missing something, it does not look like we use cookie compression. It seems like a reasonable high level estimate of gzip's rate of compression of ASCII text is 1:11 [1]. With the cookie size being limited as per this RFC to 4Kb, that could raise the limit to something like 44Kb of uncompressed ASCII text. @jimishs, I know you have done some research on use cases around SAML assertions. Do you have a sense for what reasonable upper bound for the cookie size we should consider in this design? |
|
@nibix regarding "Filter JWT attributes stored in the cookie" the backend trusting the JWT: At least in my case, the the id_token is the big thing, because it contains all the group memberships. |
|
@domruf Actually, it is the other way round. The id_token is needed by the OpenSearch backend to determine the user's roles from the group membership and thus their authorization. The access_token, on the other hand, is only meaningful to the OIDC IdP service. |
|
Besides discussing the workarounds, IMHO, we should also have a closer look at how an actual solution might look like. As a kind of kickoff for a discussion, I have drafted the following document which outlines how such a solution could look like. Any comments are very appreciated! Proposal: Architecture for session based login in DashboardsGoals and basic requirements
Non-goals
Open: Goals or non-goalsFor these items, it needs to be clarified whether it shall be a goal or not:
Fundamental design
Open design decisions
|
|
I've run some tests with actual SAML payloads with 200 SAML roles in them, and after compressing them they are still under 4Kb: 80 -rw-r--r-- 1 davelago 38091 Feb 15 13:32 saml_response_200_roles.txt
8 -rw-r--r-- 1 davelago 3723 Feb 15 13:32 saml_response_200_roles.txt.gzFor reference, the js I used to test compression was: const { createGzip } = require('node:zlib');
const { pipeline } = require('node:stream');
const {
createReadStream,
createWriteStream,
} = require('node:fs');
const gzip = createGzip();
const source = createReadStream('saml_response_200_roles.txt');
const destination = createWriteStream('saml_response_200_roles.txt.gz');
pipeline(source, gzip, destination, (err) => {
if (err) {
console.error('An error occurred:', err);
process.exitCode = 1;
}
});It seems like a worthy fix to add compression to solve the immediate painpoint, and we can tackle the larger server-side session management as part of those ongoing efforts (opensearch-project/OpenSearch-Dashboards#1215, opensearch-project/OpenSearch-Dashboards#1441). |
|
+1 to the idea of cookie compression and optimizing cookie to exclude parameters that arent needed / can be removed. Regarding the question of upper bound - im aware of customers with 800-1000 workgroup attributes (that may or may not map to one or more OpenSearch security roles). So while the size could vary, 20-30 kb was what was estimated for such large assertions. If we can go upto 44kb, uncompressed, that could solve near term concerns for large customers. |
|
Just a small clarification regarding SAML responses: The Dashboards security plugin never stores SAML responses in its cookies. Rather, it converts these into a JWT using an API in the OpenSearch backend. Still, as these JWTs are not encrypted, these should also have a good compression ratio, but maybe not as big as for the XML based and very verbose SAML. Just looking quickly at the OIDC case: For OIDC, the id_token is stored in the cookie. The id_token can be compressed as well. However, some IdPs might choose to provide encrypted id_tokens. As encrypted data is not well compressible, this will not provide such a strong benefit (it will bring some benefit by compensating for the base64 encoding size gain). |
|
I have filed a draft pull request with a first shot at the compression solution. See #1338 Observations so far:
We are still looking into other ways to serialize and store the payload to avoid all these encoding losses. |
|
I have created a sample SAML AuthnResponse to serve as a test case for our research. The AuthnResponse contains 200 roles encoded in a DN format. I understand that even though this is quite a big amount, it is still something that is realistic and must be expected. The encoded SAML AuthnResponse (28 kb): The JWT created by the security plugin from the AuthnResponse (14kb): When using the compression solution in #1338 this JWT goes through the following stages:
Maximum cross-browser safe cookie size is 4093 bytes. Thus, the solution in #1338 won't work with the reference AuthnResponse. Still, this size is blown up due to the double base64 encoding. We are still investigating ways to avoid the double encoding. |
|
I guess in my previous post there was a confusion on Long term solution on Server based session management Vs Local Browser Session storage(another short term solution). Can we look into immediate short term solution alternative - using Local Session storage, as Cookie has limitations of 4MB, but Session Storage can go upto 10MB or more in some browsers. The effort required are still the same, the place of compression and decompression - we need to put logic to put in browser session storage and pull out at the same entry and exit points. |
As the possibly bloated JWT is the authentication token, we must pass it to Dashboards (and the OpenSearch backend) for every request. Otherwise, these requests are unauthenticated. |
|
True, @nibix - all that remains as is, instead of storing the JWT in the cookie, store in something like session.setAttribute("autheticatedJWT", actualJWT).
|
|
@prabhat-chaturvedi If we use local storage or any other means of storing the data instead of Cookie on browser side, then we need to read it and attach in every request sent from Dashboards front-end to Dashboards backend. Currently, browser takes care of attaching the cookie with every request for us. |
|
@anijain-Amazon I will have to dig deep onto it and limited to my knowledge specific to OpenSearch. Session 1 : User session at Dashboard frontend - where I propose to use browser session storage and passed on to below |
|
@nibix Great details with the reference cookie, 4409 byte is on the order of 4093 bytes. If we employed a cookie splitter, seems like the base case without implementing the server side solution. Do you think that is a viable approach for the near term? (Even setting aside the double base64 encoding issue - which means there might be even more headroom!) |
I believe that cookie splitting/stitching would be viable - but also tricky. The base solution linked in the PR above has the advantage that it is limited to the Dashboards security plugin. For more intrusive solutions, it is likely that we need to make changes in core Dashboards. |
@prabhat-chaturvedi If we use Browser Session storage, then passing the credentials to every request to backend will require code changes at lots of places. |
|
We have added a second PR to investigate the viability of splitting up a large payload into multiple cookies. See #1352 For this PR the focus was on doing this without any modifications to core Dashboards. This approach comes with some drawbacks though. There are some more details in the PR, but in short, creating a cookie from a plugin is a bit harder because of the abstraction layer on top of Hapi and the PR breaks this abstraction in a somewhat hacky way:
This could be improved a little by using a separate instance of Hapi Statehood, which manages the cookies for Hapi under the hood, but even with this I haven't (yet) been able to find a better way to write the cookie values. Anyhow, I hope this PR at least serves as a basis for further discussions. |
|
A short status update on this: We've implemented this for OIDC as well, and we've made the number of additional cookies configurable. One interesting finding that probably should be mentioned in the docs when this goes live is that if the id token from the IdP is very large, we may run into the following error: I ran into this while testing an id token containing 200 roles. After receiving the token from the IdP, the I believe this is something that the customers would need to configure together with the cookie compression/splitting, otherwise the request will fail before we squeeze the token into the additional cookies. Not sure if changing this setting has any negative side effects...? |
|
We've updated the pull request with the cookie stitching: #1352 The main difference is that it is now implemented for both OpenId and SAML. The main questions going forward whether you would like the number of cookies to be configurable. There are some drawbacks to that, mainly related to if the customer changes the number of cookies. We could also give making the number of used cookies dynamic another try. In other words don't register any cookies with the Hapi server, and instead try to write as many cookies as needed by the actual content. Anyhow, happy for suggestions for improvements! |
|
We can consider keeping this implementation as optional via configuration since most of the users of OpenSearch might not have a requirement of cookies more than 4Kb. Also, we might be introducing some overhead in latency by extra base64 encoding, so turning this off if required can be desirable. |
|
@anijain-Amazon
This base64 encoding also occurs now and is kind of essential as the encrypted data is likely binary and cannot be stored as such in a cookie. Also, I don't believe that such a overhead is significant here. |
|
@nibix I believe there was an additional layer of encoding added for spilt cookie approach. Agree that the latency might not be very high but the functionality of cookie splitting can be behind a configuration flag and disabled by default. |
|
The only additional encoding step is the compression. |
Given that the ones turning this off would be users with small cookies to pass around, the overhead in their case would be quite negligible, and IMHO not worth the additional settings and code paths (that would also need to be tested) to make this an optional feature. |
|
I am one of the many sufferers of getting booted out of dashboards after an hour. I have tried making the two recommended changes of extending the JWT time and cookie session length, but neither appears to make any meaningful change. Is there anything I can do as an admin/user today to increase this timeout value before getting kicked out to my IDP and having to re-auth. Other SAML based applications don't seem to suffer from this issue. |
|
@nobletrout The expiry of the JWT Token is determined using the SAML Response. CodePointer Maybe you can try increasing the Session TTL at your IdP. PS at the first glance, your issue seems to be not related to this RFC. Please feel free to open a separate Github issue for the same, so that the discussion can be continued there. |
|
I believe #828 is the one you might be thinking of @anijain-Amazon... this issue links it at the top as one of the many things we'd like to fix with better session management. The cookie splitting work addresses just a few of these and more as a workaround for now, but yeah, there are still plenty of the ones linked above that won't be fixed this upcoming release. |
|
Closing this RFC after #1352 got released in 2.7.0. If we decide to further improve session management (for example, adding server-side session management) we can track that as a separate issue/feature. |
What is the problem?
OpenSearch uses cookies for session management. Cookies have a size limit (4Kb). Some extremely large session payloads (think of large SAML assertions) hit this limit and we don't currently have a path forward for those use cases.
Additional problems of the current session management implementation (or feature requests) are captured in the issues below. We should aim to solve for the main problem above, but if in doing so we can kill any of these "birds" with that stone, and they can inform the solution for this one, that would be great. Otherwise, we can tackle those separately:
What options could we consider?
There are may ways to skin this cat, from short term stopgaps to larger overhauls of how we manage sessions in the product.
Filter JWT attributes stored in the cookie
Right now, there are a number of attributes coming from the IdP that we store in the cookie, whether they are needed or not. Providing an allow-list of attributes and storing just these in the cookies will make the 4Kb limit go a longer way.
Use compression
Compressing the cookie payloads could relax the constrain of that 4Kb limit, as text attributes from the session are generally highly compressible (are we doing that today already? if so, disregard this option).
Stitch multiple cookies together
Another relatively low effort fix for this would be to split cookie payloads into several chunks, and stitching them together when we receive them. If we have, say, 4 chunks... would 16Kb be a high enough limit to enable even the most extreme SAML assertions?
Server-side session management
Storing the session in the backend (whether in OpenSearch Dashboards' backend or OpenSearch's), will remove the size limit, and could potentially address some of the shortcomings of using cookies such as sessions carrying over from device to device, or the ability to invalidate sessions from server side, but it adds complexity and potential use cases we need to think about carefully:
Next steps
Let's use this issue to have a discussion of options, trade-offs, and come up with a design for what a new session management strategy should look like.
The text was updated successfully, but these errors were encountered: