diff --git a/src/server/utils/firewall.ts b/src/server/utils/firewall.ts index 13d14cc5..02b4c4ec 100644 --- a/src/server/utils/firewall.ts +++ b/src/server/utils/firewall.ts @@ -21,6 +21,18 @@ type ParsedEntry = { proto?: 'tcp' | 'udp' | 'both'; }; +/** + * Sanitize a client identifier for use in an iptables comment. + * Strips all characters except ASCII alphanumeric, space, underscore, hyphen, and dot. + * Combines with client ID for a safe, identifiable comment. + * Truncates to 256 bytes (iptables comment module limit). + */ +function sanitizeComment(clientId: number, clientName: string): string { + const safe = clientName.replace(/[^a-zA-Z0-9 _.-]/g, ''); + const comment = `client ${clientId}: ${safe}`; + return comment.slice(0, 256); +} + /** * Parse a firewall entry string into its components. * Supports formats: @@ -109,22 +121,28 @@ function parseFirewallEntry(entry: string): ParsedEntry { function generateRuleArgs( clientIp: string, entry: ParsedEntry, + comment?: string, action: 'A' | 'D' = 'A' ): string[] { const rules: string[] = []; + const commentArg = comment ? ` -m comment --comment "${comment}"` : ''; const baseArgs = `-${action} ${CHAIN_NAME} -s ${clientIp} -d ${entry.ip}`; if (entry.port) { // Port-specific rules if (entry.proto === 'tcp' || entry.proto === 'both') { - rules.push(`${baseArgs} -p tcp --dport ${entry.port} -j ACCEPT`); + rules.push( + `${baseArgs} -p tcp --dport ${entry.port}${commentArg} -j ACCEPT` + ); } if (entry.proto === 'udp' || entry.proto === 'both') { - rules.push(`${baseArgs} -p udp --dport ${entry.port} -j ACCEPT`); + rules.push( + `${baseArgs} -p udp --dport ${entry.port}${commentArg} -j ACCEPT` + ); } } else { // No port - allow all traffic to destination - rules.push(`${baseArgs} -j ACCEPT`); + rules.push(`${baseArgs}${commentArg} -j ACCEPT`); } return rules; @@ -181,19 +199,21 @@ export const firewall = { `Applying firewall rules for client ${client.name} (${client.id}): ${effectiveIps.join(', ')}` ); + const comment = sanitizeComment(client.id, client.name); + for (const ipEntry of effectiveIps) { const parsed = parseFirewallEntry(ipEntry); const destIsIpv6 = isIPv6(parsed.ip.split('/')[0]); // Handle CIDR by checking base IP if (destIsIpv6) { if (enableIpv6) { - const rules = generateRuleArgs(client.ipv6Address, parsed); + const rules = generateRuleArgs(client.ipv6Address, parsed, comment); for (const rule of rules) { await exec(`ip6tables ${rule}`); } } } else { - const rules = generateRuleArgs(client.ipv4Address, parsed); + const rules = generateRuleArgs(client.ipv4Address, parsed, comment); for (const rule of rules) { await exec(`iptables ${rule}`); } @@ -324,7 +344,8 @@ export const firewall = { }, }; -export const testExports = { +export const firewallTestExports = { parseFirewallEntry, generateRuleArgs, + sanitizeComment, }; diff --git a/src/server/utils/types.ts b/src/server/utils/types.ts index 886d1753..029637a0 100644 --- a/src/server/utils/types.ts +++ b/src/server/utils/types.ts @@ -259,4 +259,4 @@ export function assertUnreachable(_: never): never { throw new Error("Didn't expect to get here"); } -export const testExports = { FirewallIpEntrySchema }; +export const typesTestExports = { FirewallIpEntrySchema }; diff --git a/src/test/unit/firewall.spec.ts b/src/test/unit/firewall.spec.ts index fb9775ab..00fd76a7 100644 --- a/src/test/unit/firewall.spec.ts +++ b/src/test/unit/firewall.spec.ts @@ -1,103 +1,107 @@ import { describe, expect, test } from 'vitest'; -import { testExports } from '../../server/utils/firewall'; -import { testExports as typesTextExports } from '../../server/utils/types'; +import { firewallTestExports } from '../../server/utils/firewall'; +import { typesTestExports } from '../../server/utils/types'; describe('firewall', () => { describe('isValidFirewallEntry', () => { test('invalid ips', () => { - expect(() => typesTextExports.FirewallIpEntrySchema.parse('')).toThrow(); + expect(() => typesTestExports.FirewallIpEntrySchema.parse('')).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('255.255.255.256') + typesTestExports.FirewallIpEntrySchema.parse('255.255.255.256') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.256') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.256') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1.1') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1.1') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('[]:443/udp') + typesTestExports.FirewallIpEntrySchema.parse('[]:443/udp') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('[]:443') + typesTestExports.FirewallIpEntrySchema.parse('[]:443') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('[::1]/32') + typesTestExports.FirewallIpEntrySchema.parse('[::1]/32') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('[1.1.1.1]/32') + typesTestExports.FirewallIpEntrySchema.parse('[1.1.1.1]/32') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('[::g]/32') + typesTestExports.FirewallIpEntrySchema.parse('[::g]/32') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('2001:dbx::1') + typesTestExports.FirewallIpEntrySchema.parse('2001:dbx::1') ).toThrow(); }); test('invalid port, protocol or cidr', () => { expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1:80/tcpp') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1:80/tcpp') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1:65536') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1:65536') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1:0') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1:0') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1/33') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1/33') ).toThrow(); expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1/32:0') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1/32:0') ).toThrow(); }); test('protocol without port', () => { expect(() => - typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1/tcp') + typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1/tcp') ).toThrow(); }); test('valid entries', () => { - expect(typesTextExports.FirewallIpEntrySchema.parse('1.1.1.1')).toBe( + expect(typesTestExports.FirewallIpEntrySchema.parse('1.1.1.1')).toBe( '1.1.1.1' ); - expect(typesTextExports.FirewallIpEntrySchema.parse('::/0')).toBe('::/0'); - expect(typesTextExports.FirewallIpEntrySchema.parse('::0/0')).toBe( + expect(typesTestExports.FirewallIpEntrySchema.parse('::/0')).toBe('::/0'); + expect(typesTestExports.FirewallIpEntrySchema.parse('::0/0')).toBe( '::0/0' ); - expect(typesTextExports.FirewallIpEntrySchema.parse('2001:db8::1')).toBe( + expect(typesTestExports.FirewallIpEntrySchema.parse('2001:db8::1')).toBe( '2001:db8::1' ); - expect(typesTextExports.FirewallIpEntrySchema.parse('::1')).toBe('::1'); + expect(typesTestExports.FirewallIpEntrySchema.parse('::1')).toBe('::1'); expect( - typesTextExports.FirewallIpEntrySchema.parse('2001:db8::1/32') + typesTestExports.FirewallIpEntrySchema.parse('2001:db8::1/32') ).toBe('2001:db8::1/32'); - expect(typesTextExports.FirewallIpEntrySchema.parse('[::1]')).toBe( + expect(typesTestExports.FirewallIpEntrySchema.parse('[::1]')).toBe( '[::1]' ); - expect(testExports.parseFirewallEntry('[::1]')).toEqual({ + expect(firewallTestExports.parseFirewallEntry('[::1]')).toEqual({ ip: '::1', }); }); }); describe('parseFirewallEntry', () => { test('IPv4 with port and protocol', () => { - expect(() => testExports.parseFirewallEntry('1.1.1.1/tcp')).toThrow(); - expect(() => testExports.parseFirewallEntry('1.1.1.1/udp')).toThrow(); - expect(testExports.parseFirewallEntry('1.1.1.1')).toEqual({ + expect(() => + firewallTestExports.parseFirewallEntry('1.1.1.1/tcp') + ).toThrow(); + expect(() => + firewallTestExports.parseFirewallEntry('1.1.1.1/udp') + ).toThrow(); + expect(firewallTestExports.parseFirewallEntry('1.1.1.1')).toEqual({ ip: '1.1.1.1', }); - expect(testExports.parseFirewallEntry('1.1.1.1:80')).toEqual({ + expect(firewallTestExports.parseFirewallEntry('1.1.1.1:80')).toEqual({ ip: '1.1.1.1', port: 80, proto: 'both', }); - expect(testExports.parseFirewallEntry('1.1.1.1:80/tcp')).toEqual({ + expect(firewallTestExports.parseFirewallEntry('1.1.1.1:80/tcp')).toEqual({ ip: '1.1.1.1', port: 80, proto: 'tcp', }); - expect(testExports.parseFirewallEntry('1.1.1.1:80/udp')).toEqual({ + expect(firewallTestExports.parseFirewallEntry('1.1.1.1:80/udp')).toEqual({ ip: '1.1.1.1', port: 80, proto: 'udp', @@ -106,34 +110,102 @@ describe('firewall', () => { test('IPv6 with port and protocol', () => { expect(() => - testExports.parseFirewallEntry('[2001:db8::1]/tcp') + firewallTestExports.parseFirewallEntry('[2001:db8::1]/tcp') ).toThrow(); expect(() => - testExports.parseFirewallEntry('[2001:db8::1]/udp') + firewallTestExports.parseFirewallEntry('[2001:db8::1]/udp') ).toThrow(); - expect(testExports.parseFirewallEntry('[2001:db8::1]')).toEqual({ + expect(firewallTestExports.parseFirewallEntry('[2001:db8::1]')).toEqual({ ip: '2001:db8::1', }); - expect(testExports.parseFirewallEntry('[2001:db8::1]:443')).toEqual({ + expect( + firewallTestExports.parseFirewallEntry('[2001:db8::1]:443') + ).toEqual({ ip: '2001:db8::1', port: 443, proto: 'both', }); - expect(testExports.parseFirewallEntry('[2001:db8::1]:443/tcp')).toEqual({ + expect( + firewallTestExports.parseFirewallEntry('[2001:db8::1]:443/tcp') + ).toEqual({ ip: '2001:db8::1', port: 443, proto: 'tcp', }); - expect(testExports.parseFirewallEntry('[2001:db8::1]:443/udp')).toEqual({ + expect( + firewallTestExports.parseFirewallEntry('[2001:db8::1]:443/udp') + ).toEqual({ ip: '2001:db8::1', port: 443, proto: 'udp', }); - expect(testExports.parseFirewallEntry('::0/0')).toEqual({ + expect(firewallTestExports.parseFirewallEntry('::0/0')).toEqual({ ip: '::0/0', }); - expect(() => testExports.parseFirewallEntry('::0/0/tcp')).toThrow(); + expect(() => + firewallTestExports.parseFirewallEntry('::0/0/tcp') + ).toThrow(); + }); + }); + describe('sanitizeComment', () => { + test('basic ASCII name', () => { + expect(firewallTestExports.sanitizeComment(1, 'My Laptop')).toBe( + 'client 1: My Laptop' + ); + }); + test('strips non-ASCII and shell metacharacters', () => { + expect(firewallTestExports.sanitizeComment(42, 'café')).toBe( + 'client 42: caf' + ); + expect(firewallTestExports.sanitizeComment(5, 'a"; rm -rf /')).toBe( + 'client 5: a rm -rf ' + ); + expect(firewallTestExports.sanitizeComment(7, 'test$(cmd)`id`')).toBe( + 'client 7: testcmdid' + ); + }); + test('preserves allowed punctuation', () => { + expect(firewallTestExports.sanitizeComment(3, 'phone-2.lan_home')).toBe( + 'client 3: phone-2.lan_home' + ); + }); + test('truncates to 256 bytes', () => { + const longName = 'a'.repeat(300); + const result = firewallTestExports.sanitizeComment(1, longName); + expect(result.length).toBeLessThanOrEqual(256); + expect(result).toBe('client 1: ' + 'a'.repeat(246)); + }); + }); + describe('generateRuleArgs', () => { + test('includes comment when provided', () => { + const rules = firewallTestExports.generateRuleArgs( + '10.8.0.2', + { ip: '10.0.0.1' }, + 'client 1: test' + ); + expect(rules).toEqual([ + '-A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -m comment --comment "client 1: test" -j ACCEPT', + ]); + }); + test('omits comment when not provided', () => { + const rules = firewallTestExports.generateRuleArgs('10.8.0.2', { + ip: '10.0.0.1', + }); + expect(rules).toEqual([ + '-A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -j ACCEPT', + ]); + }); + test('comment with port generates two rules for both proto', () => { + const rules = firewallTestExports.generateRuleArgs( + '10.8.0.2', + { ip: '10.0.0.1', port: 443, proto: 'both' }, + 'client 2: phone' + ); + expect(rules).toEqual([ + '-A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -p tcp --dport 443 -m comment --comment "client 2: phone" -j ACCEPT', + '-A WG_CLIENTS -s 10.8.0.2 -d 10.0.0.1 -p udp --dport 443 -m comment --comment "client 2: phone" -j ACCEPT', + ]); }); }); });