fix partial password masking and ambiguous ice server restore

- Mask userinfo before query params so a credential key inside a password can't leave the rest of it visible
- Reject restoring redacted ice servers when saved servers share urls but hold different accounts

frigate/api/app.py

@@ -894,19 +894,28 @@ def _restore_redacted_ice_servers(
             or REDACTED_CREDENTIAL_SENTINEL not in server.values()
         ):
             continue
-        match = next(
+        redacted = [
-            (
+            key
-                saved
+            for key, value in server.items()
-                for saved in saved_servers
+            if value == REDACTED_CREDENTIAL_SENTINEL
-                if isinstance(saved, dict) and saved.get("urls") == server.get("urls")
+        ]
-            ),
+        matches = [
-            None,
+            saved
-        )
+            for saved in saved_servers
-        if match is None:
+            if isinstance(saved, dict) and saved.get("urls") == server.get("urls")
+        ]
+
+        # Servers sharing urls but holding different accounts can't be told apart
+        if not matches or any(
+            other.get(key) != matches[0].get(key)
+            for other in matches[1:]
+            for key in redacted
+        ):
             return (
-                "An ICE server contains saved credentials but its urls changed; "
+                "An ICE server contains saved credentials that could not be matched "
-                "resend it with real values"
+                "by urls; resend it with real values"
             )
+        match = matches[0]
         for key, value in list(server.items()):
             if value != REDACTED_CREDENTIAL_SENTINEL:
                 continue

frigate/test/http_api/test_http_config_set.py

@@ -798,6 +798,39 @@ class TestConfigSetWildcardPropagation(BaseTestHttp):
         finally:
             os.unlink(config_path)
 
+    @patch("frigate.api.app.find_config_file")
+    def test_redacted_ice_servers_sharing_urls_are_rejected(self, mock_find_config):
+        self._add_credentials()
+        self.minimal_config["go2rtc"]["webrtc"]["ice_servers"].append(
+            {
+                "urls": ["turn:turn.example.com:3478"],
+                "username": "otheruser",
+                "credential": "otherpass",
+            }
+        )
+        config_path = self._write_config_file()
+        mock_find_config.return_value = config_path
+
+        try:
+            app, _ = self._create_app_with_publisher()
+
+            with AuthTestClient(app) as client:
+                ice_servers = client.get("/config").json()["go2rtc"]["webrtc"][
+                    "ice_servers"
+                ]
+                resp = self._save(
+                    client, {"go2rtc": {"webrtc": {"ice_servers": ice_servers}}}
+                )
+                self.assertEqual(resp.status_code, 400)
+
+            _, saved = self._read_config_file(config_path)
+            self.assertEqual(
+                [s["credential"] for s in saved["go2rtc"]["webrtc"]["ice_servers"]],
+                ["turnpass", "otherpass"],
+            )
+        finally:
+            os.unlink(config_path)
+
     @patch("frigate.api.app.find_config_file")
     def test_masked_inputs_restore_by_value(self, mock_find_config):
         main = "rtsp://u:MAIN@10.0.0.1:554/main"

frigate/test/test_camera_pw.py

@@ -64,6 +64,8 @@ class TestUserPassMasking(unittest.TestCase):
             "rtsp://admin:12#abc@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
             "rtsp://admin:7?x@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
             "rtsps://u:1/2@h/x": "rtsps://*:*@h/x",
+            "rtsp://admin:pre&token=suf@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
+            "rtsp://admin:a?pwd=b@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
             "http://h/x?api_key=a&apikey=b&access_token=c": (
                 "http://h/x?api_key=*&apikey=*&access_token=*"
             ),

frigate/util/builtin.py

@@ -121,9 +121,10 @@ def deep_merge(dct1: dict, dct2: dict, override=False, merge_lists=False) -> dic
 def clean_camera_user_pass(line: str) -> str:
     """Mask URL userinfo and credential query parameters in line."""
 
-    # Query values go first so an "@" inside one can't extend the userinfo match.
+    # Userinfo goes first so a credential key inside a password can't expose the
-    line = re.sub(REGEX_CREDENTIAL_QUERY_PARAM, r"\1*", line)
+    # rest of it. An "@" in a later query value only over-masks the host.
-    return re.sub(REGEX_URL_USERINFO, "://*:*@", line)
+    line = re.sub(REGEX_URL_USERINFO, "://*:*@", line)
+    return re.sub(REGEX_CREDENTIAL_QUERY_PARAM, r"\1*", line)
 
 
 def is_masked_camera_path(path: str) -> bool:

web/src/utils/credentialMask.ts

@@ -21,8 +21,8 @@ export const maskCredentials = (value: string): string => {
   }
 
   return value
-    .replace(CREDENTIAL_QUERY_PATTERN, "$1*")
+    .replace(URL_USERINFO_PATTERN, "://*:*@")
-    .replace(URL_USERINFO_PATTERN, "://*:*@");
+    .replace(CREDENTIAL_QUERY_PATTERN, "$1*");
 };
 
 export const hasCredentials = (value: string): boolean =>