Fixes lp978896 -- Session fixation security fix
Rotates session tokens on logout, and properly clears sessions to prevent data leakage. Change-Id: I52d03576d07b1e023a7730857156d0da1887b1df
This commit is contained in:
parent
0f952bcdc5
commit
041b1c44c7
@ -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
|
||||
|
@ -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}
|
||||
|
||||
|
@ -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)))
|
||||
|
@ -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."))
|
||||
|
||||
|
@ -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')
|
||||
|
@ -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."),
|
||||
|
Loading…
Reference in New Issue
Block a user