Support special characters in passwords, redacted logs & debug config (…

…blakeblackshear#4057)

* Consts for regex

* Add regex for camera username and password

* Redact user:pass from ffmpeg logs

* Redact ffmpeg commands

* Move common function to util

* Add tests

* Formatting

* Remove unused imports

* Fix test

* Add port to test

* Support special characters in passwords

* Add tests for special character handling

* Remove docs about not supporting special characters

docs/docs/guides/getting_started.md

@@ -45,12 +45,6 @@ More details on available detectors can be found [here](/configuration/detectors
 
 Now let's add the first camera:
 
-:::caution
-
-Note that passwords that contain special characters often cause issues with ffmpeg connecting to the camera. If receiving `end-of-file` or `unauthorized` errors with a verified correct password, try changing the password to something simple to rule out the possibility that the password is the issue.
-
-:::
-
 ```yaml
 mqtt:
   host: <ip of your mqtt server>

frigate/config.py

@@ -12,8 +12,18 @@
 from pydantic import BaseModel, Extra, Field, validator
 from pydantic.fields import PrivateAttr
 
-from frigate.const import BASE_DIR, CACHE_DIR, YAML_EXT
-from frigate.util import create_mask, deep_merge, load_labels
+from frigate.const import (
+    BASE_DIR,
+    CACHE_DIR,
+    REGEX_CAMERA_NAME,
+    YAML_EXT,
+)
+from frigate.util import (
+    create_mask,
+    deep_merge,
+    escape_special_characters,
+    load_labels,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -540,7 +550,7 @@ class CameraUiConfig(FrigateBaseModel):
 
 
 class CameraConfig(FrigateBaseModel):
-    name: Optional[str] = Field(title="Camera name.", regex="^[a-zA-Z0-9_-]+$")
+    name: Optional[str] = Field(title="Camera name.", regex=REGEX_CAMERA_NAME)
     enabled: bool = Field(default=True, title="Enable camera.")
     ffmpeg: CameraFfmpegConfig = Field(title="FFmpeg configuration for the camera.")
     best_image_timeout: int = Field(
@@ -695,7 +705,7 @@ def _get_ffmpeg_cmd(self, ffmpeg_input: CameraInput):
             + global_args
             + hwaccel_args
             + input_args
-            + ["-i", ffmpeg_input.path]
+            + ["-i", escape_special_characters(ffmpeg_input.path)]
             + ffmpeg_output_args
         )
 

frigate/const.py

@@ -5,3 +5,8 @@
 YAML_EXT = (".yaml", ".yml")
 PLUS_ENV_VAR = "PLUS_API_KEY"
 PLUS_API_HOST = "https://api.frigate.video"
+
+# Regex Consts
+
+REGEX_CAMERA_NAME = "^[a-zA-Z0-9_-]+$"
+REGEX_CAMERA_USER_PASS = "[a-zA-Z0-9_-]+:[a-zA-Z0-9!*'();:@&=+$,?%#_-]+@"

frigate/http.py

@@ -30,6 +30,7 @@
 from frigate.models import Event, Recordings
 from frigate.object_processing import TrackedObject, TrackedObjectProcessor
 from frigate.stats import stats_snapshot
+from frigate.util import clean_camera_user_pass
 from frigate.version import VERSION
 
 logger = logging.getLogger(__name__)
@@ -581,7 +582,7 @@ def config():
         camera_dict = config["cameras"][camera_name]
         camera_dict["ffmpeg_cmds"] = copy.deepcopy(camera.ffmpeg_cmds)
         for cmd in camera_dict["ffmpeg_cmds"]:
-            cmd["cmd"] = " ".join(cmd["cmd"])
+            cmd["cmd"] = clean_camera_user_pass(" ".join(cmd["cmd"]))
 
     config["plus"] = {"enabled": current_app.plus_api.is_active()}
 

frigate/log.py

@@ -2,14 +2,15 @@
 import logging
 import threading
 import os
-import signal
 import queue
 from multiprocessing.queues import Queue
 from logging import handlers
 from setproctitle import setproctitle
 from typing import Deque
 from collections import deque
 
+from frigate.util import clean_camera_user_pass
+
 
 def listener_configurer() -> None:
     root = logging.getLogger()
@@ -55,14 +56,19 @@ def __init__(self, log_name: str):
         self.pipeReader = os.fdopen(self.fdRead)
         self.start()
 
+    def cleanup_log(self, log: str) -> str:
+        """Cleanup the log line to remove sensitive info and string tokens."""
+        log = clean_camera_user_pass(log).strip("\n")
+        return log
+
     def fileno(self) -> int:
         """Return the write file descriptor of the pipe"""
         return self.fdWrite
 
     def run(self) -> None:
         """Run the thread, logging everything."""
         for line in iter(self.pipeReader.readline, ""):
-            self.deque.append(line.strip("\n"))
+            self.deque.append(self.cleanup_log(line))
 
         self.pipeReader.close()
 

frigate/test/test_camera_pw.py

@@ -0,0 +1,33 @@
+"""Test camera user and password cleanup."""
+
+import unittest
+
+from frigate.util import clean_camera_user_pass, escape_special_characters
+
+
+class TestUserPassCleanup(unittest.TestCase):
+    def setUp(self) -> None:
+        self.rtsp_with_pass = "rtsp://user:password@192.168.0.2:554/live"
+        self.rtsp_with_special_pass = "rtsp://user:password#$@@192.168.0.2:554/live"
+        self.rtsp_no_pass = "rtsp://192.168.0.3:554/live"
+
+    def test_cleanup(self):
+        """Test that user / pass are cleaned up."""
+        clean = clean_camera_user_pass(self.rtsp_with_pass)
+        assert clean != self.rtsp_with_pass
+        assert "user:password" not in clean
+
+    def test_no_cleanup(self):
+        """Test that nothing changes when no user / pass are defined."""
+        clean = clean_camera_user_pass(self.rtsp_no_pass)
+        assert clean == self.rtsp_no_pass
+
+    def test_special_char_password(self):
+        """Test that special characters in pw are escaped, but not others."""
+        escaped = escape_special_characters(self.rtsp_with_special_pass)
+        assert escaped == "rtsp://user:password%23%24%40@192.168.0.2:554/live"
+
+    def test_no_special_char_password(self):
+        """Test that no change is made to path with no special characters."""
+        escaped = escape_special_characters(self.rtsp_with_pass)
+        assert escaped == self.rtsp_with_pass

frigate/util.py

@@ -1,25 +1,22 @@
 import copy
 import datetime
-import hashlib
-import json
 import logging
-import math
+import re
 import signal
-import subprocess as sp
-import threading
-import time
 import traceback
+import urllib.parse
 from abc import ABC, abstractmethod
 from collections.abc import Mapping
 from multiprocessing import shared_memory
 from typing import AnyStr
 
 import cv2
-import matplotlib.pyplot as plt
 import numpy as np
 import os
 import psutil
 
+from frigate.const import REGEX_CAMERA_USER_PASS
+
 logger = logging.getLogger(__name__)
 
 
@@ -625,6 +622,23 @@ def load_labels(path, encoding="utf-8"):
             return {index: line.strip() for index, line in enumerate(lines)}
 
 
+def clean_camera_user_pass(line: str) -> str:
+    """Removes user and password from line."""
+    # todo also remove http password like reolink
+    return re.sub(REGEX_CAMERA_USER_PASS, "*:*@", line)
+
+
+def escape_special_characters(path: str) -> str:
+    """Cleans reserved characters to encodings for ffmpeg."""
+    try:
+        found = re.search(REGEX_CAMERA_USER_PASS, path).group(0)[:-1]
+        pw = found[(found.index(":") + 1) :]
+        return path.replace(pw, urllib.parse.quote_plus(pw))
+    except AttributeError:
+        # path does not have user:pass
+        return path
+
+
 class FrameManager(ABC):
     @abstractmethod
     def create(self, name, size) -> AnyStr: