From 34672893b44a5af3722bb7edf75bb9ed0a4cd0e0 Mon Sep 17 00:00:00 2001 From: Bertie Date: Tue, 30 Jun 2020 10:33:55 -0400 Subject: [PATCH] fix(core-p2p): Restrict internal routes to whitelisted IPs (#3853) --- .../plugins/whitelist-forger.test.ts | 120 ++++++++++++++++++ .../src/contracts/p2p/peer-processor.ts | 2 + packages/core-p2p/src/peer-processor.ts | 4 + .../socket-server/plugins/whitelist-forger.ts | 33 +++++ packages/core-p2p/src/socket-server/server.ts | 4 +- 5 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 __tests__/unit/core-p2p/socket-server/plugins/whitelist-forger.test.ts create mode 100644 packages/core-p2p/src/socket-server/plugins/whitelist-forger.ts diff --git a/__tests__/unit/core-p2p/socket-server/plugins/whitelist-forger.test.ts b/__tests__/unit/core-p2p/socket-server/plugins/whitelist-forger.test.ts new file mode 100644 index 0000000000..e974560dac --- /dev/null +++ b/__tests__/unit/core-p2p/socket-server/plugins/whitelist-forger.test.ts @@ -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); + 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 }); + }); +}); diff --git a/packages/core-kernel/src/contracts/p2p/peer-processor.ts b/packages/core-kernel/src/contracts/p2p/peer-processor.ts index 0d069c4428..3ab104d18a 100644 --- a/packages/core-kernel/src/contracts/p2p/peer-processor.ts +++ b/packages/core-kernel/src/contracts/p2p/peer-processor.ts @@ -11,4 +11,6 @@ export interface PeerProcessor { validateAndAcceptPeer(peer: Peer, options?: AcceptNewPeerOptions): Promise; validatePeerIp(peer, options?: AcceptNewPeerOptions): boolean; + + isWhitelisted(peer): boolean; } diff --git a/packages/core-p2p/src/peer-processor.ts b/packages/core-p2p/src/peer-processor.ts index 05301de271..8381a18e21 100644 --- a/packages/core-p2p/src/peer-processor.ts +++ b/packages/core-p2p/src/peer-processor.ts @@ -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("remoteAccess", []), peer.ip); + } + public async validateAndAcceptPeer( peer: Contracts.P2P.Peer, options: Contracts.P2P.AcceptNewPeerOptions = {}, diff --git a/packages/core-p2p/src/socket-server/plugins/whitelist-forger.ts b/packages/core-p2p/src/socket-server/plugins/whitelist-forger.ts new file mode 100644 index 0000000000..39bd6e6785 --- /dev/null +++ b/packages/core-p2p/src/socket-server/plugins/whitelist-forger.ts @@ -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; + } + }, + }); + } +} diff --git a/packages/core-p2p/src/socket-server/server.ts b/packages/core-p2p/src/socket-server/server.ts index 37c57ab26a..e7bf7ac864 100644 --- a/packages/core-p2p/src/socket-server/server.ts +++ b/packages/core-p2p/src/socket-server/server.ts @@ -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"; @@ -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); } /**