diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0c93a2ca..707702eb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1377,6 +1377,27 @@ jobs: MOBILEGL_ITEST_REQUIRE_GPU: "1" run: ctest --output-on-failure -L unit --no-tests=error -j "$(nproc)" + # P7 wave 2-F, PH-7 (1). THE TCP SUPERVISOR LANE NEEDS flatc, AND CI HAD NONE. + # + # `TcpLane.SupervisorProtocolControls` carries LABELS integration-tcp, so it has been in the + # selection below since wave 0 - as a SKIP, every run. The script builds the protocol's + # Python bindings with flatc, flatc is deliberately absent from the build graph + # (gen_protocol.py's header says why), and with none found it exits 77 and ctest reports + # the entry skipped. Skipped is what the entry looked like for the whole time it did not + # exist, which is the failure mode the plan's own row named ("referenced by NOTHING"). + # + # So the job builds the SAME pinned flatc that flatc-check builds, from the same submodule, + # and the lane step below names the entry to junit_tally.py --require-entry-passed. If the + # build ever stops producing one, the lane reds by name rather than going green having run + # the supervisor's Busy / token / listen / version / build-policy / reap controls zero times. + # --check is passed so this step cannot quietly regenerate a different committed header; + # keeping that honest is flatc-check's job and this step must not overlap with it. + - name: Check out the FlatBuffers submodule for the TCP supervisor lane + run: git submodule update --init 3rdparty/flatbuffers + + - name: Build the pinned flatc for the TCP supervisor lane + run: python3 scripts/gen_protocol.py --build-dir "${RUNNER_TEMP}/flatc-build" --check + # --no-tests=error is half the gate, exactly as in integration-verify: the integration-split # entries exist only when the library was configured with -DMOBILEGL_BUILD_DISAGGREGATED=ON, # so a build that lost the option matches nothing and reds here instead of reporting a green @@ -1392,6 +1413,8 @@ jobs: run: | ulimit -c unlimited sudo sysctl -w kernel.core_pattern='/tmp/core.%e.%p' + export MOBILEGL_FLATC_EXECUTABLE="${RUNNER_TEMP}/flatc-build/flatc" + test -x "${MOBILEGL_FLATC_EXECUTABLE}" ctest --output-on-failure -L integration-split --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/split-baseline.xml" python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/split-baseline.xml" --require-split-ran # Exit gate 9.3: the spawn arm green at the same count. It runs the same cases through a @@ -1402,6 +1425,7 @@ jobs: ctest --show-only=json-v1 -L '^integration-tcp$' > "${RUNNER_TEMP}/tcp-discovery.json" ctest --output-on-failure -L '^integration-tcp$' --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/tcp-baseline.xml" python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/tcp-baseline.xml" --require-tcp-ran --require-run-ahead --discovery "${RUNNER_TEMP}/tcp-discovery.json" + python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/tcp-baseline.xml" --require-entry-passed TcpLane.SupervisorProtocolControls # Both full transport runs are hard gates. The per-case comparison below # remains diagnostic because explicit mechanism controls retain their own mode. diff --git a/MobileGL/MG_Remote/Handshake.h b/MobileGL/MG_Remote/Handshake.h index 3b99e9af..8e33064c 100644 --- a/MobileGL/MG_Remote/Handshake.h +++ b/MobileGL/MG_Remote/Handshake.h @@ -2,8 +2,10 @@ #pragma once #include "CapsCodec.h" #include "Protocol/generated/protocol_generated.h" +#include "Transport/AuthToken.h" #include "Transport/ITransport.h" #include +#include #include #include @@ -37,6 +39,35 @@ namespace MobileGL::MG_Remote { return true; } + // PH-7 (1)(2), ID-P7-3. THE TOKEN POLICY, ONCE, FOR BOTH SERVER-SIDE SITES. + // + // ServerSession::Accept and ServerMain::RunSession each had their own reading of it (see + // Transport/AuthToken.h for what the three readings were). They now call this, so the answer + // to "is this peer allowed to open a session" is one function with one comparison, and it is + // the constant-time one. + // + // THE FOUR STATES, stated here because they were implicit and therefore different at each + // site: + // in-process - there is no peer; the "connection" is this process. Never authenticated. + // no token - not an empty credential: the LISTEN policy has already confined this + // server to loopback (SocketTransport.cpp's ListenTcp), so there is nothing + // left for a token to add. A peer that sends one anyway is not refused for + // it; ServerMain used to do that and no document ever said so. + // token, match - welcomed. + // token, mismatch - `Refuse{Authentication}`, detail "token mismatch", and the value is + // not in the line. A missing token field is a mismatch, not a separate case. + inline MobileGLResult AuthenticatePeerToken(Transport::ITransport& transport, + const ::flatbuffers::String* presented) { + if (transport.Role() == Transport::TransportRole::InProcess) return MOBILEGL_OK; + const char* expected = Transport::ConfiguredAuthToken(); + if (expected == nullptr) return MOBILEGL_OK; + const char* bytes = presented == nullptr ? "" : presented->c_str(); + const std::size_t size = presented == nullptr ? 0u : presented->size(); + if (Transport::ConstantTimeTokenMatch(expected, bytes, size)) return MOBILEGL_OK; + return RefuseHandshake(transport, ::MobileGL::Wire::RefuseCode::Authentication, + "token mismatch"); + } + inline MobileGLResult ValidatePeerHandshake(Transport::ITransport& transport, Uint32 major, Uint32 minor, Uint64 fingerprint, const char* stamp, ::MobileGL::Wire::DialMode dial) { diff --git a/MobileGL/MG_Remote/Server/ServerMain.cpp b/MobileGL/MG_Remote/Server/ServerMain.cpp index 35823ade..611d58bc 100644 --- a/MobileGL/MG_Remote/Server/ServerMain.cpp +++ b/MobileGL/MG_Remote/Server/ServerMain.cpp @@ -87,14 +87,14 @@ void SendLogAck(void* pointer) { if (ValidatePeerHandshake(*control, hello->abiMajor(), hello->abiMinor(), hello->wireFingerprint(), hello->buildFingerprint() ? hello->buildFingerprint()->c_str() : nullptr, hello->dialMode()) != MOBILEGL_OK) ::_exit(0); - if (tcp) { - const char* expected = std::getenv("MOBILEGL_IPC_TOKEN"); - const std::string token = hello->token() ? hello->token()->str() : std::string(); - if (token != (expected ? expected : "")) { - Refuse(*control, Protocol::RefuseCode::Authentication, "TCP token mismatch"); - ::_exit(0); - } - } + // PH-7 (1). The SAME policy ServerSession::Accept applies, from Handshake.h, so the + // supervisor's pre-fork answer and the session's post-fork answer cannot drift. What went: + // a `std::string` comparison (byte-at-a-time, early-returning) that ALSO refused a peer for + // presenting a token to a server that had configured none, and a `tcp` guard that made the + // unix control socket exempt by construction rather than by the policy's own reasoning. + // RefuseHandshake logs `Refuse{Authentication}` and sends the peer the frame; the supervisor's + // local Refuse() helper stays for the refusals that are its own (Busy, backend). + if (AuthenticatePeerToken(*control, hello->token()) != MOBILEGL_OK) ::_exit(0); MobileGL::MG_ConfigLoader::Init(); MobileGL::MG_Config::Transport = MobileGL::MG_Config::TransportMode::Spawn; MobileGL::MG_Pipe::MGPipeSetServerProcessRole(true); diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 2de331cb..50a76f49 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -545,12 +545,11 @@ namespace MobileGL::MG_Remote::Server { if (stream && transport.Role() == Transport::TransportRole::InProcess) return RefuseHandshake(transport, ::MobileGL::Wire::RefuseCode::LinkTerms, "stream requires a data connection"); - const char* expectedToken = std::getenv("MOBILEGL_IPC_TOKEN"); - if (transport.Role() != Transport::TransportRole::InProcess && expectedToken != nullptr && - expectedToken[0] != '\0' && (hello->token() == nullptr || - std::strcmp(hello->token()->c_str(), expectedToken) != 0)) - return RefuseHandshake(transport, ::MobileGL::Wire::RefuseCode::Authentication, - "token mismatch"); + // PH-7 (1). One policy, one constant-time comparison, shared with ServerMain's supervisor + // child (Handshake.h AuthenticatePeerToken). This site used std::strcmp, which returns at + // the first differing byte. + const MobileGLResult authenticated = AuthenticatePeerToken(transport, hello->token()); + if (authenticated != MOBILEGL_OK) return authenticated; if (hello->backendType() >= static_cast(BackendType::BackendTypeCount) || (m_backend != nullptr && hello->backendType() != static_cast(m_backend->GetBackendType()))) return RefuseHandshake(transport, ::MobileGL::Wire::RefuseCode::Backend, diff --git a/MobileGL/MG_Remote/Transport/AuthToken.h b/MobileGL/MG_Remote/Transport/AuthToken.h new file mode 100644 index 00000000..301daa92 --- /dev/null +++ b/MobileGL/MG_Remote/Transport/AuthToken.h @@ -0,0 +1,90 @@ +// MobileGL - MobileGL/MG_Remote/Transport/AuthToken.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// PH-7 (1)(2)(3), ID-P7-3. THE ONE PLACE THAT KNOWS WHAT `MOBILEGL_IPC_TOKEN` MEANS. +// +// It was three places, and the three did not agree: +// +// SocketTransport.cpp's ListenTcp asked only "is it non-empty", and a one-byte token +// therefore unlocked a `tcp://0.0.0.0` listen. +// ServerSession.cpp compared with `std::strcmp` under "the role is not InProcess AND a token +// is configured". +// ServerMain.cpp compared with `std::string::operator==` under "the control connection is +// TCP", which additionally demanded that a peer present an EMPTY token to a server that had +// configured none - a rule the other site did not have and no document states. +// +// Two of the three compared byte-at-a-time and returned at the first differing byte. That is the +// textbook remote timing oracle: an attacker who can retry learns the token one byte at a time +// instead of guessing 2^128. Whether it is exploitable over a LAN at these timings is beside the +// point; the comparison costs the same either way, and a security property nobody has to +// re-derive is worth more than a measurement. +// +// WHY A MINIMUM LENGTH IS A LISTEN-TIME REFUSAL AND NOT A WARNING. Ph's threat model (ID-P7-3) +// is "anybody who can reach the port has a remote code path". The answer to it is the token, so +// a token an attacker can enumerate is not an answer - it is the appearance of one, which is +// worse than no token at all, because `no token` is already handled: it confines the listener to +// loopback. A four-byte token, before this file, did not confine anything. 16 bytes is the +// floor, it is checked where the listener is opened, and a short token FAILS THE LISTEN rather +// than silently degrading to loopback-only: an operator who set a token and got a loopback +// server would have no way to tell that from success, which is the exact silence Ph exists to +// remove. +// +// The token is never logged, at any level, on any path. The refusals name the policy, not the +// secret (CONTRACT-P65: "错令牌返回 Refuse，不打印令牌值"). + +#pragma once + +#include +#include +#include + +namespace MobileGL::MG_Remote::Transport { + + // 128 bits of secret, in the encoding an environment variable can carry. The same width as + // `Welcome.dataNonce` (PH-7 (4)) for the same reason: below it, guessing is cheaper than + // attacking anything else in this design. + inline constexpr std::size_t kMinimumAuthTokenBytes = 16; + + // The configured token, or nullptr when none is configured. An EMPTY value is "none": every + // TCP lane in this tree sets `MOBILEGL_IPC_TOKEN=` precisely to say so (see + // MG_IntegrationTest/CMakeLists.txt's three arm environments), and an empty string is not a + // secret under any reading. + inline const char* ConfiguredAuthToken() { + const char* value = std::getenv("MOBILEGL_IPC_TOKEN"); + return value != nullptr && value[0] != '\0' ? value : nullptr; + } + + // Long enough to be a credential. Asked only of a CONFIGURED token; "none" is a separate + // state with its own policy (loopback-only), not a zero-length token. + inline bool AuthTokenIsLongEnough(const char* token) { + return token != nullptr && std::strlen(token) >= kMinimumAuthTokenBytes; + } + + // Constant time in the CONTENTS of both operands. + // + // The loop runs over the expected token's length and never leaves early, so the time depends + // on that length - which is this process's own configuration, fixed before any peer connects + // and not attacker-controlled - and on nothing the peer sent. The two lengths are folded into + // the accumulator rather than branched on, so a peer cannot learn the token's length from a + // faster answer either. Past the presented token's end the comparison reads a zero, which is + // a value, not a short-circuit: `presented` is never indexed out of its own bounds. + inline bool ConstantTimeTokenMatch(const char* expected, const char* presented, + std::size_t presentedSize) { + if (expected == nullptr) return false; + const std::size_t expectedSize = std::strlen(expected); + std::size_t difference = expectedSize ^ presentedSize; + for (std::size_t index = 0; index < expectedSize; ++index) { + const unsigned char theirs = + index < presentedSize ? static_cast(presented[index]) : 0u; + difference |= static_cast( + static_cast(expected[index]) ^ theirs); + } + return difference == 0; + } + +} // namespace MobileGL::MG_Remote::Transport diff --git a/MobileGL/MG_Remote/Transport/SocketTransport.cpp b/MobileGL/MG_Remote/Transport/SocketTransport.cpp index 526fadd6..3a589d39 100644 --- a/MobileGL/MG_Remote/Transport/SocketTransport.cpp +++ b/MobileGL/MG_Remote/Transport/SocketTransport.cpp @@ -8,6 +8,7 @@ #include "SocketTransport.h" +#include "AuthToken.h" #include "Doorbell.h" // kWaitForever #include "FdPassing.h" #include "WireLog.h" @@ -153,8 +154,24 @@ namespace MobileGL::MG_Remote::Transport { MobileGLResult ListenTcp(const std::string& name, int* outFd) { addrinfo* addresses = nullptr; if (!TcpAddress(name, &addresses)) return MOBILEGL_ERR_INVALID_ARGUMENT; - const char* token = std::getenv("MOBILEGL_IPC_TOKEN"); - const bool authenticated = token != nullptr && token[0] != '\0'; + // PH-7 (3), ID-P7-3. A CONFIGURED TOKEN IS EITHER A CREDENTIAL OR A REFUSAL. + // + // This read `getenv(...) != nullptr && token[0] != '\0'`, so ANY non-empty value - + // one byte - opened a `tcp://0.0.0.0` listen. The listen is the only place that can + // still say no cheaply and to the operator rather than to a peer, so the length is + // asked here, and a short token fails the listen outright instead of quietly + // becoming a loopback-only server that looks exactly like a working one. + // AuthToken.h holds the constant and the reason. + const char* token = ConfiguredAuthToken(); + if (token != nullptr && !AuthTokenIsLongEnough(token)) { + WireLogError("MG_Remote: Refuse{Authentication} MOBILEGL_IPC_TOKEN is %zu bytes " + "and the minimum is %zu (PH-7 (3)); refusing to listen on %s rather " + "than authenticating with a guessable secret", + std::strlen(token), kMinimumAuthTokenBytes, name.c_str()); + ::freeaddrinfo(addresses); + return MOBILEGL_ERR_PROTOCOL_MISMATCH; + } + const bool authenticated = token != nullptr; MobileGLResult result = MOBILEGL_ERR_UNSUPPORTED; for (auto* address = addresses; address; address = address->ai_next) { if (!authenticated && !Loopback(address->ai_addr)) { diff --git a/MobileGL/MG_Test/Wire/SocketTransportTest.cpp b/MobileGL/MG_Test/Wire/SocketTransportTest.cpp index 95dda313..22e5e12d 100644 --- a/MobileGL/MG_Test/Wire/SocketTransportTest.cpp +++ b/MobileGL/MG_Test/Wire/SocketTransportTest.cpp @@ -36,6 +36,7 @@ // is one this side already reassembled out of bytes nobody else can re-deliver. #include // RefuseCode, for the word below +#include // PH-7 (3): the minimum and the comparison #include // kWaitForever #include #include @@ -131,6 +132,55 @@ TEST(SocketTransportTest, TcpRejectsMalformedNamesAndUnauthenticatedWildcard) { if (wasSet) ::setenv("MOBILEGL_IPC_TOKEN", saved.c_str(), 1); } +// PH-7 (3), ID-P7-3. A CONFIGURED TOKEN IS EITHER A CREDENTIAL OR A REFUSAL. +// +// The listen policy used to ask only "non-empty", so a one-byte MOBILEGL_IPC_TOKEN opened a +// wildcard listen. The end-to-end half of this - the supervisor PROCESS dying with a named +// reason rather than degrading to a loopback server that looks like success - is +// TcpLane.SupervisorProtocolControls; this is the cheap half, and it also pins that the refusal +// applies on LOOPBACK, where a short token would otherwise be harmless. Silently accepting it +// there is what would let a short token reach a deployment. +TEST(SocketTransportTest, TcpRefusesToListenWithATokenShorterThanTheMinimum) { + const char* prior = std::getenv("MOBILEGL_IPC_TOKEN"); + const std::string saved = prior ? prior : ""; + const bool wasSet = prior != nullptr; + const std::string tooShort(kMinimumAuthTokenBytes - 1, 'x'); + ::setenv("MOBILEGL_IPC_TOKEN", tooShort.c_str(), 1); + int listener = -1; + EXPECT_EQ(SocketTransport::Listen("tcp://127.0.0.1:40697", &listener), + MOBILEGL_ERR_PROTOCOL_MISMATCH); + EXPECT_EQ(listener, -1); + // One more byte and the same endpoint comes up, so the refusal above is the LENGTH and not + // the port, the address family or the environment variable's mere presence. + const std::string longEnough(kMinimumAuthTokenBytes, 'x'); + ::setenv("MOBILEGL_IPC_TOKEN", longEnough.c_str(), 1); + EXPECT_EQ(SocketTransport::Listen("tcp://127.0.0.1:40697", &listener), MOBILEGL_OK); + EXPECT_GE(listener, 0); + if (listener >= 0) ::close(listener); + if (wasSet) ::setenv("MOBILEGL_IPC_TOKEN", saved.c_str(), 1); + else ::unsetenv("MOBILEGL_IPC_TOKEN"); +} + +// The comparison itself. `std::strcmp` and `std::string::operator==` - the two this replaced - +// both stop at the first differing byte; these cases pin the answers, and the loop in +// AuthToken.h is what makes the time not depend on WHERE the difference is. +TEST(SocketTransportTest, TheTokenComparisonAnswersLengthAndPrefixTheSameWay) { + const char* expected = "0123456789abcdef"; + EXPECT_TRUE(ConstantTimeTokenMatch(expected, "0123456789abcdef", 16)); + EXPECT_FALSE(ConstantTimeTokenMatch(expected, "0123456789abcdeF", 16)); + EXPECT_FALSE(ConstantTimeTokenMatch(expected, "Z123456789abcdef", 16)); + // A correct PREFIX is not a match, and neither is a correct token with anything after it. + EXPECT_FALSE(ConstantTimeTokenMatch(expected, "0123456789abcde", 15)); + EXPECT_FALSE(ConstantTimeTokenMatch(expected, "0123456789abcdefg", 17)); + EXPECT_FALSE(ConstantTimeTokenMatch(expected, "", 0)); + // An embedded NUL is compared, not truncated at: `presented` carries its own length because + // a flatbuffers string can hold one and c_str()-plus-strcmp would have stopped there. + const char embedded[] = {'0', '\0', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'}; + EXPECT_FALSE(ConstantTimeTokenMatch(expected, embedded, sizeof(embedded))); + EXPECT_FALSE(ConstantTimeTokenMatch(nullptr, "anything", 8)); +} + namespace { MobileGLByteSpan Span(const std::string& text) { diff --git a/scripts/ci/junit_tally.py b/scripts/ci/junit_tally.py index 4a4a7941..d3932f14 100755 --- a/scripts/ci/junit_tally.py +++ b/scripts/ci/junit_tally.py @@ -61,6 +61,34 @@ def require_tcp_proof(path, prefix, discovery, require_run_ahead=False): raise ValueError(f"{case.get('name')}: TCP runtime did not keep requested run-ahead armed") +def require_entries_passed(path, names): + """P7 wave 2-F, PH-7 (1). Named entries must have RUN and passed - a SKIP is a failure here. + + `TcpLane.SupervisorProtocolControls` is registered with SKIP_RETURN_CODE 77 because the + script it runs needs flatc, which is deliberately not in the build graph. That is the right + behaviour for a developer who has not built one; it is the wrong behaviour for CI, where the + lane's whole job is to run the supervisor's protocol controls and "skipped" is + indistinguishable from the three commits the entry did not exist. The workflow builds flatc + and then names the entry here, so a CI run in which the tool went missing reds by name + instead of reporting a green lane that measured less than it says. + """ + seen = {} + for case in ET.parse(path).getroot().iter('testcase'): + name = case.get('name') + if name not in names: + continue + if case.find('failure') is not None or case.find('error') is not None: + seen[name] = 'failed' + elif case.find('skipped') is not None or case.get('status') in ('notrun', 'disabled'): + seen[name] = 'skipped' + else: + seen[name] = 'passed' + for name in names: + state = seen.get(name, 'absent') + if state != 'passed': + raise ValueError(f'{name}: required to pass, was {state}') + + def tally(path, prefix=None): passed = failed = skipped = 0 for case in ET.parse(path).getroot().iter('testcase'): @@ -84,6 +112,8 @@ def main(): parser.add_argument('--discovery', help='CTest --show-only=json-v1 output (required for TCP proof)') parser.add_argument('--require-run-ahead', action='store_true', help='Additionally prove actual run-ahead, without fallback or demotion, on each TCP case') + parser.add_argument('--require-entry-passed', action='append', default=[], metavar='NAME', + help='This ctest entry must be present and passed; a SKIP fails (repeatable)') args = parser.parse_args() if args.prefix in ('DirectGLES.Tcp.', 'DirectGLES.TcpDevice.') and not args.discovery: parser.error('TCP arm proof requires --discovery to locate each private role log') @@ -94,6 +124,8 @@ def main(): passed, failed, skipped = tally(args.junit, prefix=prefix) if prefix in ('DirectGLES.Tcp.', 'DirectGLES.TcpDevice.'): require_tcp_proof(args.junit, prefix, args.discovery, args.require_run_ahead) + if args.require_entry_passed: + require_entries_passed(args.junit, args.require_entry_passed) except Exception as exc: # a malformed file is not "zero of everything" print(f"junit_tally: cannot prove {args.junit}: {exc}", file=sys.stderr) return 1 diff --git a/scripts/ci/tcp_lane_tools_test.py b/scripts/ci/tcp_lane_tools_test.py index 4918f7be..8c13fe42 100644 --- a/scripts/ci/tcp_lane_tools_test.py +++ b/scripts/ci/tcp_lane_tools_test.py @@ -87,6 +87,35 @@ class Accounting(unittest.TestCase): with self.assertRaises(ValueError): tally.require_tcp_proof(junit, 'DirectGLES.Tcp.', discovery) + def test_a_skipped_required_entry_is_not_a_pass(self): + """P7 wave 2-F, PH-7 (1). The four states of a NAMED entry, and only one of them passes. + + `TcpLane.SupervisorProtocolControls` skips itself (exit 77) when flatc is absent, which + is correct for a developer and wrong for CI - the workflow now builds flatc and names the + entry, so `skipped` has to red here exactly as `failed` and `absent` do. Written as four + cases rather than one because the one that matters is `skipped`, and a check that only + looked for would have let it through, which is the whole defect. + """ + tally = module('junit_tally') + name = 'TcpLane.SupervisorProtocolControls' + bodies = { + 'passed': f'', + 'skipped': f'', + 'notrun': f'', + 'failed': f'', + 'absent': '', + } + with tempfile.TemporaryDirectory() as directory: + junit = Path(directory) / 'junit.xml' + for state, body in bodies.items(): + junit.write_text(f'{body}') + if state == 'passed': + tally.require_entries_passed(junit, [name]) + continue + with self.assertRaises(ValueError, msg=state) as raised: + tally.require_entries_passed(junit, [name]) + self.assertIn(name, str(raised.exception)) + def test_equal_counts_do_not_hide_a_missing_case(self): parity = module('spawn_lane_parity') a, b = 'TriangleScenario.Draw', 'TriangleScenario.Read' diff --git a/scripts/ci/tcp_supervisor_smoke.py b/scripts/ci/tcp_supervisor_smoke.py index ed9497ca..4931f379 100644 --- a/scripts/ci/tcp_supervisor_smoke.py +++ b/scripts/ci/tcp_supervisor_smoke.py @@ -1,11 +1,18 @@ #!/usr/bin/env python3 -"""Exercise a real TCP supervisor's Busy, token, version, build-policy and REAP controls. +"""Exercise a real TCP supervisor's LISTEN, Busy, token, version, build-policy and REAP controls. P7 wave 0 registered this script as `TcpLane.SupervisorProtocolControls` in MG_IntegrationTest/CMakeLists.txt. Until then it was referenced by NOTHING in the tree - no ctest entry, no workflow step - so the four controls it asserts had been green or red for an unknown number of commits and nobody would have known which. +P7 wave 2-F, PH-7 (1)(2)(3) (ID-P7-3), added the two LISTEN controls at the top of main(). They +are the negative controls for the policy AuthToken.h now owns, and both are end-to-end against a +real supervisor process because that is the only place the policy is reachable: an in-process +unit test of `SocketTransport::Listen` can assert the return code but not that an operator who +misconfigured a token gets a dead server with a named reason rather than a live one that silently +serves loopback only. + It needs flatc, which is deliberately absent from the default build graph (gen_protocol.py's header block says why). Resolution order is gen_protocol.py's, minus the build-it-for-you arm: --flatc, then $MOBILEGL_FLATC_EXECUTABLE, then /../flatc-build/flatc. With none of them @@ -34,6 +41,18 @@ ROOT = Path(__file__).resolve().parents[2] # uses and ctest has no opinion of its own. kSkipExit = 77 +# PH-7 (3): >= AuthToken.h's kMinimumAuthTokenBytes (16). The old value, `p65-smoke-token`, was +# fifteen bytes - one short - which is a pleasing accident and exactly the kind of token the +# minimum exists to refuse. The value is arbitrary; its LENGTH is the fixture. +kToken = 'p7-ph-f-supervisor-token' +assert len(kToken) >= 16, kToken +# A token that is a credential in every respect except length. +kShortToken = 'p7-ph-f-short' +assert len(kShortToken) < 16, kShortToken + +# ServerMain's `SocketTransport::Listen(...) != MOBILEGL_OK` arm. +kListenRefusedExit = 72 + def resolve_flatc(explicit): """gen_protocol.py's order, without its build-it-for-you arm. None when there is none.""" @@ -77,7 +96,7 @@ def receive(peer, schema): raise RuntimeError(f'unexpected first control reply {kind}') -def hello(schema, flatbuffers, fingerprint=0, major=1, token='p65-smoke-token', identifier=b'MGLC'): +def hello(schema, flatbuffers, fingerprint=0, major=1, token=kToken, identifier=b'MGLC'): builder = flatbuffers.Builder(512) build = builder.CreateString('intentionally-different-build-for-p65-control') auth = builder.CreateString(token) @@ -108,18 +127,62 @@ def hello(schema, flatbuffers, fingerprint=0, major=1, token='p65-smoke-token', return struct.pack('<4sI', b'MGLF', len(payload)) + payload -@contextmanager -def supervisor(server, log, same_build=False): - with socket.socket() as reserve: - reserve.bind(('127.0.0.1', 0)) - port = reserve.getsockname()[1] - base = log.with_suffix('.library.log') +def supervisor_environment(log, token, same_build=False): + """The env a supervisor is started with, minus everything ServerMain's scrub catches.""" env = dict(os.environ, MOBILEGL_IPC_ROLE='server', MOBILEGL_IPC_DIAL='no', - MOBILEGL_IPC_TOKEN='p65-smoke-token', MOBILEGL_IPC_REQUIRE_SAME_BUILD=str(int(same_build)), - MOBILEGL_IPC_LOG_FORWARD='0', MOBILEGL_LOG_FILE_PATH=str(base)) + MOBILEGL_IPC_REQUIRE_SAME_BUILD=str(int(same_build)), + MOBILEGL_IPC_LOG_FORWARD='0', MOBILEGL_LOG_FILE_PATH=str(log)) + if token is None: + env.pop('MOBILEGL_IPC_TOKEN', None) + else: + env['MOBILEGL_IPC_TOKEN'] = token for name in ('MOBILEGL_TRANSPORT', 'MOBILEGL_IPC_SERVER_PATH', 'MOBILEGL_IPC_RING_MB', 'MOBILEGL_IPC_STAGE_MB', 'MOBILEGL_IPC_CONTROL', 'MOBILEGL_IPC_ENDPOINT', 'MOBILEGL_BACKEND_TYPE'): env.pop(name, None) + return env + + +def free_loopback_port(): + with socket.socket() as reserve: + reserve.bind(('127.0.0.1', 0)) + return reserve.getsockname()[1] + + +def refused_listen(server, log, endpoint, token): + """Start a supervisor that must NOT come up, and return (exit code, everything it said). + + PH-7 (2)(3), ID-P7-3. Both listen refusals are asserted this way rather than through + SocketTransport::Listen's return code, because the property is not "the function returns + PROTOCOL_MISMATCH" - it is that the PROCESS dies, says which policy killed it, and does not + leave a half-configured server listening. A supervisor that degraded to loopback-only after a + misconfigured token would pass a return-code test and fail an operator. + """ + base = log.with_suffix('.library.log') + env = supervisor_environment(base, token) + with log.open('wb') as output: + child = subprocess.Popen([server, endpoint, '--serve'], env=env, + stdout=output, stderr=output, start_new_session=True) + try: + code = child.wait(timeout=30) + except subprocess.TimeoutExpired: + # THE RED SHAPE OF THIS CONTROL IS A SUPERVISOR THAT CAME UP, so it is still listening + # and would answer the next control's peers. Killed here rather than left for the + # assertion below, which is what makes the red a red and not a cascade. + os.killpg(child.pid, signal.SIGKILL) + child.wait(timeout=10) + code = None + text = log.read_text(errors='replace') + role = base.with_name(base.stem + '.server' + base.suffix) + if role.is_file(): + text += role.read_text(errors='replace') + return code, text + + +@contextmanager +def supervisor(server, log, same_build=False): + port = free_loopback_port() + base = log.with_suffix('.library.log') + env = supervisor_environment(base, kToken, same_build=same_build) with log.open('wb') as output: child = subprocess.Popen([server, f'tcp://127.0.0.1:{port}', '--serve'], env=env, stdout=output, stderr=output, start_new_session=True) @@ -247,6 +310,32 @@ def main(): schema = {name: importlib.import_module('MobileGL.Wire.' + name) for name in ('CtrlEnvelope', 'CtrlMsg', 'Hello', 'LinkTerms', 'Refuse', 'Welcome')} evidence = {} + + # ---- PH-7 (2)(3), ID-P7-3: THE TWO LISTEN REFUSALS ------------------------------- + # + # Both are the SAME policy from two directions: a listener that can be reached from off + # this machine has to have a credential, and a credential has to be one. + # + # (a) No token, non-loopback bind. The mechanism has been in the tree since P6.5 + # (SocketTransport.cpp's ListenTcp) and the plan's own gate row called it "done + # (mechanism)" with the caveat that nothing referenced it. This is the reference. + # (b) A token shorter than 16 bytes, on LOOPBACK, where a short token is otherwise + # harmless. It still fails, and that is the ruling: a configured token is either a + # credential or a refusal, never a warning. Before this package a one-byte token + # unlocked `tcp://0.0.0.0`. + code, said = refused_listen(args.server, args.out / 'wildcard-no-token.log', + 'tcp://0.0.0.0:40699', None) + assert code == kListenRefusedExit, (code, said) + assert 'Refuse{Authentication}' in said, said + evidence['wildcard_listen_without_a_token'] = {'exit': code, 'line': next( + l.strip() for l in said.splitlines() if 'Refuse{Authentication}' in l)} + short = f'tcp://127.0.0.1:{free_loopback_port()}' + code, said = refused_listen(args.server, args.out / 'short-token.log', short, kShortToken) + assert code == kListenRefusedExit, (code, said) + assert 'Refuse{Authentication}' in said and 'minimum is 16' in said, said + assert kShortToken not in said, 'the refusal printed the token' + evidence['short_token_listen'] = {'exit': code, 'line': next( + l.strip() for l in said.splitlines() if 'Refuse{Authentication}' in l)} with supervisor(args.server, args.out / 'supervisor.log') as (port, process, serverLog): with pair(port) as held: with pair(port) as second: