Skip to content

Commit

Permalink
Merge pull request #4432 from LiskHQ/4363_penalty_malformed_peerlist
Browse files Browse the repository at this point in the history 
Implement penalty for malformed peerList response - Closes #4363 & #4475 & #4447
  • Loading branch information
@shuse2
shuse2 committed Nov 12, 2019
2 parents e24f604 + d3c3eb2 commit e6c6bf2
Show file tree
Hide file tree
Showing 26 changed files with 799 additions and 286 deletions.
8 changes: 8 additions & 0 deletions elements/lisk-p2p/src/constants.ts
View file
Expand Up @@ -54,6 +54,10 @@ export const DEFAULT_PRODUCTIVITY = {
lastResponded: 0,
};

// Penalty constants
export const INVALID_PEER_LIST_PENALTY = 100;
export const INVALID_PEER_INFO_PENALTY = 100;

// Peer inbound constants
export const DEFAULT_PING_INTERVAL_MAX = 60000;
export const DEFAULT_PING_INTERVAL_MIN = 20000;
Expand Down Expand Up @@ -84,6 +88,8 @@ export const INCOMPATIBLE_PROTOCOL_VERSION_REASON =
export const INCOMPATIBLE_PEER_CODE = 4104;
export const INCOMPATIBLE_PEER_UNKNOWN_REASON =
'Peer is incompatible with the node for unknown reasons';
export const INCOMPATIBLE_PEER_INFO_CODE = 4105;
export const INCOMPATIBLE_PEER_INFO_CODE_REASON = 'Peer has invalid PeerInfo';
// First case to follow HTTP status codes
export const FORBIDDEN_CONNECTION = 4403;
export const FORBIDDEN_CONNECTION_REASON = 'Peer is not allowed to connect';
Expand All @@ -92,6 +98,8 @@ export const DUPLICATE_CONNECTION = 4404;
export const DUPLICATE_CONNECTION_REASON = 'Peer has a duplicate connection';

// Peer info constants
export const INVALID_PEER_INFO_LIST_REASON = 'PeerInfo list has invalid value';
export const PEER_INFO_LIST_TOO_LONG_REASON = 'PeerInfo list is too long';
export enum ConnectionKind {
OUTBOUND = 'outbound',
INBOUND = 'inbound',
Expand Down
18 changes: 16 additions & 2 deletions elements/lisk-p2p/src/errors.ts
View file
Expand Up @@ -68,10 +68,24 @@ export class ExistingPeerError extends Error {
}
}

