ExternalVendorCode/balena-supervisor
1
0
Fork 0
mirror of https://github.com/balena-os/balena-supervisor.git synced 2024-12-19 05:37:53 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Replace BALENA-FIREWALL rule in INPUT chain instead of flushing

Browse Source 
The issue with the original Supervisor implementation of the firewall is that
on Supervisor start, the Supervisor flushes the INPUT chain of the filter table.
This doesn't play well with services that add to the INPUT chain on startup that
may start up before the Supervisor, such as certain NetworkManager connection
profiles. This change only replaces the BALENA-FIREWALL rule in the INPUT chain,
preserving the other rules as well as their order.

Closes: #1482
Change-type: patch
Signed-off-by: Christina Ying Wang <christina@balena.io>
This commit is contained in:
Christina Ying Wang 2023-02-28 12:30:57 -08:00
parent 935a4fba59
commit 84a9e7e9ac
4 changed files with 153 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/lib/firewall.ts
View File

@ -170,6 +170,18 @@ export async function applyFirewallMode(mode: string) {
}
: [];
// Get position of BALENA-FIREWALL rule in the INPUT chain for both iptables & ip6tables
const v4Position = await iptables.getRulePosition(
'INPUT',
'BALENA-FIREWALL',
4,
);
const v6Position = await iptables.getRulePosition(
'INPUT',
'BALENA-FIREWALL',
6,
);
// get an adaptor to manipulate iptables rules...
const ruleAdaptor = iptables.getDefaultRuleAdaptor();
@ -178,6 +190,37 @@ export async function applyFirewallMode(mode: string) {
.build()
.forTable('filter', (filter) =>
filter
.forChain('INPUT', (chain) => {
// Delete & insert to v4 tables
if (v4Position !== -1) {
chain.addRule({
action: iptables.RuleAction.Delete,
target: 'BALENA-FIREWALL',
family: 4,
});
}
chain.addRule({
action: iptables.RuleAction.Insert,
id: v4Position > 0 ? v4Position : 1,
target: 'BALENA-FIREWALL',
family: 4,
});
// Delete & insert to v6 tables
if (v6Position !== -1) {
chain.addRule({
action: iptables.RuleAction.Delete,
target: 'BALENA-FIREWALL',
family: 6,
});
}
chain.addRule({
action: iptables.RuleAction.Insert,
id: v6Position > 0 ? v6Position : 1,
target: 'BALENA-FIREWALL',
family: 6,
});
return chain;
})
.forChain(BALENA_FIREWALL_CHAIN, (chain) =>
chain
.addRule(prepareChain)
@ -190,16 +233,6 @@ export async function applyFirewallMode(mode: string) {
action: iptables.RuleAction.Append,
target: 'REJECT',
}),
)
.forChain('INPUT', (chain) =>
chain
.addRule({
action: iptables.RuleAction.Flush,
})
.addRule({
action: iptables.RuleAction.Append,
target: 'BALENA-FIREWALL',
}),
),
)
.apply(ruleAdaptor);

43
src/lib/iptables.ts
View File

