From f5c185fe21fd44b583e0a2a872a1a4995c618951 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 23 Aug 2019 02:03:13 +0300 Subject: [PATCH 1/5] feat(core-p2p): use compression on the p2p level (#2886) --- packages/core-p2p/src/peer-connector.ts | 1 + packages/core-p2p/src/socket-server/index.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/core-p2p/src/peer-connector.ts b/packages/core-p2p/src/peer-connector.ts index 582e2b9a2a..6ce13ccb7b 100644 --- a/packages/core-p2p/src/peer-connector.ts +++ b/packages/core-p2p/src/peer-connector.ts @@ -24,6 +24,7 @@ export class PeerConnector implements P2P.IPeerConnector { connection = create({ port: peer.port, hostname: peer.ip, + perMessageDeflate: true, }); this.connections.set(peer.ip, connection); diff --git a/packages/core-p2p/src/socket-server/index.ts b/packages/core-p2p/src/socket-server/index.ts index 400caf079d..cccede240d 100644 --- a/packages/core-p2p/src/socket-server/index.ts +++ b/packages/core-p2p/src/socket-server/index.ts @@ -22,6 +22,7 @@ export const startSocketServer = async (service: P2P.IPeerService, config: Recor workerController: __dirname + `${relativeSocketPath}/worker.js`, workers: 2, wsEngine: "ws", + perMessageDeflate: true, }, ...config.server, }); From 8e8034044a09b734d9254cb54532ca6c2db9b004 Mon Sep 17 00:00:00 2001 From: Edgar Goetzendorff Date: Fri, 23 Aug 2019 13:21:41 +0200 Subject: [PATCH 2/5] fix(core-webhooks): cast params in condition checks (#2887) --- .../unit/core-webhooks/conditions.test.ts | 72 +++++++++++++++++++ packages/core-webhooks/src/conditions.ts | 22 +++--- 2 files changed, 85 insertions(+), 9 deletions(-) diff --git a/__tests__/unit/core-webhooks/conditions.test.ts b/__tests__/unit/core-webhooks/conditions.test.ts index 036e00e239..74b58758c5 100644 --- a/__tests__/unit/core-webhooks/conditions.test.ts +++ b/__tests__/unit/core-webhooks/conditions.test.ts @@ -22,6 +22,12 @@ describe("Conditions - between", () => { max: 2, }), ).toBeTrue(); + expect( + between("1.5", { + min: "1", + max: "2", + }), + ).toBeTrue(); }); it("should be false", () => { @@ -31,6 +37,12 @@ describe("Conditions - between", () => { max: 2, }), ).toBeFalse(); + expect( + between("3", { + min: "1", + max: "2", + }), + ).toBeFalse(); }); }); @@ -47,30 +59,46 @@ describe("Conditions - contains", () => { describe("Conditions - equal", () => { it("should be true", () => { expect(eq(1, 1)).toBeTrue(); + expect(eq("1", "1")).toBeTrue(); }); it("should be false", () => { expect(eq(1, 2)).toBeFalse(); + expect(eq("1", "2")).toBeFalse(); }); }); describe("Conditions - falsy", () => { it("should be true", () => { expect(falsy(false)).toBeTrue(); + expect(falsy("false")).toBeTrue(); + expect(falsy("FaLsE")).toBeTrue(); }); it("should be false", () => { expect(falsy(true)).toBeFalse(); + expect(falsy("true")).toBeFalse(); + expect(falsy("TrUe")).toBeFalse(); }); }); describe("Conditions - greater than", () => { it("should be true", () => { expect(gt(2, 1)).toBeTrue(); + expect(gt("2", "1")).toBeTrue(); + expect(gt("10", "2")).toBeTrue(); }); it("should be false", () => { expect(gt(1, 2)).toBeFalse(); + expect(gt("1", "2")).toBeFalse(); + expect(gt("2", "10")).toBeFalse(); + expect(gt(undefined, NaN)).toBeFalse(); + expect(gt(1, NaN)).toBeFalse(); + expect(gt(undefined, 1)).toBeFalse(); + expect(gt("null", "NaN")).toBeFalse(); + expect(gt("1", "NaN")).toBeFalse(); + expect(gt("null", "1")).toBeFalse(); }); }); @@ -78,20 +106,37 @@ describe("Conditions - greater than or equal", () => { it("should be true", () => { expect(gte(2, 1)).toBeTrue(); expect(gte(2, 2)).toBeTrue(); + expect(gte("2", "1")).toBeTrue(); + expect(gte("2", "2")).toBeTrue(); }); it("should be false", () => { expect(gte(1, 2)).toBeFalse(); + expect(gte("1", "2")).toBeFalse(); + expect(gt(undefined, NaN)).toBeFalse(); + expect(gt(1, NaN)).toBeFalse(); + expect(gt(undefined, 1)).toBeFalse(); + expect(gt("null", "NaN")).toBeFalse(); + expect(gt("1", "NaN")).toBeFalse(); + expect(gt("null", "1")).toBeFalse(); }); }); describe("Conditions - less than", () => { it("should be true", () => { expect(lt(1, 2)).toBeTrue(); + expect(lt("1", "2")).toBeTrue(); }); it("should be false", () => { expect(lt(2, 1)).toBeFalse(); + expect(lt("2", "1")).toBeFalse(); + expect(gt(undefined, NaN)).toBeFalse(); + expect(gt(1, NaN)).toBeFalse(); + expect(gt(undefined, 1)).toBeFalse(); + expect(gt("null", "NaN")).toBeFalse(); + expect(gt("1", "NaN")).toBeFalse(); + expect(gt("null", "1")).toBeFalse(); }); }); @@ -99,20 +144,31 @@ describe("Conditions - less than or equal", () => { it("should be true", () => { expect(lte(1, 2)).toBeTrue(); expect(lte(1, 1)).toBeTrue(); + expect(lte("1", "2")).toBeTrue(); + expect(lte("1", "1")).toBeTrue(); }); it("should be false", () => { expect(lte(2, 1)).toBeFalse(); + expect(lte("2", "1")).toBeFalse(); + expect(gt(undefined, NaN)).toBeFalse(); + expect(gt(1, NaN)).toBeFalse(); + expect(gt(undefined, 1)).toBeFalse(); + expect(gt("null", "NaN")).toBeFalse(); + expect(gt("1", "NaN")).toBeFalse(); + expect(gt("null", "1")).toBeFalse(); }); }); describe("Conditions - not equal", () => { it("should be true", () => { expect(ne(1, 2)).toBeTrue(); + expect(ne("1", "2")).toBeTrue(); }); it("should be false", () => { expect(ne(1, 1)).toBeFalse(); + expect(ne("1", "1")).toBeFalse(); }); }); @@ -124,6 +180,12 @@ describe("Conditions - not-between", () => { max: 2, }), ).toBeTrue(); + expect( + notBetween("3", { + min: "1", + max: "2", + }), + ).toBeTrue(); }); it("should be false", () => { @@ -133,6 +195,12 @@ describe("Conditions - not-between", () => { max: 2, }), ).toBeFalse(); + expect( + notBetween("1.5", { + min: "1", + max: "2", + }), + ).toBeFalse(); }); }); @@ -149,9 +217,13 @@ describe("Conditions - regexp", () => { describe("Conditions - truthy", () => { it("should be true", () => { expect(truthy(true)).toBeTrue(); + expect(truthy("true")).toBeTrue(); + expect(truthy("TrUe")).toBeTrue(); }); it("should be false", () => { expect(truthy(false)).toBeFalse(); + expect(truthy("false")).toBeFalse(); + expect(truthy("FaLsE")).toBeFalse(); }); }); diff --git a/packages/core-webhooks/src/conditions.ts b/packages/core-webhooks/src/conditions.ts index 764257076b..316934a91e 100644 --- a/packages/core-webhooks/src/conditions.ts +++ b/packages/core-webhooks/src/conditions.ts @@ -1,12 +1,16 @@ -export const between = (actual, expected): boolean => actual > expected.min && actual < expected.max; +import { Utils } from "@arkecosystem/crypto"; + +const toBoolean = (value): boolean => value.toString().toLowerCase().trim() === "true" ? true : false; + +export const between = (actual, expected): boolean => gt(actual, expected.min) && lt(actual, expected.max); export const contains = (actual, expected): boolean => actual.includes(expected); -export const eq = (actual, expected): boolean => actual === expected; -export const falsy = (actual): boolean => actual === false; -export const gt = (actual, expected): boolean => actual > expected; -export const gte = (actual, expected): boolean => actual >= expected; -export const lt = (actual, expected): boolean => actual < expected; -export const lte = (actual, expected): boolean => actual <= expected; -export const ne = (actual, expected): boolean => actual !== expected; +export const eq = (actual, expected): boolean => JSON.stringify(actual) === JSON.stringify(expected); +export const falsy = (actual): boolean => actual === false || !toBoolean(actual); +export const gt = (actual, expected): boolean => Utils.BigNumber.make(actual).gt(expected); +export const gte = (actual, expected): boolean => Utils.BigNumber.make(actual).gte(expected); +export const lt = (actual, expected): boolean => Utils.BigNumber.make(actual).lt(expected); +export const lte = (actual, expected): boolean => Utils.BigNumber.make(actual).lte(expected); +export const ne = (actual, expected): boolean => !eq(actual, expected); export const notBetween = (actual, expected): boolean => !between(actual, expected); export const regexp = (actual, expected): boolean => new RegExp(expected).test(actual); -export const truthy = (actual): boolean => actual === true; +export const truthy = (actual): boolean => actual === true || toBoolean(actual); From c3aebbf75c707c424c07a10a82066abcd054a5b6 Mon Sep 17 00:00:00 2001 From: supaiku <1311798+supaiku0@users.noreply.github.com> Date: Wed, 28 Aug 2019 06:05:52 +0200 Subject: [PATCH 3/5] fix(crypto): use `anyOf` for transactions schema (#2894) --- packages/crypto/src/validation/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/crypto/src/validation/index.ts b/packages/crypto/src/validation/index.ts index ebf40c4373..5601090c72 100644 --- a/packages/crypto/src/validation/index.ts +++ b/packages/crypto/src/validation/index.ts @@ -98,7 +98,7 @@ export class Validator { $id: "transactions", type: "array", additionalItems: false, - items: { oneOf: [...this.transactionSchemas].map(schema => ({ $ref: `${schema}Signed` })) }, + items: { anyOf: [...this.transactionSchemas].map(schema => ({ $ref: `${schema}Signed` })) }, }); this.ajv.addSchema(schemas.block); } From 5ac0c41b03de8d640b6a8d6dbc812c3361efade7 Mon Sep 17 00:00:00 2001 From: alessiodf <35549818+alessiodf@users.noreply.github.com> Date: Tue, 27 Aug 2019 03:14:13 +0000 Subject: [PATCH 4/5] fix(core-p2p): drop connections with malformed messages --- .../core-p2p/socket-server/peer.test.ts | 41 +++++++++++++++++++ packages/core-p2p/src/enums.ts | 1 + packages/core-p2p/src/socket-server/worker.ts | 32 +++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/__tests__/integration/core-p2p/socket-server/peer.test.ts b/__tests__/integration/core-p2p/socket-server/peer.test.ts index df4c589b35..eb4765a1b1 100644 --- a/__tests__/integration/core-p2p/socket-server/peer.test.ts +++ b/__tests__/integration/core-p2p/socket-server/peer.test.ts @@ -17,7 +17,9 @@ Managers.configManager.setFromPreset("unitnet"); let server: SocketCluster; let socket; +let connect; let emit; +let send; const headers = { version: "2.1.0", @@ -43,11 +45,15 @@ beforeAll(async () => { // }); + connect = () => socket.connect(); + emit = (event, data) => new Promise((resolve, reject) => { socket.emit(event, data, (err, val) => (err ? reject(err) : resolve(val))); }); + send = data => socket.send(data); + jest.spyOn(processor, "validateAndAcceptPeer").mockImplementation(jest.fn()); }); @@ -127,6 +133,41 @@ describe("Peer socket endpoint", () => { }), ).toResolve(); }); + + it("should disconnect the client if it sends an invalid message payload", async () => { + await delay(1000); + + expect(socket.state).toBe("open"); + + send('{"event": "#handshake", "data": {}, "cid": 1}'); + await delay(500); + + send("Invalid payload"); + await delay(1000); + + expect(socket.state).toBe("closed"); + }); + + it("should disconnect the client if it sends too many pongs too quickly", async () => { + connect(); + await delay(1000); + + expect(socket.state).toBe("open"); + + send('{"event": "#handshake", "data": {}, "cid": 1}'); + await delay(500); + + send("#2"); + await delay(1000); + + expect(socket.state).toBe("open"); + + send("#2"); + send("#2"); + await delay(1000); + + expect(socket.state).toBe("closed"); + }); }); }); diff --git a/packages/core-p2p/src/enums.ts b/packages/core-p2p/src/enums.ts index c82f15229d..ba76351823 100644 --- a/packages/core-p2p/src/enums.ts +++ b/packages/core-p2p/src/enums.ts @@ -31,4 +31,5 @@ export enum SocketErrors { Validation = "CoreValidationError", RateLimitExceeded = "CoreRateLimitExceededError", Forbidden = "CoreForbiddenError", + InvalidMessagePayload = "CoreInvalidMessagePayloadError", } diff --git a/packages/core-p2p/src/socket-server/worker.ts b/packages/core-p2p/src/socket-server/worker.ts index bfa6f7dd84..8e1998857b 100644 --- a/packages/core-p2p/src/socket-server/worker.ts +++ b/packages/core-p2p/src/socket-server/worker.ts @@ -11,6 +11,8 @@ export class Worker extends SCWorker { await this.loadConfiguration(); + // @ts-ignore + this.scServer.wsServer.on("connection", (ws, req) => this.handlePayload(ws, req)); this.scServer.on("connection", socket => this.handleConnection(socket)); this.scServer.addMiddleware(this.scServer.MIDDLEWARE_HANDSHAKE_WS, (req, next) => this.handleHandshake(req, next), @@ -55,6 +57,36 @@ export class Worker extends SCWorker { }); } + private handlePayload(ws, req) { + ws.on("message", message => { + try { + const InvalidMessagePayloadError: Error = this.createError( + SocketErrors.InvalidMessagePayload, + "The message contained an invalid payload", + ); + if (message === "#2") { + const timeNow: number = new Date().getTime() / 1000; + if (ws._lastPingTime && timeNow - ws._lastPingTime < 1) { + throw InvalidMessagePayloadError; + } + ws._lastPingTime = timeNow; + } else { + const parsed = JSON.parse(message); + if ( + typeof parsed.event !== "string" || + typeof parsed.data !== "object" || + (typeof parsed.cid !== "number" && + (parsed.event === "#disconnect" && typeof parsed.cid !== "undefined")) + ) { + throw InvalidMessagePayloadError; + } + } + } catch (error) { + ws.terminate(); + } + }); + } + private async handleConnection(socket): Promise { const { data } = await this.sendToMasterAsync("p2p.utils.getHandlers"); From 76f06acaa7b8ea709b1df6d08dd965b2aebc70e2 Mon Sep 17 00:00:00 2001 From: alessiodf <35549818+alessiodf@users.noreply.github.com> Date: Sat, 31 Aug 2019 14:22:12 +0000 Subject: [PATCH 5/5] fix(core-p2p): terminate blocked client connections --- .../core-p2p/socket-server/peer.test.ts | 27 ++++++++++++++++++- packages/core-p2p/src/socket-server/worker.ts | 2 +- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/__tests__/integration/core-p2p/socket-server/peer.test.ts b/__tests__/integration/core-p2p/socket-server/peer.test.ts index eb4765a1b1..5d1a47f08e 100644 --- a/__tests__/integration/core-p2p/socket-server/peer.test.ts +++ b/__tests__/integration/core-p2p/socket-server/peer.test.ts @@ -34,7 +34,7 @@ beforeAll(async () => { const { service, processor } = createPeerService(); - server = await startSocketServer(service, { server: { port: 4007 } }); + server = await startSocketServer(service, { server: { port: 4007, workers: 1 } }); await delay(1000); socket = socketCluster.create({ @@ -270,5 +270,30 @@ describe("Peer socket endpoint", () => { }), ).rejects.toHaveProperty("name", "BadConnectionError"); }); + + it("should close the connection and prevent reconnection if blocked", async () => { + await delay(1000); + + await emit("p2p.peer.getPeers", { + headers, + }); + + expect(socket.state).toBe("open"); + + for (let i = 0; i < 100; i++) { + await expect( + emit("p2p.peer.getPeers", { + headers, + }), + ).rejects.toContainAnyEntries([["name", "CoreRateLimitExceededError"], ["name", "BadConnectionError"]]); + } + + expect(socket.state).not.toBe("open"); + + socket.connect(); + await delay(1000); + + expect(socket.state).not.toBe("open"); + }); }); }); diff --git a/packages/core-p2p/src/socket-server/worker.ts b/packages/core-p2p/src/socket-server/worker.ts index 8e1998857b..725f8cbf0c 100644 --- a/packages/core-p2p/src/socket-server/worker.ts +++ b/packages/core-p2p/src/socket-server/worker.ts @@ -118,7 +118,7 @@ export class Worker extends SCWorker { private async handleEmit(req, next): Promise { if (await this.rateLimiter.hasExceededRateLimit(req.socket.remoteAddress, req.event)) { if (await this.rateLimiter.isBlocked(req.socket.remoteAddress)) { - req.socket.disconnect(4403, "Forbidden"); + req.socket.terminate(); return; }