From cf67ff6473232314a145a85bd14dc0d4808d15b9 Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 1 May 2026 09:32:52 -0700 Subject: [PATCH 1/9] auth: branch for authentication and authorization --- deps/moxygen | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/moxygen b/deps/moxygen index e032ad85..36c314ed 160000 --- a/deps/moxygen +++ b/deps/moxygen @@ -1 +1 @@ -Subproject commit e032ad850e0f70fcda5283a0c4a5d60f5c9a40f5 +Subproject commit 36c314edb9b42345fd4ad918b463743edad21a87 From 27fd5a793f3ca92c9ce919baac3e6a81fa6d8c97 Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 1 May 2026 10:14:47 -0700 Subject: [PATCH 2/9] auth: add opt-in CAT token authorization --- CMakeLists.txt | 3 + config.example.yaml | 8 + src/MoqxPicoRelayServer.cpp | 2 +- src/MoqxRelay.cpp | 135 ++++++++- src/MoqxRelay.h | 22 +- src/MoqxRelayContext.cpp | 26 +- src/MoqxRelayContext.h | 2 +- src/MoqxRelayServer.cpp | 2 +- src/auth/Auth.cpp | 453 +++++++++++++++++++++++++++++++ src/auth/Auth.h | 95 +++++++ src/config/Config.h | 15 + src/config/ConfigResolver.cpp | 58 ++++ src/config/loader/ParsedConfig.h | 24 ++ test/AuthTest.cpp | 146 ++++++++++ test/CMakeLists.txt | 9 + 15 files changed, 990 insertions(+), 10 deletions(-) create mode 100644 src/auth/Auth.cpp create mode 100644 src/auth/Auth.h create mode 100644 test/AuthTest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f9117af..7fc8477e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,6 +35,7 @@ if(POLICY CMP0167) endif() find_package(moxygen REQUIRED) +find_package(OpenSSL REQUIRED) # --- yaml-cpp (FetchContent) --- @@ -73,6 +74,7 @@ FetchContent_MakeAvailable(reflectcpp) # --- Core library --- add_library(moqx_core STATIC + src/auth/Auth.cpp src/NamespaceTree.cpp src/MoqxRelay.cpp src/MoqxRelayContext.cpp @@ -109,6 +111,7 @@ target_link_libraries(moqx_core PUBLIC # FizzAcceptorHandshakeHelper but omits this dep from its cmake config. Folly::folly_io_async_fdsock_async_fd_socket moxygen::moxygen_moqclient + OpenSSL::Crypto ) target_compile_options(moqx_core PRIVATE -Wall -Wextra -Wpedantic) diff --git a/config.example.yaml b/config.example.yaml index 46479f3f..afcf8b79 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -55,6 +55,14 @@ services: # tls: # insecure: false # true = skip cert verification (dev only) # # ca_cert: /path/to/ca.pem # Custom CA cert (mutually exclusive with insecure: true) + # auth: # Optional: enable CAT-style authorization for this service + # enabled: true + # token_type: 0 # MOQT AUTHORIZATION_TOKEN token type + # hmac_keys: + # - id: "key-1" + # secret: "replace-with-secret" + # require_setup_token: true + # allow_request_token_override: true testing: match: diff --git a/src/MoqxPicoRelayServer.cpp b/src/MoqxPicoRelayServer.cpp index 8aea11e0..24761bdf 100644 --- a/src/MoqxPicoRelayServer.cpp +++ b/src/MoqxPicoRelayServer.cpp @@ -113,7 +113,7 @@ void MoqxPicoRelayServer::onNewSession(std::shared_ptr clientSession } void MoqxPicoRelayServer::terminateClientSession(std::shared_ptr session) { - context_->onSessionEnd(); + context_->onSessionEnd(session); MoQPicoQuicEventBaseServer::terminateClientSession(std::move(session)); } diff --git a/src/MoqxRelay.cpp b/src/MoqxRelay.cpp index ef644e36..9f6ab393 100644 --- a/src/MoqxRelay.cpp +++ b/src/MoqxRelay.cpp @@ -95,6 +95,66 @@ void MoqxRelay::onUpstreamDisconnect() { upstreamSubNsHandle_.reset(); } +folly::Expected MoqxRelay::authenticateSession( + const ClientSetup& clientSetup, + std::shared_ptr session +) { + if (!authVerifier_.enabled()) { + return folly::unit; + } + auto token = auth::findAuthToken(clientSetup.params, authVerifier_.tokenType()); + if (!token) { + if (!authVerifier_.requireSetupToken()) { + return folly::unit; + } + return folly::makeUnexpected(auth::AuthError::Missing); + } + auto grants = authVerifier_.verify(*token); + if (grants.hasError()) { + return folly::makeUnexpected(grants.error()); + } + if (!auth::allows(grants.value(), auth::Action::ClientSetup, TrackNamespace{})) { + return folly::makeUnexpected(auth::AuthError::Forbidden); + } + sessionAuth_.insert_or_assign(session.get(), std::move(grants.value())); + return folly::unit; +} + +folly::Expected MoqxRelay::authorize( + auth::Action action, + const Parameters& params, + const TrackNamespace& ns, + std::optional trackName +) { + if (!authVerifier_.enabled()) { + return folly::unit; + } + + auto session = MoQSession::getRequestSession(); + auto token = authVerifier_.allowRequestTokenOverride() + ? auth::findAuthToken(params, authVerifier_.tokenType()) + : std::nullopt; + if (token) { + auto grants = authVerifier_.verify(*token); + if (grants.hasError()) { + return folly::makeUnexpected(grants.error()); + } + if (auth::allows(grants.value(), action, ns, trackName)) { + return folly::unit; + } + return folly::makeUnexpected(auth::AuthError::Forbidden); + } + + auto it = sessionAuth_.find(session.get()); + if (it == sessionAuth_.end()) { + return folly::makeUnexpected(auth::AuthError::Missing); + } + if (!auth::allows(it->second, action, ns, trackName)) { + return folly::makeUnexpected(auth::AuthError::Forbidden); + } + return folly::unit; +} + // Sends SUBSCRIBE_UPDATE to update forwarding state. Called from: // - subscribeNamespace: forwarder was empty, new subscriber added // (forward=true) @@ -141,7 +201,6 @@ std::shared_ptr MoqxRelay::doPublishNamespac std::string peerID ) { XLOG(DBG1) << __func__ << " ns=" << pubNs.trackNamespace; - // check auth if (!pubNs.trackNamespace.startsWith(allowedNamespacePrefix_)) { return nullptr; } @@ -193,6 +252,14 @@ folly::coro::Task MoqxRelay::publishNamespac // TODO: store auth for forwarding on future SubscribeNamespace? auto session = MoQSession::getRequestSession(); auto requestID = pubNs.requestID; + auto authRes = authorize(auth::Action::PublishNamespace, pubNs.params, pubNs.trackNamespace); + if (authRes.hasError()) { + co_return folly::makeUnexpected(PublishNamespaceError{ + requestID, + PublishNamespaceErrorCode::UNAUTHORIZED, + auth::toString(authRes.error()) + }); + } auto result = doPublishNamespace(std::move(pubNs), session, std::move(callback)); if (!result) { co_return folly::makeUnexpected( @@ -285,6 +352,17 @@ Subscriber::PublishResult MoqxRelay::publish(PublishRequest pub, std::shared_ptr handle) { XLOG(DBG1) << __func__ << " ftn=" << pub.fullTrackName; XCHECK(handle) << "Publish handle cannot be null"; + auto authRes = authorize( + auth::Action::Publish, + pub.params, + pub.fullTrackName.trackNamespace, + pub.fullTrackName.trackName + ); + if (authRes.hasError()) { + return folly::makeUnexpected( + PublishError{pub.requestID, PublishErrorCode::UNAUTHORIZED, auth::toString(authRes.error())} + ); + } if (!pub.fullTrackName.trackNamespace.startsWith(allowedNamespacePrefix_)) { return folly::makeUnexpected( PublishError{pub.requestID, PublishErrorCode::UNINTERESTED, "bad namespace"} @@ -579,6 +657,21 @@ folly::coro::Task MoqxRelay::subscribeNames // Fall through: register the peer as a normal subNs subscriber so it // receives namespace announcements as publishers connect. } + if (incomingPeerID.empty()) { + auto authRes = authorize( + auth::Action::SubscribeNamespace, + subNs.params, + subNs.trackNamespacePrefix, + std::nullopt + ); + if (authRes.hasError()) { + co_return folly::makeUnexpected(SubscribeNamespaceError{ + subNs.requestID, + SubscribeNamespaceErrorCode::UNAUTHORIZED, + auth::toString(authRes.error()) + }); + } + } auto maybeNegotiatedVersion = session->getNegotiatedVersion(); CHECK(maybeNegotiatedVersion.has_value()); @@ -723,6 +816,19 @@ MoqxRelay::PublishState MoqxRelay::findPublishState(const FullTrackName& ftn) { folly::coro::Task MoqxRelay::subscribe(SubscribeRequest subReq, std::shared_ptr consumer) { auto session = MoQSession::getRequestSession(); + auto authRes = authorize( + auth::Action::Subscribe, + subReq.params, + subReq.fullTrackName.trackNamespace, + subReq.fullTrackName.trackName + ); + if (authRes.hasError()) { + co_return folly::makeUnexpected(SubscribeError{ + subReq.requestID, + SubscribeErrorCode::UNAUTHORIZED, + auth::toString(authRes.error()) + }); + } auto subscriptionIt = subscriptions_.find(subReq.fullTrackName); // TOCTOU fix: if we might be the first subscriber, wait for the upstream // connection before branching. waitForConnected() is a suspension point — @@ -895,8 +1001,17 @@ folly::coro::Task MoqxRelay::fetch(Fetch fetch, std::shared_ptr consumer) { auto session = MoQSession::getRequestSession(); - // check auth - // get trackNamespace + auto authRes = authorize( + auth::Action::Fetch, + fetch.params, + fetch.fullTrackName.trackNamespace, + fetch.fullTrackName.trackName + ); + if (authRes.hasError()) { + co_return folly::makeUnexpected( + FetchError({fetch.requestID, FetchErrorCode::UNAUTHORIZED, auth::toString(authRes.error())}) + ); + } if (fetch.fullTrackName.trackNamespace.empty()) { co_return folly::makeUnexpected( FetchError({fetch.requestID, FetchErrorCode::TRACK_NOT_EXIST, "namespace required"}) @@ -964,6 +1079,20 @@ MoqxRelay::fetch(Fetch fetch, std::shared_ptr consumer) { folly::coro::Task MoqxRelay::trackStatus(TrackStatus trackStatus) { XLOG(DBG1) << __func__ << " ftn=" << trackStatus.fullTrackName; + auto authRes = authorize( + auth::Action::TrackStatus, + trackStatus.params, + trackStatus.fullTrackName.trackNamespace, + trackStatus.fullTrackName.trackName + ); + if (authRes.hasError()) { + co_return folly::makeUnexpected(TrackStatusError{ + trackStatus.requestID, + TrackStatusErrorCode::UNAUTHORIZED, + auth::toString(authRes.error()) + }); + } + if (trackStatus.fullTrackName.trackNamespace.empty()) { co_return folly::makeUnexpected(TrackStatusError( {trackStatus.requestID, TrackStatusErrorCode::TRACK_NOT_EXIST, "namespace required"} diff --git a/src/MoqxRelay.h b/src/MoqxRelay.h index cc39440e..6b794e74 100644 --- a/src/MoqxRelay.h +++ b/src/MoqxRelay.h @@ -10,9 +10,11 @@ #include "NamespaceTree.h" #include "UpstreamProvider.h" +#include "auth/Auth.h" #include "config/Config.h" #include "relay/PropertyRanking.h" #include "relay/TopNFilter.h" +#include #include #include #include @@ -101,12 +103,13 @@ class MoqxRelay : public moxygen::Publisher, explicit MoqxRelay( config::CacheConfig cache = {}, std::string relayID = {}, + config::AuthConfig auth = {}, uint64_t maxDeselected = kDefaultMaxDeselected, std::chrono::milliseconds idleTimeout = kDefaultIdleTimeout, std::chrono::milliseconds activityThreshold = kDefaultActivityThreshold ) - : relayID_(std::move(relayID)), maxDeselected_(maxDeselected), idleTimeout_(idleTimeout), - activityThreshold_(activityThreshold) { + : relayID_(std::move(relayID)), authVerifier_(std::move(auth)), maxDeselected_(maxDeselected), + idleTimeout_(idleTimeout), activityThreshold_(activityThreshold) { if (cache.maxCachedTracks > 0) { cache_ = std::make_unique(cache.maxCachedTracks, cache.maxCachedGroupsPerTrack); @@ -122,6 +125,12 @@ class MoqxRelay : public moxygen::Publisher, allowedNamespacePrefix_ = std::move(allowed); } + folly::Expected authenticateSession( + const moxygen::ClientSetup& clientSetup, + std::shared_ptr session + ); + void removeSessionAuth(moxygen::MoQSession* session) { sessionAuth_.erase(session); } + // Store the upstream provider. The provider must have been constructed with // publishHandler=this and subscribeHandler=this so that the upstream relay's // reciprocal subNs and namespace announcements route through MoqxRelay. @@ -331,8 +340,17 @@ class MoqxRelay : public moxygen::Publisher, void onTrackEvicted(const moxygen::FullTrackName& ftn, std::shared_ptr session); + folly::Expected authorize( + auth::Action action, + const moxygen::Parameters& params, + const moxygen::TrackNamespace& ns, + std::optional trackName = std::nullopt + ); + moxygen::TrackNamespace allowedNamespacePrefix_; std::string relayID_; + auth::AuthTokenVerifier authVerifier_; + folly::F14FastMap sessionAuth_; std::shared_ptr upstream_; // Holds the peer subNs handle for the upstream (initiating) direction. diff --git a/src/MoqxRelayContext.cpp b/src/MoqxRelayContext.cpp index d1d9336f..be7ed4bc 100644 --- a/src/MoqxRelayContext.cpp +++ b/src/MoqxRelayContext.cpp @@ -22,7 +22,10 @@ MoqxRelayContext::MoqxRelayContext( ) : serviceMatcher_(services), relayID_(relayID) { for (const auto& [name, svc] : services) { - services_.emplace(name, ServiceEntry{svc, std::make_shared(svc.cache, relayID)}); + services_.emplace( + name, + ServiceEntry{svc, std::make_shared(svc.cache, relayID, svc.auth)} + ); } } @@ -138,10 +141,13 @@ void MoqxRelayContext::onNewSession(std::shared_ptr clientSession) { } } -void MoqxRelayContext::onSessionEnd() { +void MoqxRelayContext::onSessionEnd(std::shared_ptr session) { if (statsCollector_) { statsCollector_->onSessionEnd(); } + for (auto& [name, entry] : services_) { + entry.relay->removeSessionAuth(session.get()); + } } folly::Expected MoqxRelayContext::validateAuthority( @@ -161,6 +167,22 @@ folly::Expected MoqxRelayContext::validateAu // Route: set per-service relay as handler auto it = services_.find(*matchedName); CHECK(it != services_.end()) << "Service '" << *matchedName << "' matched but no entry found"; + auto authRes = it->second.relay->authenticateSession(clientSetup, session); + if (authRes.hasError()) { + XLOG(ERR) << "Authorization failed for authority=" << authority << " path=" << path + << " reason=" << auth::toString(authRes.error()); + switch (authRes.error()) { + case auth::AuthError::Expired: + return folly::makeUnexpected(SessionCloseErrorCode::EXPIRED_AUTH_TOKEN); + case auth::AuthError::Malformed: + return folly::makeUnexpected(SessionCloseErrorCode::MALFORMED_AUTH_TOKEN); + case auth::AuthError::BadSignature: + case auth::AuthError::Forbidden: + case auth::AuthError::Missing: + case auth::AuthError::WrongTokenType: + return folly::makeUnexpected(SessionCloseErrorCode::UNAUTHORIZED); + } + } session->setPublishHandler(it->second.relay); session->setSubscribeHandler(it->second.relay); return folly::unit; diff --git a/src/MoqxRelayContext.h b/src/MoqxRelayContext.h index 2fec04b5..538932f9 100644 --- a/src/MoqxRelayContext.h +++ b/src/MoqxRelayContext.h @@ -99,7 +99,7 @@ class MoqxRelayContext { // --- Delegation targets for MoqxRelayServer virtual overrides --- void onNewSession(std::shared_ptr session); - void onSessionEnd(); + void onSessionEnd(std::shared_ptr session); folly::Expected validateAuthority( const moxygen::ClientSetup& clientSetup, diff --git a/src/MoqxRelayServer.cpp b/src/MoqxRelayServer.cpp index 35d520fa..8499f2cc 100644 --- a/src/MoqxRelayServer.cpp +++ b/src/MoqxRelayServer.cpp @@ -129,7 +129,7 @@ void MoqxRelayServer::onNewSession(std::shared_ptr clientSession) { } void MoqxRelayServer::terminateClientSession(std::shared_ptr session) { - context_->onSessionEnd(); + context_->onSessionEnd(session); MoQServer::terminateClientSession(std::move(session)); } diff --git a/src/auth/Auth.cpp b/src/auth/Auth.cpp new file mode 100644 index 00000000..88bd386d --- /dev/null +++ b/src/auth/Auth.cpp @@ -0,0 +1,453 @@ +/* + * Copyright (c) OpenMOQ contributors. + * This source code is licensed under the Apache 2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "auth/Auth.h" + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +using namespace moxygen; + +namespace openmoq::moqx::auth { +namespace { + +constexpr uint8_t kEnvelopeVersion = 1; +constexpr size_t kHmacSha256Len = 32; + +std::string hmacSha256(std::string_view secret, std::string_view payload) { + unsigned int len = 0; + std::string out(kHmacSha256Len, '\0'); + HMAC( + EVP_sha256(), + secret.data(), + static_cast(secret.size()), + reinterpret_cast(payload.data()), + payload.size(), + reinterpret_cast(out.data()), + &len + ); + out.resize(len); + return out; +} + +uint32_t readU32BE(std::string_view data, size_t offset) { + return (static_cast(static_cast(data[offset])) << 24) | + (static_cast(static_cast(data[offset + 1])) << 16) | + (static_cast(static_cast(data[offset + 2])) << 8) | + static_cast(static_cast(data[offset + 3])); +} + +void appendU32BE(std::string& out, uint32_t value) { + out.push_back(static_cast((value >> 24) & 0xff)); + out.push_back(static_cast((value >> 16) & 0xff)); + out.push_back(static_cast((value >> 8) & 0xff)); + out.push_back(static_cast(value & 0xff)); +} + +std::string canonicalNamespace(const TrackNamespace& ns) { + std::string out; + for (const auto& field : ns.trackNamespace) { + appendU32BE(out, static_cast(field.size())); + out.append(field); + } + return out; +} + +bool bytesMatch(std::string_view actual, const MatchRule& rule) { + const auto expected = std::string_view(rule.value); + switch (rule.type) { + case MatchRule::Type::Exact: + return actual == expected; + case MatchRule::Type::Prefix: + return actual.starts_with(expected); + case MatchRule::Type::Suffix: + return actual.size() >= expected.size() && + actual.substr(actual.size() - expected.size()) == expected; + case MatchRule::Type::Contains: + return actual.find(expected) != std::string_view::npos; + } + return false; +} + +bool allRulesMatch(std::string_view actual, const std::vector& rules) { + return std::all_of(rules.begin(), rules.end(), [&](const auto& rule) { + return bytesMatch(actual, rule); + }); +} + +class CborReader { +public: + explicit CborReader(std::string_view data) : data_(data) {} + + bool eof() const { return pos_ == data_.size(); } + + bool readUInt(uint64_t& out) { + uint8_t major = 0; + uint64_t value = 0; + if (!readType(major, value) || major != 0) { + return false; + } + out = value; + return true; + } + + bool readInt(int64_t& out) { + uint8_t major = 0; + uint64_t value = 0; + if (!readType(major, value)) { + return false; + } + if (major == 0) { + if (value > static_cast(std::numeric_limits::max())) { + return false; + } + out = static_cast(value); + return true; + } + if (major == 1) { + if (value > static_cast(std::numeric_limits::max())) { + return false; + } + out = -1 - static_cast(value); + return true; + } + return false; + } + + bool readBytes(std::string& out) { + uint8_t major = 0; + uint64_t len = 0; + if (!readType(major, len) || (major != 2 && major != 3) || len > data_.size() - pos_) { + return false; + } + out.assign(data_.data() + pos_, static_cast(len)); + pos_ += static_cast(len); + return true; + } + + bool readArrayLen(uint64_t& len) { + uint8_t major = 0; + return readType(major, len) && major == 4; + } + + bool readMapLen(uint64_t& len) { + uint8_t major = 0; + return readType(major, len) && major == 5; + } + + bool skip() { + uint8_t major = 0; + uint64_t value = 0; + if (!readType(major, value)) { + return false; + } + switch (major) { + case 0: + case 1: + return true; + case 2: + case 3: + if (value > data_.size() - pos_) { + return false; + } + pos_ += static_cast(value); + return true; + case 4: + for (uint64_t i = 0; i < value; ++i) { + if (!skip()) { + return false; + } + } + return true; + case 5: + for (uint64_t i = 0; i < value * 2; ++i) { + if (!skip()) { + return false; + } + } + return true; + default: + return false; + } + } + +private: + bool readType(uint8_t& major, uint64_t& value) { + if (pos_ >= data_.size()) { + return false; + } + const auto first = static_cast(data_[pos_++]); + major = first >> 5; + const auto addl = first & 0x1f; + if (addl < 24) { + value = addl; + return true; + } + size_t bytes = 0; + if (addl == 24) { + bytes = 1; + } else if (addl == 25) { + bytes = 2; + } else if (addl == 26) { + bytes = 4; + } else if (addl == 27) { + bytes = 8; + } else { + return false; + } + if (bytes > data_.size() - pos_) { + return false; + } + value = 0; + for (size_t i = 0; i < bytes; ++i) { + value = (value << 8) | static_cast(data_[pos_++]); + } + return true; + } + + std::string_view data_; + size_t pos_{0}; +}; + +bool parseAction(CborReader& reader, std::vector& actions) { + int64_t action = 0; + if (!reader.readInt(action) || action < 0) { + return false; + } + actions.push_back(static_cast(static_cast(action))); + return true; +} + +bool parseActions(CborReader& reader, std::vector& actions) { + uint64_t len = 0; + auto copy = reader; + if (copy.readArrayLen(len)) { + reader = copy; + for (uint64_t i = 0; i < len; ++i) { + if (!parseAction(reader, actions)) { + return false; + } + } + return true; + } + return parseAction(reader, actions); +} + +bool parseMatchRules(CborReader& reader, std::vector& rules) { + uint64_t len = 0; + if (!reader.readMapLen(len)) { + return false; + } + for (uint64_t i = 0; i < len; ++i) { + int64_t type = 0; + std::string value; + if (!reader.readInt(type) || type < 0 || type > 3 || !reader.readBytes(value)) { + return false; + } + rules.push_back(MatchRule{ + .type = static_cast(static_cast(type)), + .value = std::move(value), + }); + } + return true; +} + +bool parseScope(CborReader& reader, Scope& scope) { + uint64_t len = 0; + if (!reader.readArrayLen(len) || len != 3) { + return false; + } + return parseActions(reader, scope.actions) && parseMatchRules(reader, scope.namespaceMatches) && + parseMatchRules(reader, scope.trackMatches); +} + +bool parseMoqt(CborReader& reader, Grants& grants) { + uint64_t len = 0; + if (!reader.readArrayLen(len)) { + return false; + } + for (uint64_t i = 0; i < len; ++i) { + Scope scope; + if (!parseScope(reader, scope)) { + return false; + } + grants.scopes.push_back(std::move(scope)); + } + return true; +} + +bool parseClaims(std::string_view data, Grants& grants, bool strictClaims) { + CborReader reader(data); + uint64_t len = 0; + if (!reader.readMapLen(len)) { + return false; + } + for (uint64_t i = 0; i < len; ++i) { + auto keyReader = reader; + int64_t intKey = 0; + std::string textKey; + bool hasIntKey = keyReader.readInt(intKey); + bool hasTextKey = false; + if (hasIntKey) { + reader = keyReader; + } else { + keyReader = reader; + hasTextKey = keyReader.readBytes(textKey); + if (!hasTextKey) { + return false; + } + reader = keyReader; + } + + if ((hasIntKey && intKey == 4) || (hasTextKey && textKey == "exp")) { + int64_t exp = 0; + if (!reader.readInt(exp) || exp < 0) { + return false; + } + grants.expiresAt = std::chrono::system_clock::time_point(std::chrono::seconds(exp)); + } else if (hasTextKey && textKey == "moqt") { + if (!parseMoqt(reader, grants)) { + return false; + } + } else if (hasTextKey && textKey == "moqt-reval") { + int64_t reval = 0; + if (!reader.readInt(reval) || reval < 0) { + return false; + } + grants.revalidateEvery = std::chrono::seconds(reval); + } else if (strictClaims) { + return false; + } else if (!reader.skip()) { + return false; + } + } + return reader.eof(); +} + +} // namespace + +AuthTokenVerifier::AuthTokenVerifier(config::AuthConfig config) : config_(std::move(config)) {} + +folly::Expected AuthTokenVerifier::verify(const AuthToken& token) const { + if (!config_.enabled) { + return Grants{}; + } + if (token.tokenType != config_.tokenType) { + return folly::makeUnexpected(AuthError::WrongTokenType); + } + const auto raw = std::string_view(token.tokenValue); + if (raw.size() < 1 + 1 + 4 + kHmacSha256Len || static_cast(raw[0]) != kEnvelopeVersion) { + return folly::makeUnexpected(AuthError::Malformed); + } + const auto keyLen = static_cast(static_cast(raw[1])); + if (raw.size() < 1 + 1 + keyLen + 4 + kHmacSha256Len) { + return folly::makeUnexpected(AuthError::Malformed); + } + const auto keyID = raw.substr(2, keyLen); + const auto claimsLenOffset = 2 + keyLen; + const auto claimsLen = readU32BE(raw, claimsLenOffset); + const auto signedLen = claimsLenOffset + 4 + claimsLen; + if (raw.size() != signedLen + kHmacSha256Len) { + return folly::makeUnexpected(AuthError::Malformed); + } + + const auto* key = std::find_if(config_.hmacKeys.begin(), config_.hmacKeys.end(), [&](auto& k) { + return k.id == keyID; + }); + if (key == config_.hmacKeys.end()) { + return folly::makeUnexpected(AuthError::BadSignature); + } + const auto expected = hmacSha256(key->secret, raw.substr(0, signedLen)); + if (CRYPTO_memcmp(expected.data(), raw.data() + signedLen, kHmacSha256Len) != 0) { + return folly::makeUnexpected(AuthError::BadSignature); + } + + Grants grants; + if (!parseClaims(raw.substr(claimsLenOffset + 4, claimsLen), grants, config_.strictClaims)) { + return folly::makeUnexpected(AuthError::Malformed); + } + if (grants.expiresAt <= std::chrono::system_clock::now()) { + return folly::makeUnexpected(AuthError::Expired); + } + return grants; +} + +std::string AuthTokenVerifier::signForTest( + std::string_view keyID, + std::string_view secret, + std::string_view cborClaims +) { + std::string out; + out.push_back(static_cast(kEnvelopeVersion)); + out.push_back(static_cast(keyID.size())); + out.append(keyID); + appendU32BE(out, static_cast(cborClaims.size())); + out.append(cborClaims); + out.append(hmacSha256(secret, out)); + return out; +} + +std::optional findAuthToken(const Parameters& params, uint64_t tokenType) { + const auto authKey = folly::to_underlying(TrackRequestParamKey::AUTHORIZATION_TOKEN); + for (const auto& param : params) { + if (param.key == authKey && param.asAuthToken.tokenType == tokenType) { + return param.asAuthToken; + } + } + return std::nullopt; +} + +bool allows( + const Grants& grants, + Action action, + const TrackNamespace& ns, + std::optional trackName, + std::chrono::system_clock::time_point now +) { + if (grants.expiresAt <= now) { + return false; + } + const auto nsBytes = canonicalNamespace(ns); + const auto trackBytes = trackName.value_or(std::string_view()); + for (const auto& scope : grants.scopes) { + if (std::find(scope.actions.begin(), scope.actions.end(), action) == scope.actions.end()) { + continue; + } + if (allRulesMatch(nsBytes, scope.namespaceMatches) && + allRulesMatch(trackBytes, scope.trackMatches)) { + return true; + } + } + return false; +} + +const char* toString(AuthError error) { + switch (error) { + case AuthError::Missing: + return "missing authorization token"; + case AuthError::WrongTokenType: + return "wrong authorization token type"; + case AuthError::Malformed: + return "malformed authorization token"; + case AuthError::BadSignature: + return "invalid authorization token signature"; + case AuthError::Expired: + return "expired authorization token"; + case AuthError::Forbidden: + return "authorization token does not permit action"; + } + return "authorization failed"; +} + +} // namespace openmoq::moqx::auth diff --git a/src/auth/Auth.h b/src/auth/Auth.h new file mode 100644 index 00000000..89c087b4 --- /dev/null +++ b/src/auth/Auth.h @@ -0,0 +1,95 @@ +/* + * Copyright (c) OpenMOQ contributors. + * This source code is licensed under the Apache 2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include "config/Config.h" + +#include +#include + +#include +#include +#include +#include +#include +#include + +namespace openmoq::moqx::auth { + +enum class Action : uint64_t { + ClientSetup = 0, + ServerSetup = 1, + PublishNamespace = 2, + SubscribeNamespace = 3, + Subscribe = 4, + RequestUpdate = 5, + Publish = 6, + Fetch = 7, + TrackStatus = 8, +}; + +enum class AuthError { + Missing, + WrongTokenType, + Malformed, + BadSignature, + Expired, + Forbidden, +}; + +struct MatchRule { + enum class Type : uint64_t { Exact = 0, Prefix = 1, Suffix = 2, Contains = 3 }; + Type type{Type::Exact}; + std::string value; +}; + +struct Scope { + std::vector actions; + std::vector namespaceMatches; + std::vector trackMatches; +}; + +struct Grants { + std::chrono::system_clock::time_point expiresAt{std::chrono::system_clock::time_point::max()}; + std::chrono::seconds revalidateEvery{0}; + std::vector scopes; +}; + +class AuthTokenVerifier { +public: + explicit AuthTokenVerifier(config::AuthConfig config); + + bool enabled() const { return config_.enabled; } + uint64_t tokenType() const { return config_.tokenType; } + bool requireSetupToken() const { return config_.requireSetupToken; } + bool allowRequestTokenOverride() const { return config_.allowRequestTokenOverride; } + + folly::Expected verify(const moxygen::AuthToken& token) const; + + // Internal v1 envelope used by tests and by local token issuers: + // 0x01 | key-id-len:u8 | key-id | claims-len:u32be | CBOR claims | HMAC-SHA256. + static std::string + signForTest(std::string_view keyID, std::string_view secret, std::string_view cborClaims); + +private: + config::AuthConfig config_; +}; + +std::optional +findAuthToken(const moxygen::Parameters& params, uint64_t tokenType); + +bool allows( + const Grants& grants, + Action action, + const moxygen::TrackNamespace& ns, + std::optional trackName = std::nullopt, + std::chrono::system_clock::time_point now = std::chrono::system_clock::now() +); + +const char* toString(AuthError error); + +} // namespace openmoq::moqx::auth diff --git a/src/config/Config.h b/src/config/Config.h index a05e674b..ce9e29d4 100644 --- a/src/config/Config.h +++ b/src/config/Config.h @@ -79,6 +79,20 @@ struct UpstreamConfig { std::chrono::milliseconds idleTimeout{5000}; }; +struct AuthConfig { + struct HmacKey { + std::string id; + std::string secret; + }; + + bool enabled{false}; + uint64_t tokenType{0}; + std::vector hmacKeys; + bool requireSetupToken{true}; + bool allowRequestTokenOverride{true}; + bool strictClaims{false}; +}; + struct ServiceConfig { struct MatchEntry { struct ExactAuthority { @@ -104,6 +118,7 @@ struct ServiceConfig { std::vector match; CacheConfig cache; std::optional upstream; // set if this service chains to an upstream relay + AuthConfig auth; }; struct AdminConfig { diff --git a/src/config/ConfigResolver.cpp b/src/config/ConfigResolver.cpp index 20c29da6..0bb2772a 100644 --- a/src/config/ConfigResolver.cpp +++ b/src/config/ConfigResolver.cpp @@ -312,6 +312,60 @@ UpstreamConfig resolveUpstream(const ParsedUpstreamConfig& upstream) { }; } +void validateAuth( + const std::string& serviceName, + const ParsedAuthConfig& auth, + std::vector& errors +) { + if (!auth.enabled.value()) { + return; + } + const auto& keys = auth.hmac_keys.value(); + if (!keys.has_value() || keys->empty()) { + errors.push_back( + "Service '" + serviceName + "': auth.hmac_keys is required when auth is enabled" + ); + return; + } + std::unordered_set keyIDs; + for (size_t i = 0; i < keys->size(); ++i) { + const auto& key = (*keys)[i]; + const auto prefix = "Service '" + serviceName + "': auth.hmac_keys[" + std::to_string(i) + "]"; + if (key.id.value().empty()) { + errors.push_back(prefix + ".id must be non-empty"); + } else if (!keyIDs.insert(key.id.value()).second) { + errors.push_back(prefix + ".id duplicates another auth key"); + } + if (key.secret.value().empty()) { + errors.push_back(prefix + ".secret must be non-empty"); + } + } + if (auth.token_type.value().value_or(0) >= (uint64_t{1} << 62)) { + errors.push_back("Service '" + serviceName + "': auth.token_type must fit in a QUIC varint"); + } +} + +AuthConfig resolveAuth(const std::optional& parsed) { + AuthConfig out; + if (!parsed) { + return out; + } + out.enabled = parsed->enabled.value(); + out.tokenType = parsed->token_type.value().value_or(0); + out.requireSetupToken = parsed->require_setup_token.value().value_or(true); + out.allowRequestTokenOverride = parsed->allow_request_token_override.value().value_or(true); + out.strictClaims = parsed->strict_claims.value().value_or(false); + if (parsed->hmac_keys.value()) { + for (const auto& key : *parsed->hmac_keys.value()) { + out.hmacKeys.push_back(AuthConfig::HmacKey{ + .id = key.id.value(), + .secret = key.secret.value(), + }); + } + } + return out; +} + // --- Service validation --- void validateService( @@ -392,6 +446,9 @@ void validateService( if (svc.upstream.value().has_value()) { validateUpstream(*svc.upstream.value(), errors); } + if (svc.auth.value().has_value()) { + validateAuth(name, *svc.auth.value(), errors); + } } std::string generateRelayID() { @@ -606,6 +663,7 @@ ServiceConfig resolveService(const ParsedServiceConfig& svc, const ParsedCacheCo .match = std::move(entries), .cache = resolveCacheConfig(cache), .upstream = std::move(upstream), + .auth = resolveAuth(svc.auth.value()), }; } diff --git a/src/config/loader/ParsedConfig.h b/src/config/loader/ParsedConfig.h index c4a68c57..ce9135d3 100644 --- a/src/config/loader/ParsedConfig.h +++ b/src/config/loader/ParsedConfig.h @@ -157,6 +157,26 @@ struct ParsedUpstreamConfig { idle_timeout_ms; }; +struct ParsedAuthConfig { + struct HmacKey { + rfl::Description<"Key identifier embedded in the token envelope", std::string> id; + rfl::Description<"Shared HMAC secret for this key id", std::string> secret; + }; + + rfl::Description<"Enable per-service authorization", bool> enabled; + rfl::Description<"Expected MOQT AUTHORIZATION_TOKEN token type", std::optional> + token_type; + rfl::Description<"Accepted HMAC signing keys", std::optional>> hmac_keys; + rfl::Description<"Require a valid setup token during CLIENT_SETUP", std::optional> + require_setup_token; + rfl::Description< + "Allow request AUTHORIZATION_TOKEN to override setup credentials", + std::optional> + allow_request_token_override; + rfl::Description<"Reject unsupported token claims instead of ignoring them", std::optional> + strict_claims; +}; + struct ParsedServiceConfig { struct MatchRule { struct ExactAuthority { @@ -202,6 +222,10 @@ struct ParsedServiceConfig { "Upstream MoQ server for this service (optional; enables relay chaining)", std::optional> upstream; + rfl::Description< + "Authentication and authorization settings for this service", + std::optional> + auth; }; struct ParsedListenerDefaultsConfig { diff --git a/test/AuthTest.cpp b/test/AuthTest.cpp new file mode 100644 index 00000000..f7f0d19c --- /dev/null +++ b/test/AuthTest.cpp @@ -0,0 +1,146 @@ +/* + * Copyright (c) OpenMOQ contributors. + * This source code is licensed under the Apache 2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "auth/Auth.h" + +#include + +using namespace openmoq::moqx; +using namespace openmoq::moqx::auth; +using namespace moxygen; + +namespace { + +void appendTypeLen(std::string& out, uint8_t major, uint64_t len) { + if (len < 24) { + out.push_back(static_cast((major << 5) | len)); + } else if (len <= 0xff) { + out.push_back(static_cast((major << 5) | 24)); + out.push_back(static_cast(len)); + } else if (len <= 0xffff) { + out.push_back(static_cast((major << 5) | 25)); + out.push_back(static_cast((len >> 8) & 0xff)); + out.push_back(static_cast(len & 0xff)); + } else { + out.push_back(static_cast((major << 5) | 26)); + out.push_back(static_cast((len >> 24) & 0xff)); + out.push_back(static_cast((len >> 16) & 0xff)); + out.push_back(static_cast((len >> 8) & 0xff)); + out.push_back(static_cast(len & 0xff)); + } +} + +std::string cborUInt(uint64_t value) { + std::string out; + appendTypeLen(out, 0, value); + return out; +} + +std::string cborText(std::string_view value) { + std::string out; + appendTypeLen(out, 3, value.size()); + out.append(value); + return out; +} + +std::string cborBytes(std::string_view value) { + std::string out; + appendTypeLen(out, 2, value.size()); + out.append(value); + return out; +} + +std::string cborArray(std::initializer_list values) { + std::string out; + appendTypeLen(out, 4, values.size()); + for (auto value : values) { + out.append(value); + } + return out; +} + +std::string cborMap(std::initializer_list> entries) { + std::string out; + appendTypeLen(out, 5, entries.size()); + for (const auto& [key, value] : entries) { + out.append(key); + out.append(value); + } + return out; +} + +std::string namespaceBytes(const TrackNamespace& ns) { + std::string out; + for (const auto& field : ns.trackNamespace) { + out.push_back(0); + out.push_back(0); + out.push_back(0); + out.push_back(static_cast(field.size())); + out.append(field); + } + return out; +} + +AuthToken +makeToken(std::string claims, std::string_view secret = "secret", std::string_view keyID = "k1") { + return AuthToken{ + .tokenType = 77, + .tokenValue = AuthTokenVerifier::signForTest(keyID, secret, claims), + .alias = AuthToken::DontRegister, + }; +} + +config::AuthConfig makeConfig() { + return config::AuthConfig{ + .enabled = true, + .tokenType = 77, + .hmacKeys = {config::AuthConfig::HmacKey{.id = "k1", .secret = "secret"}}, + .requireSetupToken = true, + .allowRequestTokenOverride = true, + }; +} + +std::string claimsFor(Action action, const TrackNamespace& ns, std::string_view track) { + auto actions = cborArray({cborUInt(static_cast(action))}); + auto nsMatch = cborMap({{cborUInt(0), cborBytes(namespaceBytes(ns))}}); + auto trackMatch = cborMap({{cborUInt(0), cborBytes(track)}}); + auto scope = cborArray({actions, nsMatch, trackMatch}); + auto moqt = cborArray({scope}); + auto exp = cborUInt(4'102'444'800ULL); // 2100-01-01 + return cborMap({{cborUInt(4), exp}, {cborText("moqt"), moqt}}); +} + +} // namespace + +TEST(AuthTest, VerifiesSignedTokenAndAllowsMatchingAction) { + TrackNamespace ns{{"live", "event"}}; + AuthTokenVerifier verifier(makeConfig()); + auto grants = verifier.verify(makeToken(claimsFor(Action::Subscribe, ns, "video"))); + ASSERT_TRUE(grants.hasValue()); + EXPECT_TRUE(allows(grants.value(), Action::Subscribe, ns, "video")); + EXPECT_FALSE(allows(grants.value(), Action::Publish, ns, "video")); + EXPECT_FALSE(allows(grants.value(), Action::Subscribe, ns, "audio")); +} + +TEST(AuthTest, RejectsBadSignature) { + auto token = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); + token.tokenValue.back() ^= 0x01; + AuthTokenVerifier verifier(makeConfig()); + auto grants = verifier.verify(token); + ASSERT_TRUE(grants.hasError()); + EXPECT_EQ(grants.error(), AuthError::BadSignature); +} + +TEST(AuthTest, RejectsExpiredToken) { + auto scope = cborArray( + {cborArray({cborUInt(static_cast(Action::ClientSetup))}), cborMap({}), cborMap({})} + ); + auto claims = cborMap({{cborUInt(4), cborUInt(1)}, {cborText("moqt"), cborArray({scope})}}); + AuthTokenVerifier verifier(makeConfig()); + auto grants = verifier.verify(makeToken(claims)); + ASSERT_TRUE(grants.hasError()); + EXPECT_EQ(grants.error(), AuthError::Expired); +} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c268f372..2bc30449 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -118,6 +118,15 @@ target_link_libraries(moqx_service_matcher_test PRIVATE ) gtest_discover_tests(moqx_service_matcher_test) +add_executable(moqx_auth_test + AuthTest.cpp +) +target_link_libraries(moqx_auth_test PRIVATE + moqx_core + GTest::gtest_main +) +gtest_discover_tests(moqx_auth_test) + add_executable(moqx_relay_context_test MoqxRelayContextTest.cpp ) From 9fab6322aa25018e11d3d1c8237b4aa51abe9b89 Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 1 May 2026 10:16:26 -0700 Subject: [PATCH 3/9] Add agents to ignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index cb37fd0a..7ed815bd 100644 --- a/.gitignore +++ b/.gitignore @@ -28,6 +28,8 @@ _CPack_Packages/ # IDE / tools .vscode/ .claude/ +.agents/ +AGENTS.md # Docker secrets (never commit) docker/.env From 4c73fc811d8971830b0105773aa374b06744772c Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 1 May 2026 10:33:54 -0700 Subject: [PATCH 4/9] fix: address auth docker build errors --- src/MoqxRelayContext.cpp | 2 +- src/auth/Auth.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/MoqxRelayContext.cpp b/src/MoqxRelayContext.cpp index be7ed4bc..3b2a952b 100644 --- a/src/MoqxRelayContext.cpp +++ b/src/MoqxRelayContext.cpp @@ -151,7 +151,7 @@ void MoqxRelayContext::onSessionEnd(std::shared_ptr session) { } folly::Expected MoqxRelayContext::validateAuthority( - const ClientSetup& /*clientSetup*/, + const ClientSetup& clientSetup, uint64_t /*negotiatedVersion*/, std::shared_ptr session ) { diff --git a/src/auth/Auth.cpp b/src/auth/Auth.cpp index 88bd386d..e0fc338c 100644 --- a/src/auth/Auth.cpp +++ b/src/auth/Auth.cpp @@ -362,9 +362,10 @@ folly::Expected AuthTokenVerifier::verify(const AuthToken& to return folly::makeUnexpected(AuthError::Malformed); } - const auto* key = std::find_if(config_.hmacKeys.begin(), config_.hmacKeys.end(), [&](auto& k) { - return k.id == keyID; - }); + const auto key = + std::find_if(config_.hmacKeys.begin(), config_.hmacKeys.end(), [&](const auto& k) { + return std::string_view(k.id) == keyID; + }); if (key == config_.hmacKeys.end()) { return folly::makeUnexpected(AuthError::BadSignature); } From 052b21aefb561741c62c0931366eb5888f05d1d7 Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 1 May 2026 10:35:35 -0700 Subject: [PATCH 5/9] chore: ignore docker dependency staging --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7ed815bd..d8ad19e8 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ _CPack_Packages/ # Dependency scratch dir /.scratch/ +/.docker-deps/ # OS .DS_Store From e27b4dfca99b45db37e6b918ebc59ea762fc564c Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Mon, 4 May 2026 15:11:33 -0700 Subject: [PATCH 6/9] auth: address review coverage and validation --- config.example.yaml | 1 + src/MoqxRelayContext.cpp | 1 + src/auth/Auth.cpp | 55 ++++++++- src/auth/Auth.h | 1 - src/config/ConfigResolver.cpp | 27 +++-- src/config/loader/ParsedConfig.h | 10 +- test/AuthTest.cpp | 187 +++++++++++++++++++++++++++++ test/config/ConfigResolverTest.cpp | 166 +++++++++++++++++++++++++ 8 files changed, 428 insertions(+), 20 deletions(-) diff --git a/config.example.yaml b/config.example.yaml index afcf8b79..437899f1 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -63,6 +63,7 @@ services: # secret: "replace-with-secret" # require_setup_token: true # allow_request_token_override: true + # strict_claims: false # Reject unsupported token claims instead of ignoring them testing: match: diff --git a/src/MoqxRelayContext.cpp b/src/MoqxRelayContext.cpp index 3b2a952b..373c5b33 100644 --- a/src/MoqxRelayContext.cpp +++ b/src/MoqxRelayContext.cpp @@ -182,6 +182,7 @@ folly::Expected MoqxRelayContext::validateAu case auth::AuthError::WrongTokenType: return folly::makeUnexpected(SessionCloseErrorCode::UNAUTHORIZED); } + return folly::makeUnexpected(SessionCloseErrorCode::UNAUTHORIZED); } session->setPublishHandler(it->second.relay); session->setSubscribeHandler(it->second.relay); diff --git a/src/auth/Auth.cpp b/src/auth/Auth.cpp index e0fc338c..6ca80f6b 100644 --- a/src/auth/Auth.cpp +++ b/src/auth/Auth.cpp @@ -10,12 +10,19 @@ #include #include #include +#include +#include #include +#include #include +#include +#include #include +#include #include #include +#include using namespace moxygen; @@ -26,6 +33,46 @@ constexpr uint8_t kEnvelopeVersion = 1; constexpr size_t kHmacSha256Len = 32; std::string hmacSha256(std::string_view secret, std::string_view payload) { +#if OPENSSL_VERSION_MAJOR >= 3 + auto* mac = EVP_MAC_fetch(nullptr, "HMAC", nullptr); + XCHECK(mac) << "EVP_MAC_fetch(HMAC) failed"; + auto macGuard = std::unique_ptr(mac, EVP_MAC_free); + + auto* ctx = EVP_MAC_CTX_new(macGuard.get()); + XCHECK(ctx) << "EVP_MAC_CTX_new failed"; + auto ctxGuard = std::unique_ptr(ctx, EVP_MAC_CTX_free); + + auto digestName = std::array{'S', 'H', 'A', '2', '5', '6', '\0'}; + OSSL_PARAM params[] = { + OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, digestName.data(), 0), + OSSL_PARAM_construct_end(), + }; + XCHECK_EQ( + EVP_MAC_init( + ctxGuard.get(), + reinterpret_cast(secret.data()), + secret.size(), + params + ), + 1 + ) << "EVP_MAC_init failed"; + XCHECK_EQ( + EVP_MAC_update( + ctxGuard.get(), + reinterpret_cast(payload.data()), + payload.size() + ), + 1 + ) << "EVP_MAC_update failed"; + size_t len = kHmacSha256Len; + std::string out(kHmacSha256Len, '\0'); + XCHECK_EQ( + EVP_MAC_final(ctxGuard.get(), reinterpret_cast(out.data()), &len, out.size()), + 1 + ) << "EVP_MAC_final failed"; + out.resize(len); + return out; +#else unsigned int len = 0; std::string out(kHmacSha256Len, '\0'); HMAC( @@ -39,6 +86,7 @@ std::string hmacSha256(std::string_view secret, std::string_view payload) { ); out.resize(len); return out; +#endif } uint32_t readU32BE(std::string_view data, size_t offset) { @@ -231,6 +279,7 @@ bool parseAction(CborReader& reader, std::vector& actions) { bool parseActions(CborReader& reader, std::vector& actions) { uint64_t len = 0; + // Copy the small reader as a backtracking checkpoint: actions may be a scalar or array. auto copy = reader; if (copy.readArrayLen(len)) { reader = copy; @@ -321,11 +370,10 @@ bool parseClaims(std::string_view data, Grants& grants, bool strictClaims) { return false; } } else if (hasTextKey && textKey == "moqt-reval") { - int64_t reval = 0; - if (!reader.readInt(reval) || reval < 0) { + int64_t ignoredReval = 0; + if (!reader.readInt(ignoredReval) || ignoredReval < 0) { return false; } - grants.revalidateEvery = std::chrono::seconds(reval); } else if (strictClaims) { return false; } else if (!reader.skip()) { @@ -389,6 +437,7 @@ std::string AuthTokenVerifier::signForTest( std::string_view secret, std::string_view cborClaims ) { + XDCHECK_LE(keyID.size(), 255u); std::string out; out.push_back(static_cast(kEnvelopeVersion)); out.push_back(static_cast(keyID.size())); diff --git a/src/auth/Auth.h b/src/auth/Auth.h index 89c087b4..102c8434 100644 --- a/src/auth/Auth.h +++ b/src/auth/Auth.h @@ -55,7 +55,6 @@ struct Scope { struct Grants { std::chrono::system_clock::time_point expiresAt{std::chrono::system_clock::time_point::max()}; - std::chrono::seconds revalidateEvery{0}; std::vector scopes; }; diff --git a/src/config/ConfigResolver.cpp b/src/config/ConfigResolver.cpp index 0bb2772a..469af9be 100644 --- a/src/config/ConfigResolver.cpp +++ b/src/config/ConfigResolver.cpp @@ -325,19 +325,20 @@ void validateAuth( errors.push_back( "Service '" + serviceName + "': auth.hmac_keys is required when auth is enabled" ); - return; - } - std::unordered_set keyIDs; - for (size_t i = 0; i < keys->size(); ++i) { - const auto& key = (*keys)[i]; - const auto prefix = "Service '" + serviceName + "': auth.hmac_keys[" + std::to_string(i) + "]"; - if (key.id.value().empty()) { - errors.push_back(prefix + ".id must be non-empty"); - } else if (!keyIDs.insert(key.id.value()).second) { - errors.push_back(prefix + ".id duplicates another auth key"); - } - if (key.secret.value().empty()) { - errors.push_back(prefix + ".secret must be non-empty"); + } else { + std::unordered_set keyIDs; + for (size_t i = 0; i < keys->size(); ++i) { + const auto& key = (*keys)[i]; + const auto prefix = + "Service '" + serviceName + "': auth.hmac_keys[" + std::to_string(i) + "]"; + if (key.id.value().empty()) { + errors.push_back(prefix + ".id must be non-empty"); + } else if (!keyIDs.insert(key.id.value()).second) { + errors.push_back(prefix + ".id duplicates another auth key"); + } + if (key.secret.value().empty()) { + errors.push_back(prefix + ".secret must be non-empty"); + } } } if (auth.token_type.value().value_or(0) >= (uint64_t{1} << 62)) { diff --git a/src/config/loader/ParsedConfig.h b/src/config/loader/ParsedConfig.h index ce9135d3..d7250fe9 100644 --- a/src/config/loader/ParsedConfig.h +++ b/src/config/loader/ParsedConfig.h @@ -167,13 +167,17 @@ struct ParsedAuthConfig { rfl::Description<"Expected MOQT AUTHORIZATION_TOKEN token type", std::optional> token_type; rfl::Description<"Accepted HMAC signing keys", std::optional>> hmac_keys; - rfl::Description<"Require a valid setup token during CLIENT_SETUP", std::optional> + rfl::Description< + "Require a valid setup token during CLIENT_SETUP (default: true)", + std::optional> require_setup_token; rfl::Description< - "Allow request AUTHORIZATION_TOKEN to override setup credentials", + "Allow request AUTHORIZATION_TOKEN to override setup credentials (default: true)", std::optional> allow_request_token_override; - rfl::Description<"Reject unsupported token claims instead of ignoring them", std::optional> + rfl::Description< + "Reject unsupported token claims instead of ignoring them (default: false)", + std::optional> strict_claims; }; diff --git a/test/AuthTest.cpp b/test/AuthTest.cpp index f7f0d19c..e7c83af0 100644 --- a/test/AuthTest.cpp +++ b/test/AuthTest.cpp @@ -103,6 +103,21 @@ config::AuthConfig makeConfig() { }; } +Grants makeGrants( + std::vector actions, + std::vector namespaceMatches, + std::vector trackMatches +) { + Grants grants; + grants.expiresAt = std::chrono::system_clock::now() + std::chrono::hours(1); + grants.scopes.push_back(Scope{ + .actions = std::move(actions), + .namespaceMatches = std::move(namespaceMatches), + .trackMatches = std::move(trackMatches), + }); + return grants; +} + std::string claimsFor(Action action, const TrackNamespace& ns, std::string_view track) { auto actions = cborArray({cborUInt(static_cast(action))}); auto nsMatch = cborMap({{cborUInt(0), cborBytes(namespaceBytes(ns))}}); @@ -113,6 +128,13 @@ std::string claimsFor(Action action, const TrackNamespace& ns, std::string_view return cborMap({{cborUInt(4), exp}, {cborText("moqt"), moqt}}); } +std::string +claimsWithScope(std::string_view actions, std::string_view nsMatch, std::string_view trackMatch) { + auto scope = cborArray({actions, nsMatch, trackMatch}); + auto exp = cborUInt(4'102'444'800ULL); // 2100-01-01 + return cborMap({{cborUInt(4), exp}, {cborText("moqt"), cborArray({scope})}}); +} + } // namespace TEST(AuthTest, VerifiesSignedTokenAndAllowsMatchingAction) { @@ -125,6 +147,81 @@ TEST(AuthTest, VerifiesSignedTokenAndAllowsMatchingAction) { EXPECT_FALSE(allows(grants.value(), Action::Subscribe, ns, "audio")); } +TEST(AuthTest, AllowsPrefixSuffixAndContainsMatchRules) { + TrackNamespace ns{{"live", "event"}}; + const auto canonicalNs = namespaceBytes(ns); + + auto prefixGrants = makeGrants( + {Action::Subscribe}, + {MatchRule{.type = MatchRule::Type::Prefix, .value = canonicalNs.substr(0, 8)}}, + {MatchRule{.type = MatchRule::Type::Prefix, .value = "vid"}} + ); + EXPECT_TRUE(allows(prefixGrants, Action::Subscribe, ns, "video")); + EXPECT_FALSE(allows(prefixGrants, Action::Subscribe, TrackNamespace{{"vod"}}, "video")); + + auto suffixGrants = makeGrants( + {Action::Fetch}, + {MatchRule{ + .type = MatchRule::Type::Suffix, + .value = canonicalNs.substr(canonicalNs.size() - 9) + }}, + {MatchRule{.type = MatchRule::Type::Suffix, .value = ".mp4"}} + ); + EXPECT_TRUE(allows(suffixGrants, Action::Fetch, ns, "clip.mp4")); + EXPECT_FALSE(allows(suffixGrants, Action::Fetch, ns, "clip.m4s")); + + auto containsGrants = makeGrants( + {Action::Publish}, + {MatchRule{.type = MatchRule::Type::Contains, .value = "live"}}, + {MatchRule{.type = MatchRule::Type::Contains, .value = "main"}} + ); + EXPECT_TRUE(allows(containsGrants, Action::Publish, ns, "camera-main")); + EXPECT_FALSE(allows(containsGrants, Action::Publish, ns, "camera-side")); +} + +TEST(AuthTest, EmptyNamespaceAndTrackRulesMatchEverything) { + auto grants = makeGrants({Action::Subscribe}, {}, {}); + + EXPECT_TRUE(allows(grants, Action::Subscribe, TrackNamespace{{"live"}}, "video")); + EXPECT_TRUE(allows(grants, Action::Subscribe, TrackNamespace{}, std::nullopt)); +} + +TEST(AuthTest, AllowsRejectsEmptyScopesAndExpiredGrants) { + Grants emptyScopes; + emptyScopes.expiresAt = std::chrono::system_clock::now() + std::chrono::hours(1); + EXPECT_FALSE(allows(emptyScopes, Action::Subscribe, TrackNamespace{{"live"}}, "video")); + + auto grants = makeGrants({Action::Subscribe}, {}, {}); + const auto now = std::chrono::system_clock::now(); + grants.expiresAt = now - std::chrono::seconds(1); + EXPECT_FALSE(allows(grants, Action::Subscribe, TrackNamespace{{"live"}}, "video", now)); +} + +TEST(AuthTest, FindAuthTokenSelectsMatchingAuthorizationToken) { + Parameters params(FrameType::SUBSCRIBE); + ASSERT_TRUE( + params + .insertParam(Parameter( + static_cast(TrackRequestParamKey::AUTHORIZATION_TOKEN), + AuthToken{.tokenType = 76, .tokenValue = "wrong", .alias = AuthToken::DontRegister} + )) + .hasValue() + ); + ASSERT_TRUE( + params + .insertParam(Parameter( + static_cast(TrackRequestParamKey::AUTHORIZATION_TOKEN), + AuthToken{.tokenType = 77, .tokenValue = "right", .alias = AuthToken::DontRegister} + )) + .hasValue() + ); + + auto token = findAuthToken(params, 77); + ASSERT_TRUE(token.has_value()); + EXPECT_EQ(token->tokenValue, "right"); + EXPECT_FALSE(findAuthToken(params, 78).has_value()); +} + TEST(AuthTest, RejectsBadSignature) { auto token = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); token.tokenValue.back() ^= 0x01; @@ -134,6 +231,71 @@ TEST(AuthTest, RejectsBadSignature) { EXPECT_EQ(grants.error(), AuthError::BadSignature); } +TEST(AuthTest, SelectsConfiguredKeyByID) { + auto config = makeConfig(); + config.hmacKeys.push_back(config::AuthConfig::HmacKey{.id = "k2", .secret = "secret-2"}); + + AuthTokenVerifier verifier(config); + auto grants = verifier.verify( + makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, ""), "secret-2", "k2") + ); + ASSERT_TRUE(grants.hasValue()); + + auto missingKey = verifier.verify( + makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, ""), "secret-3", "k3") + ); + ASSERT_TRUE(missingKey.hasError()); + EXPECT_EQ(missingKey.error(), AuthError::BadSignature); +} + +TEST(AuthTest, RejectsWrongTokenType) { + auto token = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); + token.tokenType = 78; + AuthTokenVerifier verifier(makeConfig()); + + auto grants = verifier.verify(token); + ASSERT_TRUE(grants.hasError()); + EXPECT_EQ(grants.error(), AuthError::WrongTokenType); +} + +TEST(AuthTest, RejectsMalformedTokenEnvelope) { + AuthTokenVerifier verifier(makeConfig()); + + auto empty = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); + empty.tokenValue.clear(); + auto emptyRes = verifier.verify(empty); + ASSERT_TRUE(emptyRes.hasError()); + EXPECT_EQ(emptyRes.error(), AuthError::Malformed); + + auto wrongVersion = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); + wrongVersion.tokenValue[0] = '\x02'; + auto wrongVersionRes = verifier.verify(wrongVersion); + ASSERT_TRUE(wrongVersionRes.hasError()); + EXPECT_EQ(wrongVersionRes.error(), AuthError::Malformed); + + auto truncated = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); + truncated.tokenValue.resize(3); + auto truncatedRes = verifier.verify(truncated); + ASSERT_TRUE(truncatedRes.hasError()); + EXPECT_EQ(truncatedRes.error(), AuthError::Malformed); + + auto claimsLenOverflow = makeToken(claimsFor(Action::ClientSetup, TrackNamespace{}, "")); + const auto claimsLenOffset = size_t{2} + std::string_view("k1").size(); + claimsLenOverflow.tokenValue[claimsLenOffset] = '\x7f'; + auto claimsLenOverflowRes = verifier.verify(claimsLenOverflow); + ASSERT_TRUE(claimsLenOverflowRes.hasError()); + EXPECT_EQ(claimsLenOverflowRes.error(), AuthError::Malformed); +} + +TEST(AuthTest, RejectsMalformedClaims) { + AuthTokenVerifier verifier(makeConfig()); + auto token = makeToken(cborArray({})); + + auto grants = verifier.verify(token); + ASSERT_TRUE(grants.hasError()); + EXPECT_EQ(grants.error(), AuthError::Malformed); +} + TEST(AuthTest, RejectsExpiredToken) { auto scope = cborArray( {cborArray({cborUInt(static_cast(Action::ClientSetup))}), cborMap({}), cborMap({})} @@ -144,3 +306,28 @@ TEST(AuthTest, RejectsExpiredToken) { ASSERT_TRUE(grants.hasError()); EXPECT_EQ(grants.error(), AuthError::Expired); } + +TEST(AuthTest, ParsesScalarActionAndMoqtRevalClaim) { + auto claims = claimsWithScope( + cborUInt(static_cast(Action::ClientSetup)), + cborMap({}), + cborMap({}) + ); + auto withReval = cborMap({ + {cborUInt(4), cborUInt(4'102'444'800ULL)}, + {cborText("moqt"), + cborArray({cborArray( + {cborUInt(static_cast(Action::ClientSetup)), cborMap({}), cborMap({})} + )})}, + {cborText("moqt-reval"), cborUInt(60)}, + }); + + AuthTokenVerifier verifier(makeConfig()); + auto scalarGrants = verifier.verify(makeToken(claims)); + ASSERT_TRUE(scalarGrants.hasValue()); + EXPECT_TRUE(allows(scalarGrants.value(), Action::ClientSetup, TrackNamespace{})); + + auto revalGrants = verifier.verify(makeToken(withReval)); + ASSERT_TRUE(revalGrants.hasValue()); + EXPECT_TRUE(allows(revalGrants.value(), Action::ClientSetup, TrackNamespace{})); +} diff --git a/test/config/ConfigResolverTest.cpp b/test/config/ConfigResolverTest.cpp index 1dd3966f..43edb12f 100644 --- a/test/config/ConfigResolverTest.cpp +++ b/test/config/ConfigResolverTest.cpp @@ -61,6 +61,30 @@ ParsedServiceConfig makeDefaultService() { return svc; } +ParsedAuthConfig::HmacKey makeAuthKey(std::string id = "key-1", std::string secret = "secret") { + ParsedAuthConfig::HmacKey key; + key.id = std::move(id); + key.secret = std::move(secret); + return key; +} + +ParsedAuthConfig makeAuthConfig(std::vector keys = {makeAuthKey()}) { + ParsedAuthConfig auth; + auth.enabled = true; + auth.token_type = std::optional{77}; + auth.hmac_keys = std::optional>{std::move(keys)}; + auth.require_setup_token = std::optional{false}; + auth.allow_request_token_override = std::optional{false}; + auth.strict_claims = std::optional{true}; + return auth; +} + +ParsedServiceConfig makeAuthService(ParsedAuthConfig auth = makeAuthConfig()) { + auto svc = makeDefaultService(); + svc.auth = std::move(auth); + return svc; +} + // Build a minimal valid insecure config with one any-authority service and admin. ParsedConfig makeMinimalInsecureConfig(std::string name = "test") { ParsedConfig cfg; @@ -384,6 +408,148 @@ TEST(ResolveConfig, SameAuthorityDifferentPaths) { ASSERT_TRUE(result.hasValue()); } +TEST(ResolveConfig, AuthEnabledRequiresHmacKeys) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + + ParsedAuthConfig auth; + auth.enabled = true; + cfg.services.value().emplace("svc", makeAuthService(std::move(auth))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasError()); + EXPECT_THAT(result.error(), HasSubstr("auth.hmac_keys is required")); +} + +TEST(ResolveConfig, AuthEnabledRejectsEmptyHmacKeys) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + + ParsedAuthConfig auth; + auth.enabled = true; + std::vector keys; + auth.hmac_keys = std::move(keys); + cfg.services.value().emplace("svc", makeAuthService(std::move(auth))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasError()); + EXPECT_THAT(result.error(), HasSubstr("auth.hmac_keys is required")); +} + +TEST(ResolveConfig, AuthRejectsDuplicateKeyIDs) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + cfg.services.value().emplace( + "svc", + makeAuthService(makeAuthConfig({makeAuthKey("key-1", "a"), makeAuthKey("key-1", "b")})) + ); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasError()); + EXPECT_THAT(result.error(), HasSubstr("duplicates another auth key")); +} + +TEST(ResolveConfig, AuthRejectsEmptyKeyIDAndSecret) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + cfg.services.value().emplace("svc", makeAuthService(makeAuthConfig({makeAuthKey("", "")}))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasError()); + EXPECT_THAT(result.error(), HasSubstr(".id must be non-empty")); + EXPECT_THAT(result.error(), HasSubstr(".secret must be non-empty")); +} + +TEST(ResolveConfig, AuthRejectsTokenTypeOutsideQuicVarint) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + auto auth = makeAuthConfig(); + auth.token_type = std::optional{uint64_t{1} << 62}; + cfg.services.value().emplace("svc", makeAuthService(std::move(auth))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasError()); + EXPECT_THAT(result.error(), HasSubstr("auth.token_type must fit in a QUIC varint")); +} + +TEST(ResolveConfig, AuthAccumulatesMissingKeysAndInvalidTokenType) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + ParsedAuthConfig auth; + auth.enabled = true; + auth.token_type = std::optional{uint64_t{1} << 62}; + cfg.services.value().emplace("svc", makeAuthService(std::move(auth))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasError()); + EXPECT_THAT(result.error(), HasSubstr("auth.hmac_keys is required")); + EXPECT_THAT(result.error(), HasSubstr("auth.token_type must fit in a QUIC varint")); +} + +TEST(ResolveConfig, AuthDisabledResolvesCleanly) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + ParsedAuthConfig auth; + auth.enabled = false; + cfg.services.value().emplace("svc", makeAuthService(std::move(auth))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasValue()); + EXPECT_FALSE(result.value().config.services.at("svc").auth.enabled); +} + +TEST(ResolveConfig, AuthValidConfigRoundTripsAllFields) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + cfg.services.value().emplace( + "svc", + makeAuthService( + makeAuthConfig({makeAuthKey("key-1", "secret-1"), makeAuthKey("key-2", "secret-2")}) + ) + ); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasValue()); + const auto& auth = result.value().config.services.at("svc").auth; + EXPECT_TRUE(auth.enabled); + EXPECT_EQ(auth.tokenType, 77u); + EXPECT_FALSE(auth.requireSetupToken); + EXPECT_FALSE(auth.allowRequestTokenOverride); + EXPECT_TRUE(auth.strictClaims); + ASSERT_EQ(auth.hmacKeys.size(), 2); + EXPECT_EQ(auth.hmacKeys[0].id, "key-1"); + EXPECT_EQ(auth.hmacKeys[0].secret, "secret-1"); + EXPECT_EQ(auth.hmacKeys[1].id, "key-2"); + EXPECT_EQ(auth.hmacKeys[1].secret, "secret-2"); +} + +TEST(ResolveConfig, AuthOptionalFieldsDefaultCorrectly) { + auto cfg = makeMinimalInsecureConfig(); + cfg.services.value().clear(); + ParsedAuthConfig auth; + auth.enabled = true; + std::vector keys{makeAuthKey()}; + auth.hmac_keys = std::move(keys); + cfg.services.value().emplace("svc", makeAuthService(std::move(auth))); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasValue()); + const auto& authOut = result.value().config.services.at("svc").auth; + EXPECT_TRUE(authOut.enabled); + EXPECT_EQ(authOut.tokenType, 0u); + EXPECT_TRUE(authOut.requireSetupToken); + EXPECT_TRUE(authOut.allowRequestTokenOverride); + EXPECT_FALSE(authOut.strictClaims); +} + +TEST(ResolveConfig, AuthAbsentDefaultsDisabled) { + auto cfg = makeMinimalInsecureConfig(); + + auto result = resolveConfig(cfg); + ASSERT_TRUE(result.hasValue()); + EXPECT_FALSE(result.value().config.services.at("default").auth.enabled); +} + // --- Resolution tests --- TEST(ResolveConfig, MinimalInsecure) { From 99fa2bd01a5e82a9b226a1a596a949ba6f6b8d00 Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 29 May 2026 21:50:28 -0700 Subject: [PATCH 7/9] test: pass AuthConfig to MoqxRelay ctor in MoqxTrackFilterTest The MoqxRelay constructor gained a config::AuthConfig parameter in position 3 (before maxDeselected). MoqxTrackFilterTest still used the old argument order, so the uint64_t landed in the AuthConfig slot and failed to compile, breaking every linux/macos/asan/conformance CI job. Pass config::AuthConfig{} explicitly at all four make_shared() call sites. --- test/MoqxTrackFilterTest.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/MoqxTrackFilterTest.cpp b/test/MoqxTrackFilterTest.cpp index 50d6f69c..e70f8f9f 100644 --- a/test/MoqxTrackFilterTest.cpp +++ b/test/MoqxTrackFilterTest.cpp @@ -36,6 +36,7 @@ class MoqxTrackFilterTest : public moxygen::test::MoQRelayTest { relay_ = std::make_shared( config::CacheConfig{0, 0}, // no cache /*relayID=*/"", + config::AuthConfig{}, /*maxDeselected=*/0 ); relay_->setAllowedNamespacePrefix(kPrefix); @@ -524,6 +525,7 @@ TEST_F(MoqxTrackFilterTest, FirstObjectCycle_DoesNotEvictSelectedTracksBeforeFir relay_ = std::make_shared( config::CacheConfig{0, 0}, /*relayID=*/"", + config::AuthConfig{}, /*maxDeselected=*/0, /*idleTimeout=*/std::chrono::seconds(10), /*activityThreshold=*/std::chrono::milliseconds(1) @@ -738,6 +740,7 @@ TEST_F(MoqxTrackFilterTest, DeselectedQueueEviction_EvictsOldestEntry) { relay_ = std::make_shared( config::CacheConfig{0, 0}, // no cache /*relayID=*/"", + config::AuthConfig{}, /*maxDeselected=*/2 ); relay_->setAllowedNamespacePrefix(kPrefix); @@ -827,6 +830,7 @@ TEST_F(MoqxTrackFilterTest, IdleEviction_SilentTrackReplacedByActiveOutsider) { relay_ = std::make_shared( config::CacheConfig{0, 0}, // no cache /*relayID=*/"", + config::AuthConfig{}, /*maxDeselected=*/5, /*idleTimeout=*/std::chrono::milliseconds(10), /*activityThreshold=*/std::chrono::milliseconds(1) From d2b9e592e1b972e66c1559598fd1e2cd11a8cc49 Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 29 May 2026 21:50:34 -0700 Subject: [PATCH 8/9] auth: move CborReader into its own header Extract the internal CBOR decoder from Auth.cpp into a self-contained header-only src/auth/CborReader.h (namespace openmoq::moqx::auth). Auth.cpp now includes it; the claim-parsing helpers are unchanged. No build wiring needed since moqx_core picks up headers via the src/ include dir. --- src/auth/Auth.cpp | 135 +------------------------------------ src/auth/CborReader.h | 153 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 134 deletions(-) create mode 100644 src/auth/CborReader.h diff --git a/src/auth/Auth.cpp b/src/auth/Auth.cpp index 6ca80f6b..fe49303f 100644 --- a/src/auth/Auth.cpp +++ b/src/auth/Auth.cpp @@ -5,6 +5,7 @@ */ #include "auth/Auth.h" +#include "auth/CborReader.h" #include #include @@ -134,140 +135,6 @@ bool allRulesMatch(std::string_view actual, const std::vector& rules) }); } -class CborReader { -public: - explicit CborReader(std::string_view data) : data_(data) {} - - bool eof() const { return pos_ == data_.size(); } - - bool readUInt(uint64_t& out) { - uint8_t major = 0; - uint64_t value = 0; - if (!readType(major, value) || major != 0) { - return false; - } - out = value; - return true; - } - - bool readInt(int64_t& out) { - uint8_t major = 0; - uint64_t value = 0; - if (!readType(major, value)) { - return false; - } - if (major == 0) { - if (value > static_cast(std::numeric_limits::max())) { - return false; - } - out = static_cast(value); - return true; - } - if (major == 1) { - if (value > static_cast(std::numeric_limits::max())) { - return false; - } - out = -1 - static_cast(value); - return true; - } - return false; - } - - bool readBytes(std::string& out) { - uint8_t major = 0; - uint64_t len = 0; - if (!readType(major, len) || (major != 2 && major != 3) || len > data_.size() - pos_) { - return false; - } - out.assign(data_.data() + pos_, static_cast(len)); - pos_ += static_cast(len); - return true; - } - - bool readArrayLen(uint64_t& len) { - uint8_t major = 0; - return readType(major, len) && major == 4; - } - - bool readMapLen(uint64_t& len) { - uint8_t major = 0; - return readType(major, len) && major == 5; - } - - bool skip() { - uint8_t major = 0; - uint64_t value = 0; - if (!readType(major, value)) { - return false; - } - switch (major) { - case 0: - case 1: - return true; - case 2: - case 3: - if (value > data_.size() - pos_) { - return false; - } - pos_ += static_cast(value); - return true; - case 4: - for (uint64_t i = 0; i < value; ++i) { - if (!skip()) { - return false; - } - } - return true; - case 5: - for (uint64_t i = 0; i < value * 2; ++i) { - if (!skip()) { - return false; - } - } - return true; - default: - return false; - } - } - -private: - bool readType(uint8_t& major, uint64_t& value) { - if (pos_ >= data_.size()) { - return false; - } - const auto first = static_cast(data_[pos_++]); - major = first >> 5; - const auto addl = first & 0x1f; - if (addl < 24) { - value = addl; - return true; - } - size_t bytes = 0; - if (addl == 24) { - bytes = 1; - } else if (addl == 25) { - bytes = 2; - } else if (addl == 26) { - bytes = 4; - } else if (addl == 27) { - bytes = 8; - } else { - return false; - } - if (bytes > data_.size() - pos_) { - return false; - } - value = 0; - for (size_t i = 0; i < bytes; ++i) { - value = (value << 8) | static_cast(data_[pos_++]); - } - return true; - } - - std::string_view data_; - size_t pos_{0}; -}; - bool parseAction(CborReader& reader, std::vector& actions) { int64_t action = 0; if (!reader.readInt(action) || action < 0) { diff --git a/src/auth/CborReader.h b/src/auth/CborReader.h new file mode 100644 index 00000000..79e288cf --- /dev/null +++ b/src/auth/CborReader.h @@ -0,0 +1,153 @@ +/* + * Copyright (c) OpenMOQ contributors. + * This source code is licensed under the Apache 2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace openmoq::moqx::auth { + +// Minimal CBOR (RFC 8949) decoder used to parse CAT token claims. +// Internal to the auth subsystem. +class CborReader { +public: + explicit CborReader(std::string_view data) : data_(data) {} + + bool eof() const { return pos_ == data_.size(); } + + bool readUInt(uint64_t& out) { + uint8_t major = 0; + uint64_t value = 0; + if (!readType(major, value) || major != 0) { + return false; + } + out = value; + return true; + } + + bool readInt(int64_t& out) { + uint8_t major = 0; + uint64_t value = 0; + if (!readType(major, value)) { + return false; + } + if (major == 0) { + if (value > static_cast(std::numeric_limits::max())) { + return false; + } + out = static_cast(value); + return true; + } + if (major == 1) { + if (value > static_cast(std::numeric_limits::max())) { + return false; + } + out = -1 - static_cast(value); + return true; + } + return false; + } + + bool readBytes(std::string& out) { + uint8_t major = 0; + uint64_t len = 0; + if (!readType(major, len) || (major != 2 && major != 3) || len > data_.size() - pos_) { + return false; + } + out.assign(data_.data() + pos_, static_cast(len)); + pos_ += static_cast(len); + return true; + } + + bool readArrayLen(uint64_t& len) { + uint8_t major = 0; + return readType(major, len) && major == 4; + } + + bool readMapLen(uint64_t& len) { + uint8_t major = 0; + return readType(major, len) && major == 5; + } + + bool skip() { + uint8_t major = 0; + uint64_t value = 0; + if (!readType(major, value)) { + return false; + } + switch (major) { + case 0: + case 1: + return true; + case 2: + case 3: + if (value > data_.size() - pos_) { + return false; + } + pos_ += static_cast(value); + return true; + case 4: + for (uint64_t i = 0; i < value; ++i) { + if (!skip()) { + return false; + } + } + return true; + case 5: + for (uint64_t i = 0; i < value * 2; ++i) { + if (!skip()) { + return false; + } + } + return true; + default: + return false; + } + } + +private: + bool readType(uint8_t& major, uint64_t& value) { + if (pos_ >= data_.size()) { + return false; + } + const auto first = static_cast(data_[pos_++]); + major = first >> 5; + const auto addl = first & 0x1f; + if (addl < 24) { + value = addl; + return true; + } + size_t bytes = 0; + if (addl == 24) { + bytes = 1; + } else if (addl == 25) { + bytes = 2; + } else if (addl == 26) { + bytes = 4; + } else if (addl == 27) { + bytes = 8; + } else { + return false; + } + if (bytes > data_.size() - pos_) { + return false; + } + value = 0; + for (size_t i = 0; i < bytes; ++i) { + value = (value << 8) | static_cast(data_[pos_++]); + } + return true; + } + + std::string_view data_; + size_t pos_{0}; +}; + +} // namespace openmoq::moqx::auth From f34554caf21970b37efc79c4826805e377ccccbd Mon Sep 17 00:00:00 2001 From: Paul Gregoire Date: Fri, 29 May 2026 22:39:58 -0700 Subject: [PATCH 9/9] fix(review): address PR #264 auth feedback from afrind - auth: split allows() into TrackNamespace and FullTrackName overloads over a shared allowsImpl; update relay and tests accordingly - relay: consolidate authorize() to resolve grants from one source then a single allows() check; pass session in instead of getRequestSession(); log at DBG1 on auth failures and when ignoring a request token override - auth: use folly::Endian for the u32be claims-length helpers - auth: name CBOR major-type / additional-info constants in CborReader - auth: document the CAT "moqt" claim scope CBOR layout; clarify the signForTest comment (test/tooling helper, relay never issues tokens) - auth: reject duplicate well-known claim keys (exp/moqt/moqt-reval) - config: name the QUIC varint upper bound; clarify token_type 0 is a valid out-of-band type; add descriptive comments to auth keys in config.example - test: restore CborReaderTest.cpp and add duplicate-claim-key coverage --- config.example.yaml | 14 +-- src/MoqxRelay.cpp | 61 +++++++--- src/MoqxRelay.h | 1 + src/auth/Auth.cpp | 71 ++++++++++-- src/auth/Auth.h | 14 ++- src/auth/CborReader.h | 61 ++++++---- src/config/ConfigResolver.cpp | 15 ++- test/AuthTest.cpp | 58 +++++++--- test/CMakeLists.txt | 13 +++ test/CborReaderTest.cpp | 206 ++++++++++++++++++++++++++++++++++ 10 files changed, 446 insertions(+), 68 deletions(-) create mode 100644 test/CborReaderTest.cpp diff --git a/config.example.yaml b/config.example.yaml index 437899f1..398c25a4 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -56,13 +56,13 @@ services: # insecure: false # true = skip cert verification (dev only) # # ca_cert: /path/to/ca.pem # Custom CA cert (mutually exclusive with insecure: true) # auth: # Optional: enable CAT-style authorization for this service - # enabled: true - # token_type: 0 # MOQT AUTHORIZATION_TOKEN token type - # hmac_keys: - # - id: "key-1" - # secret: "replace-with-secret" - # require_setup_token: true - # allow_request_token_override: true + # enabled: true # Turn auth on for this service (default: false) + # token_type: 0 # MOQT AUTHORIZATION_TOKEN type; 0 = out-of-band/private type (draft-ietf-moq-transport, "Token Type") + # hmac_keys: # Keys that sign/verify CAT tokens (HMAC-SHA256) + # - id: "key-1" # Key ID carried in the token envelope; selects which secret verifies it + # secret: "replace-with-secret" # Shared secret (use a long, random value) + # require_setup_token: true # Require a token at CLIENT_SETUP, not just on individual requests + # allow_request_token_override: true # Allow a per-request token to override the session's setup grants # strict_claims: false # Reject unsupported token claims instead of ignoring them testing: diff --git a/src/MoqxRelay.cpp b/src/MoqxRelay.cpp index b205f014..388fed7a 100644 --- a/src/MoqxRelay.cpp +++ b/src/MoqxRelay.cpp @@ -124,32 +124,52 @@ folly::Expected MoqxRelay::authorize( auth::Action action, const Parameters& params, const TrackNamespace& ns, + const std::shared_ptr& session, std::optional trackName ) { if (!authVerifier_.enabled()) { return folly::unit; } - auto session = MoQSession::getRequestSession(); - auto token = authVerifier_.allowRequestTokenOverride() - ? auth::findAuthToken(params, authVerifier_.tokenType()) - : std::nullopt; + auto token = auth::findAuthToken(params, authVerifier_.tokenType()); + if (token && !authVerifier_.allowRequestTokenOverride()) { + // Request carried a per-request token but override is disabled, so the + // grants established at session setup govern. This is expected behavior; + // log at DBG1 for visibility rather than failing the request. + XLOG(DBG1) << "authorize: ignoring request AUTHORIZATION_TOKEN for action=" + << static_cast(action) << " (allow_request_token_override is disabled)"; + token.reset(); + } + + // Resolve grants from exactly one source (request token or session), then run + // a single auth::allows() check below. + const auth::Grants* grants = nullptr; + auth::Grants verified; if (token) { - auto grants = authVerifier_.verify(*token); - if (grants.hasError()) { - return folly::makeUnexpected(grants.error()); + auto res = authVerifier_.verify(*token); + if (res.hasError()) { + XLOG(DBG1) << "authorize: request token verification failed for action=" + << static_cast(action) << ": " << auth::toString(res.error()); + return folly::makeUnexpected(res.error()); } - if (auth::allows(grants.value(), action, ns, trackName)) { - return folly::unit; + verified = std::move(res.value()); + grants = &verified; + } else { + auto it = sessionAuth_.find(session.get()); + if (it == sessionAuth_.end()) { + XLOG(DBG1) << "authorize: no session grants for action=" << static_cast(action) + << " ns=" << ns; + return folly::makeUnexpected(auth::AuthError::Missing); } - return folly::makeUnexpected(auth::AuthError::Forbidden); + grants = &it->second; } - auto it = sessionAuth_.find(session.get()); - if (it == sessionAuth_.end()) { - return folly::makeUnexpected(auth::AuthError::Missing); - } - if (!auth::allows(it->second, action, ns, trackName)) { + const bool permitted = + trackName ? auth::allows(*grants, action, FullTrackName{ns, std::string(*trackName)}) + : auth::allows(*grants, action, ns); + if (!permitted) { + XLOG(DBG1) << "authorize: action=" << static_cast(action) + << " not permitted for ns=" << ns; return folly::makeUnexpected(auth::AuthError::Forbidden); } return folly::unit; @@ -250,7 +270,8 @@ folly::coro::Task MoqxRelay::publishNamespac // TODO: store auth for forwarding on future SubscribeNamespace? auto session = MoQSession::getRequestSession(); auto requestID = pubNs.requestID; - auto authRes = authorize(auth::Action::PublishNamespace, pubNs.params, pubNs.trackNamespace); + auto authRes = + authorize(auth::Action::PublishNamespace, pubNs.params, pubNs.trackNamespace, session); if (authRes.hasError()) { co_return folly::makeUnexpected(PublishNamespaceError{ requestID, @@ -341,10 +362,12 @@ Subscriber::PublishResult MoqxRelay::publish(PublishRequest pub, std::shared_ptr handle) { XLOG(DBG1) << __func__ << " ftn=" << pub.fullTrackName; XCHECK(handle) << "Publish handle cannot be null"; + auto session = MoQSession::getRequestSession(); auto authRes = authorize( auth::Action::Publish, pub.params, pub.fullTrackName.trackNamespace, + session, pub.fullTrackName.trackName ); if (authRes.hasError()) { @@ -638,7 +661,7 @@ folly::coro::Task MoqxRelay::subscribeNames auth::Action::SubscribeNamespace, subNs.params, subNs.trackNamespacePrefix, - std::nullopt + session ); if (authRes.hasError()) { co_return folly::makeUnexpected(SubscribeNamespaceError{ @@ -795,6 +818,7 @@ MoqxRelay::subscribe(SubscribeRequest subReq, std::shared_ptr con auth::Action::Subscribe, subReq.params, subReq.fullTrackName.trackNamespace, + session, subReq.fullTrackName.trackName ); if (authRes.hasError()) { @@ -930,6 +954,7 @@ MoqxRelay::fetch(Fetch fetch, std::shared_ptr consumer) { auth::Action::Fetch, fetch.params, fetch.fullTrackName.trackNamespace, + session, fetch.fullTrackName.trackName ); if (authRes.hasError()) { @@ -1002,10 +1027,12 @@ MoqxRelay::fetch(Fetch fetch, std::shared_ptr consumer) { folly::coro::Task MoqxRelay::trackStatus(TrackStatus trackStatus) { XLOG(DBG1) << __func__ << " ftn=" << trackStatus.fullTrackName; + auto session = MoQSession::getRequestSession(); auto authRes = authorize( auth::Action::TrackStatus, trackStatus.params, trackStatus.fullTrackName.trackNamespace, + session, trackStatus.fullTrackName.trackName ); if (authRes.hasError()) { diff --git a/src/MoqxRelay.h b/src/MoqxRelay.h index 365c6a78..6bd6892d 100644 --- a/src/MoqxRelay.h +++ b/src/MoqxRelay.h @@ -309,6 +309,7 @@ class MoqxRelay : public moxygen::Publisher, auth::Action action, const moxygen::Parameters& params, const moxygen::TrackNamespace& ns, + const std::shared_ptr& session, std::optional trackName = std::nullopt ); diff --git a/src/auth/Auth.cpp b/src/auth/Auth.cpp index fe49303f..ebc13922 100644 --- a/src/auth/Auth.cpp +++ b/src/auth/Auth.cpp @@ -91,17 +91,14 @@ std::string hmacSha256(std::string_view secret, std::string_view payload) { } uint32_t readU32BE(std::string_view data, size_t offset) { - return (static_cast(static_cast(data[offset])) << 24) | - (static_cast(static_cast(data[offset + 1])) << 16) | - (static_cast(static_cast(data[offset + 2])) << 8) | - static_cast(static_cast(data[offset + 3])); + uint32_t netOrder = 0; + std::memcpy(&netOrder, data.data() + offset, sizeof(netOrder)); + return folly::Endian::big(netOrder); } void appendU32BE(std::string& out, uint32_t value) { - out.push_back(static_cast((value >> 24) & 0xff)); - out.push_back(static_cast((value >> 16) & 0xff)); - out.push_back(static_cast((value >> 8) & 0xff)); - out.push_back(static_cast(value & 0xff)); + const uint32_t netOrder = folly::Endian::big(value); + out.append(reinterpret_cast(&netOrder), sizeof(netOrder)); } std::string canonicalNamespace(const TrackNamespace& ns) { @@ -179,6 +176,21 @@ bool parseMatchRules(CborReader& reader, std::vector& rules) { return true; } +// A scope is the CBOR encoding of one access grant in the CAT "moqt" claim +// (see draft-law-moq-cat4moqt). It is a fixed 3-element array: +// +// [ actions, namespace-matches, track-matches ] +// +// actions - either a single action integer or a CBOR array of them +// (auth::Action enum values; parsed by parseActions). +// namespace-matches - a CBOR map { match-type => bytes } evaluated against the +// track-matches canonicalized namespace / track name respectively. The +// match-type key is a MatchRule::Type (0=exact, 1=prefix, +// 2=suffix, 3=contains); every entry in the map must match +// (logical AND). An empty map matches everything. +// +// The scope grants the action when its action set contains the requested action +// and both match maps are satisfied (see auth::allows / allowsImpl). bool parseScope(CborReader& reader, Scope& scope) { uint64_t len = 0; if (!reader.readArrayLen(len) || len != 3) { @@ -209,6 +221,11 @@ bool parseClaims(std::string_view data, Grants& grants, bool strictClaims) { if (!reader.readMapLen(len)) { return false; } + // Each well-known claim key may appear at most once; a duplicate is rejected + // rather than silently last-wins. + bool seenExp = false; + bool seenMoqt = false; + bool seenMoqtReval = false; for (uint64_t i = 0; i < len; ++i) { auto keyReader = reader; int64_t intKey = 0; @@ -227,16 +244,28 @@ bool parseClaims(std::string_view data, Grants& grants, bool strictClaims) { } if ((hasIntKey && intKey == 4) || (hasTextKey && textKey == "exp")) { + if (seenExp) { + return false; + } + seenExp = true; int64_t exp = 0; if (!reader.readInt(exp) || exp < 0) { return false; } grants.expiresAt = std::chrono::system_clock::time_point(std::chrono::seconds(exp)); } else if (hasTextKey && textKey == "moqt") { + if (seenMoqt) { + return false; + } + seenMoqt = true; if (!parseMoqt(reader, grants)) { return false; } } else if (hasTextKey && textKey == "moqt-reval") { + if (seenMoqtReval) { + return false; + } + seenMoqtReval = true; int64_t ignoredReval = 0; if (!reader.readInt(ignoredReval) || ignoredReval < 0) { return false; @@ -325,7 +354,11 @@ std::optional findAuthToken(const Parameters& params, uint64_t tokenT return std::nullopt; } -bool allows( +namespace { + +// Shared implementation for both allows() overloads. A namespace-level check +// passes std::nullopt for trackName (the track match rules then see empty bytes). +bool allowsImpl( const Grants& grants, Action action, const TrackNamespace& ns, @@ -349,6 +382,26 @@ bool allows( return false; } +} // namespace + +bool allows( + const Grants& grants, + Action action, + const TrackNamespace& ns, + std::chrono::system_clock::time_point now +) { + return allowsImpl(grants, action, ns, std::nullopt, now); +} + +bool allows( + const Grants& grants, + Action action, + const FullTrackName& ftn, + std::chrono::system_clock::time_point now +) { + return allowsImpl(grants, action, ftn.trackNamespace, std::string_view(ftn.trackName), now); +} + const char* toString(AuthError error) { switch (error) { case AuthError::Missing: diff --git a/src/auth/Auth.h b/src/auth/Auth.h index 102c8434..04f43037 100644 --- a/src/auth/Auth.h +++ b/src/auth/Auth.h @@ -69,7 +69,9 @@ class AuthTokenVerifier { folly::Expected verify(const moxygen::AuthToken& token) const; - // Internal v1 envelope used by tests and by local token issuers: + // Builds the internal v1 token envelope. Test/tooling helper only -- the relay + // verifies tokens but never issues them; this lives alongside verify() so the + // two stay in sync on the envelope layout: // 0x01 | key-id-len:u8 | key-id | claims-len:u32be | CBOR claims | HMAC-SHA256. static std::string signForTest(std::string_view keyID, std::string_view secret, std::string_view cborClaims); @@ -81,11 +83,19 @@ class AuthTokenVerifier { std::optional findAuthToken(const moxygen::Parameters& params, uint64_t tokenType); +// Namespace-level authorization (e.g. PublishNamespace, SubscribeNamespace, setup). bool allows( const Grants& grants, Action action, const moxygen::TrackNamespace& ns, - std::optional trackName = std::nullopt, + std::chrono::system_clock::time_point now = std::chrono::system_clock::now() +); + +// Track-level authorization (e.g. Subscribe, Publish, Fetch, TrackStatus). +bool allows( + const Grants& grants, + Action action, + const moxygen::FullTrackName& ftn, std::chrono::system_clock::time_point now = std::chrono::system_clock::now() ); diff --git a/src/auth/CborReader.h b/src/auth/CborReader.h index 79e288cf..486e8eb0 100644 --- a/src/auth/CborReader.h +++ b/src/auth/CborReader.h @@ -14,6 +14,27 @@ namespace openmoq::moqx::auth { +// CBOR (RFC 8949 §3) initial-byte encoding. The high 3 bits select the major +// type; the low 5 bits ("additional information") either hold the argument +// directly (< 24) or name how many big-endian bytes follow (24..27 => 1/2/4/8). +namespace cbor { +constexpr unsigned kMajorShift = 5; +constexpr uint8_t kAddlInfoMask = 0x1f; + +constexpr uint8_t kMajorUnsigned = 0; // unsigned integer +constexpr uint8_t kMajorNegative = 1; // negative integer +constexpr uint8_t kMajorByteString = 2; // byte string +constexpr uint8_t kMajorTextString = 3; // text string +constexpr uint8_t kMajorArray = 4; // array +constexpr uint8_t kMajorMap = 5; // map + +constexpr uint8_t kAddlInfoDirectMax = 24; // additional info < this is the value itself +constexpr uint8_t kAddlInfo1Byte = 24; +constexpr uint8_t kAddlInfo2Byte = 25; +constexpr uint8_t kAddlInfo4Byte = 26; +constexpr uint8_t kAddlInfo8Byte = 27; +} // namespace cbor + // Minimal CBOR (RFC 8949) decoder used to parse CAT token claims. // Internal to the auth subsystem. class CborReader { @@ -25,7 +46,7 @@ class CborReader { bool readUInt(uint64_t& out) { uint8_t major = 0; uint64_t value = 0; - if (!readType(major, value) || major != 0) { + if (!readType(major, value) || major != cbor::kMajorUnsigned) { return false; } out = value; @@ -38,14 +59,14 @@ class CborReader { if (!readType(major, value)) { return false; } - if (major == 0) { + if (major == cbor::kMajorUnsigned) { if (value > static_cast(std::numeric_limits::max())) { return false; } out = static_cast(value); return true; } - if (major == 1) { + if (major == cbor::kMajorNegative) { if (value > static_cast(std::numeric_limits::max())) { return false; } @@ -58,7 +79,9 @@ class CborReader { bool readBytes(std::string& out) { uint8_t major = 0; uint64_t len = 0; - if (!readType(major, len) || (major != 2 && major != 3) || len > data_.size() - pos_) { + if (!readType(major, len) || + (major != cbor::kMajorByteString && major != cbor::kMajorTextString) || + len > data_.size() - pos_) { return false; } out.assign(data_.data() + pos_, static_cast(len)); @@ -68,12 +91,12 @@ class CborReader { bool readArrayLen(uint64_t& len) { uint8_t major = 0; - return readType(major, len) && major == 4; + return readType(major, len) && major == cbor::kMajorArray; } bool readMapLen(uint64_t& len) { uint8_t major = 0; - return readType(major, len) && major == 5; + return readType(major, len) && major == cbor::kMajorMap; } bool skip() { @@ -83,24 +106,24 @@ class CborReader { return false; } switch (major) { - case 0: - case 1: + case cbor::kMajorUnsigned: + case cbor::kMajorNegative: return true; - case 2: - case 3: + case cbor::kMajorByteString: + case cbor::kMajorTextString: if (value > data_.size() - pos_) { return false; } pos_ += static_cast(value); return true; - case 4: + case cbor::kMajorArray: for (uint64_t i = 0; i < value; ++i) { if (!skip()) { return false; } } return true; - case 5: + case cbor::kMajorMap: for (uint64_t i = 0; i < value * 2; ++i) { if (!skip()) { return false; @@ -118,20 +141,20 @@ class CborReader { return false; } const auto first = static_cast(data_[pos_++]); - major = first >> 5; - const auto addl = first & 0x1f; - if (addl < 24) { + major = first >> cbor::kMajorShift; + const auto addl = first & cbor::kAddlInfoMask; + if (addl < cbor::kAddlInfoDirectMax) { value = addl; return true; } size_t bytes = 0; - if (addl == 24) { + if (addl == cbor::kAddlInfo1Byte) { bytes = 1; - } else if (addl == 25) { + } else if (addl == cbor::kAddlInfo2Byte) { bytes = 2; - } else if (addl == 26) { + } else if (addl == cbor::kAddlInfo4Byte) { bytes = 4; - } else if (addl == 27) { + } else if (addl == cbor::kAddlInfo8Byte) { bytes = 8; } else { return false; diff --git a/src/config/ConfigResolver.cpp b/src/config/ConfigResolver.cpp index 8410a549..56064741 100644 --- a/src/config/ConfigResolver.cpp +++ b/src/config/ConfigResolver.cpp @@ -312,6 +312,16 @@ UpstreamConfig resolveUpstream(const ParsedUpstreamConfig& upstream) { }; } +// Minimum HMAC-SHA256 shared-secret length. Short secrets undermine the MAC, so +// reject them at config load when auth is enabled. +constexpr size_t kMinHmacSecretBytes = 16; + +// MOQT carries token_type as a QUIC variable-length integer (RFC 9000 Section +// 16), whose maximum encodable value is 2^62 - 1. This bound is a property of +// the wire encoding, not a moqx policy, so it is stable across MOQT drafts: a +// value at or above 2^62 simply cannot be serialized as a varint. +constexpr uint64_t kQuicVarintExclusiveUpperBound = uint64_t{1} << 62; + void validateAuth( const std::string& serviceName, const ParsedAuthConfig& auth, @@ -341,7 +351,10 @@ void validateAuth( } } } - if (auth.token_type.value().value_or(0) >= (uint64_t{1} << 62)) { + // token_type 0 is a valid MOQT AUTHORIZATION_TOKEN type and is accepted as-is; + // it is not treated as "unset". An omitted token_type also resolves to 0 (see + // resolveAuth), so omitting it and setting token_type: 0 behave identically. + if (auth.token_type.value().value_or(0) >= kQuicVarintExclusiveUpperBound) { errors.push_back("Service '" + serviceName + "': auth.token_type must fit in a QUIC varint"); } } diff --git a/test/AuthTest.cpp b/test/AuthTest.cpp index e7c83af0..43f2bbf1 100644 --- a/test/AuthTest.cpp +++ b/test/AuthTest.cpp @@ -142,9 +142,9 @@ TEST(AuthTest, VerifiesSignedTokenAndAllowsMatchingAction) { AuthTokenVerifier verifier(makeConfig()); auto grants = verifier.verify(makeToken(claimsFor(Action::Subscribe, ns, "video"))); ASSERT_TRUE(grants.hasValue()); - EXPECT_TRUE(allows(grants.value(), Action::Subscribe, ns, "video")); - EXPECT_FALSE(allows(grants.value(), Action::Publish, ns, "video")); - EXPECT_FALSE(allows(grants.value(), Action::Subscribe, ns, "audio")); + EXPECT_TRUE(allows(grants.value(), Action::Subscribe, FullTrackName{ns, "video"})); + EXPECT_FALSE(allows(grants.value(), Action::Publish, FullTrackName{ns, "video"})); + EXPECT_FALSE(allows(grants.value(), Action::Subscribe, FullTrackName{ns, "audio"})); } TEST(AuthTest, AllowsPrefixSuffixAndContainsMatchRules) { @@ -156,8 +156,10 @@ TEST(AuthTest, AllowsPrefixSuffixAndContainsMatchRules) { {MatchRule{.type = MatchRule::Type::Prefix, .value = canonicalNs.substr(0, 8)}}, {MatchRule{.type = MatchRule::Type::Prefix, .value = "vid"}} ); - EXPECT_TRUE(allows(prefixGrants, Action::Subscribe, ns, "video")); - EXPECT_FALSE(allows(prefixGrants, Action::Subscribe, TrackNamespace{{"vod"}}, "video")); + EXPECT_TRUE(allows(prefixGrants, Action::Subscribe, FullTrackName{ns, "video"})); + EXPECT_FALSE( + allows(prefixGrants, Action::Subscribe, FullTrackName{TrackNamespace{{"vod"}}, "video"}) + ); auto suffixGrants = makeGrants( {Action::Fetch}, @@ -167,34 +169,38 @@ TEST(AuthTest, AllowsPrefixSuffixAndContainsMatchRules) { }}, {MatchRule{.type = MatchRule::Type::Suffix, .value = ".mp4"}} ); - EXPECT_TRUE(allows(suffixGrants, Action::Fetch, ns, "clip.mp4")); - EXPECT_FALSE(allows(suffixGrants, Action::Fetch, ns, "clip.m4s")); + EXPECT_TRUE(allows(suffixGrants, Action::Fetch, FullTrackName{ns, "clip.mp4"})); + EXPECT_FALSE(allows(suffixGrants, Action::Fetch, FullTrackName{ns, "clip.m4s"})); auto containsGrants = makeGrants( {Action::Publish}, {MatchRule{.type = MatchRule::Type::Contains, .value = "live"}}, {MatchRule{.type = MatchRule::Type::Contains, .value = "main"}} ); - EXPECT_TRUE(allows(containsGrants, Action::Publish, ns, "camera-main")); - EXPECT_FALSE(allows(containsGrants, Action::Publish, ns, "camera-side")); + EXPECT_TRUE(allows(containsGrants, Action::Publish, FullTrackName{ns, "camera-main"})); + EXPECT_FALSE(allows(containsGrants, Action::Publish, FullTrackName{ns, "camera-side"})); } TEST(AuthTest, EmptyNamespaceAndTrackRulesMatchEverything) { auto grants = makeGrants({Action::Subscribe}, {}, {}); - EXPECT_TRUE(allows(grants, Action::Subscribe, TrackNamespace{{"live"}}, "video")); - EXPECT_TRUE(allows(grants, Action::Subscribe, TrackNamespace{}, std::nullopt)); + EXPECT_TRUE(allows(grants, Action::Subscribe, FullTrackName{TrackNamespace{{"live"}}, "video"})); + EXPECT_TRUE(allows(grants, Action::Subscribe, TrackNamespace{})); } TEST(AuthTest, AllowsRejectsEmptyScopesAndExpiredGrants) { Grants emptyScopes; emptyScopes.expiresAt = std::chrono::system_clock::now() + std::chrono::hours(1); - EXPECT_FALSE(allows(emptyScopes, Action::Subscribe, TrackNamespace{{"live"}}, "video")); + EXPECT_FALSE( + allows(emptyScopes, Action::Subscribe, FullTrackName{TrackNamespace{{"live"}}, "video"}) + ); auto grants = makeGrants({Action::Subscribe}, {}, {}); const auto now = std::chrono::system_clock::now(); grants.expiresAt = now - std::chrono::seconds(1); - EXPECT_FALSE(allows(grants, Action::Subscribe, TrackNamespace{{"live"}}, "video", now)); + EXPECT_FALSE( + allows(grants, Action::Subscribe, FullTrackName{TrackNamespace{{"live"}}, "video"}, now) + ); } TEST(AuthTest, FindAuthTokenSelectsMatchingAuthorizationToken) { @@ -296,6 +302,32 @@ TEST(AuthTest, RejectsMalformedClaims) { EXPECT_EQ(grants.error(), AuthError::Malformed); } +TEST(AuthTest, RejectsDuplicateWellKnownClaimKeys) { + AuthTokenVerifier verifier(makeConfig()); + + // exp present twice must be rejected rather than silently last-wins. + auto dupExp = cborMap({ + {cborUInt(4), cborUInt(4'102'444'800ULL)}, + {cborUInt(4), cborUInt(4'102'444'800ULL)}, + }); + auto dupExpRes = verifier.verify(makeToken(dupExp)); + ASSERT_TRUE(dupExpRes.hasError()); + EXPECT_EQ(dupExpRes.error(), AuthError::Malformed); + + // moqt present twice must be rejected too. + auto scope = cborArray( + {cborArray({cborUInt(static_cast(Action::ClientSetup))}), cborMap({}), cborMap({})} + ); + auto dupMoqt = cborMap({ + {cborUInt(4), cborUInt(4'102'444'800ULL)}, + {cborText("moqt"), cborArray({scope})}, + {cborText("moqt"), cborArray({scope})}, + }); + auto dupMoqtRes = verifier.verify(makeToken(dupMoqt)); + ASSERT_TRUE(dupMoqtRes.hasError()); + EXPECT_EQ(dupMoqtRes.error(), AuthError::Malformed); +} + TEST(AuthTest, RejectsExpiredToken) { auto scope = cborArray( {cborArray({cborUInt(static_cast(Action::ClientSetup))}), cborMap({}), cborMap({})} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1a7f0820..43b801ff 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -154,6 +154,19 @@ target_link_libraries(moqx_auth_test PRIVATE ) gtest_discover_tests(moqx_auth_test) +# CborReader unit tests (header-only decoder, tested in isolation) +add_executable(moqx_cbor_reader_test + CborReaderTest.cpp +) +target_include_directories(moqx_cbor_reader_test PRIVATE + ${PROJECT_SOURCE_DIR}/src +) +target_link_libraries(moqx_cbor_reader_test PRIVATE + GTest::gtest_main + GTest::gmock +) +gtest_discover_tests(moqx_cbor_reader_test) + add_executable(moqx_relay_context_test MoqxRelayContextTest.cpp ) diff --git a/test/CborReaderTest.cpp b/test/CborReaderTest.cpp new file mode 100644 index 00000000..c2bc9be5 --- /dev/null +++ b/test/CborReaderTest.cpp @@ -0,0 +1,206 @@ +/* + * Copyright (c) OpenMOQ contributors. + * This source code is licensed under the Apache 2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "auth/CborReader.h" + +#include + +#include +#include +#include + +using openmoq::moqx::auth::CborReader; + +namespace { + +// Encode a CBOR data item head (major type + argument) using the minimal-width +// integer encoding, mirroring how real encoders lay out the wire bytes. +void putHead(std::string& out, uint8_t major, uint64_t value) { + const uint8_t hi = static_cast(major << 5); + if (value < 24) { + out.push_back(static_cast(hi | value)); + } else if (value <= 0xff) { + out.push_back(static_cast(hi | 24)); + out.push_back(static_cast(value)); + } else if (value <= 0xffff) { + out.push_back(static_cast(hi | 25)); + out.push_back(static_cast((value >> 8) & 0xff)); + out.push_back(static_cast(value & 0xff)); + } else if (value <= 0xffffffffULL) { + out.push_back(static_cast(hi | 26)); + for (int shift = 24; shift >= 0; shift -= 8) { + out.push_back(static_cast((value >> shift) & 0xff)); + } + } else { + out.push_back(static_cast(hi | 27)); + for (int shift = 56; shift >= 0; shift -= 8) { + out.push_back(static_cast((value >> shift) & 0xff)); + } + } +} + +std::string uint64Cbor(uint64_t value) { + std::string out; + putHead(out, 0, value); + return out; +} + +} // namespace + +TEST(CborReaderTest, ReadUIntDecodesAllArgumentWidths) { + const uint64_t values[] = { + 0, + 23, + 24, + 0xff, + 0x100, + 0xffff, + 0x10000, + 0xffffffffULL, + 0x100000000ULL, + }; + for (uint64_t v : values) { + CborReader reader(uint64Cbor(v)); + uint64_t out = 1; + ASSERT_TRUE(reader.readUInt(out)) << "value=" << v; + EXPECT_EQ(out, v); + EXPECT_TRUE(reader.eof()); + } +} + +TEST(CborReaderTest, ReadUIntRejectsNonUnsignedMajorType) { + std::string neg; + putHead(neg, 1, 5); // negative integer + CborReader reader(neg); + uint64_t out = 0; + EXPECT_FALSE(reader.readUInt(out)); +} + +TEST(CborReaderTest, ReadIntDecodesPositiveAndNegative) { + { + CborReader reader(uint64Cbor(5)); + int64_t out = 0; + ASSERT_TRUE(reader.readInt(out)); + EXPECT_EQ(out, 5); + } + { + std::string neg; + putHead(neg, 1, 0); // -1 - 0 = -1 + CborReader reader(neg); + int64_t out = 0; + ASSERT_TRUE(reader.readInt(out)); + EXPECT_EQ(out, -1); + } + { + std::string neg; + putHead(neg, 1, 9); // -1 - 9 = -10 + CborReader reader(neg); + int64_t out = 0; + ASSERT_TRUE(reader.readInt(out)); + EXPECT_EQ(out, -10); + } +} + +TEST(CborReaderTest, ReadIntRejectsValuesThatOverflowInt64) { + const uint64_t tooBig = static_cast(std::numeric_limits::max()) + 1; + { + std::string pos; + putHead(pos, 0, tooBig); + CborReader reader(pos); + int64_t out = 0; + EXPECT_FALSE(reader.readInt(out)); + } + { + std::string neg; + putHead(neg, 1, tooBig); + CborReader reader(neg); + int64_t out = 0; + EXPECT_FALSE(reader.readInt(out)); + } +} + +TEST(CborReaderTest, ReadBytesHandlesByteAndTextStrings) { + for (uint8_t major : {uint8_t{2}, uint8_t{3}}) { + std::string data; + putHead(data, major, 3); + data.append("abc"); + CborReader reader(data); + std::string out; + ASSERT_TRUE(reader.readBytes(out)) << "major=" << int(major); + EXPECT_EQ(out, "abc"); + EXPECT_TRUE(reader.eof()); + } +} + +TEST(CborReaderTest, ReadBytesRejectsLengthPastEndOfBuffer) { + std::string data; + putHead(data, 2, 5); // claims 5 bytes... + data.append("ab"); // ...but only 2 are present + CborReader reader(data); + std::string out; + EXPECT_FALSE(reader.readBytes(out)); +} + +TEST(CborReaderTest, ReadArrayAndMapLengths) { + { + std::string arr; + putHead(arr, 4, 7); + CborReader reader(arr); + uint64_t len = 0; + ASSERT_TRUE(reader.readArrayLen(len)); + EXPECT_EQ(len, 7u); + EXPECT_FALSE(reader.readMapLen(len)); // nothing left, and wrong major anyway + } + { + std::string map; + putHead(map, 5, 2); + CborReader reader(map); + uint64_t len = 0; + ASSERT_TRUE(reader.readMapLen(len)); + EXPECT_EQ(len, 2u); + } +} + +TEST(CborReaderTest, SkipTraversesNestedContainers) { + // [ 1, "hi", { 2: 3 } ] + std::string data; + putHead(data, 4, 3); + putHead(data, 0, 1); + putHead(data, 3, 2); + data.append("hi"); + putHead(data, 5, 1); + putHead(data, 0, 2); + putHead(data, 0, 3); + // Trailing sentinel so we can confirm skip() consumed exactly the array. + putHead(data, 0, 99); + + CborReader reader(data); + ASSERT_TRUE(reader.skip()); + uint64_t sentinel = 0; + ASSERT_TRUE(reader.readUInt(sentinel)); + EXPECT_EQ(sentinel, 99u); + EXPECT_TRUE(reader.eof()); +} + +TEST(CborReaderTest, RejectsTruncatedMultiByteHeader) { + std::string data; + data.push_back(static_cast((0 << 5) | 25)); // major 0, expects 2 arg bytes + data.push_back(static_cast(0x01)); // only 1 present + CborReader reader(data); + uint64_t out = 0; + EXPECT_FALSE(reader.readUInt(out)); +} + +TEST(CborReaderTest, EofReflectsConsumption) { + CborReader empty(std::string_view{}); + EXPECT_TRUE(empty.eof()); + + CborReader reader(uint64Cbor(42)); + EXPECT_FALSE(reader.eof()); + uint64_t out = 0; + ASSERT_TRUE(reader.readUInt(out)); + EXPECT_TRUE(reader.eof()); +}