Session ID not changed after privilege level change

[André Kablu]

Hi guys,

I was creating some code here and could see that session id is not been changed after any privilege level change.

Example:
If I go to my application a session id is set
After login the session is not changed
After logout the session still not changed
If some another user login in the same web-browser window, it will still keep same session id

According to OSWASP

Renew the Session ID After Any Privilege Level Change

The session ID must be renewed or regenerated by the web application after any privilege level change within the associated user session. The most common scenario where the session ID regeneration is mandatory is during the authentication process, as the privilege level of the user changes from the unauthenticated (or anonymous) state to the authenticated state. Other common scenarios must also be considered, such as password changes, permission changes or switching from a regular user role to an administrator role within the web application. For all these web application critical pages, previous session IDs have to be ignored, a new session ID must be assigned to every new request received for the critical resource, and the old or previous session ID must be destroyed.

The most common web development frameworks provide session functions and methods to renew the session ID, such as “request.getSession(true) & HttpSession.invalidate()” (J2EE), “Session.Abandon() & Response.Cookies.Add(new…)“ (ASP .NET), or “session_start() & session_regenerate_id(true)” (PHP).

The session ID regeneration is mandatory to prevent session fixation attacks [3], where an attacker sets the session ID on the victims user web browser instead of gathering the victims session ID, as in most of the other session-based attacks, and independently of using HTTP or HTTPS. This protection mitigates the impact of other web-based vulnerabilities that can also be used to launch session fixation attacks, such as HTTP response splitting or XSS [4].

A complementary recommendation is to use a different session ID or token name (or set of session IDs) pre and post authentication, so that the web application can keep track of anonymous users and authenticated users without the risk of exposing or binding the user session between both states.

https://www.owasp.org/index.php/Session_Management_Cheat_Sheet#Renew_the_Session_ID_After_Any_Privilege_Level_Change

In a short thinking I can realize 2 situations of low security in apps:

1) there is some session variable called session_is_admin
    if admin logs in the browser the variable is_admin will be set and remain set after logoff, then I can simply login again with my user and get admin privileges

2) I can let the app opened in the browser, copy the actual session ID, and after that any user who logs in that window after me will be vulnerable, cause I can reuse this session ID to impersonate this user in another machine, performing a Session hijack. This vulnerability is wide known and commonly exploited in cases that there are shared computers used.

I understand that if I set on_logoff to clear the actual session will solve the problem, however as web2py is secure by default, it should implement this code automatically.

In addition there is no mention of this behavior in the book, what may lead unwarned coders to create vulnerable apps by not knowing of this particularity.

Adding a default/automatic session.clear after logoff and renewing session ID after any privilege level change (including login and logoff) will enhance the "secure by default" title of web2py.

What do you guys think about it?

;)

[Massimo DiPierro]

Hello Andre',

thanks for looking into this.

the session fixation attacks assume the attacker has the ability to "set" the victim session id.  That is not the case of web2py since the ID is a uuid assigned by the server which the client cannot change.

I would like to better understand how an attack would work and if it is possible, in the current web2py scenario. The only possibility is session stealing but changing the session id does not mitigate that in my opinion.

Massimo

--
-- mail from:GoogleGroups "web2py-developers" mailing list
make speech: web2py-d...@googlegroups.com
unsubscribe: web2py-develop...@googlegroups.com
details : http://groups.google.com/group/web2py-developers
the project: http://code.google.com/p/web2py/
official : http://www.web2py.com/
---
You received this message because you are subscribed to the Google Groups "web2py-developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to web2py-develop...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Anthony]

I would like to better understand how an attack would work and if it is possible, in the current web2py scenario. The only possibility is session stealing but changing the session id does not mitigate that in my opinion.

I think both of the scenarios Andre described would be possible with web2py sessions, no?

[Massimo DiPierro]

Both scenarios assume cookie stealing, not session fixation (which is what Andre was concerned about). If the attacker has the ability to steal a cookie (because has access to the communication or the client browser) what difference does it make that the session id was changed? Anyway, if the user logs off, than the session becomes invalid (even if stolen).

