Univention Bugzilla – Full Text Bug Listing |
Summary: | UMC creates unnecessary amounts of sessions | ||
---|---|---|---|
Product: | UCS | Reporter: | Jürn Brodersen <brodersen> |
Component: | UMC (Generic) | Assignee: | Florian Best <best> |
Status: | CLOSED FIXED | QA Contact: | Jürn Brodersen <brodersen> |
Severity: | normal | ||
Priority: | P5 | CC: | best |
Version: | UCS 4.4 | ||
Target Milestone: | UCS 4.4-1-errata | ||
Hardware: | Other | ||
OS: | Linux | ||
What kind of report is it?: | Bug Report | What type of bug is this?: | 4: Minor Usability: Impairs usability in secondary scenarios |
Who will be affected by this bug?: | 5: Will affect all installed domains | How will those affected feel about the bug?: | 2: A Pain – users won’t like this once they notice it |
User Pain: | 0.229 | Enterprise Customer affected?: | |
School Customer affected?: | ISV affected?: | ||
Waiting Support: | Flags outvoted (downgraded) after PO Review: | ||
Ticket number: | Bug group (optional): | ||
Max CVSS v3 score: | |||
Bug Depends on: | 47106 | ||
Bug Blocks: | 48002 | ||
Attachments: | possible patch |
Description
Jürn Brodersen
2019-07-29 11:18:10 CEST
(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...) 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 |