Compare commits

..
Author SHA1 Message Date
Josh Hawkins e23b6b9a6c check the nginx-normalized path in the media auth dot-segment fallback
The dot-segment fallback only checked the raw first segment, so a role restricted to one camera could request `/x/../clips/back_door-1.jpg` and nginx served back_door's clip. The fallback now resolves the path the way nginx does before checking for a media root, and the URI split ends at a literal `#` since nginx ignores everything after it.
2026-10-14 20:58:21 -05:00
Josh Hawkins b99977192a merge repeated slashes in the media auth path check
X-Original-URL carries only the raw request path since #24602, and extract_path parsed it with urlparse, which reads a path starting with // as a host. A restricted role could request another camera's clips or recordings with a leading // and the media check saw a non-media path and allowed it, while nginx merged the slashes and served the file. extract_path now splits the raw URI the same way the go2rtc stream check does and merges repeated slashes as nginx does before matching locations. The split helper moves from auth.py into media_auth.py so both checks share it.
2026-10-14 19:44:58 -05:00
a3ca8742b0 Security patches (#24658)
CI / AMD64 Build (push) Waiting to run
CI / ARM Build (push) Waiting to run
CI / Jetson Jetpack 6 (push) Waiting to run
CI / AMD64 Extra Build (push) Blocked by required conditions
CI / ARM Extra Build (push) Blocked by required conditions
CI / Synaptics Build (push) Blocked by required conditions
CI / Assemble and push default build (push) Blocked by required conditions
* update version

* Restrict ffmpeg: go2rtc sources added through the API

go2rtc expands ffmpeg: sources into an exec: command line built from the
source's #key=value parameters, so the echo/expr/exec prefix check on
PUT /go2rtc/streams could be bypassed to run arbitrary ffmpeg arguments.

Sources submitted through the API now only accept the ffmpeg: parameters
the UI and docs use (video, audio, hardware with preset names, plus
numeric width/height/rotate/bitrate/timeout and async). Quotes,
whitespace, #raw, #input, #drawtext and unknown parameters are rejected
unless GO2RTC_ALLOW_ARBITRARY_EXEC is set. Sources defined in the config
file are unchanged.

Fixes GHSA-qjjw-3v66-279f

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Only allow existing streams on proxied go2rtc live paths

nginx proxies /live/mse/api/ws, /live/webrtc/api/ws and /api/go2rtc/webrtc
straight to go2rtc, which creates a new ad hoc stream from any src that is
not an existing stream name and starts the producer it describes. Any
authenticated user, including the built-in viewer role, could use this to
launch arbitrary go2rtc sources without going through the restricted-source
checks on PUT /go2rtc/streams.

The /auth subrequest now denies these paths unless every src names an
existing stream: a camera or its live stream, a go2rtc stream from the
config, or a stream go2rtc reports at runtime. This applies to every role,
and the runtime lookup fails closed when go2rtc cannot be reached.

Fixes GHSA-93xj-hg88-ffg7

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Ignore GO2RTC_ALLOW_ARBITRARY_EXEC in environment_vars

environment_vars installs every key into the Frigate process environment
at startup, so an admin could enable GO2RTC_ALLOW_ARBITRARY_EXEC from the
config and turn off the restricted go2rtc source checks on the API. The
override is meant to be set by the operator in the container environment
(or the Home Assistant add-on options), so it is now skipped with a
warning when it appears in environment_vars.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Apply go2rtc source restrictions to config/save and config/set

go2rtc.streams written through config/save or config/set were only
filtered at startup, which blocks echo/expr/exec and the map form but not
the ffmpeg: parameters that PUT /go2rtc/streams now rejects. Saving a
stream this way and restarting bypassed the API restriction.

Both endpoints now reject a change that adds a go2rtc stream source that
fails is_restricted_go2rtc_api_source, or a map-form source, unless
GO2RTC_ALLOW_ARBITRARY_EXEC is set. Sources already in the saved config
for the same stream are left alone, so streams configured on disk keep
working and resubmitting the whole file from the config editor succeeds.
config/set restores the previous file when it rejects a change.

Follow-up to GHSA-qjjw-3v66-279f

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Match go2rtc stream-proxy paths the way nginx does

/auth compared the raw $request_uri path against the three go2rtc
stream-proxy paths exactly, while nginx matches its prefix locations
against the normalized URI. Encoded characters, repeated slashes, dot
segments or a suffix on the path skipped the stream check even though
nginx still proxied the request to go2rtc.

The go2rtc stream locations now set a variable that the auth subrequest
forwards as X-Go2rtc-Stream-Proxy (client headers are not forwarded, so it
cannot be spoofed). As a fallback, /auth also normalizes the path like
nginx (percent-decoding, merging slashes, resolving dot segments) and
prefix matches it, and splits the raw URI by hand since urlparse reads a
leading // as a host.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Parse config stream changes with Frigate's YAML loader

The go2rtc stream check on config/save and config/set parsed the config
with the safe loader and treated a parse error as an empty config, while
Frigate and the go2rtc startup script use the round-trip loader. Input
that only the round-trip loader accepts, such as a tagged root map, hid
new stream sources from the check.

The check now uses the same round-trip loader and rejects the request
when the new config cannot be parsed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Resolve go2rtc stream placeholders the same way everywhere

The go2rtc startup script expanded stream sources with str.format(),
which also supports attribute and index access, while the config API
check used substitute_frigate_vars and fell back to the raw string when a
placeholder could not be resolved. A placeholder the API check could not
resolve could therefore expand into a restricted source at startup.

The startup script now uses substitute_frigate_vars, like the rest of the
config, and the API check rejects a stream source whose placeholders
cannot be resolved. The startup script's own copy of the FRIGATE_* env
var map is removed; the shared one also honors CREDENTIALS_DIRECTORY.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Accept codec sample rates in ffmpeg: go2rtc sources

The ffmpeg: parameter allowlist only accepted plain names for video,
audio and hardware, so the documented HomeKit source with
#audio=opus/16000 was rejected by PUT /go2rtc/streams, config/save and
config/set. Names may now carry a numeric /rate suffix. A rate without a
name is still rejected. All ffmpeg: sources in the docs pass the check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Restore the environment after go2rtc exec override tests

Tests that removed or set GO2RTC_ALLOW_ARBITRARY_EXEC did not restore a
value that was already present, so later tests could run with a changed
security setting. They now use patch.dict on os.environ.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Revert the go2rtc startup script to str.format substitution

Restores create_config.py to its state on master. The config API check
rejects any stream source whose {FRIGATE_*} placeholders it cannot
resolve with substitute_frigate_vars, and for every source it accepts,
str.format() at startup either produces the same string or fails and
stops go2rtc startup. Changing the startup script is therefore not
needed for the fix, and leaving it alone avoids a conflict with dev,
which already moved this script to the new env helpers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Explain why a go2rtc stream change was rejected

A stream source with an undefined or malformed {FRIGATE_*} placeholder
was reported as a restricted source, which reads like a security block
when the user made a typo. The config API now reports unknown variables
and invalid placeholders separately from restricted sources.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 15:10:05 -04:00
3 changed files with 89 additions and 44 deletions
+5 -34
View File
@@ -11,7 +11,7 @@ import secrets
import time import time
from datetime import datetime from datetime import datetime
from pathlib import Path from pathlib import Path
from urllib.parse import parse_qs, unquote from urllib.parse import parse_qs
import requests import requests
from fastapi import APIRouter, Depends, HTTPException, Request, Response from fastapi import APIRouter, Depends, HTTPException, Request, Response
@@ -31,6 +31,8 @@ from frigate.api.media_auth import (
check_camera_access, check_camera_access,
deny_response_for_media_uri, deny_response_for_media_uri,
is_role_restricted, is_role_restricted,
normalize_request_path,
split_request_uri,
) )
from frigate.config import AuthConfig, ProxyConfig from frigate.config import AuthConfig, ProxyConfig
from frigate.const import CONFIG_DIR, JWT_SECRET_ENV_VAR, PASSWORD_HASH_ALGORITHM from frigate.const import CONFIG_DIR, JWT_SECRET_ENV_VAR, PASSWORD_HASH_ALGORITHM
@@ -1185,37 +1187,6 @@ GO2RTC_STREAM_PROXY_PATHS = frozenset(
} }
) )
_URI_SCHEME_AND_AUTHORITY = re.compile(r"^[A-Za-z][A-Za-z0-9+.-]*://[^/?]*")
def _split_request_uri(request_uri: str) -> tuple[str, str]:
"""Split a raw request URI into its path and query string.
urlparse is not used since it reads a path starting with `//` as a host.
"""
uri = _URI_SCHEME_AND_AUTHORITY.sub("", request_uri, count=1)
path, _, query = uri.partition("?")
return path, query
def _normalize_request_path(path: str) -> str:
"""Normalize a raw request path the way nginx does before matching locations.
nginx percent-decodes the path, merges repeated slashes and resolves dot
segments, while `X-Original-URL` carries the raw `$request_uri`.
"""
segments: list[str] = []
for segment in unquote(path).split("/"):
if segment in ("", "."):
continue
if segment == "..":
if segments:
segments.pop()
continue
segments.append(segment)
return "/" + "/".join(segments)
def _is_go2rtc_stream_proxy_request(path: str, request: Request) -> bool: def _is_go2rtc_stream_proxy_request(path: str, request: Request) -> bool:
# nginx marks requests routed through the go2rtc stream locations, and the # nginx marks requests routed through the go2rtc stream locations, and the
@@ -1223,7 +1194,7 @@ def _is_go2rtc_stream_proxy_request(path: str, request: Request) -> bool:
if request.headers.get("x-go2rtc-stream-proxy") == "1": if request.headers.get("x-go2rtc-stream-proxy") == "1":
return True return True
normalized = _normalize_request_path(path) normalized = normalize_request_path(path)
return any(normalized.startswith(p) for p in GO2RTC_STREAM_PROXY_PATHS) return any(normalized.startswith(p) for p in GO2RTC_STREAM_PROXY_PATHS)
@@ -1242,7 +1213,7 @@ def deny_response_for_go2rtc_stream(
if not original_url: if not original_url:
return None return None
path, query = _split_request_uri(original_url) path, query = split_request_uri(original_url)
if not _is_go2rtc_stream_proxy_request(path, request): if not _is_go2rtc_stream_proxy_request(path, request):
return None return None
+42 -10
View File
@@ -13,8 +13,9 @@ from __future__ import annotations
import logging import logging
import os import os
import re
from enum import Enum from enum import Enum
from urllib.parse import unquote, urlparse from urllib.parse import unquote
from peewee import DoesNotExist from peewee import DoesNotExist
@@ -24,6 +25,8 @@ from frigate.models import Export, User
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
_URI_SCHEME_AND_AUTHORITY = re.compile(r"^[A-Za-z][A-Za-z0-9+.-]*://[^/?]*")
class MediaAuthResolution(str, Enum): class MediaAuthResolution(str, Enum):
"""Classification of an `X-Original-URL` path for media-auth purposes.""" """Classification of an `X-Original-URL` path for media-auth purposes."""
@@ -41,6 +44,36 @@ class MediaAuthResolution(str, Enum):
UNKNOWN = "unknown" UNKNOWN = "unknown"
def split_request_uri(request_uri: str) -> tuple[str, str]:
"""Split a raw request URI into its path and query string.
urlparse is not used since it reads a path starting with `//` as a host.
nginx ends the URI at a literal `#`, so anything after it is dropped.
"""
uri = _URI_SCHEME_AND_AUTHORITY.sub("", request_uri, count=1)
path, _, query = uri.partition("#")[0].partition("?")
return path, query
def normalize_request_path(path: str) -> str:
"""Normalize a raw request path the way nginx does before matching locations.
nginx percent-decodes the path, merges repeated slashes and resolves dot
segments, while `X-Original-URL` carries the raw `$request_uri`.
"""
segments: list[str] = []
for segment in unquote(path).split("/"):
if segment in ("", "."):
continue
if segment == "..":
if segments:
segments.pop()
continue
segments.append(segment)
return "/" + "/".join(segments)
def extract_path(original_url: str | None) -> str | None: def extract_path(original_url: str | None) -> str | None:
"""Return the decoded path component of nginx's `X-Original-URL` header. """Return the decoded path component of nginx's `X-Original-URL` header.
@@ -49,14 +82,14 @@ def extract_path(original_url: str | None) -> str | None:
request like `/recordings/.../allowed_cam/../forbidden_cam/file.mp4` request like `/recordings/.../allowed_cam/../forbidden_cam/file.mp4`
would (1) parse as the allowed camera in our auth check, (2) be served would (1) parse as the allowed camera in our auth check, (2) be served
as the forbidden camera by nginx. To close the bypass we reject any URI as the forbidden camera by nginx. To close the bypass we reject any URI
whose path contains `.` or `..` segments outright. whose path contains `.` or `..` segments outright. Repeated slashes are
merged, as nginx does before matching locations.
""" """
if not original_url: if not original_url:
return None return None
parsed = urlparse(original_url) path, _ = split_request_uri(original_url)
raw_path = parsed.path or original_url decoded = re.sub(r"/{2,}", "/", unquote(path))
decoded = unquote(raw_path)
if not decoded: if not decoded:
return None return None
@@ -252,12 +285,11 @@ def deny_response_for_media_uri(
path = extract_path(original_url) path = extract_path(original_url)
# `extract_path` returns None for URIs containing `.` or `..` segments. # `extract_path` returns None for URIs containing `.` or `..` segments.
# For media-root URIs that's a traversal attempt — deny outright. For # Deny them when nginx would resolve them under a media root, and pass
# non-media URIs, pass through (nginx / the backend handle them). # the rest through to nginx and the backend.
if path is None: if path is None:
raw = urlparse(original_url).path or original_url normalized = normalize_request_path(split_request_uri(original_url)[0])
decoded = unquote(raw) first = normalized.lstrip("/").split("/", 1)[0]
first = decoded.lstrip("/").split("/", 1)[0] if decoded else ""
if first in ("clips", "recordings", "exports"): if first in ("clips", "recordings", "exports"):
return 403 return 403
return None return None
+42
View File
@@ -76,6 +76,20 @@ class TestExtractPath(unittest.TestCase):
self.assertIsNone(extract_path(None)) self.assertIsNone(extract_path(None))
self.assertIsNone(extract_path("")) self.assertIsNone(extract_path(""))
def test_leading_double_slash_is_path(self):
self.assertEqual(
extract_path("//clips/front_door-1.jpg"), "/clips/front_door-1.jpg"
)
def test_repeated_slashes_merged(self):
self.assertEqual(
extract_path("/recordings//2026-05-11/14//front_door/00.00.mp4"),
"/recordings/2026-05-11/14/front_door/00.00.mp4",
)
self.assertEqual(
extract_path("/%2Fclips/front_door-1.jpg"), "/clips/front_door-1.jpg"
)
class TestResolveMediaUri(unittest.TestCase): class TestResolveMediaUri(unittest.TestCase):
def setUp(self): def setUp(self):
@@ -376,6 +390,34 @@ class TestDenyResponseForMediaUri(unittest.TestCase):
self.assertIsNone(self._deny("/clips/nonexistent-1.jpg", "admin")) self.assertIsNone(self._deny("/clips/nonexistent-1.jpg", "admin"))
self.assertIsNone(self._deny("/clips/nonexistent-1.jpg", "viewer")) self.assertIsNone(self._deny("/clips/nonexistent-1.jpg", "viewer"))
def test_repeated_slashes_resolve_to_owning_camera(self):
# nginx merges repeated slashes before matching the media locations
for url in (
"//clips/back_door-1.jpg",
"/clips//back_door-1.jpg",
"//recordings/2026-05-11/14/back_door/00.00.mp4",
"/recordings//2026-05-11/14/back_door/00.00.mp4",
):
self.assertEqual(self._deny(url, "limited_user"), 403, url)
self.assertIsNone(self._deny("//clips/front_door-1.jpg", "limited_user"))
def test_traversal_with_leading_double_slash_denied(self):
self.assertEqual(
self._deny("//clips/front_door/../back_door-1.jpg", "admin"), 403
)
def test_traversal_from_non_media_prefix_denied(self):
# nginx resolves these into a media root before serving
for url in (
"/x/../clips/back_door-1.jpg",
"/x/%2E%2E/clips/back_door-1.jpg",
"/./clips/front_door/../back_door-1.jpg",
"/api/../recordings/2026-05-11/14/back_door/00.00.mp4",
"/clips/back_door-1.jpg#/../../x",
):
self.assertEqual(self._deny(url, "limited_user"), 403, url)
self.assertIsNone(self._deny("/x/../api/events", "limited_user"))
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()