16218 SEC Fix 2FA bypass via RestAPI

CMK-18988

Change-Id: I11d746709c614fb21aee578229b274487f182731

.werks/16218

@@ -0,0 +1,21 @@
+Title: Fix 2FA bypass via RestAPI
+Class: security
+Compatible: compat
+Component: wato
+Date: 1725874171
+Edition: cre
+Level: 1
+Version: 2.2.0p34
+
+Previous to this Werk the RestAPI did not properly check if a user that is supposed to authenticated with multiple factors indeed authenticated fully.
+
+This issue was found during internal review.
+
+<em>Affected Versions</em>:
+
+LI: 2.3.0
+LI: 2.2.0
+
+<em>Vulnerability Management</em>:
+
+We have rated the issue with a CVSS Score of 9.2 High (<code>CVSS:4.0/AV:N/AC:H/AT:N/PR:N/UI:N/VC:H/VI:H/VA:H/SC:N/SI:N/SA:N</code>) and assigned <code>CVE-2024-8606</code>.

cmk/gui/session.py

@@ -173,6 +173,14 @@ def persist(self) -> None:
     def invalidate(self) -> None:
         self.session_info.logged_out = True
 
+    def two_factor_pending(self) -> bool:
+        if isinstance(self.user, (LoggedInNobody, LoggedInSuperUser)):
+            return False
+        return (
+            userdb.is_two_factor_login_enabled(self.user.ident)
+            and not self.session_info.two_factor_completed
+        )
+
 
 class FileBasedSession(SessionInterface):
     """A "session" which loads its information from a .mk file

cmk/gui/wsgi/applications/rest_api.py

@@ -322,6 +322,8 @@ def ensure_authenticated(persist: bool = True) -> typing.Iterator[None]:
         if session.session.exc:
             raise session.session.exc
         raise MKAuthException("You need to be logged in to access this resource.")
+    if session.session.two_factor_pending():
+        raise MKAuthException("Two-factor authentication required.")
     yield
 
 

cmk/gui/wsgi/applications/utils.py

@@ -68,11 +68,7 @@ def _call_auth() -> Response:
                 "user_webauthn_login_complete",
             )
 
-            if (
-                not two_factor_ok
-                and userdb.is_two_factor_login_enabled(user_id)
-                and not session.session_info.two_factor_completed
-            ):
+            if not two_factor_ok and session.two_factor_pending():
                 raise HTTPRedirect(
                     "user_login_two_factor.py?_origtarget=%s" % urlencode(makeuri(request, []))
                 )

tests/integration/cmk/gui/test_login.py

@@ -9,6 +9,8 @@
 from tests.testlib import CMKWebSession
 from tests.testlib.site import Site
 
+from cmk.gui.type_defs import TwoFactorCredentials, WebAuthnCredential
+
 
 def test_01_login_and_logout(site: Site) -> None:
     web = CMKWebSession(site)
@@ -249,7 +251,11 @@ def test_failed_login_counter_human(site: Site) -> None:
         # Login form
         session.post(
             "login.py",
-            params={"_username": username, "_password": "wrong_password", "_login": "Login"},
+            params={
+                "_username": username,
+                "_password": "wrong_password",
+                "_login": "Login",
+            },
             allow_redirect_to_login=True,
         )
 
@@ -283,3 +289,49 @@ def test_failed_login_counter_automation(site: Site) -> None:
             expected_code=401,
         )
         assert 0 == _get_failed_logins(site, username)
+
+
+@contextlib.contextmanager
+def enable_2fa(site: Site, username: str) -> Iterator[None]:
+    """This will mimic 2fa. it will not work in the UI and lead to crashes...
+    In master/2.3 we use totp which is easier to mimic...
+
+    Caution, this overrides any previous 2fa configs"""
+
+    site.write_text_file(
+        f"var/check_mk/web/{username}/two_factor_credentials.mk",
+        repr(
+            TwoFactorCredentials(
+                webauthn_credentials={
+                    "foo": WebAuthnCredential(
+                        credential_id="foo",
+                        registered_at=0,
+                        alias="foo",
+                        credential_data=b"foo",
+                    )
+                },
+                backup_codes=[],
+            )
+        ),
+    )
+    try:
+        yield
+    finally:
+        site.delete_file(f"var/check_mk/web/{username}/two_factor_credentials.mk")
+
+
+def test_rest_api_access_with_enabled_2fa(site: Site) -> None:
+    """you're not supposed to access the rest api if you have 2fa enabled (except for cookie auth)
+
+    See: CMK-18988"""
+    username = "cmkadmin"
+    password = site.admin_password
+    with enable_2fa(site, "cmkadmin"):
+        session = CMKWebSession(site)
+        response = session.get(
+            f"/{site.id}/check_mk/api/1.0/version",
+            auth=(username, password),
+            expected_code=401,
+        )
+        assert not "site" in response.json()
+        assert not session.is_logged_in()