diff --git a/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java b/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java index 62f0244..e9012f1 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java +++ b/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java @@ -262,11 +262,15 @@ public final class CallerResolver { return token.isEmpty() ? null : token; } + /** + * fleetd #305: delegates to {@link ConnectionIdentity#isLoopback}. This used to be a second, + * independent copy of the same rule, and the two drifted: this one accepted all of + * {@code 127.0.0.0/8}, {@code ConnectionIdentity}'s accepted only {@code 127.0.0.1}. A caller + * from {@code 127.0.0.2} therefore had its identity skipped (so it had no terminal) and was + * then read as loopback here — which under loopback-trust is the primary. Sharing the inputs + * would not have prevented that; only sharing the computation does. + */ private static boolean isLoopback(String remoteAddr) { - if (remoteAddr == null) { - return false; - } - return remoteAddr.equals("127.0.0.1") || remoteAddr.equals("::1") - || remoteAddr.equals("0:0:0:0:0:0:0:1") || remoteAddr.startsWith("127."); + return ConnectionIdentity.isLoopback(remoteAddr); } } diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java index 60b66d1..16986bb 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java @@ -60,7 +60,30 @@ public final class ConnectionIdentity { return pid > 0 ? cwds.cwdForPid(pid) : null; } - private static boolean isLoopback(String addr) { - return "127.0.0.1".equals(addr) || "::1".equals(addr) || "0:0:0:0:0:0:0:1".equals(addr); + /** + * Whether {@code addr} is a same-host address, and therefore one whose peer PID is worth + * looking up. This is the one definition of loopback in the daemon — + * {@code CallerResolver} calls it rather than keeping its own, because the two used to differ + * and that difference was a privilege escalation (fleetd #305). + * + *

The whole of {@code 127.0.0.0/8} counts, not just {@code 127.0.0.1}. On Linux every + * address in that range is bound to {@code lo} by default, so a process can connect to + * {@code 127.0.0.1:8765} with a source address of {@code 127.0.0.2} — measured on the Linux + * fleet host, where binding that source succeeds. + * + *

Being strict here does not make the daemon safer; it makes it unsafe. + * That reads backwards, so it is worth stating plainly. This predicate does not decide whether + * a caller is trusted — it decides whether the caller's identity is resolved at all. + * Returning false means {@link #resolve} answers "no terminal", and downstream a caller with no + * terminal is treated as the primary under loopback-trust. So every address excluded here is an + * address on which a worker silently becomes the lead. Widening a check normally weakens it; + * widening this one is what closes the hole. + */ + public static boolean isLoopback(String addr) { + if (addr == null) { + return false; + } + String a = addr.startsWith("::ffff:") ? addr.substring(7) : addr; // IPv4-mapped IPv6 + return a.startsWith("127.") || "::1".equals(a) || "0:0:0:0:0:0:0:1".equals(a); } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java b/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java index 31a3bce..c0d4755 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java @@ -413,4 +413,29 @@ class CallerResolverTest { assertThrows(IllegalArgumentException.class, () -> new CallerResolver(id, true, null)); assertThrows(IllegalArgumentException.class, () -> new CallerResolver(id, true, " ")); } + @Test + void aWorkerOnAnyLoopbackSourceAddressIsStillAWorkerNotThePrimary() { + // fleetd #305: the escalation. ConnectionIdentity used to accept only 127.0.0.1, so a + // worker connecting from 127.0.0.2 resolved to no terminal, and this resolver's own + // (wider) loopback check then made it the PRIMARY — granting spawn, stop, send and drain. + // Measured on the Linux fleet host: binding a source of 127.0.0.2 succeeds there, so the + // path is real and not theoretical. + CallerResolver r = new CallerResolver(workerIdentity(), false, null); + for (String src : new String[]{"127.0.0.1", "127.0.0.2", "127.1.2.3", "::ffff:127.0.0.2"}) { + Principal p = r.resolve(src, 55555, null); + assertEquals(Role.WORKER, p.role(), "a worker must stay a worker from source " + src); + assertEquals("term_a", p.terminal(), "worker terminal from source " + src); + } + } + + @Test + void aNonWorkerOnAnyLoopbackSourceAddressIsStillThePrimary() { + // The other direction of the same fix: widening the identity check must not demote a + // legitimate same-host primary that happens to connect from another 127.* address. + CallerResolver r = new CallerResolver(nonWorkerIdentity(), false, null); + for (String src : new String[]{"127.0.0.1", "127.0.0.2", "::ffff:127.0.0.1"}) { + assertEquals(Role.PRIMARY, r.resolve(src, 55555, null).role(), "source " + src); + } + } + } diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java index 20d6c0f..94ef7c8 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java @@ -20,6 +20,17 @@ class ConnectionIdentityTest { assertEquals("term_a", with(_ -> FakeHerdr.WORKER_PID).callerTerminal("127.0.0.1", 55555)); } + @Test + void resolvesWorkerFromAnyLoopbackSourceAddressNotJust127001() { + // fleetd #305. On Linux the whole 127.0.0.0/8 is bound to lo, so a worker can connect with + // a source address of 127.0.0.2. If identity resolution skips that address the caller has + // no terminal, and a caller with no terminal is the primary under loopback-trust — so this + // must resolve the worker, not null. + assertEquals("term_a", with(_ -> FakeHerdr.WORKER_PID).callerTerminal("127.0.0.2", 55555)); + assertEquals("term_a", with(_ -> FakeHerdr.WORKER_PID).callerTerminal("127.1.2.3", 55555)); + assertEquals("term_a", with(_ -> FakeHerdr.WORKER_PID).callerTerminal("::ffff:127.0.0.2", 55555)); + } + @Test void nullForOffHostCaller() { // A non-loopback peer can't be an on-host worker → treat as primary/unknown.