session.cookie.idletime does not work with redis storage
thorstenfleischmann opened this issue · 7 comments
I tried using session.cookie.idletime together with redis and regenerate strategy. After one minute the cookie "stops" working as expected. The browser receives a new cookie (same id, different "use before", different hash) and the subsequent requests can not use the session anymore.
I added some logging here:
lua-resty-session/lib/resty/session.lua
Line 598 in 2cd1f84
err is "unable to decrypt data". It seems to me that the encrypted data is not updated with the latest "usebefore"-value since usebefore is also used as a key for encryption:
btw: thanks for this awesome library
@thorstenfleischmann, thank you for reporting. Needs to be fixed.
I am having the exact same issue using the Kong (API Gateway) session plugin that uses this library. I use the Kong storage option which uses the regenerate strategy, and I get the exact same behavior as desribed above, I will try to display my digging in where I believe the issue could come from, but I dont really know a fix
If any value is used for idletime, after 60 seconds of establishing a session, set-cookie comes back with a new session, however upon looking through the code, it seems to be based on this calculation
lua-resty-session/lib/resty/session.lua
Line 283 in 2cd1f84
which would return true in this setup I believe after the new/old difference is greater than 60 (seconds)
This makes a "touch" operation happen, and a set_cookie happen, however when i looked at the touch, it does not save the new cookie to the database (like a save does), so when the client uses the latest cookie, it is not valid (authentication fails)
so to me maybe the touch should not change the cookie, but should only be maintained as an internal timer, or a touch should only happen if the idletime is hit (not 60 seconds), I will try to work on a PR but i'm still a little confused on the implementation.
I wanted to add another comment to act as a sounding board for how I understand what the idletime logic is trying to accomplish
versus renew or lifetime, the idletime is a dynamic timer, and at the end of the day needs to be stored in the DB in this storage method, I think what is happening here is that the cookie changes because the "usebefore" is updated (right now every 60 seconds, but even if it was based on idletime, the usebefore value still needs to track the idletimer value, which is stored in the cookie/DB, the problem is if the cookie changes, it needs to be saved else its invalid in this current logic (with a touch that changes the cookie but does not save it). So how do we manage a cookie that incorporates the usebefore/idletime without it changing? I dont think you can securely, so that leaves us with storing the timer data outside the cookie, but somehow able to be looked up in relation to a given session cookie.
That way idletime can be updated per request, but the session cookie wont change until that idletime is detected OR renew/lifetime is detected.
Following up after more digging, I realized that if you select the cookie storage option, the idletime logic seems to work fine, and the cookie remains valid, so i thought "what is different between the two strategies?" after investigating the touch logic between the default and regenerate strategies, the regenerate passes a different concatenated key into the modify function here
which makes it bypass the normal processing/concatenation of the string here
So what I cant understand if the purpose of the key function here, that seems to omit the expires data?
They both pivot/use the session object, so I dont know why regenerate doesnt use the same touch as default.
Hope this helps
@JakeEmo thanks for info, I'll try to fix this within a week
@JakeEmo, this turns out to be harder to solve, still possible, it just takes time. It looks like the use before has never worked with server side session storage because of key includes the usebefore, but data on server side storages is not updated. With cookie the whole thing is updated. This leads to issues with server side storages data being encrypted with different key (difference in usebefore that is included in key). The easy solution is to drop usebefore from key, but then it would be trivial to modify the usebefore. Another solution is always write to db, but that is against the design of that touch thing. And the third is to have some alternative signature that does not contain data.
The version 4.0.0 fixes this.