diff --git a/CMakeLists.txt b/CMakeLists.txt index b1b8dd8..c125aa6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,17 +73,63 @@ set(STPLUGIN_LIVEKIT_SDK_TRIPLE "" CACHE STRING set(STPLUGIN_LIVEKIT_SDK_DIR "${CMAKE_BINARY_DIR}/_deps/livekit-sdk" CACHE PATH "Directory the client-sdk-cpp release archive is extracted into (point at a persistent path to cache it across CI builds)") +# Pinned SHA256 checksums for the client-sdk-cpp v1.10.1 release archives, +# so the download in cmake/LiveKitSDK.cmake is verified the same way the +# obs-deps bootstrap next to it already is (see +# cmake/common/buildspec_common.cmake ~line 324). Each hash below was +# computed by downloading the real GitHub release asset and running +# `sha256sum` on it (2026-09-06/07) -- none of these were guessed or copied +# from an unverified source. To add a hash for a new version or triple: +# curl -LO https://github.com/livekit/client-sdk-cpp/releases/download/v/livekit-sdk--. +# sha256sum livekit-sdk--.* +# Covers every triple _lk_default_triple() can resolve to for this pinned +# version: Linux (ubuntu-22.04-x64/arm64), macOS (macos-x64/arm64) and +# Windows (windows-x64). Verified by extracting each archive +# (tar tzf / unzip -l) and confirming a real LiveKitConfig.cmake inside -- +# only Linux was also verified by an actual local CMake configure+build in +# this environment; macOS and Windows were downloaded and hashed but not +# build-tested here. +set(_stplugin_livekit_sha256_1.10.1_ubuntu-22.04-x64 "6f4fc8143f36952d42bfd5ff8d1782cf6211ba8fd6b055877e9ef85441d66324") +set(_stplugin_livekit_sha256_1.10.1_ubuntu-22.04-arm64 "399677167b474b7f107c6937904ea01898c9ec8da648cbed669387e599c6ea45") +set(_stplugin_livekit_sha256_1.10.1_macos-x64 "7102655c1f2947be4b06a95f9fafa1a11379219328e82ad875e5ebccfc9ac7e3") +set(_stplugin_livekit_sha256_1.10.1_macos-arm64 "0822af7014519a473c5b5cd019bde58c26cc2bfe5b78e4a790395ead232dc55b") +set(_stplugin_livekit_sha256_1.10.1_windows-x64 "b9fc6b2865298d7e3d032205d7e74fb9628cfe55a2ed28cb657db0a481cd518c") + include(LiveKitSDK) +if(STPLUGIN_LIVEKIT_SDK_TRIPLE) + set(_stplugin_livekit_triple "${STPLUGIN_LIVEKIT_SDK_TRIPLE}") +else() + # Mirrors LiveKitSDK.cmake's own autodetection so the checksum lookup + # below matches whatever triple livekit_sdk_setup() will actually + # resolve to and download. + _lk_default_triple(_stplugin_livekit_triple) +endif() + +set(_stplugin_livekit_sha256_var + "_stplugin_livekit_sha256_${STPLUGIN_LIVEKIT_SDK_VERSION}_${_stplugin_livekit_triple}") +if(DEFINED ${_stplugin_livekit_sha256_var}) + set(_stplugin_livekit_sha256 "${${_stplugin_livekit_sha256_var}}") +else() + set(_stplugin_livekit_sha256 "") + message(WARNING + "LiveKitSDK: no pinned SHA256 for triple '${_stplugin_livekit_triple}' " + "at version ${STPLUGIN_LIVEKIT_SDK_VERSION} -- the downloaded archive " + "will NOT be integrity-checked. Compute one (see the comment above " + "this block) and add it to CMakeLists.txt.") +endif() + if(STPLUGIN_LIVEKIT_SDK_TRIPLE) livekit_sdk_setup( VERSION "${STPLUGIN_LIVEKIT_SDK_VERSION}" SDK_DIR "${STPLUGIN_LIVEKIT_SDK_DIR}" TRIPLE "${STPLUGIN_LIVEKIT_SDK_TRIPLE}" + SHA256 "${_stplugin_livekit_sha256}" ) else() livekit_sdk_setup( VERSION "${STPLUGIN_LIVEKIT_SDK_VERSION}" SDK_DIR "${STPLUGIN_LIVEKIT_SDK_DIR}" + SHA256 "${_stplugin_livekit_sha256}" ) endif() find_package(LiveKit CONFIG REQUIRED) diff --git a/core/include/stplugin/api_client.h b/core/include/stplugin/api_client.h index e09c954..7303bb8 100644 --- a/core/include/stplugin/api_client.h +++ b/core/include/stplugin/api_client.h @@ -115,6 +115,17 @@ public: /// with the read key replaced by "***". static std::string redactedUrl(const std::string &url); + /// Scrubs "access_token=" and "key=" out of arbitrary + /// text -- not necessarily a bare URL/query string -- replacing each + /// value with "". Unlike redactedUrl (which only has to + /// handle "&"-delimited query parameters), a value here can be followed + /// by a quote or whitespace, because the text this scrubs is a free-form + /// log line that may merely *contain* a URL. Used by the OBS adapter's + /// LiveKit SDK log bridge: LiveKit's signaling URL carries the access + /// token as a query parameter, and the SDK's own log lines could + /// include it. + static std::string redactSensitiveParams(const std::string &text); + private: std::shared_ptr http_; }; diff --git a/core/src/api_client.cpp b/core/src/api_client.cpp index 5cff058..31edd73 100644 --- a/core/src/api_client.cpp +++ b/core/src/api_client.cpp @@ -96,6 +96,10 @@ std::string ApiClient::normalizeServerUrl(const std::string &raw) const bool has_scheme = url.compare(0, 7, "http://") == 0 || url.compare(0, 8, "https://") == 0; if (!has_scheme) url = "https://" + url; + // An explicit "http://..." is left as-is on purpose: an operator who + // typed the scheme out has made a deliberate (if inadvisable) choice, + // and this function's job is only to supply a sane default, not to + // second-guess an explicit one. while (!url.empty() && url.back() == '/') url.pop_back(); @@ -108,6 +112,11 @@ std::string ApiClient::normalizeServerUrl(const std::string &raw) return url; } +// No current call site logs a request URL (the OBS adapter only logs +// ws_url/status text, never the streamer-tools API request URL itself) -- +// this exists as a deliberate guard rail for whenever request-URL logging +// is added later, so the read key can never be pasted into an OBS log by +// accident. Not dead code to be deleted. std::string ApiClient::redactedUrl(const std::string &url) { const std::size_t at = url.find("key="); @@ -120,6 +129,36 @@ std::string ApiClient::redactedUrl(const std::string &url) return url.substr(0, value) + "***" + url.substr(end); } +std::string ApiClient::redactSensitiveParams(const std::string &text) +{ + static const char *const kParams[] = {"access_token=", "key="}; + + std::string out = text; + for (const char *param : kParams) { + const std::size_t param_len = std::string(param).size(); + std::size_t pos = 0; + while ((pos = out.find(param, pos)) != std::string::npos) { + const std::size_t value_start = pos + param_len; + std::size_t value_end = value_start; + // A value ends at the next query-string delimiter, a quote (the + // URL is often embedded in a quoted/bracketed log line), or + // whitespace -- whichever comes first -- or at the end of the + // string. + while (value_end < out.size()) { + const char c = out[value_end]; + if (c == '&' || c == '"' || c == '\'' || c == ' ' || c == '\t' || c == '\n' || + c == '\r' || c == ')' || c == ']') + break; + ++value_end; + } + const std::string replacement = ""; + out.replace(value_start, value_end - value_start, replacement); + pos = value_start + replacement.size(); + } + } + return out; +} + SlotsResult ApiClient::fetchSlots(const ConnectionConfig &config, int timeout_ms) const { SlotsResult result; diff --git a/core/src/http_curl.cpp b/core/src/http_curl.cpp index 96d8933..977fd51 100644 --- a/core/src/http_curl.cpp +++ b/core/src/http_curl.cpp @@ -78,8 +78,17 @@ public: curl_easy_setopt(curl, CURLOPT_WRITEDATA, &ctx); curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, static_cast(request.timeout_ms)); curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS, static_cast(request.timeout_ms)); - curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); - curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 3L); + // Redirects are never legitimate here: this client only ever talks to + // two fixed, first-party streamer-tools API endpoints, and the read + // key travels as a URL query parameter (see api_client.cpp). Blindly + // following a redirect -- including an HTTPS->HTTP downgrade, which + // curl does not refuse by default -- would hand that key to whatever + // host the redirect points at. A redirect from our own server is a + // configuration error, so treat it as a failed request instead of + // silently following it. This also brings this backend in line with + // http_winhttp.cpp, which already refuses HTTPS->HTTP downgrades by + // default. + curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 0L); curl_easy_setopt(curl, CURLOPT_USERAGENT, "streamer-tools-obs-plugin/1.0"); // NOSIGNAL is required whenever curl is used off the main thread: // without it curl installs a SIGALRM handler for DNS timeouts, which diff --git a/core/src/session.cpp b/core/src/session.cpp index 2682cf1..6a9fb41 100644 --- a/core/src/session.cpp +++ b/core/src/session.cpp @@ -617,6 +617,15 @@ bool LiveKitSession::connect(const SessionConfig &config) livekit::RoomOptions options; // auto_subscribe is what makes track_subscribed events (and therefore any // media at all) happen; the SDK is emphatic about this. + // + // Known, measured-but-unaddressed cost: auto_subscribe pulls every + // participant's published track, not just the one camera this session + // actually wants, and this client discards the unwanted ones + // client-side. In a multi-camera room that is real, wasted bandwidth + // and decode CPU that scales with room size, not with what this source + // displays. Selectively unsubscribing from unwanted publications (the + // SDK exposes per-publication subscribe/unsubscribe) is a real + // follow-up optimization, deliberately out of scope here. options.auto_subscribe = true; options.dynacast = false; // This client never publishes, so a single peer connection is all it diff --git a/core/tests/test_api_client.cpp b/core/tests/test_api_client.cpp index 0c74260..7fc3c50 100644 --- a/core/tests/test_api_client.cpp +++ b/core/tests/test_api_client.cpp @@ -99,6 +99,38 @@ void testUrlEncodeAndRedaction() ST_ASSERT_EQ(ApiClient::redactedUrl("https://h/nothing"), std::string("https://h/nothing")); } +void testRedactSensitiveParams() +{ + // The LiveKit log bridge's actual use case: a signaling URL embedded in + // a free-form SDK log line, not a bare query string. + ST_ASSERT_EQ(ApiClient::redactSensitiveParams( + "connecting to wss://lk.example.com/rtc?access_token=eyJhbGciOiJIUzI1NiJ9.abc.def&x=1"), + std::string("connecting to wss://lk.example.com/rtc?access_token=&x=1")); + + // A value can be terminated by a quote or whitespace, not just '&', since + // this scrubs arbitrary text rather than a URL/query string. + ST_ASSERT_EQ(ApiClient::redactSensitiveParams("url=\"wss://h/rtc?access_token=secret\" state=connecting"), + std::string("url=\"wss://h/rtc?access_token=\" state=connecting")); + + // "key=" is also scrubbed, matching redactedUrl's convention. + ST_ASSERT_EQ(ApiClient::redactSensitiveParams("GET https://h/api/obs/r/slots?key=secret"), + std::string("GET https://h/api/obs/r/slots?key=")); + + // Both params can appear in the same message, and each is independently + // redacted. + ST_ASSERT_EQ( + ApiClient::redactSensitiveParams("a access_token=tok1 b key=tok2 c"), + std::string("a access_token= b key= c")); + + // Text with neither parameter passes through unchanged. + ST_ASSERT_EQ(ApiClient::redactSensitiveParams("livekit: participant joined"), + std::string("livekit: participant joined")); + + // A value at the very end of the string is still bounded correctly. + ST_ASSERT_EQ(ApiClient::redactSensitiveParams("token was access_token=trailing"), + std::string("token was access_token=")); +} + void testRequestShape() { auto fake = makeFake(200, R"({"slots":[]})"); @@ -421,6 +453,7 @@ int main() { testNormalizeServerUrl(); testUrlEncodeAndRedaction(); + testRedactSensitiveParams(); testRequestShape(); testSlotsHappyPath(); testSlotsEdgeCases(); diff --git a/obs-adapter/src/plugin-main.cpp b/obs-adapter/src/plugin-main.cpp index 91e4912..da72f01 100644 --- a/obs-adapter/src/plugin-main.cpp +++ b/obs-adapter/src/plugin-main.cpp @@ -410,7 +410,17 @@ void sourceDestroy(void *data) if (!self) return; - self->stopping.store(true); + { + // Must be set while holding `mutex`, matching how `generation` is + // mutated in applySettings: the worker's wait predicate reads + // `stopping` under this same lock, so setting it outside the lock + // can race between the worker's predicate check and it entering + // the wait, dropping the notify_all() below and leaving the worker + // asleep for its full backoff (up to kBackoffMaxMs) while this + // (OBS UI) thread blocks in worker.join(). + std::lock_guard guard(self->mutex); + self->stopping.store(true); + } self->wake.notify_all(); if (self->worker.joinable()) self->worker.join(); @@ -563,7 +573,14 @@ void livekitLogToObs(livekit::LogLevel level, const std::string &, const std::st case livekit::LogLevel::Info: obs_level = LOG_INFO; break; default: obs_level = LOG_DEBUG; break; } - obs_log(obs_level, "livekit: %s", message.c_str()); + // LiveKit's signaling connection URL carries the access token as a + // query parameter. This is defensive, not a response to a confirmed + // leak: if the SDK ever logs that URL (or anything else carrying + // "access_token=" or "key="), the token must not land verbatim in an + // OBS log file that a director might paste into a support ticket. Scrub + // unconditionally before this message ever reaches obs_log. + const std::string scrubbed = ApiClient::redactSensitiveParams(message); + obs_log(obs_level, "livekit: %s", scrubbed.c_str()); } } // namespace