I see this possibility though: The attacker steals a session form the browser. He cannot use it. If the legitimate user later logins again and the attacker uses the session at the same time, they would be sharing the session Yet this cookie stealing should not be possible if the server is remote and therefore the communication is going over https, unless perhaps, if the browser window is not closed.

Anyway, I do not oppose to changing this behavior. First of all I would add a function session.reissue(). Second we can conditionally add it to admin login/logout and auth login/logout.

We need to consider the implications of this on existing applications (think of the case of somebody building a shopping cart and then logging in to pay, perhaps session.reissue() should  change the id number but preserve the data) and on load balancers using sticky sessions.

[André Kablu]

This is more a session reuse type of attack than session fixation.

And yes you are right, web2py does not have session fixation vulnerability. In a session fixation attack, the attacker needs to have the ability to change through some parameter the session ID. In the session reuse, you only need to know the session ID of a current client browser, and use it in another browser.

Also this vulnerability only affect people who uses shared computers, or shared offices that does not have desktop lockdown enforcement policies.

The session reuse issue will be set if the attacker have access to victims computer, open the client browser, open application and then copy session ID. After that IF the next user uses the same client browser windows that may already have the application opened to login, the attacker will impersonate him. This may seem unusual but very possible to do.

However In my opinion, the big issue is related to all the session variables that were set to some user (that in my previous example it could be admin flag), they are not flushed after logoff, and as web2py reuses session to the next login they will be set to the next user who logs in the same machine. So the easy and best approach for this is to assign a fresh new session to the user just after the login.

And I agree with you about implications on existing applications that may be using session to keep values for use after login... The best approach was to keep the secure way as default and create something like session.reuse = True, that can be set if user wants to reuse the session, but it will have to be set manually on those applications. Anyway to preserve the transparent migration of the existing applications, the session.reuse = True can be set as default, and added to the documentation the possibility to turn it of.

[Massimo DiPierro]

we do not have to keep backward compatibility for security issues but we need to tell users what the implications are.

would you like to attempt a fix? My recommendation is try minimize changes to tools and admin/controllers/default.py and add one method to class Session.

[Anthony]

On Wednesday, June 12, 2013 11:30:15 AM UTC-4, Massimo Di Pierro wrote:

Both scenarios assume cookie stealing, not session fixation (which is what Andre was concerned about). If the attacker has the ability to steal a cookie (because has access to the communication or the client browser) what difference does it make that the session id was changed?

I don't think he mentioned a specific concern about session fixation (though the OWASP excerpt does mention that vulnerability). The scenarios Andre outlined don't involve remote cookie stealing but situations where someone has physical access to your computer (e.g., you're in an office and walk away from your desk).

 

Anyway, if the user logs off, than the session becomes invalid (even if stolen).

Is that the case? I think the session remains upon logoff -- we just remove the "auth" object from it, right? Any other items in the session will remain, even if they only pertain to someone with logged-in privileges. 

Anyway, I do not oppose to changing this behavior. First of all I would add a function session.reissue(). Second we can conditionally add it to admin login/logout and auth login/logout.

We need to consider the implications of this on existing applications (think of the case of somebody building a shopping cart and then logging in to pay, perhaps session.reissue() should  change the id number but preserve the data) and on load balancers using sticky sessions.

Yes, there could be issues if apps are relying on some session data persisting across pre-login/login/post-login states (particularly going from pre-login to login). Maybe this should be optional. It's probably not a major vulnerability because it seems like an exploit requires physical access to the victim's machine (assuming an HTTPS connection).

Anthony

[André Kablu]

Ok! I'll work on a fix in next days...

I was discussing this issue with a friend that is also heavy web2py coder, Fabiano Engler, and one of the best ways we thought on implement it is:

1) When user login, renew/recreate session id
2) to keep backward compatibility, we can just change the session id and copy all other session variables to the new session
3) on logout we clean session variables automatically (once there is no default need for the variables to be available after logout)
4) we can create 2 settings:  one to keep session vars after logout what the default will be false
                                           one to keep session vars after login, what the default will be true to backward compatibility
   anyway in both cases the session id is not kept, ending the session reuse problems and session vars keeping alive after logout.

