From f2a32f2a9f3fb8be1a9432ac3d81d3aacdb13095 Mon Sep 17 00:00:00 2001 From: Patrik Dufresne Date: Mon, 17 Oct 2022 18:58:27 -0400 Subject: [PATCH] Define idle and absolute session timeout with agressive default to protect usage on public computer --- README.md | 1 + doc/configuration.md | 10 +++++ rdiffweb/controller/tests/test_page_login.py | 21 ++++++++++ rdiffweb/core/config.py | 29 +++++++++----- rdiffweb/rdw_app.py | 5 ++- rdiffweb/tools/auth_form.py | 42 +++++++++++++------- 6 files changed, 80 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 11b3d588..be717e7b 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ This next release focus on two-factor-authentication as a measure to increase se * Enforce password policy new password cannot be set as new password [CVE-2022-3376](https://nvd.nist.gov/vuln/detail/CVE-2022-3376) * Enforce better rate limit on login, mfa, password change and API [CVE-2022-3439](https://nvd.nist.gov/vuln/detail/CVE-2022-3439) [CVE-2022-3456](https://nvd.nist.gov/vuln/detail/CVE-2022-3456) * Enforce 'Origin' validation [CVE-2022-3457](https://nvd.nist.gov/vuln/detail/CVE-2022-3457) +* Define idle and absolute session timeout with agressive default to protect usage on public computer [CVE-2022-3327](https://nvd.nist.gov/vuln/detail/CVE-2022-3327) Breaking changes: diff --git a/doc/configuration.md b/doc/configuration.md index ec795b05..387bcfa7 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -162,6 +162,16 @@ Here is an example of how you may limit Rdiffweb access to members of *Admin_Bac ldap-group-attribute=memberUid ldap-group-attribute-is-dn=false +## Configure User Session + +A user session is a sequence of request and response transactions associated with a single user. The user session is the means to track each authenticated user. + +| Option | Description | Example | +| --- | --- | --- | +| session-idle-timeout | This timeout defines the amount of time a session will remain active in case there is no activity in the session. User Session will be revoke after this period of inactivity, unless the user selected "remember me". Default 10 minutes. | 5 | +| session-absolute-timeout | This timeout defines the maximum amount of time a session can be active. After this period, user is forced to (re)authenticate, unless the user selected "remember me". Default 20 minutes. | 30 | +| session-persistent-timeout | This timeout defines the maximum amount of time to remember and trust a user device. This timeout is used when user select "remember me". Default 30 days | 43200 | + ## Configure email notifications Since Rdiffweb v0.9, you may configure Rdiffweb to send an email notification to the users when their backups did not complete successfully for a period of time. diff --git a/rdiffweb/controller/tests/test_page_login.py b/rdiffweb/controller/tests/test_page_login.py index 9f2b760d..1f096b42 100644 --- a/rdiffweb/controller/tests/test_page_login.py +++ b/rdiffweb/controller/tests/test_page_login.py @@ -108,6 +108,9 @@ def test_login(self, unused, original_url): # Then user is redirected to original URL self.assertStatus('303 See Other') self.assertHeaderItemValue('Location', self.baseurl + original_url) + # Then cookie is not persistent + self.assertNotIn('expires', self.cookies[0][1]) + self.assertNotIn('Max-Age', self.cookies[0][1]) # When requesting the original page self.getPage(original_url) # Then page return without error @@ -177,6 +180,24 @@ def test_login_twice(self): self.assertStatus(200) self.assertInBody(self.USERNAME) + def test_login_persistent(self): + # Given a user authenticated with persistent + self.getPage('/logout/') + self.assertStatus(303) + self.getPage( + '/login/', method='POST', body={'login': self.USERNAME, 'password': self.PASSWORD, 'persistent': '1'} + ) + self.assertStatus(303) + # Then a persistent cookie is return + self.assertIn('expires', self.cookies[0][1]) + self.assertIn('Max-Age', self.cookies[0][1]) + # Then a session is created with persistent flag + session = DbSession(id=self.session_id) + session.load() + self.assertTrue(session['login_persistent']) + # Then session timeout is 30 days in future + self.assertAlmostEqual(session.timeout, 43200, delta=2) + class LoginPageWithWelcomeMsgTest(rdiffweb.test.WebCase): diff --git a/rdiffweb/core/config.py b/rdiffweb/core/config.py index b5eef33f..b560ab86 100644 --- a/rdiffweb/core/config.py +++ b/rdiffweb/core/config.py @@ -387,25 +387,32 @@ def get_parser(): ) parser.add( - '--session-timeout', - metavar='MINUTES', - help='Sessions will be revoke after this period of inactivity, unless the user selected "remember me". Default 15 minutes.', - default=15, + '--rate-limit', + metavar='LIMIT', + type=int, + default=20, + help='maximum number of requests per hour that can be made on sensitive endpoints. When this limit is reached, an HTTP 429 message is returned to the user or the user is logged out. This security measure is used to limit brute force attacks on the login page and the RESTful API.', ) parser.add( - '--session-persistent-timeout', + '--session-idle-timeout', metavar='MINUTES', - help='Persistent sessions (remember me) will be revoke after this period of inactivity. Default 30 days.', - default=43200, + help='This timeout defines the amount of time a session will remain active in case there is no activity in the session. User Session will be revoke after this period of inactivity, unless the user selected "remember me". Default 5 minutes.', + default=5, ) parser.add( - '--rate-limit', - metavar='LIMIT', - type=int, + '--session-absolute-timeout', + metavar='MINUTES', + help='This timeout defines the maximum amount of time a session can be active. After this period, user is forced to (re)authenticate, unless the user selected "remember me". Default 20 minutes.', default=20, - help='maximum number of requests per hour that can be made on sensitive endpoints. When this limit is reached, an HTTP 429 message is returned to the user or the user is logged out. This security measure is used to limit brute force attacks on the login page and the RESTful API.', + ) + + parser.add( + '--session-persistent-timeout', + metavar='MINUTES', + help='This timeout defines the maximum amount of time to remember and trust a user device. This timeout is used when user select "remember me". Default 30 days.', + default=43200, ) parser.add( diff --git a/rdiffweb/rdw_app.py b/rdiffweb/rdw_app.py index edf02209..34a1e677 100644 --- a/rdiffweb/rdw_app.py +++ b/rdiffweb/rdw_app.py @@ -205,9 +205,10 @@ def __init__(self, cfg): 'tools.sessions.debug': cfg.debug, 'tools.sessions.storage_class': DbSession, 'tools.sessions.httponly': True, - 'tools.sessions.timeout': cfg.session_timeout, # minutes + 'tools.sessions.timeout': cfg.session_idle_timeout, # minutes 'tools.sessions.persistent': False, # auth_form should update this. - 'tools.auth_form.timeout': cfg.session_persistent_timeout, # minutes + 'tools.auth_form.persistent_timeout': cfg.session_persistent_timeout, # minutes + 'tools.auth_form.absolute_timeout': cfg.session_absolute_timeout, # minutes 'tools.ratelimit.debug': cfg.debug, 'tools.ratelimit.delay': 3600, 'tools.ratelimit.limit': cfg.rate_limit, diff --git a/rdiffweb/tools/auth_form.py b/rdiffweb/tools/auth_form.py index f98b2512..7a2442c9 100644 --- a/rdiffweb/tools/auth_form.py +++ b/rdiffweb/tools/auth_form.py @@ -46,12 +46,9 @@ def _is_login(self): return False # Verify session + # We don't need to verify the timeout value since expired session get deleted automatically. session = cherrypy.session - return ( - session.get(SESSION_KEY) is not None - and session.get(LOGIN_TIME) is not None - and session[LOGIN_TIME] + datetime.timedelta(minutes=session.timeout) > session.now() - ) + return session.get(SESSION_KEY) is not None and session.get(LOGIN_TIME) is not None def _get_redirect_url(self): """ @@ -68,11 +65,32 @@ def _set_redirect_url(self): new_url = cherrypy.url(original_url, qs=qs, base='') cherrypy.session[LOGIN_REDIRECT_URL] = new_url + def _update_session_timeout(self, persistent_timeout=43200, absolute_timeout=30): + """ + Since we have multiple timeout value (idle, absolute and persistent) We need to update the session timeout and possibly the cookie timeout. + """ + persistent_timeout = cherrypy.request.config.get('tools.auth_form.persistent_timeout', 43200) + absolute_timeout = cherrypy.request.config.get('tools.auth_form.absolute_timeout', 30) + # If login is persistent, update the cookie max-age/expires + session = cherrypy.session + if session.get(LOGIN_PERSISTENT, False): + expiration = session[LOGIN_TIME] + datetime.timedelta(minutes=persistent_timeout) + session.timeout = int((expiration - session.now()).total_seconds() / 60) + cookie = cherrypy.serving.response.cookie + cookie['session_id']['max-age'] = session.timeout * 60 + cookie['session_id']['expires'] = httputil.HTTPDate(time.time() + session.timeout * 60) + else: + session_idle_timeout = cherrypy.request.config.get('tools.sessions.timeout', 60) + expiration1 = session.now() + datetime.timedelta(minutes=session_idle_timeout) + expiration2 = session[LOGIN_TIME] + datetime.timedelta(minutes=absolute_timeout) + expiration = min(expiration1, expiration2) + session.timeout = int((expiration - session.now()).total_seconds() / 60) + def redirect_to_original_url(self): # Redirect user to original URL raise cherrypy.HTTPRedirect(self._get_redirect_url()) - def run(self, login_url='/login/', logout_url='/logout', timeout=43200): + def run(self, login_url='/login/', logout_url='/logout', persistent_timeout=43200, absolute_timeout=30): """ A tool that verify if the session is associated to a user by tracking a session key. If session is not authenticated, redirect user to login page. @@ -96,15 +114,7 @@ def run(self, login_url='/login/', logout_url='/logout', timeout=43200): # And redirect to login page raise cherrypy.HTTPRedirect(login_url) - # If login is persistent, update the cookie max-age/expires - if cherrypy.session.get(LOGIN_PERSISTENT, False): - cherrypy.session.timeout = timeout - cookie = cherrypy.serving.response.cookie - cookie['session_id']['max-age'] = timeout * 60 - cookie['session_id']['expires'] = httputil.HTTPDate(time.time() + timeout * 60) - else: - session_timeout = cherrypy.request.config.get('tools.sessions.timeout', 60) - cherrypy.session.timeout = session_timeout + self._update_session_timeout() def login(self, username, persistent=False): """ @@ -116,6 +126,8 @@ def login(self, username, persistent=False): cherrypy.session[LOGIN_TIME] = cherrypy.session.now() # Generate a new session id cherrypy.session.regenerate() + # Update the session timeout + self._update_session_timeout() def logout(self): # Clear session date and generate a new session id