changeset 1598:86c7a3a625d5

server: always start a new session on connection: The session was kept when a user was connecting from service profile (but not from other profiles), this was leading to session fixation vulnerability (an attacker on the same machine could get service profile session cookie, and use it when a victim would log-in). This patch fixes it by always starting a new session on connection. fix 443
author Goffi <goffi@goffi.org>
date Fri, 23 Feb 2024 13:35:24 +0100 (9 months ago)
parents c1c1d68d063e
children 197350e8bf3b
files libervia/web/server/server.py libervia/web/server/session_iface.py
diffstat 2 files changed, 16 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/libervia/web/server/server.py	Sun Feb 11 21:32:53 2024 +0100
+++ b/libervia/web/server/server.py	Fri Feb 23 13:35:24 2024 +0100
@@ -831,8 +831,7 @@
         state = C.PROFILE_LOGGED_EXT_JID if register_with_ext_jid else C.PROFILE_LOGGED
         return state
 
-    @defer.inlineCallbacks
-    def connect(self, request, login, password):
+    async def connect(self, request, login, password):
         """log user in
 
         If an other user was already logged, it will be unlogged first
@@ -854,7 +853,6 @@
         @raise ValueError(C.PROFILE_AUTH_ERROR): invalid login and/or password
         @raise ValueError(C.XMPP_AUTH_ERROR): invalid XMPP account password
         """
-
         # XXX: all security checks must be done here, even if present in javascript
         if login.startswith("@"):
             raise failure.Failure(exceptions.DataError("No profile_key allowed"))
@@ -872,7 +870,7 @@
                 raise failure.Failure(exceptions.DataError("No profile_key allowed"))
 
             # FIXME: should it be cached?
-            new_account_domain = yield self.bridge_call("account_domain_new_get")
+            new_account_domain = await self.bridge_call("account_domain_new_get")
 
             if login_jid.host == new_account_domain:
                 # redirect "user@libervia.org" to the "user" profile
@@ -882,7 +880,7 @@
             login_jid = None
 
         try:
-            profile = yield self.bridge_call("profile_name_get", login)
+            profile = await self.bridge_call("profile_name_get", login)
         except Exception:  # XXX: ProfileUnknownError wouldn't work, it's encapsulated
             # FIXME: find a better way to handle bridge errors
             if (
@@ -914,17 +912,19 @@
 
             connect_method = "connect"
 
-        # we check if there is not already an active session
+        # we check the active session
         web_session = session_iface.IWebSession(request.getSession())
-        if web_session.profile:
-            # yes, there is
-            if web_session.profile != profile:
-                # it's a different profile, we need to disconnect it
+        if web_session.profile != profile:
+            # It's a different profile, we need to disconnect it.
+            # We always purge session even if no user was logged, to avoid the re-use of
+            # cookie (i.e. to avoid session fixation).
+            if web_session.profile:
+                # no need to log a warning if the previous profile was the service profile
                 log.warning(_(
                     "{new_profile} requested login, but {old_profile} was already "
                     "connected, disconnecting {old_profile}").format(
                         old_profile=web_session.profile, new_profile=profile))
-                self.purge_session(request)
+            self.purge_session(request)
 
         if self.waiting_profiles.get_request(profile):
             #  FIXME: check if and when this can happen
@@ -932,7 +932,7 @@
 
         self.waiting_profiles.set_request(request, profile, register_with_ext_jid)
         try:
-            connected = yield self.bridge_call(connect_method, profile, password)
+            connected = await self.bridge_call(connect_method, profile, password)
         except Exception as failure_:
             fault = getattr(failure_, 'classname', None)
             self.waiting_profiles.purge_request(profile)
@@ -971,7 +971,7 @@
                         "profile [{profile}], this should not happen!")
                             .format(session_profile=web_session.profile, profile=profile))
                     raise exceptions.InternalError("profile mismatch")
-                defer.returnValue(C.SESSION_ACTIVE)
+                return C.SESSION_ACTIVE
             log.info(
                 _(
                     "profile {profile} was already connected in backend".format(
@@ -981,8 +981,8 @@
             )
             #  no, we have to create it
 
-        state = yield defer.ensureDeferred(self._logged(profile, request))
-        defer.returnValue(state)
+        state = await self._logged(profile, request)
+        return state
 
     async def check_registration_id(self, request: server.Request) -> None:
         """Check if a valid registration ID is found in request data
--- a/libervia/web/server/session_iface.py	Sun Feb 11 21:32:53 2024 +0100
+++ b/libervia/web/server/session_iface.py	Fri Feb 23 13:35:24 2024 +0100
@@ -38,7 +38,7 @@
 
 
 class IWebSession(Interface):
-    profile = Attribute("Sat profile")
+    profile = Attribute("Libervia profile")
     jid = Attribute("JID associated with the profile")
     uuid = Attribute("uuid associated with the profile session")
     identities = Attribute("Identities of XMPP entities")