export class InvalidPeerError extends Error {
export class InvalidNodeInfoError extends Error {
public constructor(message: string) {
super(message);
this.name = 'InvalidPeerError';
this.name = 'InvalidNodeInfoError';
}
}

export class InvalidPeerInfoError extends Error {
public constructor(message: string) {
super(message);
this.name = 'InvalidPeerInfoError';
}
}

export class InvalidPeerInfoListError extends Error {
public constructor(message: string) {
super(message);
this.name = 'InvalidPeerInfoListError';
}
}

Expand Down
119 changes: 67 additions & 52 deletions elements/lisk-p2p/src/p2p.ts
View file
Expand Up @@ -16,7 +16,6 @@ import { getRandomBytes } from '@liskhq/lisk-cryptography';
import { EventEmitter } from 'events';
import * as http from 'http';
// tslint:disable-next-line no-require-imports
import shuffle = require('lodash.shuffle');
import { attach, SCServer, SCServerSocket } from 'socketcluster-server';
import * as url from 'url';
import {
Expand Down Expand Up @@ -45,6 +44,7 @@ import {
FORBIDDEN_CONNECTION,
FORBIDDEN_CONNECTION_REASON,
INCOMPATIBLE_PEER_CODE,
INCOMPATIBLE_PEER_INFO_CODE,
INCOMPATIBLE_PEER_UNKNOWN_REASON,
INVALID_CONNECTION_QUERY_CODE,
INVALID_CONNECTION_QUERY_REASON,
Expand Down Expand Up @@ -97,12 +97,15 @@ import { PeerBook } from './peer_book';
import { PeerPool, PeerPoolConfig } from './peer_pool';
import {
constructPeerId,
sanitizeOutgoingPeerInfo,
getByteSize,
sanitizeInitialPeerInfo,
sanitizePeerLists,
selectPeersForConnection,
selectPeersForRequest,
selectPeersForSend,
validateNodeInfo,
validatePeerCompatibility,
validatePeerInfo,
} from './utils';

interface SCServerUpdated extends SCServer {
Expand Down Expand Up @@ -230,38 +233,19 @@ export class P2P extends EventEmitter {
this._sanitizedPeerLists = sanitizePeerLists(
{
seedPeers: config.seedPeers
? config.seedPeers.map(peer => ({
peerId: constructPeerId(peer.ipAddress, peer.wsPort),
ipAddress: peer.ipAddress,
wsPort: peer.wsPort,
}))
? config.seedPeers.map(sanitizeInitialPeerInfo)
: [],
blacklistedPeers: config.blacklistedPeers
? config.blacklistedPeers.map(peer => ({
peerId: constructPeerId(peer.ipAddress, peer.wsPort),
ipAddress: peer.ipAddress,
wsPort: peer.wsPort,
}))
? config.blacklistedPeers.map(sanitizeInitialPeerInfo)
: [],
fixedPeers: config.fixedPeers
? config.fixedPeers.map(peer => ({
peerId: constructPeerId(peer.ipAddress, peer.wsPort),
ipAddress: peer.ipAddress,
wsPort: peer.wsPort,
}))
? config.fixedPeers.map(sanitizeInitialPeerInfo)
: [],
whitelisted: config.whitelistedPeers
? config.whitelistedPeers.map(peer => ({
peerId: constructPeerId(peer.ipAddress, peer.wsPort),
ipAddress: peer.ipAddress,
wsPort: peer.wsPort,
}))
? config.whitelistedPeers.map(sanitizeInitialPeerInfo)
: [],
previousPeers: config.previousPeers
? config.previousPeers.map(peer => ({
...peer,
peerId: constructPeerId(peer.ipAddress, peer.wsPort),
}))
? config.previousPeers.map(sanitizeInitialPeerInfo)
: [],
},
{
Expand All @@ -273,6 +257,7 @@ export class P2P extends EventEmitter {
wsPort: config.nodeInfo.wsPort,
},
);

this._config = config;
this._isActive = false;
this._hasConnected = false;
Expand All @@ -294,6 +279,7 @@ export class P2P extends EventEmitter {
if (request.procedure === REMOTE_EVENT_RPC_GET_PEERS_LIST) {
this._handleGetPeersRequest(request);
}

// Re-emit the request for external use.
this.emit(EVENT_REQUEST_RECEIVED, request);
};
Expand Down Expand Up @@ -546,9 +532,17 @@ export class P2P extends EventEmitter {
* invoke an async RPC on Peers to give them our new node status.
*/
public applyNodeInfo(nodeInfo: P2PNodeInfo): void {
validateNodeInfo(
nodeInfo,
this._config.maxPeerInfoSize
? this._config.maxPeerInfoSize
: DEFAULT_MAX_PEER_INFO_SIZE,
);

this._nodeInfo = {
...nodeInfo,
};

this._peerPool.applyNodeInfo(this._nodeInfo);
}

Expand Down Expand Up @@ -641,6 +635,7 @@ export class P2P extends EventEmitter {
private async _startPeerServer(): Promise<void> {
this._scServer.on(
'connection',
// tslint:disable-next-line: cyclomatic-complexity
(socket: SCServerSocket): void => {
// Check blacklist to avoid incoming connections from backlisted ips
if (this._sanitizedPeerLists.blacklistedPeers) {
Expand Down Expand Up @@ -738,20 +733,35 @@ export class P2P extends EventEmitter {
return;
}

// Remove these wsPort and ip from the query object
const { wsPort, ip, ...restOfQueryObject } = queryObject;
// Remove these wsPort and ipAddress from the query object
const { wsPort, ipAddress, ...restOfQueryObject } = queryObject;
const incomingPeerInfo: P2PPeerInfo = {
sharedState: {
...restOfQueryObject,
...queryOptions,
height: queryObject.height ? +queryObject.height : 0,
version: queryObject.version,
height: queryObject.height ? +queryObject.height : 0, // TODO: Remove the usage of height for choosing among peers having same ipAddress, instead use productivity and reputation
protocolVersion: queryObject.protocolVersion,
},
peerId: constructPeerId(socket.remoteAddress, remoteWSPort),
ipAddress: socket.remoteAddress,
wsPort: remoteWSPort,
};

try {
validatePeerInfo(
incomingPeerInfo,
this._config.maxPeerInfoSize
? this._config.maxPeerInfoSize
: DEFAULT_MAX_PEER_INFO_SIZE,
);
} catch (error) {
this._disconnectSocketDueToFailedHandshake(
socket,
INCOMPATIBLE_PEER_INFO_CODE,
error,
);
}

const { success, errors } = this._peerHandshakeCheck(
incomingPeerInfo,
this._nodeInfo,
Expand Down Expand Up @@ -876,32 +886,37 @@ export class P2P extends EventEmitter {
const peerDiscoveryResponseLength = this._config.peerDiscoveryResponseLength
? this._config.peerDiscoveryResponseLength
: DEFAULT_MAX_PEER_DISCOVERY_RESPONSE_LENGTH;

const knownPeers = this._peerBook.allPeers;
/* tslint:disable no-magic-numbers*/
const min = Math.ceil(
Math.min(peerDiscoveryResponseLength, knownPeers.length * 0.25),
);
const max = Math.floor(
Math.min(peerDiscoveryResponseLength, knownPeers.length * 0.5),
);
const random = Math.floor(Math.random() * (max - min + 1) + min);
const randomPeerCount = Math.max(
random,
Math.min(minimumPeerDiscoveryThreshold, knownPeers.length),
const wsMaxPayload = this._config.wsMaxPayload
? this._config.wsMaxPayload
: DEFAULT_WS_MAX_PAYLOAD;
const maxPeerInforSize = this._config.maxPeerInfoSize
? this._config.maxPeerInfoSize
: DEFAULT_MAX_PEER_INFO_SIZE;

const safeMaxPeerInfoLength =
Math.floor(DEFAULT_WS_MAX_PAYLOAD / maxPeerInforSize) - 1;

const selectedPeers = this._peerBook.getRandomizedPeerList(
minimumPeerDiscoveryThreshold,
peerDiscoveryResponseLength,
);

const selectedPeers = shuffle(knownPeers)
.slice(0, randomPeerCount)
.map(
sanitizeOutgoingPeerInfo, // Sanitize the peerInfos before responding to a peer that understand old peerInfo.
);
// Remove internal state to check byte size
const sanitizedPeerInfoList: ProtocolPeerInfo[] = selectedPeers.map(
peer => ({
ipAddress: peer.ipAddress,
wsPort: peer.wsPort,
...peer.sharedState,
}),
);

const peerInfoList = {
request.end({
success: true,
peers: selectedPeers,
};
request.end(peerInfoList);
peers:
getByteSize(sanitizedPeerInfoList) < wsMaxPayload
? sanitizedPeerInfoList
: sanitizedPeerInfoList.slice(0, safeMaxPeerInfoLength),
});
}

private _isTrustedPeer(peerId: string): boolean {
Expand Down
9 changes: 1 addition & 8 deletions elements/lisk-p2p/src/p2p_types.ts
View file
Expand Up @@ -42,8 +42,6 @@ export interface P2PPenalty {
}

export interface P2PSharedState {
readonly version: string;
readonly protocolVersion: string;
// tslint:disable-next-line: no-mixed-interface
readonly [key: string]: unknown;
}
Expand Down Expand Up @@ -74,6 +72,7 @@ export interface P2PPeersCount {
// P2PPeerInfo and P2PNodeInfo are related.
// P2PNodeInfo is the outbound info from our node.
export interface P2PNodeInfo extends P2PSharedState {
readonly protocolVersion: string;
readonly os: string;
readonly nethash: string;
readonly wsPort: number;
Expand All @@ -85,14 +84,8 @@ export interface P2PNodeInfo extends P2PSharedState {
// TODO: Include peerId as field
export interface ProtocolPeerInfo {
// To support the existing protocol
readonly ip?: string;
readonly ipAddress: string;
readonly wsPort: number;
readonly height?: number;
readonly os?: string;
readonly version?: string;
readonly protocolVersion?: string;
readonly httpPort?: number;
// tslint:disable-next-line: no-mixed-interface
readonly [key: string]: unknown;
}
Expand Down

0 comments on commit e6c6bf2

Please sign in to comment.