Skip to content

Commit

Permalink
fix(core-p2p): Restrict internal routes to whitelisted IPs (#3853)
Browse files Browse the repository at this point in the history
  • Loading branch information
bertiespell authored Jun 30, 2020
1 parent cf43417 commit 3467289
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 1 deletion.
120 changes: 120 additions & 0 deletions __tests__/unit/core-p2p/socket-server/plugins/whitelist-forger.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { Server } from "@hapi/hapi";
import Joi from "@hapi/joi";
import { Container } from "@packages/core-kernel";
import { defaults } from "@packages/core-p2p/src/defaults";
import { PeerProcessor } from "@packages/core-p2p/src/peer-processor";
import { WhitelistForgerPlugin } from "@packages/core-p2p/src/socket-server/plugins/whitelist-forger";

describe("WhitelistForgerPlugin", () => {
let whitelistForgerPlugin: WhitelistForgerPlugin;
let spyPeerProcessorWhitelisted;
const peerProcessor = new PeerProcessor();

const container = new Container.Container();

const logger = { warning: jest.fn(), debug: jest.fn() };

const pluginConfiguration = {
getOptional: (_) => defaults.remoteAccess,
};

// @ts-ignore
peerProcessor.configuration = pluginConfiguration;

const responsePayload = { status: "ok" };
const mockRouteByPath = {
"/p2p/peer/mockroute": {
id: "p2p.peer.getPeers",
handler: () => responsePayload,
validation: Joi.object().max(0),
},
};
const mockRoute = {
method: "POST",
path: "/p2p/peer/mockroute",
config: {
id: mockRouteByPath["/p2p/peer/mockroute"].id,
handler: mockRouteByPath["/p2p/peer/mockroute"].handler,
},
};
const app = { resolve: jest.fn().mockReturnValue({ getRoutesConfigByPath: () => mockRouteByPath }) };

beforeAll(() => {
container.unbindAll();
container.bind(Container.Identifiers.LogService).toConstantValue(logger);
container.bind(Container.Identifiers.Application).toConstantValue(app);
container.bind(Container.Identifiers.PeerProcessor).toConstantValue(peerProcessor);
});

beforeEach(() => {
whitelistForgerPlugin = container.resolve<WhitelistForgerPlugin>(WhitelistForgerPlugin);
spyPeerProcessorWhitelisted = jest.spyOn(peerProcessor, "isWhitelisted");
});

afterEach(() => spyPeerProcessorWhitelisted.mockClear());

it("should register the isWhitelisted plugin", async () => {
const server = new Server({ port: 4100 });
server.route(mockRoute);

const spyExt = jest.spyOn(server, "ext");

whitelistForgerPlugin.register(server);

expect(spyExt).toBeCalledWith(expect.objectContaining({ type: "onPreHandler" }));
});

it("should authorize default whitelisted IPs", async () => {
const server = new Server({ port: 4100 });
server.route(mockRoute);

const spyExt = jest.spyOn(server, "ext");

whitelistForgerPlugin.register(server);

expect(spyExt).toBeCalledWith(expect.objectContaining({ type: "onPreHandler" }));

// try the route with a valid remoteAddress
const remoteAddress = defaults.remoteAccess[0];
const responseValid = await server.inject({
method: "POST",
url: "/p2p/peer/mockroute",
payload: {},
remoteAddress,
});
expect(JSON.parse(responseValid.payload)).toEqual(responsePayload);
expect(responseValid.statusCode).toBe(200);
expect(spyPeerProcessorWhitelisted).toBeCalledTimes(1);
expect(spyPeerProcessorWhitelisted).toBeCalledWith({ ip: remoteAddress });
});

it("should not authorize none-whitelisted IPs", async () => {
const server = new Server({ port: 4100 });
server.route(mockRoute);

const spyExt = jest.spyOn(server, "ext");

whitelistForgerPlugin.register(server);

expect(spyExt).toBeCalledWith(expect.objectContaining({ type: "onPreHandler" }));

// try the route with an invalid remoteAddress
const remoteAddress = "187.166.55.44";
const responseInvalid = await server.inject({
method: "POST",
url: "/p2p/peer/mockroute",
payload: {},
remoteAddress,
});

const invalidResponsePayload = {
error: "Forbidden",
message: "IP unauthorized on internal route",
statusCode: 403,
};
expect(JSON.parse(responseInvalid.payload)).toEqual(invalidResponsePayload);
expect(responseInvalid.statusCode).not.toBe(200);
expect(spyPeerProcessorWhitelisted).toBeCalledTimes(1);
expect(spyPeerProcessorWhitelisted).toBeCalledWith({ ip: remoteAddress });
});
});
2 changes: 2 additions & 0 deletions packages/core-kernel/src/contracts/p2p/peer-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ export interface PeerProcessor {
validateAndAcceptPeer(peer: Peer, options?: AcceptNewPeerOptions): Promise<void>;

validatePeerIp(peer, options?: AcceptNewPeerOptions): boolean;

isWhitelisted(peer): boolean;
}
4 changes: 4 additions & 0 deletions packages/core-p2p/src/peer-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export class PeerProcessor implements Contracts.P2P.PeerProcessor {
this.events.listen(Enums.CryptoEvent.MilestoneChanged, this.app.resolve(DisconnectInvalidPeers));
}

public isWhitelisted(peer: Contracts.P2P.Peer): boolean {
return KernelUtils.isWhitelisted(this.configuration.getOptional<string[]>("remoteAccess", []), peer.ip);
}

public async validateAndAcceptPeer(
peer: Contracts.P2P.Peer,
options: Contracts.P2P.AcceptNewPeerOptions = {},
Expand Down
33 changes: 33 additions & 0 deletions packages/core-p2p/src/socket-server/plugins/whitelist-forger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Container, Contracts } from "@arkecosystem/core-kernel";
import Boom from "@hapi/boom";

import { InternalRoute } from "../routes/internal";

@Container.injectable()
export class WhitelistForgerPlugin {
@Container.inject(Container.Identifiers.Application)
protected readonly app!: Contracts.Kernel.Application;

@Container.inject(Container.Identifiers.PeerProcessor)
private readonly peerProcessor!: Contracts.P2P.PeerProcessor;

public register(server) {
const peerRoutesConfigByPath = this.app.resolve(InternalRoute).getRoutesConfigByPath();
const peerProcessor = this.peerProcessor;

server.ext({
type: "onPreHandler",
async method(request, h) {
if (peerRoutesConfigByPath[request.path]) {
if (peerProcessor.isWhitelisted({ ip: request.info.remoteAddress } as Contracts.P2P.Peer)) {
return h.continue;
} else {
return Boom.forbidden("IP unauthorized on internal route");
}
} else {
return h.continue;
}
},
});
}
}
4 changes: 3 additions & 1 deletion packages/core-p2p/src/socket-server/server.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Container, Contracts, Types } from "@arkecosystem/core-kernel";
import { Server as HapiServer, ServerInjectOptions, ServerInjectResponse, ServerRoute } from "@hapi/hapi";
import { plugin } from "../hapi-nes";

import { plugin } from "../hapi-nes";
import { AcceptPeerPlugin } from "./plugins/accept-peer";
import { ValidatePlugin } from "./plugins/validate";
import { WhitelistForgerPlugin } from "./plugins/whitelist-forger";
import { InternalRoute } from "./routes/internal";
import { PeerRoute } from "./routes/peer";

Expand Down Expand Up @@ -58,6 +59,7 @@ export class Server {

this.app.resolve(ValidatePlugin).register(this.server);
this.app.resolve(AcceptPeerPlugin).register(this.server);
this.app.resolve(WhitelistForgerPlugin).register(this.server);
}

/**
Expand Down

0 comments on commit 3467289

Please sign in to comment.