CVE-2022-3327: Missing Authentication for Critical Function (CWE-306)
value: Missing Authentication for Critical Function in GitHub repository ikus060/rdiffweb prior to 2.5.0a6.
CWE description: The product does not perform any authentication for functionality that requires a provable user identity or consumes a significant amount of resources.
CVSS v3 base score: 9.8
Published: 2022-10-20T00:15Z
Repository: https://github.com/ikus060/rdiffweb
Language: Python
Files changed: README.md, doc/configuration.md, rdiffweb/controller/tests/test_page_login.py, rdiffweb/core/config.py, rdiffweb/rdw_app.py, rdiffweb/tools/auth_form.py
Commit date: 2022-10-17 18:58:27 -0400
Commit message: Define idle and absolute session timeout with agressive default to protect usage on public computer

Diff: diff --git a/README.md b/README.md
index 11b3d5884..be717e7b1 100644
--- a/README.md
+++ b/README.md
@@ -135,10 +135,11 @@ This next release focus on two-factor-authentication as a measure to increase se
 * Enforce validation on fullname, username and email
 * Limit incorrect attempts to change the user's password to prevent brute force attacks #225 [CVE-2022-3273](https://nvd.nist.gov/vuln/detail/CVE-2022-3273)
 * 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:
 
 * Drop Ubuntu Hirsute & Impish (End-of-life)
 * `session-dir` is deprecated and should be replace by `rate-limit-dir`. User's session are stored in database.
diff --git a/doc/configuration.md b/doc/configuration.md
index ec795b059..387bcfa7f 100644
--- a/doc/configuration.md
+++ b/doc/configuration.md
@@ -160,10 +160,20 @@ Here is an example of how you may limit Rdiffweb access to members of *Admin_Bac
 
     ldap-required-group=cn=Admin_Backup,ou=Groups,dc=nodomain
     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.
 When enabled, Rdiffweb will also send email notification for security reason when user's password is changed.
 
diff --git a/rdiffweb/controller/tests/test_page_login.py b/rdiffweb/controller/tests/test_page_login.py
index 9f2b760d6..1f096b42f 100644
--- a/rdiffweb/controller/tests/test_page_login.py
+++ b/rdiffweb/controller/tests/test_page_login.py
@@ -106,10 +106,13 @@ class LoginPageTest(rdiffweb.test.WebCase):
         # When authentication is successful
         self.getPage('/login/', method='POST', body={'login': self.USERNAME, 'password': self.PASSWORD})
         # 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
         self.assertStatus(200)
 
@@ -175,10 +178,28 @@ class LoginPageTest(rdiffweb.test.WebCase):
         self.assertHeaderItemValue('Location', self.baseurl + "/")
         self.getPage('/')
         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):
 
     default_config = {'welcomemsg': 'default message', 'welcomemsg[fr]': 'french message'}
 
diff --git a/rdiffweb/core/config.py b/rdiffweb/core/config.py
index b5eef33f8..b560ab86a 100644
--- a/rdiffweb/core/config.py
+++ b/rdiffweb/core/config.py
@@ -385,29 +385,36 @@ def get_parser():
         metavar='FOLDER',
         help='location where to store rate-limit information. When undefined, the data is kept in memory. `--session-dir` are deprecated and kept for backward compatibility.',
     )
 
     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(
         '--ssl-certificate',
         '--sslcertificate',
diff --git a/rdiffweb/rdw_app.py b/rdiffweb/rdw_app.py
index edf022093..34a1e677a 100644
--- a/rdiffweb/rdw_app.py
+++ b/rdiffweb/rdw_app.py
@@ -203,13 +203,14 @@ class RdiffwebApp(Application):
                 'error_page.default': self.error_page,
                 'tools.sessions.on': True,
                 '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,
                 'tools.ratelimit.storage_class': rate_limit_storage_class,
                 'tools.ratelimit.storage_path': cfg.rate_limit_dir,
diff --git a/rdiffweb/tools/auth_form.py b/rdiffweb/tools/auth_form.py
index f98b25123..7a2442c9c 100644
--- a/rdiffweb/tools/auth_form.py
+++ b/rdiffweb/tools/auth_form.py
@@ -44,16 +44,13 @@ class CheckAuthForm(cherrypy.Tool):
         sessions_on = request.config.get('tools.sessions.on', False)
         if not sessions_on:
             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):
         """
         Return the original URL the user browser before getting redirect to login.
         """
@@ -66,15 +63,36 @@ class CheckAuthForm(cherrypy.Tool):
         original_url = urllib.parse.quote(request.path_info, encoding=uri_encoding)
         qs = request.query_string
         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.
         """
         request = cherrypy.serving.request
@@ -94,19 +112,11 @@ class CheckAuthForm(cherrypy.Tool):
             # Store original URL
             self._set_redirect_url()
             # 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):
         """
         Must be called by the page hanlder when the authentication is successful.
         """
@@ -114,10 +124,12 @@ class CheckAuthForm(cherrypy.Tool):
         cherrypy.session[LOGIN_PERSISTENT] = persistent
         cherrypy.session[SESSION_KEY] = username
         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
         cherrypy.session.clear()
         cherrypy.session.regenerate()

Vulnerable code:         '--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,
        '--session-persistent-timeout',
        help='Persistent sessions (remember me) will be revoke after this period of inactivity. Default 30 days.',
        default=43200,
        '--rate-limit',
        metavar='LIMIT',
        type=int,
        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.',
                'tools.sessions.timeout': cfg.session_timeout,  # minutes
                'tools.auth_form.timeout': cfg.session_persistent_timeout,  # minutes
        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()
        )
    def run(self, login_url='/login/', logout_url='/logout', timeout=43200):
        # 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
Fixed code: * 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)
## 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 |

        # Then cookie is not persistent
        self.assertNotIn('expires', self.cookies[0][1])
        self.assertNotIn('Max-Age', self.cookies[0][1])
    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)

        '--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.',
        '--session-idle-timeout',
        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,
        '--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.',
    )

    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,
                'tools.sessions.timeout': cfg.session_idle_timeout,  # minutes
                'tools.auth_form.persistent_timeout': cfg.session_persistent_timeout,  # minutes
                'tools.auth_form.absolute_timeout': cfg.session_absolute_timeout,  # minutes
        # We don't need to verify the timeout value since expired session get deleted automatically.
        return session.get(SESSION_KEY) is not None and session.get(LOGIN_TIME) is not None
    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 run(self, login_url='/login/', logout_url='/logout', persistent_timeout=43200, absolute_timeout=30):
        self._update_session_timeout()
        # Update the session timeout
        self._update_session_timeout()