This commit is contained in:
Dai Ha
@@ -676,8 +676,8 @@ guard:
|
||||
# every name here NOT also in `allow` is overlaid with a non-secret sentinel value before
|
||||
# the pane's login shell runs — real protection only for names that shell does not itself
|
||||
# re-export (see the ROUND-2 CORRECTION note above). Under allow-list: reporting only.
|
||||
# sshAuthSock → whether SSH_AUTH_SOCK may pass through under allow-list ("allow") or is omitted
|
||||
# from the member environment ("block", the default). Blocking it only omits the
|
||||
# sshAgentEnv → whether SSH_AUTH_SOCK may pass through under allow-list ("inherit") or is omitted
|
||||
# from the member environment ("omit", the default). Omitting it only omits the
|
||||
# inherited ssh-agent path. It discourages automatic use of the operator's agent.
|
||||
# It does not deny same-user access to that socket. It also does not block SSH keys that
|
||||
# are readable on disk. Git over SSH may still work from inside a member. Keep the block:
|
||||
@@ -686,7 +686,7 @@ guard:
|
||||
# confidentiality boundary. A real boundary needs a different OS user or OS-level
|
||||
# confinement, such as a container or VM. That is the open question in fleetd #184.
|
||||
#
|
||||
# Still do not set this to "allow" casually. SSH_AUTH_SOCK is a live handle to YOUR
|
||||
# Still do not set this to "inherit" casually. SSH_AUTH_SOCK is a live handle to YOUR
|
||||
# ssh-agent, so a member holding it can sign with EVERY key the agent holds. It sits in
|
||||
# no secret file and looks like no credential, which is why it slipped past three
|
||||
# earlier tickets (gitea #110). Blocking it does not contain a member, but allowing it
|
||||
@@ -704,7 +704,7 @@ guard:
|
||||
# are unaffected either way.
|
||||
# memberCredentials:
|
||||
# policy: deny-by-default # or "deny-list", or "allow-list" (CB-633) — see above
|
||||
# sshAuthSock: block # allow-list only; see the sshAuthSock note above
|
||||
# sshAgentEnv: omit # allow-list only; see the sshAgentEnv note above
|
||||
# allow:
|
||||
# - AI_GATEWAY_TOKEN # named in a profile's tokenEnv (local/gx) — a member reaching the
|
||||
# # gateway is by design, not a leak
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package dev.ltms.fleet.config;
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonCreator;
|
||||
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
|
||||
import com.fasterxml.jackson.annotation.JsonProperty;
|
||||
import com.fasterxml.jackson.core.JsonParser;
|
||||
import com.fasterxml.jackson.core.JsonToken;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
@@ -1357,19 +1359,19 @@ public record FleetConfig(
|
||||
* deny-list/deny-by-default every name here that is NOT also in {@link #allow} is
|
||||
* overlaid with a non-secret sentinel value. Under allow-list this list is
|
||||
* reporting only.
|
||||
* @param sshAuthSock whether the member may inherit {@code SSH_AUTH_SOCK} under the allow-list
|
||||
* policy ({@code "allow"}) or must have it blanked ({@code "block"}, the default).
|
||||
* @param sshAgentEnv whether the member may inherit {@code SSH_AUTH_SOCK} under the allow-list
|
||||
* policy ({@code "inherit"}) or must have it omitted ({@code "omit"}, the default).
|
||||
* This is a DECISION, never a default: {@code SSH_AUTH_SOCK} is a handle to the
|
||||
* operator's ssh-agent, and a member holding it can sign with the operator's own
|
||||
* keys — but it appears in no secret file and is credential-shaped like nothing on
|
||||
* any list, which is why three earlier tickets missed it (gitea #110 / CB-607).
|
||||
* Blocking it breaks git over SSH inside the member; allow it only when members do
|
||||
* Omitting it does not prevent git over SSH inside the member; inherit it only when members do
|
||||
* not need to authenticate as the operator over SSH. Ignored under deny-list /
|
||||
* deny-by-default, which never touch the name.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record MemberCredentials(String policy, List<String> allow, List<String> known,
|
||||
String sshAuthSock) {
|
||||
String sshAgentEnv) {
|
||||
|
||||
/** Default policy: block every {@code known} name not in {@code allow}, via the env overlay. */
|
||||
public static final String POLICY_DENY_BY_DEFAULT = "deny-by-default";
|
||||
@@ -1386,11 +1388,25 @@ public record FleetConfig(
|
||||
*/
|
||||
public static final String POLICY_ALLOW_LIST = "allow-list";
|
||||
|
||||
/** The pre-CB-633 three-field form — {@code sshAuthSock} defaults to blocked. */
|
||||
/** The pre-CB-633 three-field form — {@code sshAgentEnv} defaults to omitted. */
|
||||
public MemberCredentials(String policy, List<String> allow, List<String> known) {
|
||||
this(policy, allow, known, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads both the current {@code sshAgentEnv} key and the compatible {@code sshAuthSock} key.
|
||||
* When both keys are present, {@code sshAgentEnv} wins, even if its value is unrecognised.
|
||||
*/
|
||||
@JsonCreator
|
||||
public static MemberCredentials fromYaml(@JsonProperty("policy") String policy,
|
||||
@JsonProperty("allow") List<String> allow,
|
||||
@JsonProperty("known") List<String> known,
|
||||
@JsonProperty("sshAgentEnv") String sshAgentEnv,
|
||||
@JsonProperty("sshAuthSock") String sshAuthSock) {
|
||||
return new MemberCredentials(policy, allow, known,
|
||||
sshAgentEnv != null ? sshAgentEnv : sshAuthSock);
|
||||
}
|
||||
|
||||
public MemberCredentials {
|
||||
String normalizedPolicy = (policy == null || policy.isBlank())
|
||||
? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(java.util.Locale.ROOT);
|
||||
@@ -1399,8 +1415,9 @@ public record FleetConfig(
|
||||
policy = POLICY_DENY_LIST.equals(normalizedPolicy) ? POLICY_DENY_BY_DEFAULT : normalizedPolicy;
|
||||
allow = allow == null ? List.of() : List.copyOf(allow);
|
||||
known = known == null ? List.of() : List.copyOf(known);
|
||||
sshAuthSock = (sshAuthSock != null && "allow".equalsIgnoreCase(sshAuthSock.trim()))
|
||||
? "allow" : "block";
|
||||
sshAgentEnv = (sshAgentEnv != null
|
||||
&& ("inherit".equalsIgnoreCase(sshAgentEnv.trim())
|
||||
|| "allow".equalsIgnoreCase(sshAgentEnv.trim()))) ? "inherit" : "omit";
|
||||
}
|
||||
|
||||
/** True when this block selects the CB-633 derived-allow-list policy. */
|
||||
@@ -1409,8 +1426,8 @@ public record FleetConfig(
|
||||
}
|
||||
|
||||
/** True when {@code SSH_AUTH_SOCK} may pass through under the allow-list policy. Default: no. */
|
||||
public boolean sshAuthSockAllowed() {
|
||||
return "allow".equals(sshAuthSock);
|
||||
public boolean sshAgentEnvInherited() {
|
||||
return "inherit".equals(sshAgentEnv);
|
||||
}
|
||||
|
||||
/** {@link #allow} as a set, for membership checks. */
|
||||
|
||||
@@ -1473,9 +1473,9 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
Set<String> brokerUriEnvNames = brokerUriEnvNames();
|
||||
Set<String> allowed = new java.util.TreeSet<>(
|
||||
MemberEnvAllowList.derive(profiles.values(), creds.allowSet(), brokerUriEnvNames));
|
||||
if (creds.sshAuthSockAllowed()) {
|
||||
if (creds.sshAgentEnvInherited()) {
|
||||
allowed.add(SSH_AUTH_SOCK);
|
||||
} // blocked by default: absent from the set ⇒ blanked by the scrub like any other name
|
||||
} // omitted by default: absent from the set ⇒ blanked by the scrub like any other name
|
||||
allowed.addAll(launch.env().keySet());
|
||||
allowed.removeAll(brokerUriEnvNames);
|
||||
return allowed;
|
||||
|
||||
@@ -34,7 +34,7 @@ import java.util.TreeSet;
|
||||
*
|
||||
* <p>{@code SSH_AUTH_SOCK} is deliberately NOT here. It is a handle to the operator's ssh-agent — a
|
||||
* member holding it can sign with the operator's keys — so keeping it is a config decision
|
||||
* ({@code memberCredentials.sshAuthSock: allow}), not a derivation default.
|
||||
* ({@code memberCredentials.sshAgentEnv: inherit}), not a derivation default.
|
||||
*
|
||||
* <p><b>CB-633 follow-up:</b> the union also includes {@code memberCredentials.allow:} — the
|
||||
* operator's own explicit list. Before this, {@code policy: allow-list} silently ignored every name
|
||||
@@ -42,7 +42,7 @@ import java.util.TreeSet;
|
||||
* turning the policy on could blank credentials working members already depended on. {@code
|
||||
* SSH_AUTH_SOCK} and configured broker URI environment names are exceptions: even when the operator
|
||||
* lists them under {@code allow:}, they are excluded here. {@code SSH_AUTH_SOCK} is added back ONLY
|
||||
* by the caller when {@code sshAuthSock: allow} is explicitly set
|
||||
* by the caller when {@code sshAgentEnv: inherit} is explicitly set
|
||||
* (see {@link #SSH_AUTH_SOCK}'s javadoc) — it is a live handle to the operator's own ssh-agent, not
|
||||
* a value, so treating it like any other allow-listed name would hand a member every key the
|
||||
* operator's agent holds the moment they typed the name under {@code allow:} for an unrelated
|
||||
@@ -53,7 +53,7 @@ public final class MemberEnvAllowList {
|
||||
/**
|
||||
* The operator's ssh-agent socket path. Deliberately excluded from {@link #derive}'s union of
|
||||
* {@code memberCredentials.allow:} — see the class javadoc's CB-633 follow-up note. Governed
|
||||
* ONLY by {@code memberCredentials.sshAuthSock}, never by appearing in {@code allow:}.
|
||||
* ONLY by {@code memberCredentials.sshAgentEnv}, never by appearing in {@code allow:}.
|
||||
*/
|
||||
public static final String SSH_AUTH_SOCK = "SSH_AUTH_SOCK";
|
||||
|
||||
|
||||
@@ -2010,22 +2010,93 @@ class FleetConfigTest {
|
||||
"deny-list normalizes onto the canonical deny-by-default value");
|
||||
}
|
||||
|
||||
/**
|
||||
* CB-633: {@code SSH_AUTH_SOCK} is a decision, never a default — absent, blank, or misspelled,
|
||||
* it stays BLOCKED; only the literal "allow" (any case) passes it through. A typo like "alow"
|
||||
* failing safe here is the whole point of making it a knob.
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockDefaultsToBlockedAndOnlyExplicitAllowUnblocksIt() {
|
||||
assertTrue(new FleetConfig.MemberCredentials("allow-list", List.of(), List.of()).sshAuthSock().equals("block"),
|
||||
"absent knob blocks SSH_AUTH_SOCK");
|
||||
assertFalse(new FleetConfig.MemberCredentials("allow-list", List.of(), List.of()).sshAuthSockAllowed());
|
||||
assertFalse(new FleetConfig.MemberCredentials(null, null, null, "").sshAuthSockAllowed(),
|
||||
"blank knob blocks SSH_AUTH_SOCK");
|
||||
assertFalse(new FleetConfig.MemberCredentials(null, null, null, "alow").sshAuthSockAllowed(),
|
||||
"a misspelled value fails SAFE, not open");
|
||||
assertTrue(new FleetConfig.MemberCredentials(null, null, null, "ALLOW").sshAuthSockAllowed(),
|
||||
"the literal allow (case-insensitive) unblocks SSH_AUTH_SOCK");
|
||||
void sshAgentEnvAcceptsEveryCompatibleKeyAndValuePair(@TempDir Path dir) throws Exception {
|
||||
String[][] spellings = {
|
||||
{"sshAuthSock", "block", "omit"},
|
||||
{"sshAuthSock", "allow", "inherit"},
|
||||
{"sshAuthSock", "omit", "omit"},
|
||||
{"sshAuthSock", "inherit", "inherit"},
|
||||
{"sshAgentEnv", "block", "omit"},
|
||||
{"sshAgentEnv", "allow", "inherit"},
|
||||
{"sshAgentEnv", "omit", "omit"},
|
||||
{"sshAgentEnv", "inherit", "inherit"}
|
||||
};
|
||||
|
||||
for (int i = 0; i < spellings.length; i++) {
|
||||
Path file = dir.resolve("member-credentials-ssh-agent-" + i + ".yaml");
|
||||
Files.writeString(file, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
""" + " " + spellings[i][0] + ": " + spellings[i][1] + "\n");
|
||||
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials();
|
||||
assertEquals(spellings[i][2], credentials.sshAgentEnv(),
|
||||
spellings[i][0] + ": " + spellings[i][1] + " must normalize correctly");
|
||||
assertEquals("inherit".equals(spellings[i][2]), credentials.sshAgentEnvInherited());
|
||||
}
|
||||
}
|
||||
|
||||
/** Protects the live {@code sshAuthSock: block} allow-list configuration during the rename. */
|
||||
@Test
|
||||
void legacySshAuthSockBlockKeepsLiveAllowListConfigOmitted(@TempDir Path dir) throws Exception {
|
||||
Path file = dir.resolve("live-member-credentials.yaml");
|
||||
Files.writeString(file, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
sshAuthSock: block
|
||||
""");
|
||||
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials();
|
||||
|
||||
assertEquals("omit", credentials.sshAgentEnv());
|
||||
assertFalse(credentials.sshAgentEnvInherited(), "the live config must omit SSH_AUTH_SOCK");
|
||||
}
|
||||
|
||||
@Test
|
||||
void sshAgentEnvWinsWhenBothCompatibleKeysArePresent(@TempDir Path dir) throws Exception {
|
||||
Path file = dir.resolve("both-ssh-agent-keys.yaml");
|
||||
Files.writeString(file, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
sshAuthSock: allow
|
||||
sshAgentEnv: omit
|
||||
""");
|
||||
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials();
|
||||
|
||||
assertEquals("omit", credentials.sshAgentEnv());
|
||||
assertFalse(credentials.sshAgentEnvInherited());
|
||||
}
|
||||
|
||||
@Test
|
||||
void sshAgentEnvDefaultsToOmitAndUnknownValuesFailClosed(@TempDir Path dir) throws Exception {
|
||||
Path absent = dir.resolve("member-credentials-ssh-agent-absent.yaml");
|
||||
Files.writeString(absent, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
""");
|
||||
assertEquals("omit", FleetConfig.load(absent).memberCredentials().sshAgentEnv());
|
||||
|
||||
Path unknown = dir.resolve("member-credentials-ssh-agent-unknown.yaml");
|
||||
Files.writeString(unknown, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
sshAgentEnv: inhert
|
||||
""");
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(unknown).memberCredentials();
|
||||
assertEquals("omit", credentials.sshAgentEnv());
|
||||
assertFalse(credentials.sshAgentEnvInherited(), "an unknown value must fail closed");
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+3
-3
@@ -170,10 +170,10 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
/**
|
||||
* {@code SSH_AUTH_SOCK} is a live ssh-agent handle, not a value — it must stay blocked under
|
||||
* {@code allow-list} even when the operator lists it under {@code allow:}, because {@code
|
||||
* sshAuthSock} defaults to blocked. Governed ONLY by {@code memberCredentials.sshAuthSock}.
|
||||
* sshAgentEnv} defaults to omit. Governed ONLY by {@code memberCredentials.sshAgentEnv}.
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockStaysBlockedEvenWhenListedInMemberCredentialsAllow() {
|
||||
void sshAgentEnvStaysOmittedEvenWhenListedInMemberCredentialsAllow() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
WiringLauncher launcher = new WiringLauncher(herdr,
|
||||
allowListWithAllow(List.of("SSH_AUTH_SOCK")));
|
||||
@@ -184,7 +184,7 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
String scrub = readAll(dir.resolve(EnvAllowListScrub.SCRUB_FILE));
|
||||
assertFalse(scrub.contains("'SSH_AUTH_SOCK'"),
|
||||
"SSH_AUTH_SOCK must not be on the derived allow-list just because the operator put "
|
||||
+ "it under allow: — sshAuthSock is unset here, so it defaults to block");
|
||||
+ "it under allow: — sshAgentEnv is unset here, so it defaults to omit");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -109,11 +109,11 @@ class MemberEnvAllowListTest {
|
||||
/**
|
||||
* {@code SSH_AUTH_SOCK} is a live handle to the operator's ssh-agent, never a value — so it must
|
||||
* stay excluded from the derived set even when the operator lists it under {@code allow:} for an
|
||||
* unrelated reason. It is governed ONLY by {@code memberCredentials.sshAuthSock}, applied
|
||||
* unrelated reason. It is governed ONLY by {@code memberCredentials.sshAgentEnv}, applied
|
||||
* separately by the caller ({@code HerdrPeerLauncher}).
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockInMemberCredentialsAllowIsStillExcluded() {
|
||||
void sshAgentEnvInMemberCredentialsAllowIsStillExcluded() {
|
||||
Set<String> derived = MemberEnvAllowList.derive(List.of(), Set.of("SSH_AUTH_SOCK", "OTHER_NAME"));
|
||||
|
||||
assertFalse(derived.contains("SSH_AUTH_SOCK"),
|
||||
|
||||
Reference in New Issue
Block a user