Considering that in step 2, there will be no variables to keep from previous session as logout has cleaned them, and there will be only session vars defined for the current user before login

It will be very difficult to change this behavior on class Session, as the main functions are related to login and it will be very difficult to implement without changing Auth.
I also think that there will not be huge changes, just few lines to accomplish all this fix.

[massimo....@gmail.com]

good plan. thanks for this

From my Android phone on T-Mobile. The first nationwide 4G network.

-------- Original message -------- Subject: Re: [web2py-dev] Session ID not changed after privilege level change From: André Kablu To: web2py-d...@googlegroups.com CC:

[André Kablu]

Hi Massimo,

I had finished the fix... I had tested with mysql db, session file, etc...

Use the link to access diff and files on github, so you can do your tests too:
web2py-session-fix

Please tell me if this is what it was expected ;)

Best regards!

[Massimo DiPierro]

Thank you Andre,

I will try apply this later today.

Massimo

[André Kablu]

After putting my head on the pillow just realized that i did one line wrong. .. :/

530 + if response.session_storage_type == 'cookie' or \
531 + not response.session_id:
532 + raise Exception('No session ID to renew')

It cannot raise an exception here. ..
I only forgot to test cookie_key. ..

All the rest is working fine

I'll fix this and upload again tomorrow

Sorry for that

[massimo....@gmail.com]

no problem. tied up until tomorrow. :-(

From my Android phone on T-Mobile. The first nationwide 4G network.

-------- Original message -------- Subject: Re: [web2py-dev] Session ID not changed after privilege level change From: André Kablu To: web2py-d...@googlegroups.com CC:

[André Kablu]

Now it should be working to file, db and cookie!

I did all 3 tests!

https://github.com/KabluBR/web2py-session-fix/commit/c16b09d3c6fbd3c68f57d413390497517a634c1f


Just to describe better the changes:

1) Added session.renew:
          session.renew can be called anytime inside app
          it receives most parameters needed for it to be called manually like db, tablename, masterapp, check_client
          there is a new parameter called clear_session, if true it will perform session.clear, if false it will only change session_id
2) Added response.session_table_name = tablename to session.connect, it will make default tablename value available for use in renew (if used in connect)
3) Added 2 variables to Auth:
        keep_session_onlogin=True, (it is set default to True to keep backward compability, it will not clear session variables after login)
        keep_session_onlogout=False, (it is set default to False to clear session variables after logout, as there is no point to keep them by default)
4) Added calls to session.renew in Auth class, functions login_user and logout referencing to the new keep_session variables


So the session ID will always be changed on login and logout by default, (no need to change them for cookie stored session)
And with simple parameter auth.keep_session_(onlogin/onlogout) you can change the behavior of the clear in renew

:)

Hope everything is fine now! Please test it!

[Niphlod]

A few points of the issue weren't clear to me...
I mean, I'm pretty sure that everybody wants an "destroy it on logout" feature turned on by default.
That's what any (solid) app of mine stores any variable inside session.appname.something and then wipes session.appname with a hookup on the onlogout event....

Given that (now) onlogout trashes the session alltogether by default, why it has to destroy them even on logon ?

PS: sorry for being repetitive (and possibly naive), but I'm not an OWASP guy ^_^

PS2: noticed that client_check parameter is not used in the renew() method

[André Kablu]

No, it will not destroy session variables after login, only change the ID to prevent session reuse.

However if there is some need to destroy session variables after login, I created the setting keep_sessions_onlogin, to start fresh new session if you set to false...

This is only an option, depending of the implementation it could be needed.
Also if you want to clean sessions on login you will try to add session.clean() into onlogin, but it will not work b/c will destroy auth session... thats why I created the new setting.

