Univention Bugzilla – Bug 49929
UMC creates unnecessary amounts of sessions
Last modified: 2019-07-31 13:58:47 CEST
Created attachment 10135 [details] possible patch UMC creates unnecessary amounts of sessions What is the Problem: A new session means a new module process is created which takes time to start, needs memory and the umc server only connects to the latest process of a module, which can result in a state loss (especially for progress calls). This might also be the reason for some of the random "Forbidden" and cross site scripting warnings. How to reproduce: "ucr set umc/module/timeout=30" for convenience Open the diagnostic module "tail -f /var/log/univention/management-console-server.log" wait for "MAIN ( PROCESS ) : Processor: dying" in the log In the diagnostics module click the button to run the diagnostic again. You should now see multiple umc module processes for the diagnostic module The attached patch locks the auth request because I think I had problems with bug 47106. It also uses get_sessionid instead of create_sessionid to avoid multiple sessions. The basic problem seems to be that a new session_id is created by the create_sessionid if no session is found for the sessionid in the cookie, but the cookie is not updated immediately which can result in a new session again. I'm not sure about possible side effects because I don't really know why create_sessionid was used in the first place.
(In reply to Jürn Brodersen from comment #0) > Created attachment 10135 [details] > possible patch > > UMC creates unnecessary amounts of sessions > > What is the Problem: > A new session means a new module process is created which takes time to > start, needs memory and the umc server only connects to the latest process > of a module, which can result in a state loss (especially for progress > calls). More precisely: A new session (in the UMC-Web-Server) means one new connection to the UMC-Server. That connection may have multiple singleton-module-processes. Therefore the loss of a session means state loss and breaks e.g. progress calls. So we have to ensure that every request in the same UMC session/tab/browser window uses the same connection to the UMC-Server. > This might also be the reason for some of the random "Forbidden" and cross > site scripting warnings. Would be nice, if we find the reason for this :-) > How to reproduce: > "ucr set umc/module/timeout=30" for convenience > Open the diagnostic module > "tail -f /var/log/univention/management-console-server.log" > wait for "MAIN ( PROCESS ) : Processor: dying" in the log > In the diagnostics module click the button to run the diagnostic again. > You should now see multiple umc module processes for the diagnostic module "Processor: dying" means that the module process has ended after $module_timeout seconds of inactivity (with no open request for that module process). > The attached patch locks the auth request because I think I had problems > with bug 47106. It also uses get_sessionid instead of create_sessionid to > avoid multiple sessions. The basic problem seems to be that a new session_id > is created by the create_sessionid if no session is found for the sessionid > in the cookie, but the cookie is not updated immediately which can result in > a new session again. > I'm not sure about possible side effects because I don't really know why > create_sessionid was used in the first place. It is a security risk to use a cookie value which is generated by the client, not by the server. Therefore we must use create_session(). If there are multiple requests in the queue they have to wait for the answer of the authentication request and use that session then. I was currently searching for the cause in the UMC-Server, but your idea is an interesting approach, thanks for that analysis so far.
I think this patch is enough: diff --git a/management/univention-management-console/univention-management-console-web-server b/management/univention-management-console/univention-management-console-web-server index f3b25c92f3..1806675569 100755 --- a/management/univention-management-console/univention-management-console-web-server +++ b/management/univention-management-console/univention-management-console-web-server @@ -616,12 +616,16 @@ class Ressource(object): # we must not change the session ID cookie as this might cause # race conditions in the frontend during login, especially when logged in via SAML return self.get_session_id() + user = self.get_user() + if user: # TODO:? and user.time_remaining > 1: + return user.sessionid if random: return str(uuid.uuid4()) return sessionidhash() def get_session_id(self): """get the current session ID from cookie.""" + # caution: use this wisely! do not create a new session with this ID (value coming from the Client)! return self.get_cookie('UMCSessionId') or sessionidhash() def get_session(self):
Fixed :-) If there was already a session, we use the same sesssionid again :-) This is done already in a similar manner if there is another module process running. This is a second fallack for the case where all module processes (i.e. the connection to the UMC-Server) are ended. univention-management-console.yaml c2956332660f | Bug #49929: do not create new session ids after module process dies univention-management-console (11.0.4-31) c2956332660f | Bug #49929: do not create new session ids after module process dies
I really like the fix :) But now bug 47106 is triggered... :( Meaning some request are never answered (in my case the first five) I see two possible solutions: fix bug 47106 add a lock around the auth request (I prefer fixing 47106...)
Set to resolved again because bug 47106 is fixed
What I tested: Login with saml -> OK Login without saml -> OK @require_password decorator with saml -> OK Logout with saml -> OK Logout without saml -> OK Password change -> OK Can't reproduce comment 1 any more -> OK As fa as I can tell the UMC Session ID only changes on a logout -> OK yaml -> OK
<http://errata.software-univention.de/ucs/4.4/203.html>