diff --git a/frigate/api/app.py b/frigate/api/app.py index 1cb495c3d1..41952f4d3c 100644 --- a/frigate/api/app.py +++ b/frigate/api/app.py @@ -894,19 +894,28 @@ def _restore_redacted_ice_servers( or REDACTED_CREDENTIAL_SENTINEL not in server.values() ): continue - match = next( - ( - saved - for saved in saved_servers - if isinstance(saved, dict) and saved.get("urls") == server.get("urls") - ), - None, - ) - if match is None: + redacted = [ + key + for key, value in server.items() + if value == REDACTED_CREDENTIAL_SENTINEL + ] + matches = [ + saved + for saved in saved_servers + 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; " - "resend it with real values" + "An ICE server contains saved credentials that could not be matched " + "by urls; resend it with real values" ) + match = matches[0] for key, value in list(server.items()): if value != REDACTED_CREDENTIAL_SENTINEL: continue diff --git a/frigate/test/http_api/test_http_config_set.py b/frigate/test/http_api/test_http_config_set.py index 3a31593537..e3145e6591 100644 --- a/frigate/test/http_api/test_http_config_set.py +++ b/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" diff --git a/frigate/test/test_camera_pw.py b/frigate/test/test_camera_pw.py index 72162722ff..57b7201650 100644 --- a/frigate/test/test_camera_pw.py +++ b/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=*" ), diff --git a/frigate/util/builtin.py b/frigate/util/builtin.py index 42ca6217a6..544df2536f 100644 --- a/frigate/util/builtin.py +++ b/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. - line = re.sub(REGEX_CREDENTIAL_QUERY_PARAM, r"\1*", line) - return re.sub(REGEX_URL_USERINFO, "://*:*@", line) + # Userinfo goes first so a credential key inside a password can't expose the + # rest of it. An "@" in a later query value only over-masks the host. + 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: diff --git a/web/src/utils/credentialMask.ts b/web/src/utils/credentialMask.ts index 4e1dcae2a1..fcb9b92c28 100644 --- a/web/src/utils/credentialMask.ts +++ b/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 =>