[Niphlod]

ok, so, to be totally sure (maybe questions will pop up from newbies on web2py-users) ....

Given that the new default is to destroy variables on logout, there's no "security risk" on leaving the session the way they are on logon.

The only thing that changes is that a renew() is called on login, and that changes the session id only.

There's an option to clear session even on login, for ultra-security purposes.

On logout, instead, it's recommended for "old" apps to turn on the "clearing" to avoid potential security risks in shared environments computers.

"old" apps will have their session id renew()ed on logout by default (but not cleared )

Is there any error in these statements ?

[Kablu®]

The only thing that will change in old applications (once you upgrade your framework files) is:
- session id will be changed after login and logout
- session vars will be cleared after logout

And there will exist a possibility to change this in the parameters.

I don't see any backward compatibility issues this way, an this is fixing the 2 situations mentioned above, making sessions secure by default, wirhout need extra configuration.

You received this message because you are subscribed to a topic in the Google Groups "web2py-developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/web2py-developers/fvXIthL_0KY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to web2py-develop...@googlegroups.com.

[Kablu®]

You're right about client _check variable. .. I'd added to my first code and removed later, i forget to remove it from func def. :D

Plz just remove it.

[Massimo DiPierro]

Hello Kablu,

I have included the code from your patch but I have disabled it (added a return statement on top of the renew function). This is because the code does not pass the test in gluon/tests/test_web. The functional test, creates a new user "Homer" logins and expects to see "Welcome Homer" but it fails.

The test relies on gluon/contrib/webclient.py. Webclient detects changes in session_id and interprets them as broken session. At least that is what I would expect and I would know how to fix it. the problem is that Webclient is not detecting the change in session (even with your code) and fails because it does not see "Welcome Homer". Could you help me investigate why this is?

Once this is understood, your patch looks good.

Massimo

[Fabiano Engler Neto]

Hi Massimo,

I am working with Kablu trying to understand what is happening. I noticed two interesting things that wanted to ask you guys about.

When reading the webclient.py code I noticed that it clears self.cookies after every request and just save the cookies that are set again in that last response. That didn't make sense to me as that is not the behaviour you would expect from a browser. On a regular browser, once a cookie is set by the server (with set-cookie header), the browser will send that cookie on ALL following requests (and not only on the next one) -- unless of course that cookie expired, which is not the case here since the session cookie doesn't even have an expire attribute set.

In other words, webclient.py expect the server to set-cookie in every request and retain the cookies only for the immediate next request. For me, this assumption is wrong as that is not how web applications work and it should not work at all in any cases with more than two requests.

What surprised me the most is that actually web2py is setting the session cookie in every response, that's why the webclient.py works at all with web2py, it should not work with other apps. That's what I wanted to ask you guys, is there any particular reason why does web2py sets the cookie in every response? And why does webclient.py depend on this behaviour?

If there is no particular reason and we find that this a bug, myself and Kablu can work on fixes for these too. We are still investigating but this maybe related to why webclient.py is not working properly after Kablu's patch.

Fabiano.

[André Kablu]

Hi Massimo,

I'd found the problem on webclient. :o)
Try this:
https://github.com/KabluBR/web2py-session-fix/commit/cfed76b34ed826091407ea19ba44ea02c4da7f88

Tested on windows / linux and it runs fine now!

However I did not set an environment and changed the webclient code for BASIC AUTH behavior... Can you please try to add the same fix I did for main urllib2.build_opener to the one used by basic auth?

Thanks!

Kablu

[Fabiano Engler Neto]

Explaining Kablu's patch, it indeed had to do with the code in webclient.py that handle cookies.

What happens is that webclient.py is handling the 'set-cookie' header manually and not in a way that is compatible with urllibs handlers, meaning instead of setting a proper handler (based on class urllib2.BaseHandler) it is parsing only the headers returned by urlopen().

This is a problem because when urlopen() is used on login url it actually performs two http requests and the webclient.py code only see the headers of the second one (which is return). This happens because the http status of the first request is redirect (3xx) to index page, this first response contain the correct set-cookie with the new session id, but it is ignored.

