Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion agent-computer/src/egress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions agent-computer/tests/egress-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down