@ -3,6 +3,9 @@ import { spawn } from 'child_process';
import { Readable } from 'stream';
import { TypedError } from 'typed-error';
import { exec } from './fs-utils';
import log from './supervisor-console';
export class IPTablesRuleError extends TypedError {
public constructor(err: string | Error, public ruleset: string) {
super(err);
@ -13,6 +16,7 @@ export enum RuleAction {
Insert = '-I',
Append = '-A',
Flush = '-F',
Delete = '-D',
}
export interface Rule {
id?: number;
@ -96,6 +100,14 @@ export function convertToRestoreRulesFormat(rules: Rule[]): string {
}
if (rule.chain) {
args.push(rule.chain);
// Optionally push a rule to a specific position in the chain
if (
(rule.action === RuleAction.Insert ||
rule.action === RuleAction.Delete) &&
rule.id
) {
args.push(rule.id?.toString() ?? '1');
}
}
if (rule.proto) {
args.push(`-p ${rule.proto}`);
@ -291,3 +303,34 @@ async function applyRules(rules: Rule | Rule[], adaptor: RuleAdaptor) {
await adaptor(processedRules);
}
class StdError extends TypedError {}
export async function getRulePosition(
chain: string,
ruleMatch: string,
family: 4 | 6,
): Promise<number> {
const cmd = `ip${
family === 6 ? '6' : ''
}tables -L ${chain} -n --line-numbers`;
try {
const { stdout, stderr } = await exec(cmd);
if (stderr) {
throw new StdError(stderr);
}
const line = stdout.split('\n').find((l) => l.includes(ruleMatch));
if (!line) {
return -1;
}
return parseInt(line.split(' ')[0], 10);
} catch (e: unknown) {
log.error(
`Received ${
e instanceof StdError ? 'stderr' : 'error'
} querying iptables ${chain} chain:`,
(e as Error).message ?? e ?? 'Unknown error',
);
return -1;
}
}

68
test/integration/lib/firewall.spec.ts
View File

@ -10,6 +10,7 @@ import * as iptablesMock from '~/test-lib/mocked-iptables';
import * as dbFormat from '~/src/device-state/db-format';
import constants = require('~/lib/constants');
import * as iptables from '~/lib/iptables';
import { RuleAction, Rule } from '~/lib/iptables';
import { log } from '~/lib/supervisor-console';
@ -86,7 +87,7 @@ describe('lib/firewall', function () {
// expect that we jump to the firewall chain...
expectRule({
action: RuleAction.Append,
action: RuleAction.Insert,
target: 'BALENA-FIREWALL',
chain: 'INPUT',
family: 4,
@ -122,7 +123,7 @@ describe('lib/firewall', function () {
// expect that we jump to the firewall chain...
expectRule({
action: RuleAction.Append,
action: RuleAction.Insert,
target: 'BALENA-FIREWALL',
chain: 'INPUT',
family: 4,
@ -179,7 +180,7 @@ describe('lib/firewall', function () {
// expect that we jump to the firewall chain...
expectRule({
action: RuleAction.Append,
action: RuleAction.Insert,
target: 'BALENA-FIREWALL',
chain: 'INPUT',
family: 4,
@ -238,7 +239,7 @@ describe('lib/firewall', function () {
// expect that we jump to the firewall chain...
expectRule({
action: RuleAction.Append,
action: RuleAction.Insert,
target: 'BALENA-FIREWALL',
chain: 'INPUT',
family: 4,
@ -269,6 +270,65 @@ describe('lib/firewall', function () {
expect(loggerSpy.called).to.be.true;
}, iptablesMock.realRuleAdaptor);
});
// TODO: The way that iptables & mocked-iptables are implemented does not
// translate to well-written, side-effect free tests. While this test passes,
// it's not able to check that BALENA-FIREWALL was inserted at the correct
// position in the INPUT rule chain.
// We should refactor the test setup for iptables to write rules to a file,
// and test whether that file can be applied successfully with `iptables-restore --test`.
it('should only replace the BALENA-FIREWALL rule from INPUT chain', async () => {
const acceptRuleOnPort = (port: number) => ({
action: RuleAction.Append,
proto: 'tcp',
matches: [`--dport ${port}`],
target: 'ACCEPT',
});
await iptablesMock.whilstMocked(
async ({ hasAppliedRules, expectRule }) => {
// clear the spies...
loggerSpy.resetHistory();
logSpy.resetHistory();
// Add some rules to INPUT: 3 unrelated rules,
// and BALENA-FIREWALL at position 4.
await iptables
.build()
.forTable('filter', (filter) =>
filter.forChain('INPUT', (chain) =>
chain
.addRule(acceptRuleOnPort(80))
.addRule(acceptRuleOnPort(443))
.addRule(acceptRuleOnPort(22))
.addRule({
action: RuleAction.Append,
target: 'BALENA-FIREWALL',
chain: 'INPUT',
}),
),
)
.apply(iptablesMock.fakeRuleAdaptor);
// set the firewall to be in off mode...
await config.set({ firewallMode: 'off' });
await hasAppliedRules;
// Unrelated rules should not have been flushed
expectRule(acceptRuleOnPort(80));
expectRule(acceptRuleOnPort(443));
expectRule(acceptRuleOnPort(22));
// TODO: this should be testable but isn't because of the reason above.
// expectRule({
// action: RuleAction.Insert,
// target: 'BALENA-FIREWALL',
// chain: 'INPUT',
// id: 4,
// });
},
iptablesMock.fakeRuleAdaptor,
);
});
});
describe('Service rules', () => {

6
test/lib/mocked-iptables.ts
View File

@ -107,7 +107,7 @@ class FakeRuleAdaptor {
export const realRuleAdaptor = iptables.getDefaultRuleAdaptor();
const fakeRuleAdaptorManager = new FakeRuleAdaptor();
const fakeRuleAdaptor = fakeRuleAdaptorManager.getRuleAdaptor();
export const fakeRuleAdaptor = fakeRuleAdaptorManager.getRuleAdaptor();
// @ts-expect-error Assigning to a RO property
iptables.getDefaultRuleAdaptor = () => {
@ -121,11 +121,11 @@ export interface MockedState {
clearHistory: () => void;
}
export type MockedConext = (state: MockedState) => Promise<any>;
export type MockedContext = (state: MockedState) => Promise<any>;
const applyFirewallRules = firewall.applyFirewallMode;
export const whilstMocked = async (
context: MockedConext,
context: MockedContext,
ruleAdaptor: iptables.RuleAdaptor = fakeRuleAdaptor,
) => {
const getOriginalDefaultRuleAdaptor = iptables.getDefaultRuleAdaptor;