bug: Fix unhandled promise rejection

When invoking iptables-restore it can fail. This wasn't handled
and this makes sure that it fails gracefully.

Change-type: patch
Signed-off-by: Rich Bayliss <rich@balena.io>
This commit is contained in:
Rich Bayliss 2020-07-10 15:02:07 +01:00
parent 56a9d96b67
commit 898c7e71da
No known key found for this signature in database
GPG Key ID: E53C4B4D18499E1A
5 changed files with 158 additions and 65 deletions

View File

@ -4,6 +4,7 @@ import * as config from '../config/index';
import * as constants from './constants'; import * as constants from './constants';
import * as iptables from './iptables'; import * as iptables from './iptables';
import { log } from './supervisor-console'; import { log } from './supervisor-console';
import { logSystemMessage } from '../logger';
import * as dbFormat from '../device-state/db-format'; import * as dbFormat from '../device-state/db-format';
@ -12,7 +13,7 @@ export const initialised = (async () => {
await applyFirewall(); await applyFirewall();
// apply firewall whenever relevant config changes occur... // apply firewall whenever relevant config changes occur...
config.on('change', async ({ firewallMode, localMode }) => { config.on('change', ({ firewallMode, localMode }) => {
if (firewallMode || localMode != null) { if (firewallMode || localMode != null) {
applyFirewall({ firewallMode, localMode }); applyFirewall({ firewallMode, localMode });
} }
@ -145,48 +146,58 @@ export async function applyFirewallMode(mode: string) {
log.info(`${LOG_PREFIX} Applying firewall mode: ${mode}`); log.info(`${LOG_PREFIX} Applying firewall mode: ${mode}`);
// get an adaptor to manipulate iptables rules... try {
const ruleAdaptor = iptables.getDefaultRuleAdaptor(); // are we running services in host-network mode?
const isServicesInHostNetworkMode = await runningHostBoundServices();
// are we running services in host-network mode? // should we allow only traffic to the balena host services?
const isServicesInHostNetworkMode = await runningHostBoundServices(); const returnIfOff: iptables.Rule | iptables.Rule[] =
mode === 'off' || (mode === 'auto' && !isServicesInHostNetworkMode)
? {
comment: `Firewall disabled (${mode})`,
action: iptables.RuleAction.Insert,
target: 'RETURN',
}
: [];
// should we allow only traffic to the balena host services? // get an adaptor to manipulate iptables rules...
const returnIfOff: iptables.Rule | iptables.Rule[] = const ruleAdaptor = iptables.getDefaultRuleAdaptor();
mode === 'off' || (mode === 'auto' && !isServicesInHostNetworkMode)
? {
comment: `Firewall disabled (${mode})`,
action: iptables.RuleAction.Insert,
target: 'RETURN',
}
: [];
// configure the BALENA-FIREWALL chain... // configure the BALENA-FIREWALL chain...
await iptables await iptables
.build() .build()
.forTable('filter', (filter) => .forTable('filter', (filter) =>
filter filter
.forChain(BALENA_FIREWALL_CHAIN, (chain) => .forChain(BALENA_FIREWALL_CHAIN, (chain) =>
chain chain
.addRule(prepareChain) .addRule(prepareChain)
.addRule(supervisorAccessRules) .addRule(supervisorAccessRules)
.addRule(standardServices) .addRule(standardServices)
.addRule(standardPolicy) .addRule(standardPolicy)
.addRule(returnIfOff), .addRule(returnIfOff),
) )
.forChain('INPUT', (chain) => .forChain('INPUT', (chain) =>
chain chain
.addRule({ .addRule({
action: iptables.RuleAction.Flush, action: iptables.RuleAction.Flush,
}) })
.addRule({ .addRule({
action: iptables.RuleAction.Append, action: iptables.RuleAction.Append,
target: 'BALENA-FIREWALL', target: 'BALENA-FIREWALL',
}), }),
), ),
) )
.apply(ruleAdaptor); .apply(ruleAdaptor);
// all done! // all done!
log.success(`${LOG_PREFIX} Firewall mode applied`); log.success(`${LOG_PREFIX} Firewall mode applied`);
} catch (err) {
logSystemMessage(`${LOG_PREFIX} Firewall mode not applied due to error`);
log.error(`${LOG_PREFIX} Firewall mode not applied`);
log.error('Error applying firewall mode', err);
if (err instanceof iptables.IPTablesRuleError) {
log.debug(`Ruleset:\r\n${err.ruleset}`);
}
}
} }

View File

@ -1,8 +1,13 @@
import * as _ from 'lodash'; import * as _ from 'lodash';
import { child_process } from 'mz'; import { child_process } from 'mz';
import { Readable } from 'stream'; import { Readable } from 'stream';
import TypedError = require('typed-error');
export class IPTablesRuleError extends Error {} export class IPTablesRuleError extends TypedError {
public constructor(err: string | Error, public ruleset: string) {
super(err);
}
}
export enum RuleAction { export enum RuleAction {
Insert = '-I', Insert = '-I',
@ -98,10 +103,11 @@ export function convertToRestoreRulesFormat(rules: Rule[]): string {
if (rule.matches) { if (rule.matches) {
rule.matches.forEach((match) => args.push(match)); rule.matches.forEach((match) => args.push(match));
} }
if (rule.comment) { // TODO: Enable this once the support for it can be confirmed...
args.push('-m comment'); // if (rule.comment) {
args.push(`--comment "${rule.comment}"`); // args.push('-m comment');
} // args.push(`--comment "${rule.comment}"`);
// }
if (rule.target) { if (rule.target) {
args.push(`-j ${rule.target}`); args.push(`-j ${rule.target}`);
} }
@ -151,14 +157,14 @@ const iptablesRestoreAdaptor: RuleAdaptor = async (
return; return;
} }
const input = rulesFiles[family]; const ruleset = rulesFiles[family];
const cmd = family === 'v6' ? 'ip6tables-restore' : 'iptables-restore'; const cmd = family === 'v6' ? 'ip6tables-restore' : 'iptables-restore';
await new Promise<string>((resolve, reject) => { await new Promise<string>((resolve, reject) => {
const args = ['--noflush', '--verbose']; const args = ['--noflush', '--verbose'];
// prepare to pipe the rules into iptables-restore... // prepare to pipe the rules into iptables-restore...
const stdinStream = new Readable(); const stdinStream = new Readable();
stdinStream.push(input); stdinStream.push(ruleset);
stdinStream.push(null); stdinStream.push(null);
// run the restore... // run the restore...
@ -185,6 +191,7 @@ const iptablesRestoreAdaptor: RuleAdaptor = async (
return reject( return reject(
new IPTablesRuleError( new IPTablesRuleError(
`Error running iptables: ${stderr.join()} (${args.join(' ')})`, `Error running iptables: ${stderr.join()} (${args.join(' ')})`,
ruleset,
), ),
); );
} }

View File

@ -1,6 +1,6 @@
import { stripIndent } from 'common-tags'; import { stripIndent } from 'common-tags';
import { child_process, fs } from 'mz'; import { child_process, fs } from 'mz';
import { SinonStub, stub, spy } from 'sinon'; import { SinonStub, stub, spy, SinonSpy } from 'sinon';
import { expect } from './lib/chai-config'; import { expect } from './lib/chai-config';
import * as deviceConfig from '../src/device-config'; import * as deviceConfig from '../src/device-config';
@ -14,9 +14,10 @@ const extlinuxBackend = new ExtlinuxConfigBackend();
const rpiConfigBackend = new RPiConfigBackend(); const rpiConfigBackend = new RPiConfigBackend();
describe('Device Backend Config', () => { describe('Device Backend Config', () => {
const logSpy = spy(logger, 'logSystemMessage'); let logSpy: SinonSpy;
before(async () => { before(async () => {
logSpy = spy(logger, 'logSystemMessage');
await prepare(); await prepare();
}); });

View File

@ -1,4 +1,4 @@
import { spy } from 'sinon'; import _ = require('lodash');
import { expect } from 'chai'; import { expect } from 'chai';
import * as Docker from 'dockerode'; import * as Docker from 'dockerode';
@ -7,22 +7,33 @@ import * as sinon from 'sinon';
import * as config from '../src/config'; import * as config from '../src/config';
import * as firewall from '../src/lib/firewall'; import * as firewall from '../src/lib/firewall';
import * as logger from '../src/logger';
import * as iptablesMock from './lib/mocked-iptables'; import * as iptablesMock from './lib/mocked-iptables';
import * as targetStateCache from '../src/device-state/target-state-cache'; import * as targetStateCache from '../src/device-state/target-state-cache';
import constants = require('../src/lib/constants'); import constants = require('../src/lib/constants');
import { RuleAction } from '../src/lib/iptables'; import { RuleAction } from '../src/lib/iptables';
import { log } from '../src/lib/supervisor-console';
describe('Host Firewall', function () { describe('Host Firewall', function () {
const dockerStubs: Dictionary<sinon.SinonStub> = {};
let loggerSpy: sinon.SinonSpy;
let logSpy: sinon.SinonSpy;
let apiEndpoint: string; let apiEndpoint: string;
let listenPort: number; let listenPort: number;
let dockerStub: sinon.SinonStubbedInstance<typeof docker>;
before(async () => { before(async () => {
dockerStub = sinon.stub(docker); // spy the logs...
dockerStub.listContainers.resolves([]); loggerSpy = sinon.spy(logger, 'logSystemMessage');
dockerStub.listImages.resolves([]); logSpy = sinon.spy(log, 'error');
dockerStub.getImage.returns({
// stub the docker calls...
dockerStubs.listContainers = sinon
.stub(docker, 'listContainers')
.resolves([]);
dockerStubs.listImages = sinon.stub(docker, 'listImages').resolves([]);
dockerStubs.getImage = sinon.stub(docker, 'getImage').returns({
id: 'abcde', id: 'abcde',
inspect: async () => { inspect: async () => {
return {}; return {};
@ -37,13 +48,17 @@ describe('Host Firewall', function () {
}); });
after(async () => { after(async () => {
sinon.restore(); for (const stub of _.values(dockerStubs)) {
stub.restore();
}
loggerSpy.restore();
logSpy.restore();
}); });
describe('Basic On/Off operation', () => { describe('Basic On/Off operation', () => {
it('should confirm the `changed` event is handled', async function () { it('should confirm the `changed` event is handled', async function () {
await iptablesMock.whilstMocked(async ({ hasAppliedRules }) => { await iptablesMock.whilstMocked(async ({ hasAppliedRules }) => {
const changedSpy = spy(); const changedSpy = sinon.spy();
config.on('change', changedSpy); config.on('change', changedSpy);
// set the firewall to be in off mode... // set the firewall to be in off mode...
@ -245,6 +260,22 @@ describe('Host Firewall', function () {
}, },
); );
}); });
it('should catch errors when rule changes fail', async () => {
await iptablesMock.whilstMocked(async ({ hasAppliedRules }) => {
// clear the spies...
loggerSpy.resetHistory();
logSpy.resetHistory();
// set the firewall to be in off mode...
await config.set({ firewallMode: 'off' });
await hasAppliedRules;
// should have caught the error and logged it
expect(logSpy.calledWith('Error applying firewall mode')).to.be.true;
expect(loggerSpy.called).to.be.true;
}, iptablesMock.realRuleAdaptor);
});
}); });
describe('Supervisor API access', () => { describe('Supervisor API access', () => {

View File

@ -1,9 +1,12 @@
import _ = require('lodash'); import _ = require('lodash');
import { expect } from 'chai'; import { expect } from 'chai';
import { stub } from 'sinon';
import { child_process } from 'mz';
import * as firewall from '../../src/lib/firewall'; import * as firewall from '../../src/lib/firewall';
import * as iptables from '../../src/lib/iptables'; import * as iptables from '../../src/lib/iptables';
import { EventEmitter } from 'events'; import { EventEmitter } from 'events';
import { Writable } from 'stream';
class FakeRuleAdaptor { class FakeRuleAdaptor {
private rules: iptables.Rule[]; private rules: iptables.Rule[];
@ -80,9 +83,15 @@ class FakeRuleAdaptor {
} }
} }
const fakeRuleAdaptor = new FakeRuleAdaptor(); export const realRuleAdaptor = iptables.getDefaultRuleAdaptor();
const fakeRuleAdaptorManager = new FakeRuleAdaptor();
const fakeRuleAdaptor = fakeRuleAdaptorManager.getRuleAdaptor();
// @ts-expect-error Assigning to a RO property // @ts-expect-error Assigning to a RO property
iptables.getDefaultRuleAdaptor = () => fakeRuleAdaptor.getRuleAdaptor(); iptables.getDefaultRuleAdaptor = () => {
return fakeRuleAdaptor;
};
export interface MockedState { export interface MockedState {
hasAppliedRules: Promise<void>; hasAppliedRules: Promise<void>;
@ -94,8 +103,37 @@ export interface MockedState {
export type MockedConext = (state: MockedState) => Promise<any>; export type MockedConext = (state: MockedState) => Promise<any>;
const applyFirewallRules = firewall.applyFirewallMode; const applyFirewallRules = firewall.applyFirewallMode;
export const whilstMocked = async (context: MockedConext) => { export const whilstMocked = async (
fakeRuleAdaptor.clearHistory(); context: MockedConext,
ruleAdaptor: iptables.RuleAdaptor = fakeRuleAdaptor,
) => {
const getOriginalDefaultRuleAdaptor = iptables.getDefaultRuleAdaptor;
const spawnStub = stub(child_process, 'spawn').callsFake(() => {
const fakeProc = new EventEmitter();
(fakeProc as any).stdout = new EventEmitter();
const stdin = new Writable();
stdin._write = (
chunk: Buffer,
_encoding: string,
callback: (err?: Error) => void,
) => {
console.log(chunk.toString('utf8'));
callback();
fakeProc.emit('close', 1);
};
(fakeProc as any).stdin = stdin;
return fakeProc as any;
});
// @ts-expect-error Assigning to a RO property
iptables.getDefaultRuleAdaptor = () => {
return ruleAdaptor;
};
fakeRuleAdaptorManager.clearHistory();
const applied = new EventEmitter(); const applied = new EventEmitter();
@ -106,9 +144,9 @@ export const whilstMocked = async (context: MockedConext) => {
}; };
await context({ await context({
expectRule: (rule) => fakeRuleAdaptor.expectRule(rule), expectRule: (rule) => fakeRuleAdaptorManager.expectRule(rule),
expectNoRule: (rule) => fakeRuleAdaptor.expectNoRule(rule), expectNoRule: (rule) => fakeRuleAdaptorManager.expectNoRule(rule),
clearHistory: () => fakeRuleAdaptor.clearHistory(), clearHistory: () => fakeRuleAdaptorManager.clearHistory(),
hasAppliedRules: new Promise((resolve) => { hasAppliedRules: new Promise((resolve) => {
applied.once('applied', () => resolve()); applied.once('applied', () => resolve());
}), }),
@ -116,4 +154,9 @@ export const whilstMocked = async (context: MockedConext) => {
// @ts-expect-error Assigning to a RO property // @ts-expect-error Assigning to a RO property
firewall.applyFirewallMode = applyFirewallRules; firewall.applyFirewallMode = applyFirewallRules;
spawnStub.restore();
// @ts-expect-error Assigning to a RO property
iptables.getDefaultRuleAdaptor = getOriginalDefaultRuleAdaptor;
}; };