diff --git a/cpp/module_proxy.cpp b/cpp/module_proxy.cpp index 4e06b2f..1df0df4 100644 --- a/cpp/module_proxy.cpp +++ b/cpp/module_proxy.cpp @@ -95,18 +95,6 @@ QVariant ModuleProxy::callRemoteMethod(const QString& authToken, const QString& return m_provider->callMethod(methodName, args); } -bool ModuleProxy::informModuleToken(const QString& authToken, const QString& moduleName, const QString& token) -{ - Q_UNUSED(authToken) - - if (!m_provider) { - qWarning() << "ModuleProxy: Cannot inform token on null provider"; - return false; - } - - return m_provider->informModuleToken(moduleName, token); -} - namespace { // note: this is to ensure comparison is constant time to prevent timing attacks // Length-independent constant-time comparison of two tokens. Returns true only @@ -129,6 +117,36 @@ bool constantTimeEquals(const QString& a, const QString& b) } } // namespace +bool ModuleProxy::informModuleToken(const QString& authToken, const QString& moduleName, const QString& token) +{ + if (!m_provider) { + qWarning() << "ModuleProxy: Cannot inform token on null provider"; + return false; + } + + const QString coreToken = TokenManager::instance().getToken(QStringLiteral("core")); + const QString capToken = TokenManager::instance().getToken(QStringLiteral("capability_module")); + const bool callerIsTrusted = + (!coreToken.isEmpty() && constantTimeEquals(authToken, coreToken)) || + (!capToken.isEmpty() && constantTimeEquals(authToken, capToken)); + if (authToken.isEmpty() || !callerIsTrusted) { + qWarning() << "ModuleProxy: rejecting informModuleToken for" << moduleName + << "- caller is not the trusted core/capability_module channel"; + return false; + } + + if (moduleName.isEmpty()) { + qWarning() << "ModuleProxy: Cannot inform token with empty module name"; + return false; + } + if (token.isEmpty()) { + qWarning() << "ModuleProxy: Cannot inform empty token for module:" << moduleName; + return false; + } + + return m_provider->informModuleToken(moduleName, token); +} + bool ModuleProxy::isAuthorized(const QString& authToken) const { // Fail closed: an empty token is never valid, even if some empty value diff --git a/docs/docs.md b/docs/docs.md index bcfd81a..27dca09 100644 --- a/docs/docs.md +++ b/docs/docs.md @@ -205,13 +205,13 @@ Modules never instantiate `ModuleProxy` directly; it is created by the provider - Introspect the wrapped module’s events via `getPluginEvents()`, returning a `QJsonArray` describing each `logos_events:` declaration (name, signature, parameters, and — when documented — a `description`; no return type, since events are void). `getPluginInterface()` returns both methods and events in one array (each entry tagged with a `"type"`). All three are filtered views over the provider's single `getMethods()` call — there is no separate `getEvents()` vtable method, which keeps the provider ABI stable across SDK versions - Provide an `eventResponse` signal that the provider emits when events are forwarded to subscribers - Store tokens issued by other modules via `saveToken(fromModuleName, token)` -- Allow a module or consumer to inform another module of a token via `informModuleToken(authToken, moduleName, token)` +- Allow the trusted core / capability module to inform this module of a token via `informModuleToken(authToken, moduleName, token)`. This is a **privileged** operation: planting a token would otherwise let any peer authorize itself (see the security note below), so `informModuleToken` validates `authToken` against this module's own seed secret (stored under the `core` / `capability_module` keys by the host at module init) and rejects the call — failing closed — unless the caller presents that secret | Method | Purpose | | --------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | | `explicit ModuleProxy(QObject* module, QObject *parent = nullptr)` | Wraps `module` for remote access. | | `QVariant callRemoteMethod(const QString& authToken, const QString& methodName, const QVariantList& args = {})` | Validates `authToken`, locates `methodName` on the module and invokes it. Supports up to five arguments and multiple return types. This will forward the request to the wrapped object. | -| `bool informModuleToken(const QString& authToken, const QString& moduleName, const QString& token)` | Stores `token` for `moduleName` in the global `TokenManager`. This is used by the core and capability module to let this module know that another module will communicate using a certain token,=. | +| `bool informModuleToken(const QString& authToken, const QString& moduleName, const QString& token)` | Stores `token` for `moduleName` in the global `TokenManager`, letting this module know that another module will communicate using that token. **Privileged**: `authToken` must match this module's seed secret (the value the host stores under the `core` / `capability_module` keys at init), so only the trusted core / capability module can plant a token. Empty or non-matching tokens are rejected (fail-closed) and `false` is returned. | | `QJsonArray getPluginMethods()` | Enumerates the wrapped module’s methods and returns a JSON array with signatures and parameters. Generated provider/universal modules also include a per-method `description` (from the method's header doc comment); legacy modules introspected via Qt meta‑object have none. | | `QJsonArray getPluginEvents()` | Enumerates the wrapped module’s `logos_events:` declarations and returns a JSON array with names, signatures, and parameters (plus a per-event `description` from the declaration's doc comment). Universal modules report their declared events; legacy/provider modules return an empty array. | | `QJsonArray getPluginInterface()` | Returns the module’s whole interface — methods and events together — each entry tagged with a `"type"` (`"method"`/`"event"`). `getPluginMethods`/`getPluginEvents` are the filtered views; all three derive from one `getMethods()` call (no separate `getEvents()` vtable method, so the provider ABI stays stable). | @@ -495,7 +495,7 @@ signals: | Method | Purpose | |--------|---------| | `callRemoteMethod(authToken, methodName, args) → QVariant` | Validates `authToken`, locates `methodName` on the module and invokes it | -| `informModuleToken(authToken, moduleName, token) → bool` | Stores `token` for `moduleName` in the global `TokenManager` | +| `informModuleToken(authToken, moduleName, token) → bool` | Stores `token` for `moduleName` in the global `TokenManager`. **Privileged**: only the trusted core / capability module channel may call it — `authToken` must match this module's seed secret (the `core` / `capability_module` token the host plants at init); empty or non-matching tokens are rejected and return `false` | | `getPluginMethods() → QJsonArray` | Enumerates the wrapped module's methods (name, signature, return type, parameters, and a per-method `description` for documented provider/universal methods) | | `getPluginEvents() → QJsonArray` | Enumerates the wrapped module's `logos_events:` declarations (name, signature, parameters, and a per-event `description` for documented universal events); empty for legacy/provider modules | | `getPluginInterface() → QJsonArray` | Methods and events together, each tagged with a `"type"`; the un-filtered source that `getPluginMethods`/`getPluginEvents` slice (all three come from one `getMethods()` call — no separate `getEvents()` vtable method) | @@ -504,6 +504,9 @@ signals: - Enforce token validation on every inbound call (returns invalid `QVariant` on failure). - Dispatch to the wrapped QObject via Qt meta-object APIs and support introspection via `getPluginMethods()` / `getPluginEvents()`. - Provide the `eventResponse` signal used by providers/clients to forward events across process boundaries. +- Gate `informModuleToken()` so only the trusted core / capability module channel can plant a token (see the security note below). + +> **Security note — `informModuleToken` is privileged.** `callRemoteMethod()` authorizes a call when the presented token matches *any* token stored in this module's `TokenManager`. That means whoever can write into the token store effectively controls authorization. `informModuleToken()` is the write path, so it must not be callable by an arbitrary peer — otherwise a peer could plant a token of its own choosing and then present that same token to `callRemoteMethod()` to invoke any method, bypassing the capability gate entirely (finding F-002, CWE-862). To prevent this, `informModuleToken()` validates its `authToken` (using the same constant-time comparison as `callRemoteMethod`) against this module's seed secret — the value the host writes under the `core` and `capability_module` keys at module init. Only the trusted core / capability module knows that secret; every other caller is rejected, and an empty or unseeded secret fails closed. ### 3.4 Generated Wrappers diff --git a/tests/sdk/CMakeLists.txt b/tests/sdk/CMakeLists.txt index b748856..bb10e03 100644 --- a/tests/sdk/CMakeLists.txt +++ b/tests/sdk/CMakeLists.txt @@ -35,6 +35,7 @@ add_executable(sdk_tests test_logos_module_context.cpp test_module_proxy.cpp test_auth_token_enforcement.cpp + test_inform_module_token_auth.cpp test_logos_api_provider.cpp test_mock_transport.cpp test_local_transport_integration.cpp diff --git a/tests/sdk/test_inform_module_token_auth.cpp b/tests/sdk/test_inform_module_token_auth.cpp new file mode 100644 index 0000000..8455170 --- /dev/null +++ b/tests/sdk/test_inform_module_token_auth.cpp @@ -0,0 +1,170 @@ +// Security regression test for finding F-002: +// +// ModuleProxy::informModuleToken ignored its authToken (Q_UNUSED) and +// unconditionally planted the supplied token into this module's TokenManager. +// Because isAuthorized() accepts a match against ANY stored token value, a +// peer could plant a token of its own choosing and then present that same +// token to callRemoteMethod() to dispatch any business method — fully +// bypassing the capability gate (CWE-862 Missing Authorization). +// +// The legitimate flow only ever reaches informModuleToken from the trusted +// core / capability_module channel: +// +// * the host seeds this module's TokenManager with the module's authToken +// under the keys "core" and "capability_module" at init +// (logos-liblogos module_initializer.cpp), and +// * the only callers — logos_core's notifyCapabilityModule() and the +// capability module's requestModule() — present exactly that secret as the +// authToken argument. +// +// So the fix gates informModuleToken on that seed secret: a caller that cannot +// present the module's own "core"/"capability_module" token may not plant +// anything. These tests pin that down — the exploit must be rejected, and the +// trusted channel must still succeed. + +#include +#include +#include "logos_api.h" +#include "logos_provider_object.h" +#include "module_proxy.h" +#include "token_manager.h" + +// Minimal new-API provider whose privileged method records when it is reached. +class TokenAuthTestProvider : public LogosProviderBase { +public: + QString providerName() const override { return "token_auth_test"; } + QString providerVersion() const override { return "1.0.0"; } + + QVariant callMethod(const QString& methodName, const QVariantList& args) override + { + lastMethodCalled = methodName; + lastArgs = args; + return QVariant(QStringLiteral("dispatched")); + } + + QJsonArray getMethods() override + { + QJsonArray arr; + QJsonObject m; + m["type"] = "method"; + m["name"] = "privilegedMethod"; + arr.append(m); + return arr; + } + + QString lastMethodCalled; + QVariantList lastArgs; +}; + +class InformModuleTokenAuthTest : public ::testing::Test { +protected: + void SetUp() override + { + TokenManager::instance().clearAllTokens(); + m_provider = new TokenAuthTestProvider(); + // The provider must be initialized with a LogosAPI so its + // informModuleToken() can reach the TokenManager — this mirrors a real + // loaded module and ensures that, absent the authz gate, a planted token + // would actually be stored (i.e. the test fails for the right reason). + m_api = new LogosAPI("token_auth_test"); + m_provider->init(m_api); + } + void TearDown() override + { + delete m_provider; + delete m_api; + TokenManager::instance().clearAllTokens(); + } + + // Seed the per-module secret the host plants at module init: the module's + // own authToken stored under "core" and "capability_module". Only the + // trusted core/capability_module channel knows this value. + void seedTrustedSecret(const QString& secret) + { + TokenManager::instance().saveToken("core", secret); + TokenManager::instance().saveToken("capability_module", secret); + } + + TokenAuthTestProvider* m_provider = nullptr; + LogosAPI* m_api = nullptr; +}; + +// ── F-002 exploit: a peer plants its own token, then uses it ──────────────── +// +// This is the core regression. With the vulnerable code (Q_UNUSED(authToken)), +// the empty/garbage authToken is ignored, "PWN-TOKEN" lands in the +// TokenManager, and the subsequent callRemoteMethod is authorized — the +// EXPECT_FALSE below fails. With the fix, the plant is rejected and the +// privileged call never dispatches. +TEST_F(InformModuleTokenAuthTest, PeerCannotPlantTokenThenAuthorizeCall) +{ + ModuleProxy proxy(m_provider); + + // The module has been loaded; the host seeded its trusted secret. The peer + // does NOT know it. + seedTrustedSecret("the-module-core-secret"); + + // 1) Attacker tries to plant a token of its choosing with no/garbage auth. + bool plantedEmpty = proxy.informModuleToken("", "attacker", "PWN-TOKEN"); + bool plantedGarbage = proxy.informModuleToken("not-the-secret", "attacker", "PWN-TOKEN-2"); + + EXPECT_FALSE(plantedEmpty) + << "informModuleToken with an empty authToken must be rejected (F-002)"; + EXPECT_FALSE(plantedGarbage) + << "informModuleToken with a non-trusted authToken must be rejected (F-002)"; + + // 2) Neither planted value may exist in the token store... + EXPECT_TRUE(TokenManager::instance().getToken("attacker").isEmpty()) + << "a rejected plant must not be stored"; + + // 3) ...and presenting them to callRemoteMethod must NOT authorize a call. + QVariant r1 = proxy.callRemoteMethod("PWN-TOKEN", "privilegedMethod", {QVariant(1)}); + QVariant r2 = proxy.callRemoteMethod("PWN-TOKEN-2", "privilegedMethod", {QVariant(1)}); + EXPECT_FALSE(r1.isValid()); + EXPECT_FALSE(r2.isValid()); + EXPECT_TRUE(m_provider->lastMethodCalled.isEmpty()) + << "a planted token must never reach the provider — the capability gate " + "must hold (F-002)"; +} + +// ── The trusted channel still works ───────────────────────────────────────── +// +// The host/core (and capability_module) present the module's own seeded secret +// as the authToken. That call must succeed and store the token, so the real +// capability handshake keeps functioning. +TEST_F(InformModuleTokenAuthTest, TrustedChannelCanPlantToken) +{ + ModuleProxy proxy(m_provider); + seedTrustedSecret("the-module-core-secret"); + + bool ok = proxy.informModuleToken("the-module-core-secret", "caller_mod", "issued-tok"); + + EXPECT_TRUE(ok) << "the trusted core/capability_module channel must be able to plant tokens"; + EXPECT_EQ(TokenManager::instance().getToken("caller_mod"), "issued-tok"); + + // And the token it issued is now usable by that caller — the intended flow. + QVariant r = proxy.callRemoteMethod("issued-tok", "privilegedMethod", {QVariant(7)}); + EXPECT_EQ(r.toString(), "dispatched"); + EXPECT_EQ(m_provider->lastMethodCalled, "privilegedMethod"); +} + +// A peer cannot smuggle a plant through by presenting a *business* token it was +// legitimately issued (e.g. some other module's issued token that happens to be +// in the store). informModuleToken is privileged: only the core/capability +// secret unlocks it, not any-stored-token the way callRemoteMethod's +// isAuthorized() works. +TEST_F(InformModuleTokenAuthTest, IssuedBusinessTokenCannotUnlockPlanting) +{ + ModuleProxy proxy(m_provider); + seedTrustedSecret("the-module-core-secret"); + // A normal per-caller token exists in the store (as if previously issued). + TokenManager::instance().saveToken("some_caller", "a-business-token"); + + // Presenting that business token as the authToken must NOT authorize a plant. + bool planted = proxy.informModuleToken("a-business-token", "attacker", "PWN-TOKEN"); + + EXPECT_FALSE(planted) + << "only the core/capability_module secret may authorize informModuleToken, " + "not an arbitrary issued business token"; + EXPECT_TRUE(TokenManager::instance().getToken("attacker").isEmpty()); +} diff --git a/tests/sdk/test_local_transport_integration.cpp b/tests/sdk/test_local_transport_integration.cpp index 714e98e..9a2aee0 100644 --- a/tests/sdk/test_local_transport_integration.cpp +++ b/tests/sdk/test_local_transport_integration.cpp @@ -193,7 +193,14 @@ TEST_F(LocalTransportIntegrationTest, InformModuleTokenDelegatesToProvider) LogosObject* obj = conn.requestObject("local_mod", 5000); ASSERT_NE(obj, nullptr); - bool result = obj->informModuleToken("auth", "target_mod", "secret_tok", 5000); + // informModuleToken is privileged (F-002): only the trusted + // core/capability_module channel may plant tokens. Seed the module's auth + // secret (as the host does at init) and present it, so this drives the + // delegation path through the transport rather than hitting the authz + // rejection. + TokenManager::instance().saveToken("capability_module", "trusted-secret"); + + bool result = obj->informModuleToken("trusted-secret", "target_mod", "secret_tok", 5000); EXPECT_TRUE(result); EXPECT_EQ(TokenManager::instance().getToken("target_mod"), "secret_tok"); obj->release(); diff --git a/tests/sdk/test_module_proxy.cpp b/tests/sdk/test_module_proxy.cpp index e5af0dd..8aaa02e 100644 --- a/tests/sdk/test_module_proxy.cpp +++ b/tests/sdk/test_module_proxy.cpp @@ -169,8 +169,19 @@ TEST_F(ModuleProxyTest, InformModuleTokenDelegatesToProvider) m_provider->init(&api); ModuleProxy proxy(m_provider); - bool result = proxy.informModuleToken("auth", "target_mod", "tok123"); + // informModuleToken is privileged: only the trusted core/capability_module + // channel may plant tokens (F-002). The host seeds the module's own auth + // secret under "core"/"capability_module" at init; the legitimate caller + // presents that secret. Seed it and present it so this exercises the + // delegation path rather than the (now-enforced) authz rejection. + TokenManager::instance().clearAllTokens(); + TokenManager::instance().saveToken("capability_module", "trusted-secret"); + + bool result = proxy.informModuleToken("trusted-secret", "target_mod", "tok123"); EXPECT_TRUE(result); // Provider's informModuleToken saves via TokenManager EXPECT_EQ(TokenManager::instance().getToken("target_mod"), "tok123"); + + // Don't leak the seeded secret into sibling tests sharing the singleton. + TokenManager::instance().clearAllTokens(); }