bungle / lua-resty-session

Session library for OpenResty – flexible and secure
BSD 2-Clause "Simplified" License
320 stars 111 forks source link

persistent cookies possible? #2

Closed capr closed 9 years ago

capr commented 9 years ago

Hi,

I want my clients to stay logged in next time they open the browser, but I see that you specifically say: no persistent cookies. Why, what's the downside?

Thanks, Cosmin.

bungle commented 9 years ago

Thanks,

Yes, it is possible. Although I usually want to have these "remember me" type of cookies separately, as their usage (and data stored in them) differs. Other non-session related cookies may be stored on a client permanently (with expiration data), but it is out of scope of this library. In general non-persistent cookies are called session cookies, and that's what this library tries to provide.

Right now on SSL/TLS connections this library makes use of ngx_var.ssl_session_id as suggested in Secure Cookie Protocol paper. This id changes, if the browser is closed (so the permanent cookies won't work in that case, making them non-permanent functionality wise). We are trying to make session cookie handling as secure as practically possible (and the default settings also follow this). Also I'm not sure how Expires flags behave with HttpOnly flag (that we use), it should work, but I'm not sure how browsers ended up implementing this (so, I have to test this more - there are a lot of information and misinformation on the web around this).

Conclusion: it is possible to make session cookies permanent cookies. But I'm not sure if this promotes a good security practices. I may add this as an configuration option.

In general these remember me cookies should be:

  1. single use (one remember me cookie should work only once), and a new one is regenerated after the use (in case of session cookies, you should regenerate the session cookie whenever you change the authorization context)
  2. You should not store anything user related on a persistent remember me cookie (just identify it with some random data)
  3. You should hash in some other data as well when checking a remember me cookie's validity, like HTTP_USER_AGENT (and possibly HTTP_REMOTE_ADDR). You could certainly apply hmac/encryption here as well, as described in Secure Cookie Protocol paper.

Maybe we could add "remember me" type of functionality in lua-resty-session, but I rather still keep that cookie separately, and store only small amounts of non-user related data in it (as is the case with session cookies too).

You could send me an pull-request if you end up implementing this, but right now this is not a top-priority for me. I'm right now considering adding server side session support with ngx.shared.DICT/redis/memcached etc. backends to lua-resty-session (that currently only supports client side session data storage).

capr commented 9 years ago

Hi Aapo, thanks for replying, that was very informative.

Here's how I understand the situation. I'm no expert so correct me if I'm wrong. So right now, I just disabled the ssi check and the user agent check (to survive browser upgrades) and made the cookie persistent (2 years). I understand that this makes the cookie usable outside the SSL session, if stolen. But a remember-me cookie is by necessity usable outside the SSL session anyway, and if a user be given a secure session cookie based on that less secure cookie, then it doesn't matter that the session cookie is more secure. Even if the remember-me cookie is single-use, I imagine it would be stolen before being used yet. I can't embed the remote address in it because many people have dynamic IPs. Can't embed the user agent either because browsers self-upgrade all the time.

Anyway, I'm not sure I see the whole picture, please feel free to correct me.

PS: I made the pull request.

bungle commented 9 years ago

Yes, I think you understood this correct. And yes, this is all about making compromises. More compromises the less secure the end result is. In this case it seens you need to disable ssl_session_id, http_user_agent, remote_addr (disabled by default), and maybe even scheme checks. That's what compromises mean.

I think your request is reasonable (when you know the downsides of it, and not enabled by default), so I just made a patch to have this support in lua-resty-session as you can see in this commit:

https://github.com/bungle/lua-resty-session/commit/5d0b4a7c4dac2ac54acf6d1a37f8d63c98673dff

Now, could you please test this?

Things I added:

  1. Configuration parameter: session.cookie.persistent = true | false (default = false)
  2. Configuration parameter: session.check.ssi = true | false (default = true)

You may also want to configure session.cookie.renew to something larger than default 600 seconds if you want to have your users "logged in" as much as possible (you may also disable additional checks like User Agent, and Scheme).

You may configure these from Nginx config as well. This is not yet, documented, but please try it out, and report any bugs you find.

Please use the latest version as I did also make some small changes after that commit above (I added Max-Age to cookie in addition to Expires, and renamed permanent to persistent).

Hopefully this solves this case and doesn't break anything. If everything is fine, I will release a new version of it.

Thanks Aapo

PS. I didn't see your pull request before doing this, but it looks like we ended up with similar solution.

capr commented 9 years ago

Now, could you please test this?

Sure. I'll get back to you.

You may also want to configure session.cookie.renew to something larger than default 600 seconds if you want to have your users "logged in" as much as possible (you may also disable additional checks like User Agent, and Scheme).

Not sure I understand this point. Isn't an old cookie (older than renew) always accepted? I actually don't know why the cookie must be renewed every 10 minutes.

bungle commented 9 years ago

Hi, I made some more changes to this, and added documentation.

Why do we need to renew the cookies? We need to do this, because otherwise the cookies would expire if they are not saved (session:save sends a new cookie). Imagine that nothing in session data etc. needs changing (you only read the session data). Then we have two possibilities to not to expire users session.

  1. Send cookie back always with a new expiration time (that is session:save)
  2. Send cookie back only when certain treshold is met (that is session.cookie.renew).

Sending cookies back and forth is not always neccessary, so we don't send it if nothing changes (no need to call session:save), and use less bandwidth. Because there is no mechanism to expire client side session cookies (cookies that do not have expiration date), we store expiration date in cookie. And then we check if we need to send a new cookie if cookie's remaining lifetime is less than the session.cookie.renew (even when the user doesn't call session:save). Otherwise you could have your browser window open for say years, and it wouldn't expire the session. On server side sessions the server expires the sessions automatically, but as we don't have this with stateless client only session cookies (and even in that case we rely on the client), we don't have other options. For persistent cookies for which we have now implementation, the sessions will expire on client side depending on lifetime setting.

bungle commented 9 years ago

I just released this as 1.3 version, and everything seems to be working fine.

Thanks again. Still if you find any bugs, please let me know. If not I will be closing this tomorrow.

capr commented 9 years ago

Oh, I see. So by renewing the cookie every 10min you advance the expiration date to another hour, signaling that the user is still using the app. Kinda like a screen-saver with a password :)

It's worth adding these explanations in the docs directly IMHO. No matter how many times I deal with these issues, I still have to go over the details every time.

bungle commented 9 years ago

Yes, good suggestion and you understood correctly.

  1. We start sessions (send cookies) manually by calling session.start() (start only sends the cookie if no session has been started or if it needs to be renewed) or if we modify session data by calling session:save
  2. We expire them automatically if there is no activity (e.g. request to server) on a client side
  3. But if there is activity, we renew cookie automatically when it is close to expiring (by default 10 mins before it expires). E.g. user sends a request when there is 8 mins for it's cookie to expire. And our code calls session.start(). It finds out that 8 min is less than 10 min and automatically renews the cookie with max. lifetime (configurable with session.cookie.lifetime - by default 1 hour).

If we don't send it back when it's nearing its expiration, the expiration time that is coded in a cookie will make cookie expired and not accepted anymore. If we always send it back when most of our request just check if the user is logged in (and not modify session data) we just lose bandwidth by sending it back when nothing changed. Client will send it to server though until it expires on a client side (time on persistent cookie, or closing the browser and losing the session cookie).

We always check that cookie is not tampered (and some additional tests) and that the cookie has not expired (you cannot change the expiration time on a client side without tampering the cookie). So you cannot edit the cookie (at least not easily, unless there are weaknesses in cryptography and hmac algorithms or that you loose your session.secret, and even then we check by default the ssl_session_id that cannot be easily faked) and have it accepted by the server. Some checks are stored in a cookie, some checks are run in addition to cookie data.

We try to emulate server side sessions as closely as possible (and that is a future feature - while stateless client side sessions are really nice feature, some might want to have server side storage as well - there are limits in cookie sizes, and some think that server side storage is safer, which is somewhat true - stateless client side cookies are easier to scale and there is no need to store anything on server for session handling needs).

akunz commented 9 years ago

Hello Aapo,

thanks for sharing your lua-resty-session lib. I see there is already the encrypted-session-nginx-module which is useable from lua. Please can you give me a hint, what are the benefits of the lua-resty-session lib?

Alexander

Am 17.11.2014 18:01, schrieb Aapo Talvensaari:

  • Feature: Added support for persistent sessions. See issue #2 https://github.com/bungle/lua-resty-session/issues/2.
  • Feature: Added session.check.ssi, session.cookie.persistent and the related Nginx configuration variables.
  • Feature: Added Max-Age=0 to expiration code.

— View it on GitHub https://github.com/bungle/lua-resty-session/releases/tag/v1.3.

bungle commented 9 years ago

Hi Alexander,

The main differences as far as I know are:

  1. encrypted-session-nginx-module is implemented in C as a Nginx module, and lua-resty-session is Lua module (easier to use from Lua code)
  2. encrypted-session-nginx-module doesn't implement secure cookie protocol but something close to it, see also https://github.com/openresty/encrypted-session-nginx-module/issues/2, https://github.com/openresty/encrypted-session-nginx-module/issues/3 and https://github.com/openresty/encrypted-session-nginx-module/issues/4
  3. lua-resty-session has additional checks like ssl_session_id, user agent, remote address and scheme
  4. lua-resty-session is more configurable (you can change hmac and encrypting algorithms for example, encrypted-session-nginx-module uses predefined ones, and it also uses plain md5 as a digest, not a hmac)
  5. lua-resty-session uses different iv for each encryption
  6. lua-resty-session is going to have server side session storage in addition to client side
  7. lua-resty-session is zero-configuration (if you do not want to configure anything - both secret keys and ivs are automatically generated for each cookie independently)
  8. lua-resty-session automatically renews the sessions (but you need to call session.start() as there are no hooks for that in OpenResty).
  9. lua-resty-session is about 220 lines of Lua code and encrypted-session-nginx-module is closer to 700 lines of C code.
  10. lua-resty-session can be easily forked, modified, and contributed etc. without needing to recompile nginx
  11. data is json encoded with lua-resty-template so it is easier to decode and work on lua side (encoding and decoding is done transparently no need to worry about details, just use plain old Lua and its basic data structures and types)
  12. encrypted-session-nginx-module is built by agentzh and lua-resty-template is built by me (Yichun Zhang is one of the best developers there is and he knows the Nginx and OpenResty a lot better than me)

Generally there is a place for both of them. If you are going to build application mainly with Lua, I think lua-resty-session will be a better fit. If you are mainly working with Nginx and without Lua, encrypted-session-nginx-module can be a better choice.

I think that encrypted-session-nginx-module will always have quite basic implementation, but lua-resty-template is more likely to gain different features, as the ideas flourish, like this bug report. Right now the next step is to add support for server side session storages (and make it transparent to user - just add small amounts of configs - e.g. redis connection string, session.storage.redis_host = "localhost" and session.storage = "redis" which is going to be by default session.storage = "client").

Also agentzh, the lead developer of OpenResty, has said that modules should be preferably be built using Lua to make them JITted properly with LuaJIT (e.g. he has reimplemented a lot of ngx.* stack with lua-resty-core).

bungle commented 9 years ago

I'm closing this issue now. Please reopen it if you find bugs etc.

akunz commented 9 years ago

Hello Aapo,

thanks so much for your detailed explanation. Sometimes it's not so easy to see the differences at the first look. You are right, it's a good idea to use lua-resty-session within a lua web-application.

Thanks for putting me in the right direction :-)

Alexander