mirror
/
wg-easy
mirror of https://github.com/wg-easy/wg-easy
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

fix: create firewall rules without comments when xt_comment is unavailable 

Enabling per-client firewall filtering on hosts that ship iptables
without the xt_comment kernel module (e.g. Synology DSM 7.3) failed the
whole `POST /api/admin/interface` request. Rules are built with
`-m comment --comment "..."`, and when the kernel module is missing
iptables rejects the rule at insertion ("Extension comment revision 0
not supported, missing kernel module?"), aborting the entire rebuild.

Probe once whether iptables/ip6tables can actually insert a rule using
the comment module (the userspace extension can be present while the
kernel module is not, so `--help` is insufficient) and cache the result.
When comments are unsupported, build the rules without `-m comment` so
filtering still works, only losing the cosmetic client label. This
mirrors the existing graceful degradation when iptables is entirely
unavailable.

The rule-building logic is extracted into a pure `buildClientRuleCommands`
helper and covered by unit tests, including a regression test asserting
no rule references the comment module when it is unsupported.

Fixes #2545

Co-Authored-By: Claude Fable 5 <[email protected]>
pull/2688/head
TowyTowy 12 hours ago
parent
78b2838498
commit
59431b57e8
2 changed files with 214 additions and 29 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 161
      src/server/utils/firewall.ts
  2. 82
      src/test/unit/firewall.spec.ts

161
src/server/utils/firewall.ts
View File

@ -16,6 +16,11 @@ let rebuildQueued = false;
// Cache iptables availability check result
let iptablesAvailable: boolean | null = null;
// Cache whether iptables supports the comment module (xt_comment kernel module).
// Some systems (e.g. Synology DSM) ship iptables without the xt_comment kernel
// module, which makes `-m comment` fail at rule insertion. See #2545.
let commentSupported: boolean | null = null;
type ParsedEntry = {
ip: string;
port?: number;
@ -160,6 +165,81 @@ function generateRuleArgs(
return rules;
}
/**
* Probe whether the given iptables binary can actually insert a rule that uses
* the comment module. The userspace extension (libxt_comment) may be present
* while the xt_comment kernel module is missing, in which case parsing succeeds
* but rule insertion fails. We therefore probe by inserting a throwaway rule
* into a dedicated probe chain rather than relying on `--help`.
*/
async function probeComment(bin: 'iptables' | 'ip6tables'): Promise<boolean> {
const probeChain = `${CHAIN_NAME}_PROBE`;
try {
await exec(
`${bin} -N ${probeChain} 2>/dev/null || ${bin} -F ${probeChain}`
);
await exec(
`${bin} -A ${probeChain} -m comment --comment "wg-easy comment probe" -j RETURN`
);
return true;
} catch {
return false;
} finally {
await exec(`${bin} -F ${probeChain} 2>/dev/null || true`);
await exec(`${bin} -X ${probeChain} 2>/dev/null || true`);
}
}
/**
* Build the list of iptables/ip6tables commands needed to allow a client's
* traffic. Pure function (no side effects) so it can be unit tested.
*
* When `useComments` is false the `-m comment` argument is omitted entirely,
* which keeps rule creation working on hosts without the xt_comment kernel
* module (see #2545).
*/
function buildClientRuleCommands(
client: FirewallClient,
defaultAllowedIps: string[],
enableIpv6: boolean,
useComments: boolean
): string[] {
// Determine which IPs to use for firewall rules
// Priority: firewallIps > allowedIps > defaultAllowedIps
const effectiveIps =
client.firewallIps && client.firewallIps.length > 0
? client.firewallIps
: (client.allowedIps ?? defaultAllowedIps);
const comment = useComments
? sanitizeComment(client.id, client.name)
: undefined;
const commands: string[] = [];
for (const ipEntry of effectiveIps) {
const parsed = parseFirewallEntry(ipEntry);
const baseIp = parsed.ip.split('/')[0] ?? parsed.ip; // Handle CIDR by checking base IP
const destIsIpv6 = isIPv6(baseIp);
if (destIsIpv6) {
if (enableIpv6) {
const rules = generateRuleArgs(client.ipv6Address, parsed, comment);
for (const rule of rules) {
commands.push(`ip6tables ${rule}`);
}
}
} else {
const rules = generateRuleArgs(client.ipv4Address, parsed, comment);
for (const rule of rules) {
commands.push(`iptables ${rule}`);
}
}
}
return commands;
}
export const firewall = {
/**
* Initialize the custom chain if it doesn't exist
@ -198,39 +278,22 @@ export const firewall = {
async applyClientRules(
client: FirewallClient,
defaultAllowedIps: string[],
enableIpv6: boolean
enableIpv6: boolean,
useComments: boolean = true
): Promise<void> {
// Determine which IPs to use for firewall rules
// Priority: firewallIps > allowedIps > defaultAllowedIps
const effectiveIps =
client.firewallIps && client.firewallIps.length > 0
? client.firewallIps
: (client.allowedIps ?? defaultAllowedIps);
const commands = buildClientRuleCommands(
client,
defaultAllowedIps,
enableIpv6,
useComments
);
FW_DEBUG(
`Applying firewall rules for client ${client.name} (${client.id}): ${effectiveIps.join(', ')}`
`Applying ${commands.length} firewall rule(s) for client ${client.name} (${client.id})`
);
const comment = sanitizeComment(client.id, client.name);
for (const ipEntry of effectiveIps) {
const parsed = parseFirewallEntry(ipEntry);
const baseIp = parsed.ip.split('/')[0] ?? parsed.ip; // Handle CIDR by checking base IP
const destIsIpv6 = isIPv6(baseIp);
if (destIsIpv6) {
if (enableIpv6) {
const rules = generateRuleArgs(client.ipv6Address, parsed, comment);
for (const rule of rules) {
await exec(`ip6tables ${rule}`);
}
}
} else {
const rules = generateRuleArgs(client.ipv4Address, parsed, comment);
for (const rule of rules) {
await exec(`iptables ${rule}`);
}
}
for (const command of commands) {
await exec(command);
}
},
@ -267,13 +330,18 @@ export const firewall = {
// Flush existing rules
await this.flushChain();
// Detect once whether the comment module is usable. Without it, adding
// rules with `-m comment` fails and would abort the whole rebuild (#2545).
const useComments = await this.supportsComment(enableIpv6);
// Apply rules for each enabled client
for (const client of clients) {
if (!client.enabled) continue;
await this.applyClientRules(
client,
userConfig.defaultAllowedIps,
enableIpv6
enableIpv6,
useComments
);
}
@ -349,11 +417,45 @@ export const firewall = {
}
},
/**
* Check whether iptables (and ip6tables, when IPv6 is enabled) can insert
* rules using the comment module. The result is cached.
*
* Some systems ship iptables without the xt_comment kernel module. Adding a
* rule with `-m comment` then fails, which previously aborted the whole
* firewall rebuild and made enabling the feature impossible (#2545). When
* comments are unsupported we fall back to creating rules without them.
*
* @param enableIpv6 - If true, also require ip6tables to support comments.
*/
async supportsComment(enableIpv6: boolean = true): Promise<boolean> {
// Return cached result if we've already checked
if (commentSupported !== null) {
return commentSupported;
}
const ipv4Supported = await probeComment('iptables');
const ipv6Supported = enableIpv6 ? await probeComment('ip6tables') : true;
commentSupported = ipv4Supported && ipv6Supported;
if (commentSupported) {
FW_DEBUG('iptables comment module is available');
} else {
FW_DEBUG(
'iptables comment module (xt_comment) is not available, creating firewall rules without comments'
);
}
return commentSupported;
},
/**
* Clear the availability cache to force a re-check
*/
clearAvailabilityCache(): void {
iptablesAvailable = null;
commentSupported = null;
},
};
@ -361,4 +463,5 @@ export const firewallTestExports = {
parseFirewallEntry,
generateRuleArgs,
sanitizeComment,
buildClientRuleCommands,
};

82
src/test/unit/firewall.spec.ts
View File

@ -292,4 +292,86 @@ describe('firewall', () => {
]);
});
});
describe('buildClientRuleCommands', () => {
const baseClient = {
id: 1,
name: 'Test',
ipv4Address: '10.8.0.2',
ipv6Address: 'fdcc:ad94:bacf:61a4::2',
allowedIps: null as string[] | null,
firewallIps: null as string[] | null,
enabled: true,
};
test('includes comments when the comment module is supported', () => {
const commands = firewallTestExports.buildClientRuleCommands(
{ ...baseClient, firewallIps: ['10.0.0.1'] },
['0.0.0.0/0', '::/0'],
true,
true
);
expect(commands).toEqual([
'iptables -A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -m comment --comment "client 1: Test" -j ACCEPT',
]);
});
// Regression test for #2545: hosts without the xt_comment kernel module
// (e.g. Synology DSM) reject any rule using `-m comment`, which previously
// aborted the whole firewall rebuild. When comments are unsupported no rule
// may reference the comment module.
test('omits comments when xt_comment is unavailable (#2545)', () => {
const commands = firewallTestExports.buildClientRuleCommands(
{ ...baseClient, firewallIps: ['10.0.0.1'] },
['0.0.0.0/0', '::/0'],
true,
false
);
expect(commands).toEqual([
'iptables -A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -j ACCEPT',
]);
for (const command of commands) {
expect(command).not.toContain('-m comment');
}
});
test('routes IPv6 destinations to ip6tables and skips them when IPv6 is disabled', () => {
const client = {
...baseClient,
firewallIps: ['10.0.0.1', '2001:db8::1'],
};
expect(
firewallTestExports.buildClientRuleCommands(client, [], true, false)
).toEqual([
'iptables -A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -j ACCEPT',
'ip6tables -A WG_CLIENTS -s fdcc:ad94:bacf:61a4::2 -d 2001:db8::1 -j ACCEPT',
]);
expect(
firewallTestExports.buildClientRuleCommands(client, [], false, false)
).toEqual(['iptables -A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -j ACCEPT']);
});
test('falls back to allowedIps then defaultAllowedIps', () => {
expect(
firewallTestExports.buildClientRuleCommands(
{ ...baseClient, allowedIps: ['10.1.0.0/24'] },
['0.0.0.0/0'],
false,
false
)
).toEqual([
'iptables -A WG_CLIENTS -s 10.8.0.2 -d 10.1.0.0/24 -j ACCEPT',
]);
expect(
firewallTestExports.buildClientRuleCommands(
baseClient,
['0.0.0.0/0'],
false,
false
)
).toEqual(['iptables -A WG_CLIENTS -s 10.8.0.2 -d 0.0.0.0/0 -j ACCEPT']);
});
});
});

Write Preview
Loading…
Cancel
Save