diff --git a/CHANGELOG.md b/CHANGELOG.md index 888608113..a973b2caf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A malformed IP range in an egress rule is refused instead of widening the rule + +An egress `cidr` rule written `10.0.0.5/` was stored as `10.0.0.5/0`, which is every IPv4 +address, so a typo for one host opened an allow-list to all of them. `/0x8` and `10.0.0.0/8/9` were +accepted the same way. A zone id such as `fe80::1%eth0` was accepted too, and then threw from the +filter on the first connection that policy judged. Each is now refused when the rule is saved, with +the sentence a malformed range already got. A rule like this saved earlier matches nothing. + **Before upgrading.** Four things change for an existing deployment: - Automatic Learning is on unless an administrator saved it off. It does nothing until a Learning container is assigned; see below. diff --git a/agent-computer/src/egress.ts b/agent-computer/src/egress.ts index 7fff9bc3c..14a961a26 100644 --- a/agent-computer/src/egress.ts +++ b/agent-computer/src/egress.ts @@ -238,7 +238,11 @@ function normalizeHost(host: string): string { function parseCidr( value: string, ): { address: string; prefix: number; family: "ipv4" | "ipv6" } | null { - const [address = "", prefixText] = value.trim().split("/"); + const [address = "", prefixText, ...rest] = value.trim().split("/"); + // `Number` reads "" as 0 and takes "0x8", so `10.0.0.5/` would allow every address. A zone id + // passes `isIP` but `BlockList` throws on it when the rule is first used. + if (rest.length > 0 || address.includes("%")) return null; + if (prefixText !== undefined && !/^\d{1,3}$/.test(prefixText)) return null; const version = isIP(address); if (version === 0) return null; const max = version === 4 ? 32 : 128; diff --git a/agent-computer/tests/egress-policy.test.ts b/agent-computer/tests/egress-policy.test.ts index ed3c75c04..d5baa4bcb 100644 --- a/agent-computer/tests/egress-policy.test.ts +++ b/agent-computer/tests/egress-policy.test.ts @@ -102,6 +102,17 @@ describe("the network policy rules", () => { .ok, ).toBe(false); expect(parseEgressPolicy({ mode: "sometimes", rules: [] }).ok).toBe(false); + // Each of these once read as some other range: "" and "0x0" as /0, which is every address. + for (const value of [ + "10.0.0.5/", + "10.0.0.0/0x8", + "10.0.0.0/8/9", + "10.0.0.0/ 8", + "fe80::1%eth0", + "fe80::%eth0/64", + ]) { + expect(parseEgressRules([{ type: "cidr", value }]).ok).toBe(false); + } expect( parseEgressRules([ { type: "domain", value: "Example.COM" },