Fixes lp978896 -- Session fixation security fix

Rotates session tokens on logout, and properly clears sessions
to prevent data leakage.

Change-Id: Id11054e852b8c8a386756e9de980cb5eff64f228

horizon/exceptions.py

@@ -203,7 +203,7 @@ def handle(request, message=None, redirect=None, ignore=False, escalate=False):
     if issubclass(exc_type, UNAUTHORIZED):
         if ignore:
             return NotAuthorized
-        request.session.clear()
+        request.user_logout()
         if not handled:
             LOG.debug("Unauthorized: %s" % exc_value)
             # We get some pretty useless error messages back from

horizon/middleware.py

@@ -49,6 +49,15 @@ class HorizonMiddleware(object):
 
         Adds a :class:`~horizon.users.User` object to ``request.user``.
         """
+        # A quick and dirty way to log users out
+        def user_logout(request):
+            if hasattr(request, '_cached_user'):
+                del request._cached_user
+            # Use flush instead of clear, so we rotate session keys in
+            # addition to clearing all the session data
+            request.session.flush()
+        request.__class__.user_logout = user_logout
+
         request.__class__.user = users.LazyUser()
         request.horizon = {'dashboard': None, 'panel': None}
 

horizon/tests/auth_tests.py

@@ -18,6 +18,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import time
+
 from django import http
 from django.core.urlresolvers import reverse
 from keystoneclient import exceptions as keystone_exceptions
@@ -220,3 +222,53 @@ class AuthViewTests(test.TestCase):
 
         self.assertRedirectsNoFollow(res, reverse('splash'))
         self.assertNotIn(KEY, self.client.session)
+
+    def test_session_fixation(self):
+        session_ids = []
+        form_data = {'method': 'Login',
+                     'region': 'http://localhost:5000/v2.0',
+                     'password': self.user.password,
+                     'username': self.user.name}
+
+        self.mox.StubOutWithMock(api, 'token_create')
+        self.mox.StubOutWithMock(api, 'tenant_list_for_token')
+        self.mox.StubOutWithMock(api, 'token_create_scoped')
+
+        aToken = self.tokens.unscoped_token
+        bToken = self.tokens.scoped_token
+
+        api.token_create(IsA(http.HttpRequest), "", self.user.name,
+                         self.user.password).AndReturn(aToken)
+        api.tenant_list_for_token(IsA(http.HttpRequest),
+                                  aToken.id).AndReturn([self.tenants.first()])
+        api.token_create_scoped(IsA(http.HttpRequest),
+                                self.tenant.id,
+                                aToken.id).AndReturn(bToken)
+
+        api.token_create(IsA(http.HttpRequest), "", self.user.name,
+                         self.user.password).AndReturn(aToken)
+        api.tenant_list_for_token(IsA(http.HttpRequest),
+                                  aToken.id).AndReturn([self.tenants.first()])
+        api.token_create_scoped(IsA(http.HttpRequest),
+                                self.tenant.id,
+                                aToken.id).AndReturn(bToken)
+        self.mox.ReplayAll()
+
+        res = self.client.get(reverse('horizon:auth_login'))
+        self.assertEqual(res.cookies.get('sessionid'), None)
+        res = self.client.post(reverse('horizon:auth_login'), form_data)
+        session_ids.append(res.cookies['sessionid'].value)
+
+        self.assertEquals(self.client.session['user_name'],
+                          self.user.name)
+        self.client.session['foobar'] = 'MY TEST VALUE'
+        res = self.client.get(reverse('horizon:auth_logout'))
+        session_ids.append(res.cookies['sessionid'].value)
+        self.assertEqual(len(self.client.session.items()), 0)
+        # Sleep for 1 second so the session values are different if
+        # using the signed_cookies backend.
+        time.sleep(1)
+        res = self.client.post(reverse('horizon:auth_login'), form_data)
+        session_ids.append(res.cookies['sessionid'].value)
+        # Make sure all 3 session id values are different
+        self.assertEqual(len(session_ids), len(set(session_ids)))

horizon/users.py

@@ -59,7 +59,7 @@ def get_user_from_request(request):
         # If any of those keys are missing from the session it is
         # overwhelmingly likely that we're dealing with an outdated session.
         LOG.exception("Error while creating User from session.")
-        request.session.clear()
+        request.user_logout()
         raise exceptions.NotAuthorized(_("Your session has expired. "
                                          "Please log in again."))
 

horizon/views/auth.py

@@ -96,6 +96,6 @@ def switch_tenants(request, tenant_id):
 
 def logout(request):
     """ Clears the session and logs the current user out. """
-    request.session.clear()
+    request.user_logout()
     # FIXME(gabriel): we don't ship a view named splash
     return shortcuts.redirect('splash')

horizon/views/auth_forms.py

@@ -77,6 +77,16 @@ class Login(forms.SelfHandlingForm):
             self.fields['region'].widget = forms.widgets.HiddenInput()
 
     def handle(self, request, data):
+        if 'user_name' in request.session:
+            if request.session['user_name'] != data['username']:
+                # To avoid reusing another user's session, create a
+                # new, empty session if the existing session
+                # corresponds to a different authenticated user.
+                request.session.flush()
+        # Always cycle the session key when viewing the login form to
+        # prevent session fixation
+        request.session.cycle_key()
+
         # For now we'll allow fallback to OPENSTACK_KEYSTONE_URL if the
         # form post doesn't include a region.
         endpoint = data.get('region', None) or settings.OPENSTACK_KEYSTONE_URL
@@ -116,7 +126,7 @@ class Login(forms.SelfHandlingForm):
                 # If we get here we don't want to show a stack trace to the
                 # user. However, if we fail here, there may be bad session
                 # data that's been cached already.
-                request.session.clear()
+                request.user_logout()
                 exceptions.handle(request,
                                   message=_("An error occurred authenticating."
                                             " Please try again later."),