Hi,
forwarding an off-list discussion. Please review and comment to the list.
Thanks, Hans
---------- Forwarded message ---------- From: Hans Hübner hans@huebner.org Date: Fri, Oct 24, 2008 at 12:54 Subject: Re: hunchentoot session hijacking To: Anton Vodonosov avodonosov@yandex.ru Cc: Edi Weitz edi@agharta.de
Hi Anton,
thank you for your security analysis. I agree that, even though the weakness of the session protection is documented, it would be better to provide for a secure scheme from the beginning. The problem at hand is that there is no portable secure solution. Adding a documented hook to Hunchentoot that allows for setting up a more secure scheme is certainly possible, but I think that this would not be beneficial: Such a hook is trivial, and the concerned user can always implement HUNCHENTOOT:RESET-SESSION-SECRET in a more secure fashion, overriding the default implementation.
Another option would be to make specifying a part of the session secret mandatory at server startup time. I would find this inconvenient, in particular if it cannot be demonstrated that the scheme would be substantially more secure than the existing mechanism. Another option would be to provide for a portable secure implementation, but I certainly lack the time to look at this.
So, if you feel that this is something that needs to be addressed, can you provide us with a patch, either to the source code or to the documentation? A documentation patch could consist of a short summary of your security analysis and a description how the concerned user can make the server more secure.
Thanks, Hans
On Thu, Oct 23, 2008 at 23:38, Anton Vodonosov avodonosov@yandex.ru wrote:
Hello.
I've recently discovered that hunchentoot sessions are quite easy to hijack (while I was investigating generation of unpredictable keys for cl-openid). Therefore I want to suggest a solution to make them more secure.
I know the documentations discourages from considering sessions as really secure, but after some experiments it turned out that in normal case it is a matter of just several hours to hijack hunchentoot sessions.
For example, I know that the hunchentoot-session cookie value is "2561%3A5767E844C4A3FBEDBCA79C349F1DE52B; path=/".
2561 here is SESSION-ID. 3A5767E844C4A3FBEDBCA79C349F1DE52B is value produced by (encode-session-string (session-id session) (session-user-agent session) (session-remote-addr session) ;; usually ignored (session-start session)))
I know my SESSION-USER-AGENT. SESSION-START is most likely the value of the Date header in the server response.
The only unknown value is the *SESSION-SECRET* global used by ENCODE-SESSION-STRING. But it is easy guessable. It is generated by a series or RANDOM invocation. RANDOM implementation is usually deterministic, and therefore *SESSION-SECRET* is fully determined initial value of *THE-RANDOM-STATE*, which in turn is usually fully determined by current time (i.e. server startup time). I.e. *SESSION-SECRET* is fully determined by the server start time.
For example in SBCL, MAKE-RANDOM-STATE uses GET-UNIVERSAL-TIME. Resolution of GET-UNIVERSAL-TIME is one second i.e. if we want to consider a year-long period, we have only (* 365 24 60 60) == 31536000 values to consider. This took several hours of a moderate laptop.
After we know *SESSION-SECRET*, we may easy calculate session cookie for any given session ID (we usually do not know session start time of other people, but it is in the range of 30 minutes from current time for active sessions).
In short, to make session more secure we need unguessable *SESSION-SECRET*. It is possible to use /dev/[u]random on Linux and CryptoAPI on Windows, but the simplest way, in my opinion, would be to provide a public function to set session secret (or a part of session secret). User will set it to some passphrase.
Best regards,
- Anton
on Friday, October 24, 2008, 1:43:18 PM Hans wrote:
So, if you feel that this is something that needs to be addressed, can you provide us with a patch, either to the source code or to the documentation? A documentation patch could consist of a short summary of your security analysis and a description how the concerned user can make the server more secure.
Hi.
My suggestion is in the patch attached. It introduces new variable *session-secretizer* which is supposed to be set by the concerned user to some secret value. *session-secretizer* is used as a part of *session-secret*. If it is not set, *session-secret* is generated as before, but a WARN is issued.
Perhaps *session-secretizer* is a stupid name, but I was not able to contrive anything better, taking into account that *session-secret* is already busy.
Also attached is a small patch to cl+ssl in your repository that makes it compilable on sbcl win32.
Best regards, - Anton
Hi Anton,
thank you for the patches. I have committed the CL+SSL patch both upstream and in my repository.
For the session secret update:
- The name *SESSION-SECRETIZER* really does not cut it. I would suggest that *SESSION-SECRET-SALT* is more suitable. - The docstring and documentation file should be clear about the expected datatype of the variable (string?) - I don't like the warning if the only way to set up the salt is setting the special variable. - Maybe it would be better to supply the concerned user with a way to override the ENCODE-SESSION-STRING function instead of having them mess with the internals of the current session secret encoding function.
I would like to know what Edi thinks about this before committing this or any other patch to the repository.
-Hans
on Monday, October 27, 2008, 12:16:42 PM Hans wrote:
Hi Anton,
thank you for the patches. I have committed the CL+SSL patch both upstream and in my repository.
For the session secret update:
- The name *SESSION-SECRETIZER* really does not cut it. I would
suggest that *SESSION-SECRET-SALT* is more suitable.
Yes, that's better :)
- The docstring and documentation file should be clear about the
expected datatype of the variable (string?)
Yes, I forgot. It must be string.
- I don't like the warning if the only way to set up the salt is
setting the special variable.
- Maybe it would be better to supply the concerned user with a way to
override the ENCODE-SESSION-STRING function instead of having them mess with the internals of the current session secret encoding function.
Overriding ENCODE-SESSION-STRING is more complex responsibility for users - they must deal with four parameters: session-id, user-agent, remote-addres and session-start-time; and with *USE-USER-AGENT-FOR-SESSIONS*, *USE-REMOTE-ADDR-FOR-SESSIONS*; and they must understand that the result must unique and unguessable.
I also thought about providing a possibility to overrride RESET-SESSION-SECRET, as you suggested in the previous email (btw, how "overrride"? just redefine or clos-specialize on artificially introduced parameter). But in this case we need to export both RESET-SESSION-SECRET and *SESSION-SECRET* and explain people that generated secret must every time be not only unguessable but also unique, which is an extra responsibility for them.
Therefore, although I do not like too much introduction of the new special variable, it is the simplest way I see. The only thing we need is the salt, and users just provide it. Hunchentoot does all the remaining job.
I would like to know what Edi thinks about this before committing this or any other patch to the repository.
Ok.
Best regards, - Anton
-
Anton Vodonosov -
Hans Hübner