Another thing I'd like to comment is about the permissive behaviour of web2py session, which is usually not seen with very good eyes on the security world. This behaviour, summed up with the fact the web2py sets the cookie on every response made this issue very hard to debug. What was happening is that web2py client was sending and _old_ session id (because urlopen() had ignored the set-cookie from renew) and instead of web2py refusing that old and invalid session id, it actually accepted it and created a new session for that id (permissive behaviour) and then set the cookie with the old value. It took us some good hours to understand why web2py was returning the old session even after the renew was called and to understand why that was different from regular browsers ;)

There are other places where the handling of cookies are odd or not in compliance with HTTP standards. I will quote some for discussion:

Consider this patch:

             # move cookies to headers
+            cookies_val = []
             for key, value in cookies.iteritems():
-                req.add_header('Cookie', '%s=%s' % (key, value))
+                cookies_val.append('%s=%s' % (key, value))
+            cookies_val = ','.join(cookies_val)
+            req.add_header('Cookie', cookies_val)

For multiple cookies, current code simply add multiple 'Cookie' headers, which is against the HTTP standard. When web servers see the same header multiple times most of them usually ignore all occurrences but the last one. The patch above put all cookies in a single combine cookie header, which what the web servers expect.

Actually, multiple headers with the same name will never get to the server in this case because Request.add_header() will only retain the last one, as it can be seen on its documentation:

Request.add_header(key, val)

Add another header to the request. (...) Note that there cannot be more than one header with the same name, and later calls will overwrite previous calls in case the key collides.
  http://docs.python.org/2/library/urllib2.html#urllib2.Request.add_header

 
Consider the code below:

-        self.cookies = {}
+        #self.cookies = {}
         if 'set-cookie' in self.headers:
             for item in self.headers['set-cookie'].split(','):
                 key, value = item[:item.find(';')].split('=')
-            self.cookies[key.strip()] = value.strip()
+                self.cookies[key.strip()] = value.strip()

self.cookies is reset after every response, which is odd (see my previous e-mail), as it is expected a browser will retain cookies set on previous requests.

When a server has multiple cookies to set, it will provide only one set-cookie header with multiple cookies in it, separated by a comma char, as the code above expects (it has a split by commas). But the way code is indented bellow it makes it only save the last cookie on the list of cookies set by the server, instead of all of them, as it would be expected.

Another code that didn't quite made sense for me is the following:

        # copy headers from dict to list of key,value
        headers_list = []
        for key, value in self.default_headers.iteritems():
            if not key in headers:
                headers[key] = value

        # add headers to request
        for key, value in headers_list:
            opener.addheaders.append((key, str(value)))

These are the only two occurrences of the variable headers_list in the entire file. The variable headers_list is set to the empty list and then always used as empty, as it is never written to. Also, the comment above doesn't make sense ("copy headers from dict to list of key,value") as the list is not being used and the values are being assigned to a dict again, not a list.

Looking forward for your comments,

Fabiano.

[Massimo DiPierro]

Thank you again Andre

I merged this with some some previous changes in webclient.py. We may have lost the ability to set the Content-Type header in requests. No big deal.

@everybody, the change in session_id in login/logout is a major change in web2py. Please help us test it to make sure everything works well. It is now in trunk.

Massimo

[Massimo DiPierro]

Hello Fabiano,

this behavior is to be considered a bug. It would be great if you could fix it. :-)

massimo

[Kablu®]

Hi Massimo,

I think it was missed 1 debug line at gluon/globals.py:
 line 590       print response.session_id_name, response.session_id

[]'s

Kablu

You received this message because you are subscribed to a topic in the Google Groups "web2py-developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/web2py-developers/fvXIthL_0KY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to web2py-develop...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.
 
 

--
Atc,

André Kablukow
(41) 9215-9200

[Massimo DiPierro]

oops. I had remove it but not pushed. Pushing now.