From 42460e5b2a30c0f97ea95c679e5a8e8a08a4af1f Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Mon, 24 Aug 2026 10:06:15 -0300 Subject: [PATCH] fix(tokens): separate INBOUND from OUTBOUND, without moving a single byte MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A grant one way was a grant both ways. TokenManager was ONE flat map with no direction tag, written from both sides of every relationship: LogosAPIClient stored the token it will PRESENT to a callee under the CALLEE's name, and a token RECEIVED from a caller was stored under the CALLER's name. Same key namespace, last write wins. Measured on the shipped fleet with two ordinary modules doing nothing unusual: one grant A -> B leaves the SAME token value under both opposite-meaning keys, and the never-granted B -> A call then succeeds. Silently. A.callOther(B, ping) CALL_OK T1 A holds token for B? val=7685c776-... T1 B holds token for A? val=7685c776-... <-- one value, two meanings B.callOther(A, ping) CALL_OK <-- never granted WHY THE LAYOUT COULD NOT CHANGE. TokenManager's layout is a cross-package ABI: the host ALLOCATES the object and module/UI-plugin images MUTATE it through their own statically-linked accessors — and host and modules ship as separate .lgx that mix versions at runtime by design. The header's ABI-safety note is about ALLOCATION ("no consumer allocates one, none needs sizeof"); the hazard is MUTATION. Splitting into three members took sizeof 32 -> 64 and moved m_mutex 24 -> 56. QMutex::fastTryLock() compare-exchanges at this+24, which in that layout is m_inbound's QHash d-pointer. Empty, the old code silently borrows the hash's pointer slot as a mutex and puts it back, so it LOOKS fine; non-empty, the exchange fails and lockInternal() interprets the QHash Data* as a QMutexPrivate* and futex-waits on it — hung forever, inside a token-store write, on the module host's Qt main thread. No crash, no log line, no timeout that recovers. Reproduced by calling the shipped 0.6 plugin's own saveToken on a 0.7 object: exit=124. So direction lives in the KEY NAMESPACE instead. Outbound is the bare peer name (byte-identical to master); inbound is "\x01in\x01" + caller. m_tokens@16, m_mutex@24, sizeof 32 — measured identical to master in every shipped image, pinned by a static_assert against a reference struct that fires if a member is added. Two things a key namespace forces that separate members did not: every door REFUSES a key carrying the namespace character, or a wire-supplied caller name could forge across the direction boundary; and credential() is DERIVED from bootstrapKeys() rather than cached, because a cached field reads empty on a store another image wrote and then refuses every push. AN ANCHOR KEY IS NO LONGER SPELLED AS A MODULE NAME. scanIssuedTokens' m_tokens loop offered every matched key unconditionally while the m_store loop deliberately never offers, so "an anchor must never name a caller" was enforced on one side only. A module announcing itself as "core" — which logos-rust-sdk did unprompted — therefore authorized as kind:module name:core. The rule generalises: a store may only name a caller with a key it alone can write. Implemented as a masked operand, so the comparison count is unchanged; RefusingToNameAnAnchorKeyCostsNoComparison pins that via logos::tokenComparisonCount(). lp_token_save / lp_token_save_for now return LP_ERR_INVALID_ARG on a reserved key instead of LP_OK. Only the return code was wrong; saveToken already refused. PROTOCOL 0.8: logos_module_accept_inbound_token joins the module-impl C ABI (12 exports). onInit keeps logos_module_accept_token for the module's own anchor — that one IS outbound, and merging the two paths is what reintroduces the bug. Supersedes the field-split approach; the semantics are unchanged from it. Co-Authored-By: Claude Opus 5 --- cpp/logos_api_client.cpp | 5 + cpp/logos_api_client.h | 4 + cpp/logos_module_impl.h | 48 ++- cpp/logos_protocol.cpp | 62 ++- cpp/logos_protocol.h | 134 ++++++- cpp/module_proxy.cpp | 247 ++++++++---- cpp/module_proxy.h | 30 +- cpp/token_manager.cpp | 210 +++++++++- cpp/token_manager.h | 420 ++++++++++++++++++-- nix/default.nix | 2 +- tests/protocol/CMakeLists.txt | 39 +- tests/protocol/test_call_caller.cpp | 228 ++++++++++- tests/protocol/test_inbound_door.cpp | 277 +++++++++++++ tests/protocol/test_inbound_token_store.cpp | 64 +-- tests/protocol/test_token_direction.cpp | 380 ++++++++++++++++++ tests/protocol/test_token_manager.cpp | 66 +++ tests/protocol/test_token_manager_abi.cpp | 324 +++++++++++++++ 17 files changed, 2357 insertions(+), 183 deletions(-) create mode 100644 tests/protocol/test_inbound_door.cpp create mode 100644 tests/protocol/test_token_direction.cpp create mode 100644 tests/protocol/test_token_manager_abi.cpp diff --git a/cpp/logos_api_client.cpp b/cpp/logos_api_client.cpp index e5c27dc..1833f06 100644 --- a/cpp/logos_api_client.cpp +++ b/cpp/logos_api_client.cpp @@ -197,6 +197,11 @@ QString LogosAPIClient::mintAndCacheToken(const QString& objectName, Timeout tim // token-rotation race where overlapping requestModule calls mint fresh tokens // that overwrite each other at the target (e.g. QtRO's sync wait reentering // via a nested event loop). + // + // saveToken == the OUTBOUND half, keyed by the CALLEE. It used to be one map + // with the inbound tokens, so this write also authorized `objectName` to + // call US and an inbound push for the same peer clobbered this cache. Both + // are closed; see the DIRECTION note in token_manager.h. if (!token.isEmpty()) m_token_manager->saveToken(objectName, token); return token; diff --git a/cpp/logos_api_client.h b/cpp/logos_api_client.h index 953561c..f35c977 100644 --- a/cpp/logos_api_client.h +++ b/cpp/logos_api_client.h @@ -345,6 +345,10 @@ public: bool informModuleToken_module(const QString& authToken, const QString& originModule, const QString& moduleName, const QString& token, int timeoutMs = 20000); TokenManager* getTokenManager() const; + // The OUTBOUND token for `module_name`: what this client presents when it + // CALLS `module_name`. Never a token some caller was issued to call US — + // that half of the store has no reader here, by construction (see the + // DIRECTION note in token_manager.h). QString getToken(const QString& module_name); // nlohmann::json overloads — args is a JSON array, result is a JSON value. diff --git a/cpp/logos_module_impl.h b/cpp/logos_module_impl.h index ddf3dd1..8f4d8ed 100644 --- a/cpp/logos_module_impl.h +++ b/cpp/logos_module_impl.h @@ -81,11 +81,55 @@ LOGOS_MODULE_IMPL_EXPORT void logos_module_set_context( LOGOS_MODULE_IMPL_EXPORT void logos_module_set_emit_callback( logos_module_emit_cb cb, void* user_data); -/* Deliver an auth token for `module_name` (the provider-side - * informModuleToken). Returns 0 on acceptance. */ +/* THE OUTBOUND DOOR. Deliver the token this module will PRESENT when it CALLS + * `module_name`. Returns 0 on acceptance. + * + * ONE MEANING ONLY, and that is the change: the glue used to call this from two + * places with two opposite meanings -- seeding the module's own anchor in + * onInit (outbound, correct) and forwarding a CALLER's token from + * informModuleToken (inbound, filed as an outbound credential). The second + * write is what let a module present, to a peer, the very token that peer had + * been issued to call IT -- the direction collision, one image deeper than the + * one ModuleProxy::authorize sees. The caller path now goes through + * logos_module_accept_inbound_token below; this one is the anchor seeding and + * nothing else. Do not merge them back together. */ LOGOS_MODULE_IMPL_EXPORT int logos_module_accept_token(const char* module_name, const char* token); +/* THE INBOUND DOOR. Record that `caller` may present `token` when it calls THIS + * module. Returns 0 on acceptance. + * + * WHY IT IS A SECOND SYMBOL RATHER THAN A FLAG ON THE FIRST. A parameter is a + * value a caller can get wrong, and default; a separate name either resolves or + * does not. The two doors write two disjoint key namespaces in the image's + * TokenManager (logos-protocol cpp/token_manager.h), and nothing reachable from + * one can read the other -- which is the property that makes a grant one way + * stop being a grant the other way. + * + * WHAT A TOKEN REGISTRY GETS INSTEAD, said here because it is the one thing + * about this door that surprises. To an ordinary provider `informModuleToken` + * means "this caller may call you". To the module holding the token registry + * (capability_module) the SAME wire message means "here is module X's token, + * present it when you call X" -- outbound. So lp_token_save_inbound, which this + * forwards to, ALSO writes the outbound half when, and only when, the image has + * been granted the "token_registry" host service. Without that carve-out + * capability_module's roster (lp_token_keys) empties and every requestModule in + * the fleet is refused with "rejecting request from unknown module identity". + * The grant is the declaration of the role, so the role decides -- see + * lp_token_save_inbound in logos_protocol.h. + * + * CONDITIONAL on protocol >= 0.8, with the same teeth as every other entry + * here: the glue emits a DIRECT call, so a module generated for >= 0.8 whose + * backend omits this definition links cleanly and then fails at dlopen() on + * ELF with "undefined symbol" -- invisible on macOS. Both backends + * (logos-cpp-sdk's lidl_gen_cdylib.cpp and logos-rust-sdk's + * rustgen_provider.rs) owe it in the SAME WAVE as this declaration. + * nix/module-impl-abi.nix is what makes that fail in CI instead of at a user's + * dlopen -- but only after each backend bumps its logos-protocol lock, which is + * the real lag to watch. */ +LOGOS_MODULE_IMPL_EXPORT int logos_module_accept_inbound_token(const char* caller, + const char* token); + /* Grant the module the privileged host services named in `services_json` (a * JSON array from the closed set lp_grant_host_services documents). Returns 0 * on acceptance; the generated implementation simply forwards to diff --git a/cpp/logos_protocol.cpp b/cpp/logos_protocol.cpp index 55e22b5..cc0beb4 100644 --- a/cpp/logos_protocol.cpp +++ b/cpp/logos_protocol.cpp @@ -585,8 +585,57 @@ char* lp_token_get(const char* module_name) int lp_token_save(const char* module_name, const char* token) { if (!module_name || !token) return LP_ERR_INVALID_ARG; - TokenManager::instance().saveToken(QString::fromUtf8(module_name), - QString::fromUtf8(token)); + const QString key = QString::fromUtf8(module_name); + // SYMMETRIC WITH lp_token_save_inbound, deliberately. saveToken() refuses a + // key carrying the reserved direction namespace and can only say so in the + // log: its signature is void and is pinned by the cross-package ABI freeze + // (see the layout note in token_manager.h), so it cannot report through a + // return value. Without this the two doors disagreed about the SAME + // refusal — LP_OK here, LP_ERR_INVALID_ARG there — and a module tripping + // the guard read rc=0 and carried on believing it held a credential it does + // not hold. Asked FIRST, so the answer cannot depend on saveToken's + // internals staying in step. + if (TokenManager::isReservedKey(key)) return LP_ERR_INVALID_ARG; + TokenManager::instance().saveToken(key, QString::fromUtf8(token)); + return LP_OK; +} + +int lp_token_save_inbound(const char* caller, const char* token) +{ + if (!caller || !token) return LP_ERR_INVALID_ARG; + const QString callerName = QString::fromUtf8(caller); + const QString value = QString::fromUtf8(token); + + // The inbound half, always. saveInboundToken refuses an empty name, an + // empty token, and a name carrying the reserved direction namespace -- + // `caller` is named by capability_module over RPC, so it must not be able to + // address any key but its own. + if (!TokenManager::instance().saveInboundToken(callerName, value)) + return LP_ERR_INVALID_ARG; + + // THE TOKEN-REGISTRY CARVE-OUT. See the declaration for the argument; the + // short version is that informModuleToken means opposite things depending on + // WHO RECEIVES IT. To an ordinary provider it is "this caller may call you" + // and stops at the line above. To the module holding the registry it is + // "here is module X's token, present it when you call X", and that is + // outbound: capability_module reads lp_token_keys() for its known-caller + // gate and lp_token_get() for the credential it presents when pushing to the + // target. Both read the OUTBOUND half. + // + // The grant is the declaration of that role, so the grant decides. It is off + // by default, fail-closed, already the gate on lp_token_keys, and it lives + // in the image whose store is being written -- unlike a codegen flag, which + // would be a second place for the two to disagree and which the glue + // generator could not compute anyway (it is handed a LIDL contract, not + // metadata.json). + // + // MEASURED CONSEQUENCE OF OMITTING THIS, so nobody "simplifies" it away: + // capability_module's roster empties, every requestModule is refused with + // "rejecting request from unknown module identity", and the fleet locks out + // at the first cross-module call. Fail-closed, and total. + if (hostServiceGranted(ServiceTokenRegistry)) + TokenManager::instance().saveToken(callerName, value); + return LP_OK; } @@ -631,8 +680,15 @@ int lp_token_save_for(const char* identity, const char* module_name, const char* token) { if (!identity || !module_name || !token) return LP_ERR_INVALID_ARG; + const QString key = QString::fromUtf8(module_name); + // The same door with a store selector in front of it, so it owes the same + // answer — see lp_token_save. Checked BEFORE forIdentity(), which is not + // tidiness: forIdentity() records that a shared store was vended under + // `identity`, and that record makes a later isolateIdentity() refuse. A + // rejected argument must not be able to cost an identity its isolation. + if (TokenManager::isReservedKey(key)) return LP_ERR_INVALID_ARG; TokenManager::forIdentity(QString::fromUtf8(identity)) - .saveToken(QString::fromUtf8(module_name), QString::fromUtf8(token)); + .saveToken(key, QString::fromUtf8(token)); return LP_OK; } diff --git a/cpp/logos_protocol.h b/cpp/logos_protocol.h index baa33e3..1fb1967 100644 --- a/cpp/logos_protocol.h +++ b/cpp/logos_protocol.h @@ -160,9 +160,43 @@ // "auth token not recognized" — which is the intended failure mode for a change // that removes a credential nobody was entitled to. Ship protocol, then // logos-plugin-qt, then the hosts, with matching flake.locks. -#define LOGOS_PROTOCOL_VERSION_MINOR 7 +// 0.8: the INBOUND door -- lp_token_save_inbound(), and the module-impl export +// that carries it across the cdylib boundary +// (logos_module_accept_inbound_token, logos_module_impl.h). +// +// WHAT IT CLOSES. The generated Qt glue's informModuleToken wrote the SAME value +// through two doors: LogosProviderBase::informModuleToken (inbound, into the +// HOST image's store) and logos_module_accept_token (which forwarded to +// lp_token_save -- OUTBOUND, into the cdylib's own store). The value is a +// CALLER's token, so the second write filed it as a credential this module would +// PRESENT to that caller. Measured end to end on shipped artifacts: after +// capability_module minted B> and pushed it to B, B's own client found it +// under "A", skipped requestModule, presented it to A, was rejected, and +// re-exchanged. Every such pair paid a rejection plus a full extra round trip, +// forever, and the caller saw only success. +// +// ADDITIVE AT THE ABI, and unusually cleanly so: no existing symbol changes +// signature or behaviour, logos_module_accept_token keeps meaning exactly what +// its name says, and a module generated below 0.8 emits neither the call nor +// the definition and behaves exactly as it does today. +// +// NOT ADDITIVE FOR A TOKEN REGISTRY, which is the one thing to get right when +// reading this. See lp_token_save_inbound below. +// +// AND WHAT DID **NOT** HAPPEN AT 0.7, said here because it nearly cost the +// fleet. The inbound/outbound split shipped under 0.7 without a version of its +// own AND with three members where TokenManager had one, which moved m_mutex +// from +24 to +56 on an object the host allocates and module images mutate. +// Two versions of that object both answered "0.7.0" and mixing them deadlocked +// a host process on the first token push, with no diagnostic anywhere. The +// storage is now a key namespace inside the original single QHash, so the +// layout is back to what every shipped module was compiled against and MAJOR +// remains the only gate that has to mean anything. token_manager.h's private +// section carries the measurements; token_manager.cpp's static_assert is the +// tripwire. +#define LOGOS_PROTOCOL_VERSION_MINOR 8 #define LOGOS_PROTOCOL_VERSION_PATCH 0 -#define LOGOS_PROTOCOL_VERSION_STRING "0.7.0" +#define LOGOS_PROTOCOL_VERSION_STRING "0.8.0" /* --------------------------------------------------------------------------- * Export marking. @@ -403,18 +437,102 @@ LP_API char* lp_get_methods(lp_client* client); /* --------------------------------------------------------------------------- * Tokens + * + * THE WHOLE lp_token_* FAMILY IS THE **OUTBOUND** FAMILY, and this paragraph is + * the contract, not a description of today's callers. `module_name` everywhere + * below is the module being CALLED, and the value is the token this image will + * PRESENT to it. None of these writes authorizes anybody to call US. + * + * ONE FUNCTION HERE IS NOT OUTBOUND, and it is the last one in the section: + * lp_token_save_inbound, added at 0.8. Everything named above it is outbound; + * it is placed after the family and labelled at every mention so the paragraph + * above stays readable as the rule it is. + * + * WHY AN INBOUND DOOR EXISTS AT ALL, since an earlier version of this note + * argued one was UNREPRESENTABLE and that the absence was a stronger guarantee + * than a doc comment on a door that exists. The argument was that a cdylib has + * no LogosAPI, no ModuleProxy and never authorizes, so every read of its store + * is an outbound presentation. Both halves are true and neither is the point: + * the problem was never a cdylib READING inbound, it was the generated glue + * WRITING a caller's token through the only door available, which was the + * outbound one. With one door the direction had nowhere to go, so an inbound + * grant landed in the outbound cache and the module then presented a peer's own + * token back at that peer. Measured end to end on shipped artifacts: after + * capability_module minted B> and pushed it to B, B's client found the + * value under "A", skipped requestModule, presented it to A, was rejected, and + * re-exchanged -- a rejection plus a full extra round trip on every call of + * every two-way pair, permanently, reported to the caller as success. A second + * door is what gives the inbound value somewhere to land that no outbound read + * can reach. + * + * `logos_module_accept_token` (logos_module_impl.h) is therefore the OUTBOUND + * door and nothing else -- the module's own anchor, seeded in the glue's + * onInit. The caller path goes through logos_module_accept_inbound_token. + * Both backends owe the new definition in the same wave as the declaration: + * miss one and it links clean and dies at dlopen with an undefined symbol, on + * Linux only, invisible on macOS. logos_module_impl.h records that this has + * shipped twice at perfect version agreement. Note also that the + * module-impl-abi checks in both SDKs only go red AFTER each bumps its + * logos-protocol lock: declaring alone turns nothing red, and that lag is the + * real risk. * ------------------------------------------------------------------------- */ -/** Get the stored token for `module_name`. Returns NULL when absent; - * caller frees via lp_string_free. +/** Get the OUTBOUND token for `module_name` — what this image presents when it + * CALLS `module_name`. Returns NULL when absent; caller frees via + * lp_string_free. * * Reads this IMAGE's store — the one lp_client_create uses for every origin - * that has not been isolated. For an isolated origin, see lp_token_get_for. */ + * that has not been isolated. For an isolated origin, see lp_token_get_for. + * + * Does NOT see tokens this image issued to its own callers: those live in the + * inbound half, which has no lp_* reader by design (see above). */ LP_API char* lp_token_get(const char* module_name); -/** Store a token for `module_name` in this image's store. */ +/** Store the OUTBOUND token for `module_name` — what this image will present + * when it CALLS `module_name`. + * + * Storing a token here does not let `module_name` call US. When `module_name` + * is "core" or "capability_module" this also installs the value as this + * store's identity credential, which is how the generated glue's + * logos_module_accept_token("core") seeding keeps working unchanged. */ LP_API int lp_token_save(const char* module_name, const char* token); +/** THE INBOUND DOOR (protocol 0.8). Record that `caller` may present `token` + * when it calls THIS image. The mirror of lp_token_save, and the ONE function + * in this family that is not outbound. + * + * Writes the inbound key namespace, which lp_token_get and lp_token_keys + * cannot read and no outbound presentation can reach. Refuses an empty name or + * token, and refuses a name carrying the reserved namespace character -- + * `caller` arrives over RPC, named by capability_module, so it must not be able + * to address any key but its own. + * + * THE TOKEN-REGISTRY CARVE-OUT, and it is the whole reason this is not simply + * "the inbound half of lp_token_save". The same wire message means opposite + * things depending on WHO RECEIVES IT. To an ordinary provider, + * informModuleToken(caller, token) is "caller may present this to you" -- + * inbound. To the module holding the token registry it is "here is module X's + * token; present it when you call X" -- outbound, and the same map is also the + * roster that answers "is this caller a module I know". capability_module reads + * exactly that: lp_token_keys() for the known-caller gate, lp_token_get() for + * the credential it presents when pushing to the target. + * + * So when, and only when, this image holds the "token_registry" grant, this + * ALSO writes the outbound half. Without the carve-out, routing the glue's + * second write here empties capability_module's roster and every cross-module + * call in the fleet is refused with "rejecting request from unknown module + * identity" -- fail-closed, but a fleet-wide lockout at the first call. + * + * The grant is the right discriminator rather than a codegen flag: it IS the + * declaration of the registry role (metadata.json host_services), it is off by + * default and fail-closed, it is already what gates lp_token_keys, and it lives + * in the image whose store is being written. A per-module codegen flag would + * add a second place for the two to disagree -- and the glue generator is + * handed a LIDL contract, not metadata, so it cannot see host_services at all. + * + * Returns 0 on acceptance. */ +LP_API int lp_token_save_inbound(const char* caller, const char* token); + /* --- per-identity token stores --------------------------------------------- * * A host that loads several modules IN ONE IMAGE gives all of them the same @@ -492,8 +610,8 @@ LP_API int lp_token_reset_identity(const char* identity); * as your own is the elevation this whole surface exists to prevent). */ LP_API int lp_token_adopt_credential(const char* identity, const char* credential); -/** The module names this image's token store holds, as a JSON array. Caller - * frees via lp_string_free. +/** The module names this image's OUTBOUND token store holds, as a JSON array. + * Caller frees via lp_string_free. * * Requires the "token_registry" host service (lp_grant_host_services): an * ungranted image gets NULL. NULL is therefore "refused", never "empty" — a diff --git a/cpp/module_proxy.cpp b/cpp/module_proxy.cpp index 836461e..02ec53e 100644 --- a/cpp/module_proxy.cpp +++ b/cpp/module_proxy.cpp @@ -245,6 +245,122 @@ struct CallerFold { } }; +// ── THE AUTHORIZATION SCAN, GIVEN ONLY WHAT IT MAY SEE ─────────────────────── +// +// Free function, and the parameter list is the mechanism. It takes an +// InboundView and the credential VALUE — never the TokenManager they came from +// — so the outbound accessors are not the discouraged choice in here, they are +// unnameable. The bug this whole split removes was one line of "and also scan +// the store", written by someone who had a TokenManager* in scope; nobody in +// this function does. +// +// WHAT THE THREE SOURCES ARE, and why only one of them contributes a NAME: +// +// (1) proxyInbound ModuleProxy::m_tokens — caller-keyed by construction +// (saveToken / informModuleToken are its only writers). +// The naming oracle, EXCEPT under a bootstrapKeys() name: +// see the anchor mask at the loop itself. +// (2) storeInbound the provider identity's own inbound record. Also +// caller-keyed, and it authorizes — but it offers NO name, +// because in the Qt stack the same (caller, token) pair is +// in BOTH (1) and (2): ModuleProxy::informModuleToken +// records one and the provider it forwards to records the +// other. Folding both would make moduleHits == 2 for every +// ordinary caller and collapse every honest answer to +// Unknown. Losing a name that (1) already has costs +// nothing; double-counting would cost all of them. +// (3) credential this identity's own host-issued anchor. One value, no +// key, so there is nothing here that could be turned into +// a name even by accident — which is the point of it being +// a scalar. +// +// CONSTANT TIME. Every entry is compared exactly once, with no early exit and +// no `break`, and the credential is compared exactly once whether or not it is +// set (the emptiness test masks the RESULT, it does not skip the comparison). +// So the number of constantTimeEquals calls is |proxyInbound| + |storeInbound| +// + 1 — a function of the store sizes alone, never of the presented token, of +// where a match sits, or of whether there was one. +// logos::tokenComparisonCount() lets InboundTokenStore. +// TheComparisonCountDependsOnStoreSizeOnly say so out loud. +struct ScanOutcome { + bool authorized = false; + unsigned moduleHits = 0; // matches in the caller-keyed naming oracle + unsigned anchorHits = 0; // matches on this identity's own credential + CallerFold fold; +}; + +ScanOutcome scanIssuedTokens(const QString& authToken, + const QHash& proxyInbound, + TokenManager::InboundView storeInbound, + const QString& credential) +{ + ScanOutcome out; + + // AN ANCHOR NAME IS NOT A CALLER NAME, and this is the one place that can + // still be told otherwise. THE INVARIANT: a store may only name a caller + // with a key IT ALONE CAN WRITE. m_tokens qualifies for every ordinary + // module name — informModuleToken is its only writer and it files under the + // caller's name — and does NOT qualify for "core" or "capability_module", + // which are the role labels every OTHER store in the system keeps its + // credential under. A key spelled that way is a name two mechanisms can + // produce, so the oracle can no longer say which one did. + // + // NOT HYPOTHETICAL. logos-rust-sdk/src/plugin.rs:144 hardcodes + // `CString::new("core")` as the origin of every outbound client a Rust + // module creates, so every Rust module announces itself as an anchor name + // unprompted; capability_module then pushes the minted pair token naming + // that caller "core", informModuleToken files it here, and without the mask + // below this scan reports {"kind":"module","name":"core"} for a caller that + // is nothing of the kind. Source (2) in the block above already declines to + // name from a store it does not exclusively own; this is that same rule, + // applied to the subset of THIS store's keys it does not exclusively own + // either. + // + // THE ANSWER IS UNKNOWN, NOT HOST. Masking the fold leaves keyLen == 0, so + // the `moduleHits == 1 && keyLen > 0` test in authorize() fails and the + // verdict falls through. That is the honest one: we know we cannot name the + // caller, we do NOT know it is the host. The host arm stays reserved for a + // match against this identity's credential, a value only the host installs. + // + // COSTS NO COMPARISON. `isAnchor` compares a public store KEY against two + // public role labels — it is not a token comparison, it does not touch + // constantTimeEquals, and the mask is applied inside the same branch-free + // fold `match` already goes through. The comparison count stays + // |proxyInbound| + |storeInbound| + 1; CallCaller. + // RefusingToNameAnAnchorKeyCostsNoComparison measures it. + const QStringList anchorNames = TokenManager::bootstrapKeys(); + for (auto it = proxyInbound.constBegin(); it != proxyInbound.constEnd(); ++it) { + const int isAnchor = anchorNames.contains(it.key()) ? 1 : 0; + const int match = constantTimeEquals(authToken, it.value()) ? 1 : 0; + out.authorized |= (match != 0); + out.fold.offer(match & ~isAnchor, it.key().toUtf8()); + // Still counted. The grant is real and it authorizes; what it may not + // do is supply a name. Counting it also keeps a second, honest caller + // sharing the value from being named through the tie rule. + out.moduleHits += static_cast(match); + } + + // No fold.offer() below, and that absence is deliberate — see (2) above. + for (const QString& caller : storeInbound.keys()) { + const int match = constantTimeEquals(authToken, storeInbound.token(caller)) ? 1 : 0; + out.authorized |= (match != 0); + } + + // Always compared, never skipped: `present` masks the answer rather than + // the work, so an empty credential costs the same comparison a set one + // does. authToken is already known non-empty at the call site, so an unset + // credential could not match anyway; the mask is there so the count does + // not depend on the store's state either. + { + const int present = credential.isEmpty() ? 0 : 1; + const int match = constantTimeEquals(authToken, credential) ? 1 : 0; + out.authorized |= ((match & present) != 0); + out.anchorHits += static_cast(match & present); + } + + return out; +} + } // namespace namespace logos { @@ -284,11 +400,19 @@ bool ModuleProxy::informModuleToken(const QString& authToken, const QString& mod // succeeds and only the read comes up empty. It is unreached today: that // function runs in a module IMAGE, whose ring is the process ring. It stops // being unreached the moment a provider identity is isolated in-process. - const QString coreToken = m_store->getToken(QStringLiteral("core")); - const QString capToken = m_store->getToken(QStringLiteral("capability_module")); + // + // ONE credential, not two key reads. TokenManager::credential() is the + // value adoptCredential() installs under every bootstrapKeys() key, so this + // asks for the anchor BY THE THING IT IS instead of by looking two module- + // shaped names up in a map — which is what a store that also holds outbound + // per-target tokens made dangerous. The two keys never disagree in any + // production store: every writer in the fleet sets both from one value + // (module_initializer.cpp:169-170, seedHandshakeTrustAnchor, + // lidl_gen_cdylib_glue.cpp:412-413, LpBridge::syncFromApi, ui-host + // main.cpp:212-213). + const QString credential = m_store->credential(); const bool callerIsTrusted = - (!coreToken.isEmpty() && constantTimeEquals(authToken, coreToken)) || - (!capToken.isEmpty() && constantTimeEquals(authToken, capToken)); + !credential.isEmpty() && constantTimeEquals(authToken, credential); if (authToken.isEmpty() || !callerIsTrusted) { qWarning() << "ModuleProxy: rejecting informModuleToken for" << moduleName << "- caller is not the trusted core/capability_module channel"; @@ -310,25 +434,27 @@ bool ModuleProxy::informModuleToken(const QString& authToken, const QString& mod // module might call back into us from inside the push and be rejected with a // token we had already decided to accept. That window does not exist: the // push reaches module code only as far as a store write - // (lp_module_accept_token -> TokenManager::saveToken, logos_protocol.cpp), - // which calls nothing back. Absent a real window, mirroring the provider's + // (LogosProviderBase::informModuleToken -> TokenManager::saveInboundToken, + // and for a cdylib logos_module_accept_token -> lp_token_save), which calls + // nothing back. Absent a real window, mirroring the provider's // verdict is the smaller claim, so it is the one to make. if (!m_provider->informModuleToken(moduleName, token)) { return false; } // WHY THE PROXY KEEPS ITS OWN COPY of something the provider just stored. - // isAuthorized also scans m_store, and in the default out-of-process - // topology LogosProviderBase's write lands there — so on the happy path this - // is redundant. It is not redundant where it counts. m_store is - // direction-MIXED (LogosAPIClient writes the token it will PRESENT to a - // callee under the CALLEE's name, logos_api_client.cpp:176), so it can never - // say WHOSE a token is; m_tokens is keyed by the caller by construction and - // can, which is what a caller-identity oracle has to be built on. And - // m_store's contents have a lifetime this proxy does not control — - // TokenManager::resetIdentity() empties an isolated store on plugin reload — - // while a token this proxy was told about is good until the proxy dies with - // the module it fronts. + // authorize() also scans m_store's INBOUND half, where the provider's write + // lands — so on the happy path this is redundant. It is not redundant where + // it counts. m_tokens is the naming oracle (scanIssuedTokens takes a name + // from it and from nothing else), it survives a store this proxy does not + // control being emptied under it (TokenManager::resetIdentity() clears an + // isolated store on plugin reload), and it is the record that exists even + // when the provider writes nowhere at all. + // + // DELIBERATELY NOT ALSO m_store->saveInboundToken() here. One map, one + // writer: the provider owns its store's inbound record and this proxy owns + // m_tokens. A second writer would put the same fact in two halves of two + // objects with different lifetimes, which is the shape that rots. // // NOT a claim that a refused push leaves the token unusable. The generated // Qt glue saves to the host stack BEFORE it forwards across the C ABI and @@ -359,17 +485,27 @@ bool ModuleProxy::authorize(const QString& authToken, const QString& transportPr } // A token is valid only if THIS module actually issued it to some caller. - // Two stores hold issued tokens: - // * m_tokens — the proxy's own INBOUND record, keyed by caller - // (saveToken / informModuleToken). Direction-pure. - // * m_store — this provider identity's TokenManager: the host - // anchors, the bootstrap seed, and whatever else the - // host put there. Direction-MIXED, so it authorizes - // but must never be reverse-looked-up to NAME anyone. + // Three sources hold issued tokens, and every one of them is now + // direction-pure: + // * m_tokens — the proxy's own INBOUND record, keyed by + // caller (saveToken / informModuleToken). + // * m_store->inbound() — this provider identity's inbound record, the + // tokens ITS provider was told to accept. + // * m_store->credential() — this identity's own host-issued anchor. // We scan every issued token with a constant-time compare and never early // out, so neither a match position nor the number of issued tokens leaks // through timing. // + // WHAT IS NO LONGER HERE, AND WHY THAT IS THE FIX. The scan used to walk + // m_store's whole key set — which was one flat map holding OUTBOUND + // per-target tokens as well. A token this module cached in order to CALL + // peer_b therefore authorized peer_b to call US, so capability_module's one + // minted value for peer_b> was silently also a grant for + // me>: no handshake, nothing logged, and the access policy at + // capability_module_plugin.cpp:99-106 never consulted. The outbound half is + // now a separate map, and scanIssuedTokens() is not given anything that can + // reach it. tests/protocol/test_token_direction.cpp is the detector. + // // m_store, NOT TokenManager::instance(): the store that authorizes has to be // the store the inbound writes go to. LogosProviderBase::informModuleToken // writes to LogosAPI::getTokenManager() == TokenManager::forIdentity((match); - } - - // (2) m_store — direction-MIXED: LogosAPIClient writes the token we will - // PRESENT to a callee under the CALLEE's name (logos_api_client.cpp:176), so - // a hit here may name a module we CALL as the module CALLING us. It - // therefore contributes NO name — note there is no fold.offer() below, and - // that absence is the whole of rule (1) in the m_tokens comment above. - // - // The single thing this store can say honestly is "this is the host - // bootstrap token", because those keys have exactly one writer (the host - // initializer / seedHandshakeTrustAnchor) and exactly one meaning. - // `isAnchor` compares a public KEY, not a token, so branching on it leaks - // nothing; it is computed before the comparison so the fold stays uniform. - const QStringList anchorKeys = TokenManager::bootstrapKeys(); - for (const QString& key : m_store->getTokenKeys()) { - const int isAnchor = anchorKeys.contains(key) ? 1 : 0; - const int match = constantTimeEquals(authToken, m_store->getToken(key)) ? 1 : 0; - authorized |= (match != 0); - anchorHits += static_cast(match & isAnchor); - } + // Everything the caller-identity work adds is bookkeeping AFTER each + // comparison has already happened — a mask-select into a fixed-width buffer + // and two counter increments — so the comparison count, and with it the + // property the constant-time compare exists to provide, is unchanged by + // construction rather than by inspection. logos::tokenComparisonCount() + // lets a test say so out loud. See scanIssuedTokens() above for the + // count and for why only one of the three sources contributes a name. + const ScanOutcome scan = scanIssuedTokens(authToken, m_tokens, + m_store->inbound(), + m_store->credential()); + bool authorized = scan.authorized; + const unsigned moduleHits = scan.moduleHits; // matches in the INBOUND record + const unsigned anchorHits = scan.anchorHits; // matches on our own credential // Not one of our own issued tokens — give a host-installed validator the // chance to accept it for this transport. This is how operator-issued named @@ -451,12 +558,14 @@ bool ModuleProxy::authorize(const QString& authToken, const QString& transportPr // assert an identity the anchor's own ambiguity ("core" and // "capability_module" share one value) already forbids. *callerJson = logos::callerHostAnchorJson(); - } else if (moduleHits == 1 && fold.keyLen > 0) { - *callerJson = logos::callerModuleJson(fold.name()); + } else if (moduleHits == 1 && scan.fold.keyLen > 0) { + *callerJson = logos::callerModuleJson(scan.fold.name()); } // Everything else stays Unknown, and each case is a real one: // * zero name matches — a validator-accepted operator token, or a hit - // on a non-anchor key of the direction-mixed store. + // in the store's own inbound record that the proxy was never told + // about (the legacy QtProviderObject path, or a host that seeded + // the store directly). // * two or more — two callers were issued the same token value. // Impossible with UUIDs, but if it ever happens we do not get to // pick one. diff --git a/cpp/module_proxy.h b/cpp/module_proxy.h index b8f7a11..9350f04 100644 --- a/cpp/module_proxy.h +++ b/cpp/module_proxy.h @@ -84,8 +84,9 @@ public: using TokenValidator = std::function; - // `token_store` is the store this proxy AUTHORIZES AGAINST — the host - // anchors plus whatever else the host seeded for this provider's identity. + // `token_store` is the store this proxy AUTHORIZES AGAINST — specifically + // its INBOUND half and its CREDENTIAL; the outbound half is never consulted + // (TokenManager's DIRECTION note explains what that closed). // It must be the same store the provider's own informModuleToken writes to, // which for the Qt stack is LogosAPI::getTokenManager() == // TokenManager::forIdentity(), NOT the ambient @@ -129,9 +130,11 @@ signals: void eventResponse(const QString& eventName, const QVariantList& data); private: - // Returns true when authToken matches a token THIS module has been told about - // (via saveToken / informModuleToken), or one held in this proxy's token - // store, OR the host-installed validator accepts it for `transportProtocol`. + // Returns true when authToken matches a token THIS module has been told + // about (via saveToken / informModuleToken), or one in the INBOUND half of + // this proxy's token store, or this identity's own credential, OR the + // host-installed validator accepts it for `transportProtocol`. The store's + // OUTBOUND half is deliberately not among them — see scanIssuedTokens(). // Empty/unknown tokens are rejected. The built-in comparison is constant-time // and never early-outs, so neither a correct prefix nor the number of issued // tokens leaks through timing. @@ -165,11 +168,18 @@ private: // informModuleToken(), both of which key by the CALLER — which is what makes // it the only store here that can honestly NAME a caller. // - // Do not reverse-look-up m_store for that. TokenManager is direction-MIXED: - // LogosAPIClient writes the token we will PRESENT to a callee under the - // CALLEE's name (logos_api_client.cpp:176), while inbound tokens are written - // under the CALLER's name. A hit there may name a module we CALL as the - // module CALLING us, which is affirmatively wrong and worse than unknown. + // NOT the only inbound record any more, and still the only NAMING one. + // TokenManager now has an inbound half of its own (m_store->inbound()), + // written by the provider; authorize() scans it but takes no name from it, + // because in the Qt stack the same (caller, token) pair lands in both and + // folding both would make every ordinary caller ambiguous. See + // scanIssuedTokens() in module_proxy.cpp. + // + // Never reverse-look-up m_store's OUTBOUND half for a caller name — and + // note that authorize() is no longer given anything that could: the + // outbound map holds the token we will PRESENT to a callee, filed under the + // CALLEE's name (logos_api_client.cpp:201), so a hit there would name a + // module we CALL as the module CALLING us. QHash m_tokens; // Never null after construction; see the constructor comment. // diff --git a/cpp/token_manager.cpp b/cpp/token_manager.cpp index 79611a1..0c02036 100644 --- a/cpp/token_manager.cpp +++ b/cpp/token_manager.cpp @@ -1,7 +1,39 @@ #include "token_manager.h" +#include #include #include +/* -- THE ABI TRIPWIRE ------------------------------------------------------- + * + * A TokenManager is allocated by one image and mutated by another: a module + * plugin links its own copy of every accessor in this file and runs it on the + * object the HOST image constructed (LogosAPI::getTokenManager()). The host is + * one package, each module is its own .lgx, and they are built months apart. + * So a member added here is an ABI break that shows up as a mutex CAS at the + * wrong offset -- measured as a permanent deadlock in a shipped host process, + * with no diagnostic anywhere (evaluateProtocolGate compares MAJOR only, and + * logos_module_get_protocol_version() is called by nobody). + * + * This reference struct is the layout every module in the field was compiled + * against. Direction lives in the KEY namespace precisely so that this line + * never has to change; if it does have to change, that is a MAJOR bump and a + * fleet-wide rebuild, not a MINOR. + * + * sizeof-equality is what a static_assert can see. The ORDER of the two members + * is pinned by tests/protocol/test_token_manager_abi.cpp, which measures the + * offsets. */ +namespace { +struct TokenManagerAbiReference : QObject { + QHash tokens; + mutable QMutex mutex; +}; +} // namespace +static_assert(sizeof(TokenManager) == sizeof(TokenManagerAbiReference), + "TokenManager's layout is frozen: this object is allocated by the " + "host image and mutated by module images built against other " + "revisions of this header. Adding a member moves m_mutex and " + "deadlocks them. Encode new state in the key namespace instead."); + TokenManager& TokenManager::instance() { static TokenManager instance; @@ -57,10 +89,10 @@ namespace { bool isHostAnchorValue(const QString& value) { if (value.isEmpty()) return false; - for (const QString& key : TokenManager::bootstrapKeys()) { - if (TokenManager::instance().getToken(key) == value) return true; - } - return false; + // credential(), not getToken(bootstrapKey): the anchor is ONE value under + // two role labels, and asking for it by that name is what stops this from + // being a reverse lookup over a key namespace. + return TokenManager::instance().credential() == value; } } // namespace @@ -172,12 +204,8 @@ QStringList TokenManager::identitiesSharingHostAnchor() for (const QString& name : names) { TokenManager& store = forIdentity(name); if (&store == &instance()) continue; - for (const QString& key : bootstrapKeys()) { - if (isHostAnchorValue(store.getToken(key))) { - out.append(name); - break; - } - } + if (isHostAnchorValue(store.credential())) + out.append(name); } out.sort(); return out; @@ -207,8 +235,25 @@ TokenManager::~TokenManager() void TokenManager::saveToken(const QString& key, const QString& token) { + // The namespace guard, on the OUTBOUND door. `key` arrives from the wire on + // the paths that matter (capability_module names the peer in + // informModuleToken, and lp_token_save takes whatever the C ABI was handed), + // so a name carrying the direction-namespace character would be a way to + // write an outbound value into an inbound key and re-create the collision + // this split removes. Loud, because a legitimate module name can never + // contain a C0 control character and there is therefore no benign case. + if (isReservedKey(key)) { + qWarning() << "TokenManager: refusing to save a token under a reserved key" + << "(the direction namespace is not addressable from outside)"; + return; + } QMutexLocker locker(&m_mutex); m_tokens[key] = token; + // No separate credential to write: credential() reads whichever + // bootstrapKeys() key is set, which is exactly where this call just put it. + // That is THE CREDENTIAL SHIM -- see the note at the declaration -- and + // deriving rather than caching is what makes it survive a store written by + // an image built against a different revision of this header. emit tokenSaved(key); } @@ -219,6 +264,11 @@ void TokenManager::saveToken(const std::string& key, const std::string& token) QString TokenManager::getToken(const QString& key) const { + // The outbound namespace only. Refusing a reserved key here is the read-side + // half of the same guard: without it, getToken(inboundKey(x)) would hand a + // caller a token that caller was ISSUED, to present as if it were its own -- + // the collision this split removes, running the other way. + if (isReservedKey(key)) return QString(); QMutexLocker locker(&m_mutex); return m_tokens.value(key, QString()); } @@ -230,6 +280,7 @@ std::string TokenManager::getToken(const std::string& key) const bool TokenManager::hasToken(const QString& key) const { + if (isReservedKey(key)) return false; QMutexLocker locker(&m_mutex); return m_tokens.contains(key); } @@ -241,9 +292,18 @@ bool TokenManager::hasToken(const std::string& key) const bool TokenManager::removeToken(const QString& key) { + // Outbound door: an inbound entry is not removable through it, for the same + // reason it is not readable through it. + if (isReservedKey(key)) return false; QMutexLocker locker(&m_mutex); if (m_tokens.contains(key)) { m_tokens.remove(key); + // No credential to resync. credential() derives from whichever + // bootstrapKeys() key is still set, so dropping one of the two names can + // no longer leave a cached value asserting a token the store does not + // hold. LogosAPIClient removes a rejected per-target token on the + // re-exchange path (logos_api_client.cpp:139/:293), so that drift was + // reachable from ordinary traffic rather than only from teardown. emit tokenRemoved(key); return true; } @@ -258,6 +318,11 @@ bool TokenManager::removeToken(const std::string& key) void TokenManager::clearAllTokens() { QMutexLocker locker(&m_mutex); + // BOTH NAMESPACES, and the credential with them. resetIdentity() documents + // why: a reload re-mints and re-registers, so a surviving credential would + // be a locked-out reload wearing the appearance of a working one, and the + // inbound record names the callers of the PREVIOUS incarnation. One clear() + // takes all three because all three live in this map. m_tokens.clear(); emit allTokensCleared(); } @@ -265,7 +330,14 @@ void TokenManager::clearAllTokens() QList TokenManager::getTokenKeys() const { QMutexLocker locker(&m_mutex); - return m_tokens.keys(); + // OUTBOUND ONLY. This is the roster lp_token_keys() publishes to a granted + // token registry, and leaking the inbound namespace into it would publish + // the names of everyone who may call US as if they were modules we can call. + QList keys; + keys.reserve(m_tokens.size()); + for (auto it = m_tokens.constBegin(); it != m_tokens.constEnd(); ++it) + if (!isReservedKey(it.key())) keys.append(it.key()); + return keys; } std::vector TokenManager::getTokenKeysStd() const @@ -274,12 +346,124 @@ std::vector TokenManager::getTokenKeysStd() const std::vector keys; keys.reserve(static_cast(m_tokens.size())); for (auto it = m_tokens.constBegin(); it != m_tokens.constEnd(); ++it) - keys.push_back(it.key().toStdString()); + if (!isReservedKey(it.key())) keys.push_back(it.key().toStdString()); return keys; } int TokenManager::tokenCount() const { QMutexLocker locker(&m_mutex); - return m_tokens.size(); + int n = 0; + for (auto it = m_tokens.constBegin(); it != m_tokens.constEnd(); ++it) + if (!isReservedKey(it.key())) ++n; + return n; +} + +/* ── the INBOUND half ─────────────────────────────────────────────────────── + * + * Separate bodies rather than a direction parameter on the existing ones: a + * parameter is a value a caller can get wrong (and default), while a separate + * name is a symbol that either resolves or does not. + */ + +bool TokenManager::saveInboundToken(const QString& caller, const QString& token) +{ + // The first two refusals mirror ModuleProxy::saveToken. An empty token is + // the one that matters: it would read as PRESENT to inbound().contains() + // while authorizing nothing, which is the worst of both. + if (caller.isEmpty() || token.isEmpty()) return false; + // The third is the namespace guard on the INBOUND door. `caller` is named by + // capability_module over RPC; a name carrying the namespace character would + // let it address a key other than its own inbound slot. + if (isReservedKey(caller)) { + qWarning() << "TokenManager: refusing an inbound token for a caller name " + "carrying the reserved direction namespace"; + return false; + } + QMutexLocker locker(&m_mutex); + m_tokens[inboundKey(caller)] = token; + return true; +} + +bool TokenManager::saveInboundToken(const std::string& caller, const std::string& token) +{ + return saveInboundToken(QString::fromStdString(caller), + QString::fromStdString(token)); +} + +QString TokenManager::credential() const +{ + QMutexLocker locker(&m_mutex); + return credentialLocked(); +} + +QString TokenManager::inboundValue(const QString& caller) const +{ + if (isReservedKey(caller)) return QString(); + QMutexLocker locker(&m_mutex); + // The inbound namespace only, and this is THE line the split exists to keep + // honest: no fall-through to the bare key, however convenient it would look + // on the day some caller comes up empty here. + return m_tokens.value(inboundKey(caller), QString()); +} + +QStringList TokenManager::inboundKeyList() const +{ + QMutexLocker locker(&m_mutex); + // The stored keys carry the namespace prefix; callers -- ModuleProxy's scan + // among them -- want the bare caller names, so strip it here rather than + // letting the encoding escape the class. + // + // THIS READS "RESERVED" AS "INBOUND", AND THAT IS ONLY TRUE WHILE THERE IS + // EXACTLY ONE RESERVED NAMESPACE. isReservedKey() asks whether a key + // carries the namespace CHARACTER anywhere; inboundKey() is the only + // producer of such a key today, so the two coincide and a fixed-width strip + // is exact. Introduce a second namespace -- U+0001 "out", U+0001 "meta", + // anything -- and this silently mis-reports every key in it AS AN INBOUND + // CALLER, with a mangled name, straight into ModuleProxy's authorization + // scan. inboundCount() below makes the same assumption and would over-count + // the same way. + // + // WHAT TO DO INSTEAD, at that point and not before: match the specific + // prefix (`it.key().startsWith(inboundKey(QString()))`) rather than the + // character class, in both functions. It is not written that way now + // because the guard would be dead code today and an untested branch in a + // security scan is worse than an invariant stated where it can be read. + const int skip = inboundKey(QString()).size(); + QStringList out; + for (auto it = m_tokens.constBegin(); it != m_tokens.constEnd(); ++it) + if (isReservedKey(it.key())) out.append(it.key().mid(skip)); + return out; +} + +bool TokenManager::inboundContains(const QString& caller) const +{ + if (isReservedKey(caller)) return false; + QMutexLocker locker(&m_mutex); + return m_tokens.contains(inboundKey(caller)); +} + +int TokenManager::inboundCount() const +{ + QMutexLocker locker(&m_mutex); + // "reserved" == "inbound" only while one namespace exists -- see the note + // in inboundKeyList() above, which this shares. + int n = 0; + for (auto it = m_tokens.constBegin(); it != m_tokens.constEnd(); ++it) + if (isReservedKey(it.key())) ++n; + return n; +} + +QString TokenManager::credentialLocked() const +{ + // ONE VALUE UNDER TWO ROLE LABELS, read rather than cached. Every anchor + // writer in the fleet installs it with saveToken() under both bootstrap + // keys, so this is not an inference about where it might be -- it is the + // place it is put. Deriving it is also what makes the answer correct on a + // store written by an image built against another revision of this header. + for (const QString& key : bootstrapKeys()) { + const QString value = m_tokens.value(key, QString()); + if (!value.isEmpty()) return value; + } + return QString(); } \ No newline at end of file diff --git a/cpp/token_manager.h b/cpp/token_manager.h index ddf184f..60f9aa7 100644 --- a/cpp/token_manager.h +++ b/cpp/token_manager.h @@ -245,9 +245,96 @@ public: */ static bool resetIdentity(const QString& identity); + /* ── DIRECTION: three roles, and they are three different things ───────── + * + * OUTBOUND callee -> the token I present when I call that callee. + * Written by LogosAPIClient on the first exchange + * (logos_api_client.cpp:201, async twin :357) and read back + * at :124 / :313. Keyed by the module I am CALLING. + * INBOUND caller -> the token I issued to that caller, which it + * presents when it calls ME. Written by the provider's + * informModuleToken. Keyed by the module CALLING me. + * CREDENTIAL MY OWN host-issued credential. One value, not a map — see + * credential() below for why that is a finding rather than a + * choice. + * + * WHAT SHARING ONE MAP COST. Until this split all three lived in one flat + * QHash with no direction tag, and ModuleProxy::authorize + * accepted anything in it. So a token cached in order to CALL x authorized x + * to call ME: capability_module mints ONE value for b>, m caches it + * outbound under "b" and b records it inbound under "m", after which b may + * call m and m accepts — no handshake, nothing logged, and + * capability_module's access policy never consulted. The grant graph the + * policy is written against is directed; the enforcement was not. Reachable + * in the DEFAULT topology, not only under single-image: a module loaded by + * logos-module-loader-qt runs in its own process and one TokenManager there + * takes all three writes. tests/protocol/test_token_direction.cpp is the + * detector, with the numbers. + * + * WHAT KEEPS IT SPLIT, and it is not anyone remembering to: + * + * * ModuleProxy::authorize is handed an InboundView and a credential + * STRING — never a TokenManager* — so no expression inside the scan can + * name THIS PROXY'S outbound map. That is a property of the injected + * store, NOT of the class: TokenManager::instance() is a public static, + * so any code that wants the process-wide outbound map can still reach + * it in one expression. What the scan cannot do is reach it by + * ACCIDENT, which is the failure this split is about — the pre-split + * hole was one map serving both directions, not a deliberate lookup. + * * getToken() never falls through to the inbound map, so a token some + * caller was issued can never be presented as if it were ours. + * * The two halves cannot produce the same STORAGE KEY even for the same + * peer name: outbound is filed under the bare name, inbound under + * inboundKey(caller), and every door refuses a key that carries the + * namespace character. See the layout note in the private section -- + * the halves are two key namespaces in ONE QHash, not two members, + * because this object crosses the host/module package boundary and + * adding a member to it deadlocked real hosts. + * + * WHY THE UNADORNED SPELLINGS ARE THE OUTBOUND ONES. saveToken / getToken / + * hasToken / removeToken / getTokenKeys / tokenCount keep meaning OUTBOUND, + * deliberately: a future writer who reaches for the familiar name intending + * an inbound grant grants NOTHING, and the failure is a single visibly + * rejected call. The opposite default would over-grant, silently. + * + * THE ONE CHANGE THAT MUST NEVER BE MADE: a read-side fall-through between + * the two halves. One line puts the collision back and it would look like a + * convenience. + */ + /** - * @brief Save a token with the given key - * @param key The identifier for the token + * @brief Save an OUTBOUND token: what I present when I call `key`. + * + * `key` is the module I am CALLING. This does NOT authorize `key` to call + * me — see saveInboundToken() for that, and the DIRECTION note above for + * why reaching for this one by mistake fails closed. + * + * THE CREDENTIAL SHIM: when `key` is one of bootstrapKeys() this ALSO + * installs `token` as this store's credential(), which is what lets every + * existing anchor writer keep working untouched — logos-module-loader-qt's + * module_initializer.cpp:169-170, logos-plugin-qt's + * seedHandshakeTrustAnchor and its generated glue's + * logos_module_accept_token("core")/("capability_module"), logos-qt-sdk's + * LpBridge::syncFromApi. Retire the shim by moving those four to + * adoptCredential(); until then this is the only reason they are not a + * same-wave breaking change. + * + * The shim cannot be abused into re-creating the collision by caching a + * per-target token for a module literally NAMED "core" or + * "capability_module": LogosAPIClient only ever writes here after a MISS on + * getToken(objectName) (logos_api_client.cpp:124), and a store that has a + * credential answers both those names non-empty, so :201 is never reached + * for them. A store with no credential has nothing to clobber. + * + * REFUSES a `key` carrying the direction-namespace character, silently to + * the caller and loudly in the log. `key` reaches here from the wire -- + * capability_module names the peer in informModuleToken -- so without the + * refusal a caller could spell a name that files an OUTBOUND value into the + * INBOUND namespace, which is the pre-split collision reconstructed by hand. + * A module name cannot contain a C0 control character, so nothing legitimate + * is refused. + * + * @param key The CALLEE this token is for * @param token The token value to store */ void saveToken(const QString& key, const QString& token); @@ -264,8 +351,14 @@ public: void saveToken(const std::string& key, const std::string& token); /** - * @brief Retrieve a token by key - * @param key The identifier for the token + * @brief Retrieve an OUTBOUND token: what I present when I call `key`. + * + * Reads the outbound half and the outbound half ONLY. It does not fall + * through to the inbound map, and must never be made to: a token a caller + * was issued is that caller's to present, not ours, and answering it here + * would let any module present a peer's credential as its own. + * + * @param key The CALLEE whose token to look up * @return QString The token value, or empty string if not found */ QString getToken(const QString& key) const; @@ -324,8 +417,12 @@ public: void clearAllTokens(); /** - * @brief Get all token keys - * @return QList List of all token keys + * @brief Every OUTBOUND key: the modules this store can call. + * + * The roster lp_token_keys() publishes (gated on the "token_registry" host + * service). Inbound callers are NOT here — see inbound().keys(). + * + * @return QList List of all outbound token keys */ QList getTokenKeys() const; @@ -340,11 +437,160 @@ public: std::vector getTokenKeysStd() const; /** - * @brief Get the number of stored tokens - * @return int Number of tokens stored + * @brief Get the number of OUTBOUND tokens stored. + * @return int Number of outbound tokens stored */ int tokenCount() const; + /* ── the INBOUND half ──────────────────────────────────────────────────── + * + * caller -> the token THIS store issued to that caller. The only store here + * that can honestly answer "who is calling me", and the only one + * ModuleProxy::authorize is allowed to see. + */ + + /** + * @brief A read-only handle onto the INBOUND half, and only the inbound + * half. + * + * THIS IS THE MECHANISM, not a convenience wrapper. The authorization scan + * receives one of these instead of the TokenManager it came from, so the + * outbound accessors are not merely the wrong choice there — they cannot be + * named at all. A rename would have relied on every future caller choosing + * correctly; this does not. + * + * Copyable and non-owning: it is a pointer to a store whose address is + * stable for the lifetime of the image (see forIdentity()). Only + * TokenManager::inbound() can make one. + */ + class InboundView + { + public: + /** @brief The token issued to `caller`, or empty. NEVER falls through + * to the outbound map — that fall-through IS the bug this + * split removes. */ + inline QString token(const QString& caller) const; + /** @brief Every caller this store has issued a token to. */ + inline QStringList keys() const; + /** @brief Whether `caller` has been issued a token. */ + inline bool contains(const QString& caller) const; + /** @brief How many callers have been issued a token. */ + inline int count() const; + + private: + friend class TokenManager; + explicit InboundView(const TokenManager* owner) : m_owner(owner) {} + const TokenManager* m_owner; + }; + + /** @brief The inbound-only handle. See InboundView. */ + InboundView inbound() const { return InboundView(this); } + + /** + * @brief Whether `key` carries the direction namespace, and is therefore + * refused by every door on this class. + * + * PUBLIC SO A REFUSAL CAN BE REPORTED, not so the encoding can be used. + * saveToken() returns void — its signature is pinned by the same + * cross-package ABI freeze the layout note below describes, so it cannot + * grow a bool — and a C door that forwards to it therefore has nothing to + * turn into a return code. lp_token_save answered LP_OK for a refused write + * while lp_token_save_inbound answered LP_ERR_INVALID_ARG for the identical + * one, and a module reading rc=0 went on believing it held a credential it + * does not hold. This is what lets the outbound doors ask FIRST and answer + * the same way. + * + * It is a predicate over the KEY SPACE, not over any token: it says which + * names are addressable from outside, which is public information (no + * module name may contain a C0 control character). It vends no encoding — + * namespaceChar() and inboundKey() stay private — so nothing outside can + * construct a reserved key, only recognise one. + */ + static bool isReservedKey(const QString& key) { return key.contains(namespaceChar()); } + + /** + * @brief Record that `caller` may present `token` when calling ME. + * + * The INBOUND door. Two production writers, both downstream of + * ModuleProxy::informModuleToken, which admits nothing that did not + * authenticate on the trusted core/capability channel first: + * + * * the provider's informModuleToken in the HOST image (logos-plugin-qt + * LogosProviderBase::informModuleToken and + * QtProviderObject::informModuleToken), and + * * lp_token_save_inbound in the MODULE image, reached from the generated + * glue's informModuleToken through the module-impl export + * logos_module_accept_inbound_token (protocol 0.8). A Qt plugin links + * its own copy of this library, so the host's store and the plugin's + * are different objects and both have to be told. + * + * lp_token_save_inbound ALSO writes the outbound half when its image holds + * the "token_registry" grant -- see the note on that function in + * logos_protocol.h. That is not a leak between the halves: it is the one + * receiver for whom the same wire message genuinely means the other + * direction. + * + * Refuses an empty caller or an empty token, for the same reason + * ModuleProxy::saveToken does: an empty value reads as PRESENT to + * inbound().contains() while authorizing nothing. Refuses a `caller` + * carrying the direction-namespace character for the reason saveToken() + * gives: the name arrives over RPC and must not be able to address any key + * but its own. + * + * @return true if it was recorded + */ + bool saveInboundToken(const QString& caller, const QString& token); + + /** + * @brief saveInboundToken — const char* overload. + * + * Not sugar: without it a literal pair is AMBIGUOUS between the QString and + * std::string overloads and every `saveInboundToken("a", "b")` fails to + * compile, exactly as saveToken's own const char* overload exists to + * prevent. Found by building logos-qt-sdk's tests against this header, not + * by reading it. + */ + bool saveInboundToken(const char* caller, const char* token) + { return saveInboundToken(QString(caller), QString(token)); } + + /** @brief saveInboundToken — std::string overload. */ + bool saveInboundToken(const std::string& caller, const std::string& token); + + /** + * @brief THIS store's own host-issued credential — the trust anchor. + * + * A VALUE, not a map, and that is a finding rather than a design choice. + * The anchor is genuinely both directions — presented outbound to + * capability_module (logos_api_client.cpp:164/:341) and compared against + * inbound (module_proxy.cpp's informModuleToken gate and authorize's + * anchorHits) — which is exactly why it resists a two-way split and exactly + * why it must not be a third MAP: a key living in two maps is a rename. + * + * It already was one value under two names. adoptCredential() writes ONE + * credential under EVERY bootstrapKeys() key, those keys are role labels + * rather than module names (token_manager.cpp:38-45), and + * logos_caller_scope.h forbids the host arm from carrying a name for + * precisely that reason. Making it a scalar is what it already is. + * + * And it is what dissolves the objection: what authorize() is HANDED is a + * bare QString with no key attached, so nothing inside the scan can turn a + * credential match into a module NAME -- which is the property that matters, + * and the reason anchorHits is counted separately from moduleHits. + * + * It is DERIVED, not stored: the value under whichever bootstrapKeys() key + * is set. That is not a weakening -- the credential has always also been in + * the outbound half, because every writer in the fleet installs it with + * saveToken("core", ...) and the shim below keeps that working -- and it is + * what lets this answer correctly on a store written by an image built + * against a different revision of this header. A cached member could not: + * it read empty on a store an older image wrote, which refuses every + * informModuleToken push. + * + * Written by adoptCredential(), and by saveToken() under a bootstrap key + * (the shim documented there). + */ + QString credential() const; + signals: /** * @brief Emitted when a token is saved @@ -395,37 +641,153 @@ private: TokenManager(const TokenManager&) = delete; TokenManager& operator=(const TokenManager&) = delete; + /* -- THE LAYOUT IS FROZEN, AND IT IS FROZEN BY MEASUREMENT -------------- + * + * This object is allocated by one image and MUTATED BY ANOTHER. That is not + * a hypothetical: a module plugin statically links its own copy of this + * library and therefore its own copy of every method below, then operates + * on the TokenManager the HOST image constructed, reached through + * LogosAPI::getTokenManager(). LogosProviderBase::informModuleToken and + * logos-qt-sdk's LpBridge::syncFromApi are both plugin CODE running on a + * host OBJECT. The host ships as one package and each module ships as its + * own .lgx, installed independently, so the two are routinely built months + * apart. + * + * An earlier revision of this file argued the opposite -- "no consumer ever + * allocates a TokenManager and none needs sizeof(), so a consumer compiled + * against the old header keeps working against the new library" -- and split + * the store into three members on the strength of it. The premise is false: + * the accessor bodies are the CONSUMER's, compiled against the CONSUMER's + * header, so the member offsets they use are the consumer's too. Adding + * m_inbound and m_credential moved m_mutex from +24 to +56, and the + * version-mix matrix measured what that costs, on shipped artifacts: + * + * * OLD host + NEW module. saveInboundToken compare-exchanges at this+56 + * on a 32-byte object -- past the end, into adjacent BSS. In the + * measured host that word is boost::asio's openssl_init guard, whose + * value is 1, which is exactly Qt's dummyLocked() sentinel, so the fast + * path can never win and QBasicMutex::lockInternal() futex-waits + * forever. The module's host process deadlocks on the FIRST inbound + * token push and never serves another call -- while the daemon still + * reports it "loaded", "crashed": 0. + * * NEW host + OLD module. The old code CASes this+24, which post-split + * is m_inbound's QHash d-pointer. It survives only while that hash is + * empty (a null d reads as unlocked); the same deadlock appears the + * moment anything writes an inbound token first. + * * The ui seam, both ways. LpBridge::syncFromApi -> getToken hangs + * before the ui plugin ever reaches READY. + * + * None of it is detectable in band: evaluateProtocolGate compares MAJOR + * only, logos_module_get_protocol_version() is exported by every module and + * called by nobody, and both layouts answer "0.7.0". + * + * SO DIRECTION IS ENCODED IN THE KEY, NOT IN NEW FIELDS. One QHash and one + * QMutex, at the offsets every shipped module was compiled against. + * m_tokens holds both halves in disjoint key namespaces: an OUTBOUND entry + * is filed under the bare peer name exactly as it always was, an INBOUND + * entry under inboundKey(caller), which begins with a character no module + * name can contain. The public surface is unchanged -- saveToken/getToken + * are the outbound half, saveInboundToken/InboundView the inbound half, + * neither reachable from the other -- so what the split bought is bought + * here too; only the storage stopped being an ABI event. + * + * WHY A KEY NAMESPACE IS NOT THE OLD COLLISION WEARING A HAT. The pre-split + * bug was that ONE key served both directions and ModuleProxy::authorize + * accepted anything in the map. Here the two directions cannot produce the + * same key even when the peer name is identical; the scan is handed an + * InboundView that can only enumerate inbound keys; and every door refuses a + * key carrying the namespace character (isReservedKey), so no wire-supplied + * name can forge its way into the other half. tests/protocol/ + * test_token_direction.cpp and test_token_manager_abi.cpp are the detectors. + * + * ANYTHING ADDED HERE IS AN ABI BREAK ACROSS THE PACKAGE BOUNDARY. The + * static_assert in token_manager.cpp is the tripwire: its reference struct + * spells out the layout master shipped, so a new member cannot be added + * quietly. */ + /** - * @brief Hash map storing tokens by key. + * @brief Every token this store holds, in two disjoint key namespaces. * - * DIRECTION-MIXED, AND THEREFORE NEVER A CALLER ORACLE. One flat map with no - * direction tag, written from both sides of every relationship: + * OUTBOUND -- key is the bare peer name: the token I present when I CALL it. + * INBOUND -- key is inboundKey(caller): the token I ISSUED to that caller. * - * * OUTBOUND — LogosAPIClient stores the token it will PRESENT to a callee - * under the CALLEE's name (logos_api_client.cpp:176, async twin :332). - * * INBOUND — a token RECEIVED from a caller is stored under the CALLER's - * name (lp_module_accept_token -> logos_protocol.cpp:635, and - * LogosProviderBase::informModuleToken in logos-plugin-qt). - * - * Same key namespace, last write wins. So a reverse lookup here — "which key - * holds this token, therefore who is calling me" — can name a module we CALL - * as the module CALLING us. That is affirmatively wrong, and worse than - * declining to answer. ModuleProxy::m_tokens is the caller-keyed, - * inbound-only record; anything that needs to NAME a caller uses that. - * - * The two directions do not currently collide in the DEFAULT topology, but - * only by accident of linkage: a module cdylib links its own copy of this - * library, so its outbound writes land in the cdylib image's instance() - * while the host image's store takes the inbound ones. Any single-image - * configuration — an in-process plugin host, the shared-runtime migration, - * this test suite — puts both in one map again. + * Never read directly by ModuleProxy. That is enforced by authorize() being + * handed an InboundView and a credential string rather than this object, + * and the view enumerates the inbound namespace only. */ QHash m_tokens; /** * @brief Mutex for thread-safe access to tokens + * + * MUST REMAIN THE LAST MEMBER, AND THE ONLY ONE AFTER m_tokens. See the + * layout note above: a plugin built against a different revision of this + * header locks at whatever offset ITS header put this at. */ mutable QMutex m_mutex; + + /* -- the namespace that carries direction ------------------------------- + * + * U+0001 (START OF HEADING). A module name is a metadata.json identifier + * and a name on the wire; it cannot contain a C0 control character. Every + * public door checks isReservedKey(), so a caller-supplied name -- which on + * the inbound path arrives over RPC from capability_module -- can never be + * spelled to land in the other half. + * + * Static inline, so no storage is added and nothing here is an ABI event. + * They are also the ONLY inline bodies in this header that touch the + * encoding, and they touch no member, which keeps the rule stated at the + * top of the private section true: every member access goes through an + * out-of-line symbol. */ + static QChar namespaceChar() { return QChar(u'\u0001'); } + static QString inboundKey(const QString& caller) + { + return QString(namespaceChar()) + QStringLiteral("in") + + QString(namespaceChar()) + caller; + } + + /* The inbound reads, private so InboundView is the only way to reach them. + * A nested class is a member and has access to the enclosing class's + * private members, so no friend declaration is needed and no second public + * spelling of the inbound half exists to be picked by accident. */ + QString inboundValue(const QString& caller) const; + QStringList inboundKeyList() const; + bool inboundContains(const QString& caller) const; + int inboundCount() const; + + /* THIS store's credential, read with m_mutex already held. + * + * The credential is DERIVED, not a field, and that is what makes it answer + * correctly on a store some other image wrote. It is the value under + * whichever bootstrapKeys() key is set -- precisely where every anchor + * writer in the fleet has always put it (module_initializer.cpp:169-170, + * seedHandshakeTrustAnchor, the generated glue's + * logos_module_accept_token("core"), LpBridge::syncFromApi, ui-host + * main.cpp:212-213). A cached field got this wrong in BOTH mixed-package + * directions: written by an old image it stayed empty and every + * informModuleToken push was refused; written by a new image it landed + * outside a 32-byte object. */ + QString credentialLocked() const; }; +inline QString TokenManager::InboundView::token(const QString& caller) const +{ + return m_owner->inboundValue(caller); +} + +inline QStringList TokenManager::InboundView::keys() const +{ + return m_owner->inboundKeyList(); +} + +inline bool TokenManager::InboundView::contains(const QString& caller) const +{ + return m_owner->inboundContains(caller); +} + +inline int TokenManager::InboundView::count() const +{ + return m_owner->inboundCount(); +} + #endif // TOKEN_MANAGER_H \ No newline at end of file diff --git a/nix/default.nix b/nix/default.nix index cbeac76..444cd4b 100644 --- a/nix/default.nix +++ b/nix/default.nix @@ -12,7 +12,7 @@ in pname = "logos-protocol"; inherit isWindows; # Tracks LOGOS_PROTOCOL_VERSION_STRING in cpp/logos_protocol.h. - version = "0.7.0"; + version = "0.8.0"; # Common native build inputs nativeBuildInputs = [ diff --git a/tests/protocol/CMakeLists.txt b/tests/protocol/CMakeLists.txt index c6790c2..9a78c1a 100644 --- a/tests/protocol/CMakeLists.txt +++ b/tests/protocol/CMakeLists.txt @@ -344,11 +344,12 @@ add_executable(protocol_tests # "unknown method" (no silent shadowing), and the listing agrees with what # is callable -- plus the auth gate, which introspection deliberately lacks. test_module_identity.cpp - # The INBOUND token store. ModuleProxy::m_tokens is the only caller-keyed, - # inbound-only record here, and it had ZERO production writers: every real - # authorization decision came out of TokenManager, which is direction-mixed - # (outbound tokens keyed by callee sit in the same flat map as inbound ones - # keyed by caller). Two detectors, one per mechanism — + # The INBOUND token store. ModuleProxy::m_tokens is the proxy's own + # caller-keyed record, and it had ZERO production writers: every real + # authorization decision came out of TokenManager, which was then + # direction-mixed (outbound tokens keyed by callee sat in the same flat map + # as inbound ones keyed by caller; test_token_direction.cpp split it). Two + # detectors, one per mechanism — # AnInformedTokenLandsInTheProxysOwnStore fails when informModuleToken does # not record, AnAmbientTokenDoesNotAuthorizeAnIsolatedProxy fails when # isAuthorized scans TokenManager::instance() instead of the proxy's own @@ -360,15 +361,37 @@ add_executable(protocol_tests # by-product of the authorization scan. Production of the caller # DOCUMENT and its lifetime on the dispatching thread only — nothing # in this repo delivers it into a module cdylib yet. Most of the file - # is about the three cases that must REFUSE to name anyone; the - # direction-mixed-store case is the one an obvious simplification - # would delete. + # is about the cases that must REFUSE to name anyone; the outbound-token + # case is the one an obvious simplification would delete, and since the + # direction split it asserts a refusal to AUTHORIZE as well. test_call_caller.cpp # An isolated identity's OWN credential: the store is born empty, the host # mints/registers/adopts, and both halves — no host anchor, and no lockout — # are asserted against real ModuleProxy authorization. See the file header # for the two neutered builds that make each half a detector. test_consumer_credential.cpp + # DIRECTION: outbound vs inbound in one store. The detector for the split, + # and the pins that say what the split must not break. Three of its + # assertions also detect whether logos-plugin-qt's companion one-line move + # (informModuleToken -> saveInboundToken) has landed; see the note on + # DirectionProvider. + test_token_direction.cpp + # THE LAYOUT of TokenManager, measured on a live object. The store split + # shipped as three members, which moved m_mutex from +24 to +56 on an object + # the HOST allocates and MODULE images mutate through their own statically + # linked copy of these accessors. Mixing a host and a module .lgx built + # either side of that deadlocked the module's host process on the first + # inbound token push -- silently, with the daemon still reporting + # "loaded"/"crashed":0, because the only load gate compares MAJOR. Direction + # is now a KEY NAMESPACE inside the original single QHash; these tests are + # what keep it that way, and they are RED on e514c53. + test_token_manager_abi.cpp + # THE INBOUND DOOR across the C ABI (lp_token_save_inbound, protocol 0.8), + # and the token-registry carve-out that keeps capability_module's roster + # alive. Every case names the one-line neutering that turns it red; two of + # them detect the original bug (a caller's token filed as an outbound + # credential) and two detect its fleet-fatal overcorrection. + test_inbound_door.cpp # test_plain_transport_tcp.cpp — entirely #if 0 (see the note inside: # in-process Qt-event-loop deadlock between consumer + provider under # nix's test sandbox; exercised cross-process by the integration matrix). diff --git a/tests/protocol/test_call_caller.cpp b/tests/protocol/test_call_caller.cpp index 23a34d0..30b6b09 100644 --- a/tests/protocol/test_call_caller.cpp +++ b/tests/protocol/test_call_caller.cpp @@ -16,11 +16,13 @@ // // THE THREE REFUSALS, which are the reason the type has an Unknown arm at all: // -// * A hit in TokenManager under a NON-anchor key names nobody. That store is -// direction-MIXED — LogosAPIClient files the token it will PRESENT to a -// callee under the CALLEE's name (logos_api_client.cpp:176) — so a reverse -// lookup there can name a module we CALL as the module CALLING us. Naming -// it would be affirmatively wrong, which is worse than declining. +// * A token found ONLY in TokenManager's OUTBOUND half names nobody, and +// since the direction split no longer authorizes either. LogosAPIClient +// files the token it will PRESENT to a callee under the CALLEE's name +// (logos_api_client.cpp:201), so a reverse lookup there could name a module +// we CALL as the module CALLING us — affirmatively wrong, worse than +// declining. authorize() is now handed an inbound-only view and cannot +// reach that half at all. // * A hit on an anchor key names the HOST and carries no module name. // TokenManager::bootstrapKeys() is "core" and "capability_module" holding // one host secret under two keys, so any name from that arm is a coin flip. @@ -40,8 +42,9 @@ // at the Unknown it initialises. 6 passed, 5 FAILED. The four // Unknown-expecting cases survive, which is what makes (a) and (b) // distinguishable rather than two spellings of one detector. -// (c) THE MIXED STORE NAMING PEOPLE — a second fold.offer() in the m_store -// loop. 10 passed, 1 FAILED (the direction-purity case). +// (c) THE STORE'S INBOUND HALF NAMING PEOPLE — a second fold.offer() in +// scanIssuedTokens' storeInbound loop. 10 passed, 1 FAILED (the +// direction-purity case, before the split made that loop inbound-only). // (d) NO ANCHOR ARM. 9 passed, 2 FAILED — and the second failure is the // interesting one: the tie case reports // {"kind":"module","name":"impostor_module"} for a token that is @@ -203,17 +206,26 @@ TEST(CallCaller, TheHostAnchorNamesTheHostAndCarriesNoName) EXPECT_EQ(provider.seen.find("name"), std::string::npos); } -// ── 3. the direction-mixed store names NOBODY ──────────────────────────────── +// ── 3. an OUTBOUND token names nobody because it reaches nobody ────────────── // // THE REFUSAL THAT MATTERS MOST, and the one an "obvious simplification" would // delete. "some_callee" here is exactly the shape LogosAPIClient writes: a token // this module holds in order to CALL some_callee, filed under some_callee's -// name. Reverse-looking-up that store would report some_callee as our CALLER. +// name. Reverse-looking-up it would report some_callee as our CALLER. // -// RED BEFORE: with a second fold.offer() added to the m_store loop — the -// one-line "why not name it from there too" — this reports -// {"kind":"module","name":"some_callee"} and fails. -TEST(CallCaller, ATokenFoundOnlyInTheDirectionMixedStoreNamesNobody) +// THIS ASSERTION MOVED, and the move is the point. It used to say the token +// still AUTHORIZES and merely cannot be NAMED — pre-existing behaviour that the +// caller-identity work deliberately did not touch. The direction split is the +// change that touches it: authorize() is handed m_store->inbound() and a +// credential, so an entry in the outbound half is not reachable, let alone +// nameable. tests/protocol/test_token_direction.cpp holds the security +// argument for why that had to move; this file keeps the naming consequence +// next to the other four refusals. +// +// Two assertions rather than one, because either alone would be satisfiable by +// the wrong mechanism: refusal WITHOUT the Unknown document would mean the +// scope never opened, and Unknown WITHOUT the refusal is the old behaviour. +TEST(CallCaller, AnOutboundTokenNeitherAuthorizesNorNames) { ensureCallerApp(); TokenManager& store = privateStore(QStringLiteral("caller_mixed_store_no_name")); @@ -226,10 +238,11 @@ TEST(CallCaller, ATokenFoundOnlyInTheDirectionMixedStoreNamesNobody) CallerProbeProvider provider; ModuleProxy proxy(&provider, nullptr, &store); - // It still AUTHORIZES — that is pre-existing behaviour and this change does - // not touch it. It simply cannot NAME anyone. - ASSERT_TRUE(dispatched(proxy.callRemoteMethod(outbound, QStringLiteral("work"), {}))); - EXPECT_EQ(provider.seen, kUnknown); + EXPECT_FALSE(dispatched(proxy.callRemoteMethod(outbound, QStringLiteral("work"), {}))) + << "a token we hold in order to CALL some_callee authorized some_callee " + "to call US"; + EXPECT_EQ(provider.calls, 0); + EXPECT_EQ(provider.seen, std::string()); // never dispatched, never scoped } // ── 4. an operator token names nobody ──────────────────────────────────────── @@ -440,3 +453,184 @@ TEST(CallCaller, TheProducedDocumentsMatchTheDeclaredWireShape) // Escaping goes through a real JSON writer rather than string concatenation. EXPECT_EQ(logos::callerModuleJson("a\"b"), R"({"kind":"module","name":"a\"b"})"); } + +// ── 12. an inbound key spelled as an ANCHOR name names nobody ──────────────── +// +// THE INVARIANT: a store may only name a caller with a key it alone can write. +// +// m_tokens is the naming oracle because informModuleToken is its only writer +// and that writer files a token under the CALLER's name. The bootstrap names +// break that exclusivity: "core" and "capability_module" are role labels the +// anchor lives under in every OTHER store in the system, so a key spelled that +// way is a name two different mechanisms can produce and the oracle can no +// longer say which one did. +// +// NOT HYPOTHETICAL. logos-rust-sdk/src/plugin.rs:144 hardcodes +// `CString::new("core")` as the ORIGIN of every outbound client a Rust module +// creates, so every Rust module announces itself to capability_module as +// "core" — an anchor name — unprompted. capability_module then pushes the +// minted pair token at the victim naming the caller "core", informModuleToken +// files it in m_tokens under that key, and the fold happily reports +// {"kind":"module","name":"core"} for a caller that is nothing of the kind. +// The C++ SDK is unaffected: logos_lp_client.h passes a real m_origin. +// +// UNKNOWN, NOT HOST, and the distinction is the whole answer. We know we cannot +// name this caller; we do NOT know it is the host. The host arm is reserved for +// a match against THIS store's credential, which is a value only the host +// installs — see case 2. Answering host here would hand an attacker the very +// escalation the anchor arm exists to make unforgeable. +// +// RED BEFORE (measured, at this commit, with the fold offering every key): +// Expected equality of these values: +// provider.seen +// Which is: "{\"kind\":\"module\",\"name\":\"core\"}" +// kUnknown +// Which is: "{\"kind\":\"unknown\"}" +TEST(CallCaller, AnInboundKeySpelledAsAnAnchorNamesNobody) +{ + ensureCallerApp(); + + for (const QString& anchorName : TokenManager::bootstrapKeys()) { + TokenManager& store = privateStore( + QStringLiteral("caller_anchor_named_key_%1").arg(anchorName)); + // This store's OWN credential, distinct per iteration so a value that + // leaked between the two could not satisfy the assertions below. + const QString anchor = + QStringLiteral("caller-test-anchor-12-%1").arg(anchorName); + store.saveToken(QStringLiteral("core"), anchor); + + CallerProbeProvider provider; + ModuleProxy proxy(&provider, nullptr, &store); + + // The push capability_module makes on behalf of a caller that announced + // itself as "core". It is accepted — this is not about refusing the + // grant, which would break the fleet — it is about refusing to NAME it. + const QString granted = + QStringLiteral("caller-token-anchor-named-%1").arg(anchorName); + ASSERT_NE(granted, anchor); + ASSERT_TRUE(proxy.informModuleToken(anchor, anchorName, granted)); + + ASSERT_TRUE(dispatched(proxy.callRemoteMethod(granted, QStringLiteral("work"), {}))) + << "the grant itself must still authorize: " << anchorName.toStdString(); + EXPECT_EQ(provider.seen, kUnknown) + << "named a caller from an anchor key: " << anchorName.toStdString(); + // Spelled separately because "no name" is the property that must not be + // helpfully improved later, exactly as in case 2. + EXPECT_EQ(provider.seen.find(anchorName.toStdString()), std::string::npos); + EXPECT_EQ(provider.seen.find("host"), std::string::npos) + << "Unknown, not host: we cannot name this caller, and we do not " + "know it is the host"; + } +} + +// ── 13. an ordinary key still names, with an anchor key in the same store ──── +// +// THE CONTROL for case 12, and it is not optional: masking the fold with +// `match & ~isAnchor` on a per-entry basis is one line away from masking it for +// the whole scan, which would silently retire caller identity altogether while +// every Unknown-expecting case above stayed green. +TEST(CallCaller, AnAnchorKeyInTheStoreDoesNotSuppressOtherNames) +{ + ensureCallerApp(); + TokenManager& store = privateStore(QStringLiteral("caller_anchor_key_control")); + const QString anchor = seedAnchor(store, "caller-test-anchor-13"); + + CallerProbeProvider provider; + ModuleProxy proxy(&provider, nullptr, &store); + + const QString impostorToken = QStringLiteral("caller-token-13-impostor"); + const QString honestToken = QStringLiteral("caller-token-13-honest"); + ASSERT_TRUE(proxy.informModuleToken(anchor, QStringLiteral("core"), impostorToken)); + ASSERT_TRUE(proxy.informModuleToken(anchor, QStringLiteral("chat_module"), honestToken)); + + ASSERT_TRUE(dispatched(proxy.callRemoteMethod(honestToken, QStringLiteral("work"), {}))); + EXPECT_EQ(provider.seen, moduleDoc("chat_module")); + + ASSERT_TRUE(dispatched(proxy.callRemoteMethod(impostorToken, QStringLiteral("work"), {}))); + EXPECT_EQ(provider.seen, kUnknown); +} + +// ── 14. refusing to name an anchor key costs no comparison ─────────────────── +// +// WHAT THE REFUSAL IS ALLOWED TO BE: a mask on the FOLD, which runs after each +// constantTimeEquals has already happened. It must not be a `continue`, a +// filtered key list, or anything else that changes how many comparisons a scan +// performs — that would make the cost of an inbound call depend on how the +// store's keys are spelled, which is a property of the caller. +// +// `isAnchor` compares a public store KEY against two public role labels, so +// branching on it leaks nothing; this pins that it also does not COUNT. +// +// The formula is |m_tokens| + |m_store->inbound()| + 1 — the credential is +// always compared once. CallerProbeProvider writes to no store, so the second +// term is 0 and the expected cost is (number of informed callers) + 1. +// +// A PIN, NOT A DETECTOR OF THE MASK: it is green both before and after the +// anchor refusal exists, which is exactly what "the comparison count is +// unchanged" has to mean. What it detects is the refusal written the OTHER way. +// Measured, with the mask replaced by the `continue` anyone reaching for +// "just skip anchor-named keys" would write: +// +// withAnchorKey Which is: 4 +// withoutAnchorKey Which is: 5 +// +// — the cost of an inbound call became a function of how the store's keys are +// spelled. The same build also lost the grant entirely +// (AnInboundKeySpelledAsAnAnchorNamesNobody: "the grant itself must still +// authorize: core"), which is the second reason a skip is the wrong shape. +TEST(CallCaller, RefusingToNameAnAnchorKeyCostsNoComparison) +{ + ensureCallerApp(); + + // Two stores of IDENTICAL size whose key sets differ only in whether one + // key is spelled as an anchor name. + const QStringList anchorNamed{ QStringLiteral("a_module"), QStringLiteral("b_module"), + QStringLiteral("c_module"), QStringLiteral("core") }; + const QStringList plainNamed { QStringLiteral("a_module"), QStringLiteral("b_module"), + QStringLiteral("c_module"), QStringLiteral("d_module") }; + + auto measure = [](const QString& identity, const QStringList& callers) { + TokenManager& store = privateStore(identity); + const QString anchor = QStringLiteral("ct-anchor-%1").arg(identity); + store.saveToken(QStringLiteral("core"), anchor); + + CallerProbeProvider provider; + ModuleProxy proxy(&provider, nullptr, &store); + + QStringList issued; + for (const QString& caller : callers) { + const QString token = QStringLiteral("ct-tok-%1-%2").arg(identity, caller); + EXPECT_TRUE(proxy.informModuleToken(anchor, caller, token)); + issued << token; + } + + // The reference: a token that matches nothing, so the scan runs to the + // end of both stores. + const unsigned long long beforeMiss = logos::tokenComparisonCount(); + proxy.callRemoteMethod(QStringLiteral("ct-no-such-token"), + QStringLiteral("work"), {}); + const unsigned long long missCost = + logos::tokenComparisonCount() - beforeMiss; + + // EVERY issued token, including the anchor-named one: the cost of a hit + // must not depend on which key held it. + for (const QString& token : issued) { + const unsigned long long before = logos::tokenComparisonCount(); + EXPECT_TRUE(dispatched(proxy.callRemoteMethod(token, QStringLiteral("work"), {}))); + EXPECT_EQ(logos::tokenComparisonCount() - before, missCost) + << "identity=" << identity.toStdString() + << " token=" << token.toStdString(); + } + return missCost; + }; + + const unsigned long long withAnchorKey = + measure(QStringLiteral("ct_anchor_named"), anchorNamed); + const unsigned long long withoutAnchorKey = + measure(QStringLiteral("ct_plain_named"), plainNamed); + + EXPECT_EQ(withAnchorKey, withoutAnchorKey); + // Anti-vacuity: pin the closed form, so a counter that stopped being fed + // cannot satisfy the equality above. 4 inbound keys + 1 credential. + EXPECT_EQ(withAnchorKey, 5ull); +} diff --git a/tests/protocol/test_inbound_door.cpp b/tests/protocol/test_inbound_door.cpp new file mode 100644 index 0000000..2f317cf --- /dev/null +++ b/tests/protocol/test_inbound_door.cpp @@ -0,0 +1,277 @@ +// THE INBOUND DOOR ACROSS THE C ABI — lp_token_save_inbound, protocol 0.8. +// +// WHAT IT IS FOR. The generated Qt glue's informModuleToken used to write the +// SAME value through two doors: +// +// LogosProviderBase::informModuleToken(moduleName, token); // HOST image +// logos_module_accept_token(moduleName, token); // THIS image +// +// The value is a CALLER's token — capability_module saying "moduleName may call +// you" — and the second door forwarded to lp_token_save, the OUTBOUND family. +// So a cdylib filed every caller's token as a credential to PRESENT BACK to +// that caller. That is the direction collision logos-protocol's store split +// removed from the host image, reconstituted one image deeper, and it was +// measured end to end on shipped artifacts: after capability_module minted +// B> and pushed it to B, B's own LogosAPIClient found the value under "A", +// logged "Found token", SKIPPED requestModule, presented it to A, was rejected +// ("auth token not recognized"), and re-exchanged. Every call of every two-way +// pair paid a rejection plus a full extra round trip, permanently, and the +// caller saw only success. +// +// HOW THESE ARE DETECTORS. Three neutered builds of lp_token_save_inbound were +// run against this file; the counts below are measured, not predicted, and the +// restored control is 7/7 green. +// +// A. point it at TokenManager::saveToken — i.e. spell it the way the glue +// used to. 6 of 7 RED. This is the bug, exactly: +// AnInboundPushIsNotAnOutboundCredential +// "Which is: 0x955010 / Which is: (nullptr)" — lp_token_get answered +// AnInboundPushDoesNotClobberAnExistingOutboundCache +// "T-peer-presents-to-me" vs "T-i-present-to-peer" — the cache was +// overwritten by the inbound push, which is the direction collision +// (and the four carve-out / argument cases, which lose the inbound half +// entirely) +// +// B. delete the hostServiceGranted(ServiceTokenRegistry) carve-out. 2 RED: +// ATokenRegistryStillGetsItsOutboundRoster (roster empty, token "") +// RevokingTheGrantStopsTheCarveOut +// This is the OPPOSITE failure and it is fleet-fatal: capability_module +// reads lp_token_keys() for its known-caller gate and lp_token_get() for +// the credential it presents when pushing, so an inbound-only door empties +// its roster and every requestModule is refused with "rejecting request +// from unknown module identity" — fail-closed, and total, at the first +// cross-module call. +// +// C. make the carve-out unconditional. 4 RED, including +// AnInboundPushIsNotAnOutboundCredential +// AnUngrantedImageGetsNoOutboundEntry +// Together B and C pin the carve-out to the GRANT rather than to nothing +// or to everyone. + +#include + +#include "logos_protocol.h" +#include "token_manager.h" + +#include +#include + +#include +#include + +#include + +namespace { + +QCoreApplication* ensureInboundApp() +{ + static int argc = 0; + static char* argv[] = { nullptr }; + if (!QCoreApplication::instance()) + new QCoreApplication(argc, argv); + return QCoreApplication::instance(); +} + +std::string takeString(char* owned) +{ + if (!owned) return {}; + const std::string out = owned; + lp_string_free(owned); + return out; +} + +bool rosterContains(const std::string& dump, const std::string& key) +{ + const nlohmann::json j = nlohmann::json::parse(dump, nullptr, /*allow_exceptions=*/false); + if (!j.is_array()) return false; + return std::any_of(j.begin(), j.end(), [&](const nlohmann::json& e) { + return e.is_string() && e.get() == key; + }); +} + +class InboundDoor : public ::testing::Test { +protected: + void SetUp() override + { + ensureInboundApp(); + // The grant and the image store are both process-global, so every case + // starts ungranted and empty rather than from whatever ran before it. + ASSERT_EQ(lp_grant_host_services(nullptr), LP_OK); + TokenManager::instance().clearAllTokens(); + } + void TearDown() override + { + lp_grant_host_services(nullptr); + TokenManager::instance().clearAllTokens(); + } +}; + +} // namespace + +// ── the bug the door exists to close ──────────────────────────────────────── + +TEST_F(InboundDoor, AnInboundPushIsNotAnOutboundCredential) +{ + ASSERT_EQ(lp_token_save_inbound("caller_a", "T-issued-to-a"), LP_OK); + + // The claim, in the words of the failure it prevents: this module must not + // be able to present, to caller_a, the very token caller_a was issued to + // call THIS module. + EXPECT_EQ(lp_token_get("caller_a"), nullptr) + << "an inbound grant became an outbound credential — the collision, one " + "image below ModuleProxy::authorize"; + EXPECT_FALSE(TokenManager::instance().hasToken(QStringLiteral("caller_a"))); + + // ...and it did land where it belongs. + EXPECT_EQ(TokenManager::instance().inbound().token(QStringLiteral("caller_a")), + QStringLiteral("T-issued-to-a")); +} + +TEST_F(InboundDoor, AnInboundPushDoesNotClobberAnExistingOutboundCache) +{ + // The other half of the same key collision, and the one that produced the + // measured rejection-plus-re-exchange on every call: a cached per-target + // token for peer P, then an inbound push naming P. + ASSERT_EQ(lp_token_save("peer", "T-i-present-to-peer"), LP_OK); + ASSERT_EQ(lp_token_save_inbound("peer", "T-peer-presents-to-me"), LP_OK); + + EXPECT_EQ(takeString(lp_token_get("peer")), std::string("T-i-present-to-peer")) + << "the inbound push overwrote the outbound cache for the same peer"; + EXPECT_EQ(TokenManager::instance().inbound().token(QStringLiteral("peer")), + QStringLiteral("T-peer-presents-to-me")); +} + +TEST_F(InboundDoor, TheOutboundDoorIsStillTheAnchorSeedingDoor) +{ + // logos_module_accept_token keeps its meaning, and the glue's onInit keeps + // using it: the module's own host-issued credential, under both bootstrap + // keys. Nothing about adding an inbound door may change this. + ASSERT_EQ(lp_token_save("core", "my-anchor"), LP_OK); + ASSERT_EQ(lp_token_save("capability_module", "my-anchor"), LP_OK); + + EXPECT_EQ(TokenManager::instance().credential(), QStringLiteral("my-anchor")); + EXPECT_EQ(takeString(lp_token_get("capability_module")), std::string("my-anchor")); +} + +// ── the carve-out, in both directions ─────────────────────────────────────── + +TEST_F(InboundDoor, ATokenRegistryStillGetsItsOutboundRoster) +{ + ASSERT_EQ(lp_grant_host_services(R"(["token_registry"])"), LP_OK); + + // This is the shape of what capability_module receives: core telling it + // "module_x is loaded, here is its token". To the registry that message is + // OUTBOUND — the credential it will present when it pushes to module_x — + // and it is also the roster entry its known-caller gate reads. + ASSERT_EQ(lp_token_save_inbound("module_x", "T-x"), LP_OK); + + const std::string roster = takeString(lp_token_keys()); + ASSERT_FALSE(roster.empty()) << "the granted roster must be readable"; + EXPECT_TRUE(rosterContains(roster, "module_x")) + << "capability_module's known-caller gate reads exactly this; empty here " + "means every requestModule in the fleet is refused with 'rejecting " + "request from unknown module identity'"; + + EXPECT_EQ(takeString(lp_token_get("module_x")), std::string("T-x")) + << "capability_module authenticates its push to module_x with this value"; + + // The inbound half is written too: the registry is also a provider, and a + // module presenting its own anchor to capability_module is an inbound call. + EXPECT_EQ(TokenManager::instance().inbound().token(QStringLiteral("module_x")), + QStringLiteral("T-x")); +} + +TEST_F(InboundDoor, AnUngrantedImageGetsNoOutboundEntry) +{ + // Same call, no grant. An ordinary module is not a registry, and for it the + // message means only "this caller may call you". + ASSERT_EQ(lp_token_save_inbound("module_x", "T-x"), LP_OK); + + EXPECT_EQ(lp_token_get("module_x"), nullptr) + << "the carve-out fired for an image that was never granted the registry " + "role — that is the original bug with an extra step"; + EXPECT_EQ(lp_token_keys(), nullptr) << "and the roster stays closed"; + EXPECT_EQ(TokenManager::instance().inbound().token(QStringLiteral("module_x")), + QStringLiteral("T-x")); +} + +TEST_F(InboundDoor, RevokingTheGrantStopsTheCarveOut) +{ + ASSERT_EQ(lp_grant_host_services(R"(["token_registry"])"), LP_OK); + ASSERT_EQ(lp_token_save_inbound("early", "T-early"), LP_OK); + ASSERT_EQ(lp_grant_host_services(nullptr), LP_OK); + ASSERT_EQ(lp_token_save_inbound("late", "T-late"), LP_OK); + + // The grant is read at the moment of the write, not cached at load: an image + // that loses the role stops filing new pushes outbound. + EXPECT_EQ(takeString(lp_token_get("early")), std::string("T-early")); + EXPECT_EQ(lp_token_get("late"), nullptr); +} + +// ── argument handling ─────────────────────────────────────────────────────── + +TEST_F(InboundDoor, RefusesNullEmptyAndForgedNames) +{ + EXPECT_EQ(lp_token_save_inbound(nullptr, "t"), LP_ERR_INVALID_ARG); + EXPECT_EQ(lp_token_save_inbound("caller", nullptr), LP_ERR_INVALID_ARG); + EXPECT_EQ(lp_token_save_inbound("", "t"), LP_ERR_INVALID_ARG); + + // An empty token would read as PRESENT to inbound().contains() while + // authorizing nothing, which is the worst of both. + EXPECT_EQ(lp_token_save_inbound("caller", ""), LP_ERR_INVALID_ARG); + EXPECT_FALSE(TokenManager::instance().inbound().contains(QStringLiteral("caller"))); + + // The caller name arrives over RPC, named by capability_module. A name + // carrying the reserved direction-namespace character must not be able to + // address any key but its own. + EXPECT_EQ(lp_token_save_inbound("\001in\001victim", "forged"), LP_ERR_INVALID_ARG); + EXPECT_EQ(TokenManager::instance().inbound().token(QStringLiteral("victim")), QString()); +} + +TEST_F(InboundDoor, TheOutboundDoorReportsARefusedForgedNameToo) +{ + // SYMMETRY, and it is a report the caller can act on rather than a log line + // only an operator can read. lp_token_save_inbound already answers + // LP_ERR_INVALID_ARG for a name carrying the reserved direction namespace; + // lp_token_save answered LP_OK for the identical refusal, because + // TokenManager::saveToken returns void and the C door had nothing to + // forward. A module tripping the guard therefore saw rc=0 and went on + // believing it held a credential it does not hold — the failure mode this + // whole split exists to make loud. + // + // RED BEFORE (measured, at this commit): + // Expected equality of these values: + // lp_token_save("\001in\001victim", "forged") + // Which is: 0 + // LP_ERR_INVALID_ARG + // Which is: -1 + EXPECT_EQ(lp_token_save("\001in\001victim", "forged"), LP_ERR_INVALID_ARG); + EXPECT_EQ(TokenManager::instance().inbound().token(QStringLiteral("victim")), + QString()) + << "an outbound write addressed the inbound namespace"; + // A PIN ON THE SECOND GUARD, and it is stated in the only accessor that + // could see the failure. hasToken(), tokenCount() and getTokenKeys() all + // FILTER reserved keys, so each answers "nothing there" whether or not the + // write landed; inbound().count() is the one that counts the namespace. + // + // Removing the C-door guard alone leaves this GREEN (measured): the write + // is refused a second time inside TokenManager::saveToken, so the only + // defect was the return code — which is exactly the finding. This fires + // when BOTH guards go, which is the state the door was one line away from. + EXPECT_EQ(TokenManager::instance().inbound().count(), 0) + << "the refused key landed in the reserved namespace, invisible to every " + "outbound accessor"; + + // The per-identity twin is the same door with a store selector in front of + // it, so it owes the same answer. Kept in the same case because a fix that + // reaches one and not the other is the shape this finding already is. + EXPECT_EQ(lp_token_save_for("some_identity", "\001in\001victim", "forged"), + LP_ERR_INVALID_ARG); + + // ...and the refusal is about the NAMESPACE, not about the door: an + // ordinary name still succeeds, so a fix that simply fails everything + // cannot pass this. + EXPECT_EQ(lp_token_save("peer", "T-peer"), LP_OK); + EXPECT_EQ(takeString(lp_token_get("peer")), std::string("T-peer")); + EXPECT_EQ(lp_token_save_for("some_identity", "peer2", "T-peer2"), LP_OK); +} diff --git a/tests/protocol/test_inbound_token_store.cpp b/tests/protocol/test_inbound_token_store.cpp index 6a1d78c..3369787 100644 --- a/tests/protocol/test_inbound_token_store.cpp +++ b/tests/protocol/test_inbound_token_store.cpp @@ -8,12 +8,14 @@ // writer is saveToken(from_module_name, token)), but with // ZERO production writers anywhere in the workspace. In a // real process it was permanently empty. -// * TokenManager — direction-MIXED. LogosAPIClient writes the token it will -// PRESENT to a callee under the CALLEE's name -// (logos_api_client.cpp:176); lp_module_accept_token writes -// a token RECEIVED from a caller under the CALLER's name -// (logos_protocol.cpp:635). One flat QHash -// with no direction tag, last write wins. +// * TokenManager — direction-MIXED AT THE TIME. LogosAPIClient wrote the +// token it will PRESENT to a callee under the CALLEE's name +// (logos_api_client.cpp:201) into the same flat +// QHash that took a token RECEIVED from a +// caller under the CALLER's name. Last write won. That map +// is now two maps and a credential — see the DIRECTION note +// in cpp/token_manager.h — but everything below was written +// against the mixed version and is described as it was. // // So every inbound authorization decision rested on a store that also holds // outbound tokens, and the store that could not hold an outbound token was @@ -64,12 +66,15 @@ // on every build above, including (a). Do not read them as evidence of anything // this change added. // -// WHAT IS DELIBERATELY NOT HERE. There is no assertion that a token found ONLY -// in TokenManager is refused. It is still accepted, exactly as before — the -// host anchors and every bootstrap seed live there and nothing else authorizes -// them. This change adds a second, direction-pure record; it does not take the -// mixed store out of the authorization scan, and doing so would be a behaviour -// break rather than a bug fix. +// WHAT USED TO BE DELIBERATELY NOT HERE, AND WHERE IT WENT. This file +// originally declined to assert that a token found ONLY in TokenManager is +// refused, on the grounds that taking the mixed store out of the scan would be +// a behaviour break rather than a bug fix. It was both: the direction split +// (tests/protocol/test_token_direction.cpp) took the OUTBOUND half out, and the +// two halves that remain — the store's own inbound record and its credential — +// are what the cases below exercise. The store this file's RecordingProvider +// writes is now the inbound half, which is why nothing here needed to change +// beyond the door it spells. #include @@ -98,9 +103,10 @@ QCoreApplication* ensureInboundApp() { // // It writes into a store handed to it at construction, which is what makes the // isolation case reproducible in-process: this is the stand-in for -// LogosProviderBase::informModuleToken, whose real body is -// `m_logosAPI->getTokenManager()->saveToken(moduleName, token)` and whose real -// store is TokenManager::forIdentity(). +// LogosProviderBase::informModuleToken, whose store is +// TokenManager::forIdentity(). It writes through the +// INBOUND door, which is the door that repo owes the same move to; see the note +// on DirectionProvider in test_token_direction.cpp. class RecordingProvider : public LogosProviderObject { public: explicit RecordingProvider(TokenManager* store) : m_store(store) {} @@ -110,7 +116,7 @@ public: return QVariant(); } bool informModuleToken(const QString& moduleName, const QString& token) override { - if (m_store) m_store->saveToken(moduleName, token); + if (m_store) m_store->saveInboundToken(moduleName, token); ++informs; return acceptPushes; } @@ -313,13 +319,21 @@ TEST(InboundTokenStore, AnAmbientTokenDoesNotAuthorizeAnIsolatedProxy) TokenManager& isolated = TokenManager::forIdentity(identity); ASSERT_NE(&isolated, &TokenManager::instance()); - // Planted in the AMBIENT ring only. The key is deliberately not a bootstrap - // key: those now hold the identity's OWN credential, which is a different - // value from the host's by construction, so a bootstrap key would prove the - // same thing less directly. + // Planted in the AMBIENT ring only, through the INBOUND door — this test is + // about WHICH STORE is scanned, not about direction, so it plants the kind + // of token that authorizes. Written with saveToken() it would now be + // refused by both proxies for the unrelated reason that an outbound entry + // never authorizes (test_token_direction.cpp), and the control below would + // stop being a control. + // + // The key is deliberately not a bootstrap key: those now hold the + // identity's OWN credential, which is a different value from the host's by + // construction, so a bootstrap key would prove the same thing less + // directly. const QString ambientOnly = QStringLiteral("inbound-token-ambient-only"); - TokenManager::instance().saveToken(QStringLiteral("some_other_module"), ambientOnly); - ASSERT_TRUE(isolated.getToken(QStringLiteral("some_other_module")).isEmpty()); + ASSERT_TRUE(TokenManager::instance().saveInboundToken( + QStringLiteral("some_other_module"), ambientOnly)); + ASSERT_TRUE(isolated.inbound().token(QStringLiteral("some_other_module")).isEmpty()); RecordingProvider provider(&isolated); ModuleProxy proxy(&provider, /*parent=*/nullptr, &isolated); @@ -344,8 +358,12 @@ TEST(InboundTokenStore, TheDefaultStoreIsStillTheAmbientRing) ensureInboundApp(); seedTrustAnchor(); + // Through the inbound door, for the reason spelled out in test 5: the claim + // here is about which STORE a defaulted ModuleProxy scans, and it would be + // masked by the direction refusal if the token were planted outbound. const QString planted = QStringLiteral("inbound-token-default-store"); - TokenManager::instance().saveToken(QStringLiteral("planted_module"), planted); + ASSERT_TRUE(TokenManager::instance().saveInboundToken( + QStringLiteral("planted_module"), planted)); RecordingProvider provider(&TokenManager::instance()); ModuleProxy proxy(&provider); diff --git a/tests/protocol/test_token_direction.cpp b/tests/protocol/test_token_direction.cpp new file mode 100644 index 0000000..b287455 --- /dev/null +++ b/tests/protocol/test_token_direction.cpp @@ -0,0 +1,380 @@ +// DIRECTION. One image, one store, both directions writing the SAME name. +// +// This file is the detector for the split described in token_manager.h's +// m_tokens comment. Every test here is RED on the tree it was written against +// (c698402, "a private store is created EMPTY"); the numbers are at the bottom. +// +// THE SHAPE IT REPRODUCES IS PRODUCTION, NOT A HYPOTHETICAL. A module loaded by +// logos-module-loader-qt runs in its own process, and in THAT process one +// TokenManager takes all three writes: +// +// * OUTBOUND LogosAPIClient caches the token capability_module minted for +// B> under B's name (logos_api_client.cpp:201, async twin +// :357), reading it back at :124 / :313. +// * INBOUND LogosProviderBase::informModuleToken saves the token a caller +// will present under the CALLER's name (logos-plugin-qt +// logos_provider_object.cpp:53) — into +// LogosAPI::getTokenManager(), the same object. +// * ANCHOR the module's own host-issued credential, under BOTH +// bootstrapKeys() (module_initializer.cpp:169-170; for a cdylib +// the generated glue's logos_module_accept_token("core") / +// ("capability_module"), lidl_gen_cdylib_glue.cpp:412-413). +// +// ModuleProxy::authorize scans that whole map (module_proxy.cpp:419-424) and +// accepts ANY value in it. So an OUTBOUND token authorizes an INBOUND call. +// +// WHAT THAT COSTS, CONCRETELY: every capability grant is silently BIDIRECTIONAL. +// capability_module mints one value, hands it to M (which caches it outbound +// under "B") and pushes it to B (which records it inbound under "M"). B may then +// call M with it — M finds it in its own outbound cache and authorizes — though +// nothing ever granted B -> M. The grant graph the access policy +// (capability_module_plugin.cpp:99-106) is written against is directed; the +// enforcement is not. +#include + +#include "logos_provider_interface.h" +#include "logos_rpc_status.h" +#include "module_proxy.h" +#include "token_manager.h" + +#include +#include +#include +#include +#include + +namespace { + +QCoreApplication* ensureDirectionApp() { + static int argc = 0; + static char* argv[] = { nullptr }; + if (!QCoreApplication::instance()) + new QCoreApplication(argc, argv); + return QCoreApplication::instance(); +} + +// Stands in for LogosProviderBase::informModuleToken, which writes the SAME +// store the proxy authorizes against. That identity is the whole point: a +// provider that wrote somewhere else would not reproduce the collision. +// +// THE ONE LINE THAT MOVED, AND THE COMPANION CHANGE IT MIRRORS. The real body +// was `getTokenManager()->saveToken(moduleName, token)` — the OUTBOUND door, +// keyed by the CALLEE — and that mis-spelling is half of what this file +// detects. It is now `saveInboundToken`, and logos-plugin-qt owes the identical +// one-line move in cpp/logos_provider_object.cpp:53 and +// cpp/qt_provider_object.cpp:497 in the same wave. +// +// So this stand-in is NOT quietly moving the goalposts, and it is worth being +// exact about which assertions depend on it. Tests 1 and 5-half-two are red on +// the old tree with this provider written EITHER way: they are about the scan +// no longer seeing the outbound map, which is a logos-protocol change alone. +// Tests 2c, 3 and 5-half-one are the ones that need the door to move, and they +// are precisely the assertions ABOUT the door: an inbound push must not land in +// the outbound cache. Without the companion change those three stay red, which +// is the correct signal — they are the thing that says logos-plugin-qt has not +// landed yet. +class DirectionProvider : public LogosProviderObject { +public: + explicit DirectionProvider(TokenManager* store) : m_store(store) {} + + QVariant callMethod(const QString& method, const QVariantList&) override { + if (method == QLatin1String("work")) return QStringLiteral("worked"); + return QVariant(); + } + bool informModuleToken(const QString& moduleName, const QString& token) override { + if (m_store) m_store->saveInboundToken(moduleName, token); + return true; + } + QJsonArray getMethods() override { + QJsonObject work; + work["name"] = QStringLiteral("work"); + work["type"] = QStringLiteral("method"); + return QJsonArray{ work }; + } + void setEventListener(EventCallback) override {} + void init(void*) override {} + QString providerName() const override { return QStringLiteral("direction_module"); } + QString providerVersion() const override { return QStringLiteral("1.0.0"); } + +private: + TokenManager* m_store; +}; + +bool callSucceeds(ModuleProxy& proxy, const QString& token) { + const QVariant r = proxy.callRemoteMethod(token, QStringLiteral("work"), {}); + return !logos::isUnauthorizedSentinel(r) && r.toString() == QStringLiteral("worked"); +} + +// The ONE store a module process has: isolated so it is a distinct object from +// the suite-wide ambient ring (forIdentity returns instance() until a name is +// isolated), then given its OWN credential exactly as logos::admitConsumer does. +struct ModuleImage { + explicit ModuleImage(const QString& identity, const QString& credential) + : store(nullptr), cred(credential) + { + EXPECT_TRUE(TokenManager::isolateIdentity(identity)); + store = &TokenManager::forIdentity(identity); + EXPECT_NE(store, &TokenManager::instance()); + store->adoptCredential(credential); + } + TokenManager* store; + QString cred; +}; + +} // namespace + +// ── 1. THE DETECTOR ───────────────────────────────────────────────────────── +// +// An OUTBOUND cache entry must not authorize an INBOUND call. +// +// Nothing else in this suite asserts it. test_call_caller's +// ATokenFoundOnlyInTheDirectionMixedStoreNamesNobody covers the NAMING half — +// such a token resolves to Unknown — and deliberately asserts that it still +// AUTHORIZES. That is the line this test moves. +TEST(TokenDirection, AnOutboundTokenDoesNotAuthorizeAnInboundCall) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_outbound_only"), + QStringLiteral("direction-own-credential-1")); + DirectionProvider provider(m.store); + ModuleProxy proxy(&provider, nullptr, m.store); + + // Exactly logos_api_client.cpp:201 — the token capability_module minted for + // peer_b>, cached under the CALLEE's name. peer_b holds this + // value too; it is the value peer_b was informed of. + m.store->saveToken(QStringLiteral("peer_b"), + QStringLiteral("minted-for-me-to-call-peer_b")); + + EXPECT_FALSE(callSucceeds(proxy, QStringLiteral("minted-for-me-to-call-peer_b"))) + << "an outbound per-target token authorized an inbound call: the grant " + "me->peer_b silently also grants peer_b->me"; +} + +// ── 2. the same name from both sides, inbound written first ───────────────── +// +// The mutual-call case: I call B and B calls me. Two DIFFERENT values, one key. +// Post-split each must survive under its own direction and only its own +// direction; today one map means the second write wins and the first value is +// simply gone. +// +// Written inbound-first so the outbound value is the survivor, which is what +// makes the third expectation a detector rather than an artefact of clobbering. +TEST(TokenDirection, TheSameNameInBothDirectionsKeepsBothValues) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_both_ways"), + QStringLiteral("direction-own-credential-2")); + DirectionProvider provider(m.store); + ModuleProxy proxy(&provider, nullptr, m.store); + + const QString inbound = QStringLiteral("peer_b-presents-this-to-me"); + const QString outbound = QStringLiteral("i-present-this-to-peer_b"); + + // INBOUND first: capability_module pushes over the trusted channel, which + // this store's own credential satisfies (module_proxy.cpp:287-291). + ASSERT_TRUE(proxy.informModuleToken(m.cred, QStringLiteral("peer_b"), inbound)); + // OUTBOUND second. + m.store->saveToken(QStringLiteral("peer_b"), outbound); + + // (a) the inbound token still authorizes — the pin. + EXPECT_TRUE(callSucceeds(proxy, inbound)) + << "the token peer_b was issued no longer authorizes it"; + + // (b) the outbound token does not — the detector. + EXPECT_FALSE(callSucceeds(proxy, outbound)) + << "the token I present to peer_b authorized an inbound call"; + + // (c) the outbound value is readable back under peer_b's name — the round + // trip logos_api_client.cpp:124 makes on the next call. + EXPECT_EQ(m.store->getToken(QStringLiteral("peer_b")), outbound); +} + +// ── 3. the same pair, outbound written first ──────────────────────────────── +// +// The order that shows the FUNCTIONAL half, independent of any security claim: +// today the inbound push CLOBBERS the outbound cache, so this module's next call +// to peer_b goes out carrying peer_b's own inbound token. peer_b rejects it, and +// the client burns its one re-exchange (logos_api_client.cpp:136-143) recovering +// from a collision it caused itself. +TEST(TokenDirection, AnInboundPushDoesNotClobberTheOutboundCache) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_clobber"), + QStringLiteral("direction-own-credential-3")); + DirectionProvider provider(m.store); + ModuleProxy proxy(&provider, nullptr, m.store); + + const QString outbound = QStringLiteral("cached-for-calling-peer_c"); + const QString inbound = QStringLiteral("peer_c-presents-this-to-me"); + + m.store->saveToken(QStringLiteral("peer_c"), outbound); + ASSERT_TRUE(proxy.informModuleToken(m.cred, QStringLiteral("peer_c"), inbound)); + + EXPECT_EQ(m.store->getToken(QStringLiteral("peer_c")), outbound) + << "an inbound push overwrote the outbound per-target cache"; +} + +// ── 4. PINS. Everything the split must NOT break. ─────────────────────────── + +// The identity's own credential still authorizes inbound (the host arm), and is +// still what the trusted-channel gate compares against. +TEST(TokenDirection, TheOwnCredentialStillAuthorizesAndStillGatesPushes) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_pin_anchor"), + QStringLiteral("direction-own-credential-4")); + DirectionProvider provider(m.store); + ModuleProxy proxy(&provider, nullptr, m.store); + + EXPECT_TRUE(callSucceeds(proxy, m.cred)); + EXPECT_TRUE(proxy.informModuleToken(m.cred, QStringLiteral("peer_d"), + QStringLiteral("granted-to-peer_d"))); + EXPECT_FALSE(proxy.informModuleToken(QStringLiteral("not-the-credential"), + QStringLiteral("peer_e"), + QStringLiteral("granted-to-peer_e"))); +} + +// An inbound token authorizes, from a store nobody wrote outbound. +TEST(TokenDirection, AnInboundTokenStillAuthorizes) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_pin_inbound"), + QStringLiteral("direction-own-credential-5")); + DirectionProvider provider(m.store); + ModuleProxy proxy(&provider, nullptr, m.store); + + ASSERT_TRUE(proxy.informModuleToken(m.cred, QStringLiteral("peer_f"), + QStringLiteral("granted-to-peer_f"))); + EXPECT_TRUE(callSucceeds(proxy, QStringLiteral("granted-to-peer_f"))); +} + +// The outbound read path is untouched: what the client cached is what the client +// reads back. A split that broke this would break every second cross-module call. +TEST(TokenDirection, TheOutboundCacheStillReadsBack) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_pin_outbound"), + QStringLiteral("direction-own-credential-6")); + + m.store->saveToken(QStringLiteral("peer_g"), QStringLiteral("tok-g")); + EXPECT_EQ(m.store->getToken(QStringLiteral("peer_g")), QStringLiteral("tok-g")); + EXPECT_TRUE(m.store->hasToken(QStringLiteral("peer_g"))); + EXPECT_TRUE(m.store->getTokenKeys().contains(QStringLiteral("peer_g"))); +} + +// ── 5. THE WHOLE MECHANISM, both halves, in the shape production builds it ── +// +// capability_module mints ONE value and it lands in two places +// (capability_module_plugin.cpp:108-131): +// +// * pushed to B as "the token M will present" -> B's store, keyed "M" +// * returned to M and cached as "what I present to B" +// -> M's store, keyed "B" +// +// B's client then reads its OWN store for a token to present to M +// (logos_api_client.cpp:124 -> m_token_manager->getToken("M"), and that +// m_token_manager is the same object LogosProviderBase::informModuleToken wrote, +// logos-plugin-qt logos_api.cpp:53 / logos_provider_object.cpp:46). It finds a +// non-empty value, so it SKIPS requestModule entirely — and M authorizes it, +// because M cached the identical value outbound. +// +// So one grant M -> B silently produces B -> M, with no handshake, nothing +// logged, and capability_module's access policy +// (capability_module_plugin.cpp:99-106) never consulted. Both halves are +// asserted, because either one alone would close it. +TEST(TokenDirection, AGrantOneWayIsNotAGrantTheOtherWay) +{ + ensureDirectionApp(); + ModuleImage mi(QStringLiteral("direction_pair_m"), + QStringLiteral("direction-own-credential-m")); + ModuleImage bi(QStringLiteral("direction_pair_b"), + QStringLiteral("direction-own-credential-b")); + DirectionProvider providerM(mi.store); + DirectionProvider providerB(bi.store); + ModuleProxy proxyM(&providerM, nullptr, mi.store); + ModuleProxy proxyB(&providerB, nullptr, bi.store); + + const QString minted = QStringLiteral("capability-minted-for-M-calling-B"); + + // capability_module -> B: "M may present this". + ASSERT_TRUE(proxyB.informModuleToken(bi.cred, QStringLiteral("direction_pair_m"), + minted)); + // capability_module -> M: the return value of requestModule, cached. + mi.store->saveToken(QStringLiteral("direction_pair_b"), minted); + + // Half one: B's client must find NOTHING to present to M, so it is forced + // through requestModule — where the access policy lives. + EXPECT_TRUE(bi.store->getToken(QStringLiteral("direction_pair_m")).isEmpty()) + << "B's outbound lookup for M returned the token M was issued for " + "calling B; B's next call to M skips requestModule entirely"; + + // Half two: even handed the value, M must refuse it. M never issued it. + EXPECT_FALSE(callSucceeds(proxyM, minted)) + << "B authorized at M using the token minted for M -> B: the grant is " + "bidirectional and the access policy was never consulted"; +} + +// ── 6. THE SPLIT ITSELF: neither half may read the other ──────────────────── +// +// The two accessors are the mechanism, so the mechanism gets its own assertion. +// A read-side fall-through — `inbound().token(x)` answering from the outbound +// map when it comes up empty, or getToken(x) answering from the inbound one — +// is the single change that would put the collision back while looking like a +// convenience, and it is the change this test exists to fail on. +TEST(TokenDirection, NeitherHalfAnswersForTheOther) +{ + ensureDirectionApp(); + ModuleImage m(QStringLiteral("direction_no_fallthrough"), + QStringLiteral("direction-own-credential-7")); + + m.store->saveToken(QStringLiteral("only_outbound"), QStringLiteral("tok-out")); + ASSERT_TRUE(m.store->saveInboundToken(QStringLiteral("only_inbound"), + QStringLiteral("tok-in"))); + + EXPECT_TRUE(m.store->inbound().token(QStringLiteral("only_outbound")).isEmpty()) + << "the inbound read fell through to the outbound map"; + EXPECT_FALSE(m.store->inbound().contains(QStringLiteral("only_outbound"))); + + EXPECT_TRUE(m.store->getToken(QStringLiteral("only_inbound")).isEmpty()) + << "the outbound read fell through to the inbound map: a module would " + "present a peer's own credential as its own"; + EXPECT_FALSE(m.store->hasToken(QStringLiteral("only_inbound"))); + EXPECT_FALSE(m.store->getTokenKeys().contains(QStringLiteral("only_inbound"))); + + // The credential is a VALUE, not a key in either map, so no reverse lookup + // over either map can produce a name from it. + EXPECT_EQ(m.store->credential(), m.cred); + EXPECT_FALSE(m.store->inbound().keys().contains(QStringLiteral("core"))); + EXPECT_FALSE(m.store->inbound().keys().contains(QStringLiteral("capability_module"))); + + // An empty inbound value is refused rather than stored: it would read as + // PRESENT to contains() while authorizing nothing. + EXPECT_FALSE(m.store->saveInboundToken(QStringLiteral("empty_peer"), QString())); + EXPECT_FALSE(m.store->inbound().contains(QStringLiteral("empty_peer"))); +} + +// ── HOW THIS WAS RUN, AND WHAT IT SAID ────────────────────────────────────── +// +// BEFORE — UNMODIFIED c698402 plus this file only (with DirectionProvider still +// spelling its write `saveToken`, as logos-plugin-qt does today), so the reds +// below are the shipped behaviour and not a neutered build. +// `nix build .#checks.x86_64-linux.tests`, x86_64-linux, 24 cores. +// +// 99% tests passed, 4 tests failed out of 513 (160.7s) +// +// 422 - AnOutboundTokenDoesNotAuthorizeAnInboundCall Actual: true, want false +// 423 - TheSameNameInBothDirectionsKeepsBothValues Actual: true, want false +// 424 - AnInboundPushDoesNotClobberTheOutboundCache got the INBOUND value +// under the callee's key +// 428 - AGrantOneWayIsNotAGrantTheOtherWay BOTH halves red: +// B's outbound lookup for M returned M's token (want empty), and +// M authorized B with it (want refused) +// +// 425, 426, 427 PASSED — the three pins. They are green here and must stay +// green after the split; read none of them as evidence of anything it adds. +// +// The other 509 are untouched, which is the second half of the claim: the +// collision is reachable from a store nothing else in the suite disturbs. +// +// AFTER — see the run recorded at the top of cpp/token_manager.cpp's DIRECTION +// note and in the change's own report. diff --git a/tests/protocol/test_token_manager.cpp b/tests/protocol/test_token_manager.cpp index d528de8..c421645 100644 --- a/tests/protocol/test_token_manager.cpp +++ b/tests/protocol/test_token_manager.cpp @@ -108,6 +108,72 @@ TEST_F(TokenManagerTest, SignalAllTokensCleared) // security-relevant properties: the raw value never appears, the output is // stable for correlation, and distinct tokens map to distinct fingerprints. +// ── the store mechanics of the DIRECTION split ────────────────────────────── +// +// The behavioural consequences live in test_token_direction.cpp; these are the +// three pieces of bookkeeping that hold the two names and the one value +// together, and that nothing else exercises. + +TEST_F(TokenManagerTest, TheInboundOverloadsAllReachTheSameMap) +{ + // The const char* overload is not sugar: without it a literal pair is + // AMBIGUOUS between the QString and std::string forms and the call does not + // compile at all. This test is therefore as much a compile-time assertion + // as a runtime one — it was added because building logos-qt-sdk against + // this header failed on exactly that. + EXPECT_TRUE(TokenManager::instance().saveInboundToken("peer_lit", "tok-lit")); + EXPECT_TRUE(TokenManager::instance().saveInboundToken( + QStringLiteral("peer_q"), QStringLiteral("tok-q"))); + EXPECT_TRUE(TokenManager::instance().saveInboundToken( + std::string("peer_std"), std::string("tok-std"))); + + const TokenManager::InboundView in = TokenManager::instance().inbound(); + EXPECT_EQ(in.token(QStringLiteral("peer_lit")), QStringLiteral("tok-lit")); + EXPECT_EQ(in.token(QStringLiteral("peer_q")), QStringLiteral("tok-q")); + EXPECT_EQ(in.token(QStringLiteral("peer_std")), QStringLiteral("tok-std")); + EXPECT_EQ(in.count(), 3); + + // And none of them is visible to the outbound surface. + EXPECT_EQ(TokenManager::instance().tokenCount(), 0); + EXPECT_FALSE(TokenManager::instance().hasToken("peer_lit")); +} + +TEST_F(TokenManagerTest, RemovingABootstrapKeyDoesNotStrandTheCredential) +{ + // One value under two names. Dropping ONE name must leave the credential + // asserting the value the store still holds under the other; dropping BOTH + // must leave no credential at all. Reachable from ordinary traffic: + // LogosAPIClient::removeToken() runs on the re-exchange path. + TokenManager::instance().adoptCredential(QStringLiteral("the-credential")); + ASSERT_EQ(TokenManager::instance().credential(), QStringLiteral("the-credential")); + + ASSERT_TRUE(TokenManager::instance().removeToken("core")); + EXPECT_EQ(TokenManager::instance().credential(), QStringLiteral("the-credential")) + << "dropping one of the two bootstrap names cleared a credential the " + "store still holds under the other"; + + ASSERT_TRUE(TokenManager::instance().removeToken("capability_module")); + EXPECT_TRUE(TokenManager::instance().credential().isEmpty()) + << "the credential outlived every bootstrap key it was installed under"; +} + +TEST_F(TokenManagerTest, ClearAllTokensClearsAllThree) +{ + // resetIdentity() is documented to clear the credential too — a reload + // re-mints and re-registers, so a surviving credential is a locked-out + // reload that looks live. The inbound record goes for the same reason: it + // names the callers of the PREVIOUS incarnation. + TokenManager::instance().saveToken("callee", "out-tok"); + TokenManager::instance().saveInboundToken("caller", "in-tok"); + TokenManager::instance().adoptCredential(QStringLiteral("cred")); + + TokenManager::instance().clearAllTokens(); + + EXPECT_EQ(TokenManager::instance().tokenCount(), 0); + EXPECT_EQ(TokenManager::instance().inbound().count(), 0); + EXPECT_TRUE(TokenManager::instance().credential().isEmpty()); +} + TEST(RedactTokenTest, NeverContainsRawTokenValue) { const QString secret = "3f2a9c00-dead-beef-cafe-0123456789ab"; diff --git a/tests/protocol/test_token_manager_abi.cpp b/tests/protocol/test_token_manager_abi.cpp new file mode 100644 index 0000000..7bdc3cb --- /dev/null +++ b/tests/protocol/test_token_manager_abi.cpp @@ -0,0 +1,324 @@ +// THE LAYOUT OF TokenManager IS A CROSS-PACKAGE ABI, AND THIS FILE MEASURES IT. +// +// WHY A TEST AND NOT JUST A COMMENT. A TokenManager is allocated by ONE image +// and mutated by ANOTHER. A module plugin statically links its own copy of this +// library — its own TokenManager::saveToken, its own TokenManager::getToken — +// and runs them on the object the HOST image constructed, reached through +// LogosAPI::getTokenManager(). logos-plugin-qt's +// LogosProviderBase::informModuleToken and logos-qt-sdk's LpBridge::syncFromApi +// are both plugin CODE operating on a host OBJECT. The host ships as one +// package and each module ships as its own .lgx, installed independently, so +// the two sides are routinely built months apart. +// +// So the member offsets a plugin uses are the offsets ITS header had. An +// earlier revision of token_manager.h split the store into three members on the +// argument that "no consumer ever allocates a TokenManager and none needs +// sizeof()". That argument is about ALLOCATION; the hazard is MUTATION. Adding +// two members moved m_mutex from +24 to +56 and the version-mix matrix measured +// the result on shipped artifacts: +// +// * OLD host + NEW module: TokenManager::saveInboundToken compare-exchanges +// at this+56 on a 32-byte object — past the end, into adjacent BSS. In the +// measured host that word was boost::asio's openssl_init guard, whose value +// after static init is 1, which is exactly Qt's dummyLocked() sentinel; the +// fast path can therefore never win and QBasicMutex::lockInternal() +// futex-waits forever. The module's host process deadlocked on the FIRST +// inbound token push and never served another call, while the daemon still +// reported it "loaded", "crashed": 0. +// * NEW host + OLD module: the old code CASes this+24, which post-split is +// m_inbound's QHash d-pointer. Survivable only while that hash is empty; a +// poke setting it non-null reproduced the same permanent hang. +// * The ui seam, both ways: LpBridge::syncFromApi -> getToken hung before the +// plugin ever reached READY. +// +// Nothing in band could see any of it. evaluateProtocolGate compares MAJOR only +// and MAJOR is 0 on both sides; logos_module_get_protocol_version() is exported +// by every module and called by nobody; and both layouts answered "0.7.0". +// +// HOW EACH TEST HERE IS A DETECTOR. Every assertion below is RED on the +// three-member tree (logos-protocol e514c53) and green on this one: +// +// TheObjectIsTheSizeEveryShippedModuleWasCompiledAgainst +// sizeof 64 vs 32 — the direct measurement of the break. +// TheTokenMapIsAtTheOffsetEveryShippedModuleReadsItFrom +// measured on a live object rather than asserted from the header, so a +// reorder that preserves sizeof is caught too. +// BothDirectionsLiveInTheSameMember +// the positive form of "no member was added": an inbound write must move +// the SAME word an outbound write moves. +// +// The remaining tests are about the encoding that makes the freeze possible — +// direction as a KEY NAMESPACE — and specifically about the one thing a key +// namespace can get wrong that a separate member cannot: a wire-supplied name +// spelled to land in the other half. + +#include + +#include "token_manager.h" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace { + +QCoreApplication* ensureAbiApp() +{ + static int argc = 0; + static char* argv[] = { nullptr }; + if (!QCoreApplication::instance()) + new QCoreApplication(argc, argv); + return QCoreApplication::instance(); +} + +// The layout master shipped, spelled out. This is not a guess at what the class +// contains: it is the contract every module .lgx in the field was compiled +// against, and the thing token_manager.cpp's static_assert pins. +struct AbiReference : QObject { + QHash tokens; + mutable QMutex mutex; +}; + +// Which pointer-sized words of `obj` went from all-zero to non-zero across +// `mutate`. Reading the object representation through unsigned char* is the +// only way to ask this question from outside the class, and it is the right +// question: the header's own claim about where its members sit is exactly what +// is under test, so an offsetof() on the header under test would be circular. +std::vector wordsThatBecameNonZero(const void* obj, std::size_t size, + const std::function& mutate) +{ + const auto* bytes = static_cast(obj); + const std::size_t words = size / sizeof(void*); + + std::vector before(words); + std::memcpy(before.data(), bytes, words * sizeof(void*)); + + mutate(); + + std::vector after(words); + std::memcpy(after.data(), bytes, words * sizeof(void*)); + + std::vector moved; + for (std::size_t i = 0; i < words; ++i) + if (before[i] == 0 && after[i] != 0) + moved.push_back(static_cast(i * sizeof(void*))); + return moved; +} + +// A store nobody else in this binary has touched. instance() accumulates tokens +// from every other suite, and an already-populated QHash has a non-null d +// pointer, which is precisely the 0 -> non-zero transition the probe looks for. +TokenManager& freshStore(const char* name) +{ + const QString identity = QString::fromLatin1(name); + EXPECT_TRUE(TokenManager::isolateIdentity(identity)) + << "the probe needs a private store; this name must not have been vended shared"; + TokenManager& store = TokenManager::forIdentity(identity); + EXPECT_NE(&store, &TokenManager::instance()); + return store; +} + +// The literal an attacker would have to spell to forge an inbound key. Written +// out here rather than obtained from TokenManager (which keeps it private, and +// rightly so) — this file is the OUTSIDE, which is where the forgery would come +// from. U+0001 START OF HEADING. +const QString kForgedInboundPrefix = QString(QChar(u'\u0001')) + + QStringLiteral("in") + + QString(QChar(u'\u0001')); + +class TokenManagerAbi : public ::testing::Test { +protected: + void SetUp() override { ensureAbiApp(); } +}; + +} // namespace + +// ── the freeze ────────────────────────────────────────────────────────────── + +TEST_F(TokenManagerAbi, TheObjectIsTheSizeEveryShippedModuleWasCompiledAgainst) +{ + EXPECT_EQ(sizeof(TokenManager), sizeof(AbiReference)) + << "TokenManager grew a member. It is allocated by the host image and " + "mutated by module images built against other revisions of this " + "header, so a member added here moves m_mutex and deadlocks them — " + "measured, on shipped artifacts. Encode new state in the key " + "namespace instead."; + + // Stated separately so the failure says WHICH of the two facts broke: the + // absolute shape (QObject + two words) and the reference's agreement with + // it are different claims. + EXPECT_EQ(sizeof(AbiReference), sizeof(QObject) + 2 * sizeof(void*)); +} + +TEST_F(TokenManagerAbi, TheTokenMapIsAtTheOffsetEveryShippedModuleReadsItFrom) +{ + TokenManager& store = freshStore("abi_probe_outbound"); + + const auto moved = wordsThatBecameNonZero( + &store, sizeof(TokenManager), + [&] { store.saveToken(QStringLiteral("peer"), QStringLiteral("tok")); }); + + ASSERT_EQ(moved.size(), 1u) + << "exactly one word should have gone from null to non-null: the token " + "map's d pointer"; + EXPECT_EQ(moved.front(), static_cast(sizeof(QObject))) + << "the token map moved off the offset every shipped module reads it " + "from; the mutex moved with it"; + + // With sizeof pinned above and the map measured here, the mutex has exactly + // one place left to be — which is why this file does not need to reach into + // a private member to pin it. + EXPECT_EQ(sizeof(TokenManager), sizeof(QObject) + 2 * sizeof(void*)); +} + +TEST_F(TokenManagerAbi, BothDirectionsLiveInTheSameMember) +{ + TokenManager& store = freshStore("abi_probe_inbound"); + + const auto moved = wordsThatBecameNonZero( + &store, sizeof(TokenManager), + [&] { store.saveInboundToken(QStringLiteral("caller"), QStringLiteral("tok")); }); + + ASSERT_EQ(moved.size(), 1u) + << "an inbound write must touch exactly one word — the same map the " + "outbound write touches. Two means a second member exists."; + EXPECT_EQ(moved.front(), static_cast(sizeof(QObject))) + << "the inbound half is stored somewhere other than the one token map, " + "which is an added member however it is spelled"; +} + +// ── the encoding, and the one way a key namespace can be attacked ─────────── + +TEST_F(TokenManagerAbi, TheOutboundDoorRefusesAKeyThatForgesTheInboundNamespace) +{ + TokenManager& store = freshStore("abi_probe_forge_out"); + + // The name reaches saveToken from the wire: capability_module names the peer + // in informModuleToken, and lp_token_save passes through whatever the C ABI + // was handed. Without the refusal this is a one-line way to write an + // OUTBOUND value into an INBOUND slot — the pre-split collision, forged by + // hand rather than caused by a shared key. + store.saveToken(kForgedInboundPrefix + QStringLiteral("victim"), + QStringLiteral("forged")); + + EXPECT_EQ(store.inbound().token(QStringLiteral("victim")), QString()) + << "a forged key authorized a caller through the outbound door"; + EXPECT_FALSE(store.inbound().contains(QStringLiteral("victim"))); + EXPECT_EQ(store.inbound().count(), 0); + + // And it was not quietly filed as an ordinary outbound entry either: a + // refusal that stores the value anywhere leaves it in the roster. + EXPECT_EQ(store.tokenCount(), 0); + EXPECT_TRUE(store.getTokenKeys().isEmpty()); +} + +TEST_F(TokenManagerAbi, TheInboundDoorRefusesACallerNameThatCarriesTheNamespace) +{ + TokenManager& store = freshStore("abi_probe_forge_in"); + + EXPECT_FALSE(store.saveInboundToken(kForgedInboundPrefix + QStringLiteral("x"), + QStringLiteral("t"))) + << "a caller name is supplied by capability_module over RPC; it must " + "not be able to address any key but its own"; + EXPECT_EQ(store.inbound().count(), 0); + EXPECT_EQ(store.tokenCount(), 0); +} + +TEST_F(TokenManagerAbi, TheOutboundRosterNeverPublishesInboundKeys) +{ + TokenManager& store = freshStore("abi_probe_roster"); + + store.saveToken(QStringLiteral("callee"), QStringLiteral("out-tok")); + ASSERT_TRUE(store.saveInboundToken(QStringLiteral("caller"), QStringLiteral("in-tok"))); + + // getTokenKeys() is the roster lp_token_keys() publishes to a granted token + // registry, and capability_module treats every entry as "a module I may + // call". Leaking the inbound half into it would publish everyone who may + // call US as if we held their credential. + EXPECT_EQ(store.getTokenKeys(), QList{ QStringLiteral("callee") }); + EXPECT_EQ(store.tokenCount(), 1); + EXPECT_EQ(store.getTokenKeysStd().size(), 1u); + EXPECT_EQ(store.getTokenKeysStd().front(), std::string("callee")); + + // ...and the inbound view is the mirror image of that claim. + EXPECT_EQ(store.inbound().keys(), QStringList{ QStringLiteral("caller") }); + EXPECT_EQ(store.inbound().count(), 1); + + // Neither half answers for the other, at the door as well as in the roster. + EXPECT_EQ(store.getToken(QStringLiteral("caller")), QString()); + EXPECT_FALSE(store.hasToken(QStringLiteral("caller"))); + EXPECT_EQ(store.inbound().token(QStringLiteral("callee")), QString()); +} + +TEST_F(TokenManagerAbi, AnInboundEntryIsNotReadableOrRemovableThroughTheOutboundDoor) +{ + TokenManager& store = freshStore("abi_probe_reserved_read"); + + ASSERT_TRUE(store.saveInboundToken(QStringLiteral("caller"), QStringLiteral("in-tok"))); + + const QString forged = kForgedInboundPrefix + QStringLiteral("caller"); + EXPECT_EQ(store.getToken(forged), QString()) + << "getToken(inboundKey(x)) would hand a module a token it ISSUED, to " + "present as if it were its own"; + EXPECT_FALSE(store.hasToken(forged)); + EXPECT_FALSE(store.removeToken(forged)); + + // The refusal is a refusal, not a deletion. + EXPECT_EQ(store.inbound().token(QStringLiteral("caller")), QStringLiteral("in-tok")); +} + +// ── the credential, which is DERIVED and therefore mixed-package correct ──── + +TEST_F(TokenManagerAbi, TheCredentialIsWhateverIsUnderTheBootstrapKeys) +{ + TokenManager& store = freshStore("abi_probe_credential"); + + EXPECT_EQ(store.credential(), QString()); + + // This is what EVERY anchor writer in the fleet does — module_initializer, + // seedHandshakeTrustAnchor, the generated glue's + // logos_module_accept_token("core"), LpBridge::syncFromApi, ui-host's + // main(). Reading the credential back out of the map rather than out of a + // cached member is what makes it answer correctly on a store some OTHER + // image wrote, which is the whole mixed-package case. + for (const QString& key : TokenManager::bootstrapKeys()) + store.saveToken(key, QStringLiteral("anchor-v1")); + EXPECT_EQ(store.credential(), QStringLiteral("anchor-v1")); + + // Dropping one of the two role labels must not strand the credential on a + // value the store no longer holds. LogosAPIClient removes a rejected + // per-target token on the re-exchange path, so this is reachable from + // ordinary traffic rather than only from teardown. + ASSERT_TRUE(store.removeToken(TokenManager::bootstrapKeys().first())); + EXPECT_EQ(store.credential(), QStringLiteral("anchor-v1")); + + for (const QString& key : TokenManager::bootstrapKeys()) + store.removeToken(key); + EXPECT_EQ(store.credential(), QString()); +} + +TEST_F(TokenManagerAbi, ClearingTakesBothNamespacesAndTheCredential) +{ + TokenManager& store = freshStore("abi_probe_clear"); + + store.saveToken(QStringLiteral("callee"), QStringLiteral("out")); + ASSERT_TRUE(store.saveInboundToken(QStringLiteral("caller"), QStringLiteral("in"))); + store.adoptCredential(QStringLiteral("anchor")); + ASSERT_EQ(store.credential(), QStringLiteral("anchor")); + + store.clearAllTokens(); + + EXPECT_EQ(store.tokenCount(), 0); + EXPECT_EQ(store.inbound().count(), 0); + EXPECT_EQ(store.credential(), QString()) + << "resetIdentity() clears a store on reload precisely so a stale " + "credential cannot make a locked-out reload look like a live one"; +}