From abbcf172b49d17324293b6aae06fd5882f07aaad Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Fri, 11 Nov 2016 17:54:34 +0000 Subject: [PATCH] Backport fix for CVE-2016-8638 Signed-off-by: Patrick Uiterwijk --- ipsilon/login/common.py | 9 +++++---- ipsilon/providers/saml2/logout.py | 6 +++--- ipsilon/providers/saml2/sessions.py | 25 +++++++++++-------------- ipsilon/providers/saml2idp.py | 2 +- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/ipsilon/login/common.py b/ipsilon/login/common.py index cd4f166..8f69a16 100644 --- a/ipsilon/login/common.py +++ b/ipsilon/login/common.py @@ -283,10 +283,11 @@ class Logout(Page): def root(self, *args, **kwargs): us = UserSession() - for provider in self.handlers: - self.debug("Calling logout for provider %s" % provider) - obj = self.handlers[provider] - obj() + if us.user is not None: + for provider in self.handlers: + self.debug("Calling logout for provider %s" % provider) + obj = self.handlers[provider] + obj() us.logout(self.user) return self._template('logout.html', title='Logout') diff --git a/ipsilon/providers/saml2/logout.py b/ipsilon/providers/saml2/logout.py index cc9b777..9861c85 100644 --- a/ipsilon/providers/saml2/logout.py +++ b/ipsilon/providers/saml2/logout.py @@ -217,7 +217,7 @@ class LogoutRequest(ProviderPageBase): # Fall through to handle any remaining sessions. # Find the next SP to logout and send a LogoutRequest - session = saml_sessions.get_next_logout() + session = saml_sessions.get_next_logout(us.user) if session: self.debug('Going to log out %s' % session.provider_id) @@ -241,7 +241,7 @@ class LogoutRequest(ProviderPageBase): # log out self.debug('logging out provider id %s' % session.provider_id) indexes = saml_sessions.get_session_id_by_provider_id( - session.provider_id + session.provider_id, us.user ) self.debug('Requesting logout for sessions %s' % indexes) req = logout.get_request() @@ -262,7 +262,7 @@ class LogoutRequest(ProviderPageBase): # response we cached earlier. try: - session = saml_sessions.get_initial_logout() + session = saml_sessions.get_initial_logout(us.user) except ValueError: self.debug('SLO get_last_session() unable to find last session') raise cherrypy.HTTPError(400, 'Unable to determine logout state') diff --git a/ipsilon/providers/saml2/sessions.py b/ipsilon/providers/saml2/sessions.py index 1000a87..41f1181 100644 --- a/ipsilon/providers/saml2/sessions.py +++ b/ipsilon/providers/saml2/sessions.py @@ -103,7 +103,6 @@ class SAMLSessionFactory(Log): """ def __init__(self, database_url): self._ss = SAML2SessionStore(database_url=database_url) - self.user = None def _data_to_samlsession(self, uuidval, data): """ @@ -125,8 +124,6 @@ class SAMLSessionFactory(Log): """ Add a new login session to the table. """ - self.user = user - timeout = cherrypy_config['tools.sessions.timeout'] t = datetime.timedelta(seconds=timeout * 60) expiration_time = datetime.datetime.now() + t @@ -157,11 +154,11 @@ class SAMLSessionFactory(Log): return self._data_to_samlsession(uuidval, data) - def get_session_id_by_provider_id(self, provider_id): + def get_session_id_by_provider_id(self, provider_id, user): """ Return a tuple of logged-in session IDs by provider_id """ - candidates = self._ss.get_user_sessions(self.user) + candidates = self._ss.get_user_sessions(user) session_ids = [] for c in candidates: @@ -209,7 +206,7 @@ class SAMLSessionFactory(Log): datum = samlsession.convert() self._ss.update_session(datum) - def get_next_logout(self, peek=False): + def get_next_logout(self, user, peek=False): """ Get the next session in the logged-in state and move it to the logging_out state. Return the session that is @@ -221,7 +218,7 @@ class SAMLSessionFactory(Log): Return None if no more sessions in LOGGED_IN state. """ - candidates = self._ss.get_user_sessions(self.user) + candidates = self._ss.get_user_sessions(user) for c in candidates: key = c.keys()[0] @@ -231,13 +228,13 @@ class SAMLSessionFactory(Log): return samlsession return None - def get_initial_logout(self): + def get_initial_logout(self, user): """ Get the initial logout request. Return None if no sessions in INIT_LOGOUT state. """ - candidates = self._ss.get_user_sessions(self.user) + candidates = self._ss.get_user_sessions(user) # FIXME: what does it mean if there are multiple in init? We # just return the first one for now. How do we know @@ -253,11 +250,11 @@ class SAMLSessionFactory(Log): def wipe_data(self): self._ss.wipe_data() - def dump(self): + def dump(self, user): """ Dump all sessions to debug log """ - candidates = self._ss.get_user_sessions(self.user) + candidates = self._ss.get_user_sessions(user) count = 0 for c in candidates: @@ -280,11 +277,11 @@ if __name__ == '__main__': sess2 = factory.add_session('_789012', provider2, "testuser", "") # Test finding sessions by provider - ids = factory.get_session_id_by_provider_id(provider2) + ids = factory.get_session_id_by_provider_id(provider2, 'admin') assert(len(ids) == 1) sess3 = factory.add_session('_345678', provider2, "testuser", "") - ids = factory.get_session_id_by_provider_id(provider2) + ids = factory.get_session_id_by_provider_id(provider2, 'testuser') assert(len(ids) == 2) # Test finding sessions by session ID @@ -315,5 +312,5 @@ if __name__ == '__main__': # Even though we've started logout, make sure we can still find # all sessions for a provider. - ids = factory.get_session_id_by_provider_id(provider2) + ids = factory.get_session_id_by_provider_id(provider2, 'admin') assert(len(ids) == 2) diff --git a/ipsilon/providers/saml2idp.py b/ipsilon/providers/saml2idp.py index ad31d8f..965a960 100644 --- a/ipsilon/providers/saml2idp.py +++ b/ipsilon/providers/saml2idp.py @@ -392,7 +392,7 @@ Provides SAML 2.0 authentication infrastructure. """ user = us.get_user() saml_sessions = self.sessionfactory - session = saml_sessions.get_next_logout() + session = saml_sessions.get_next_logout(us.user) if session is None: return -- 2.10.1