From 6b63ecb439519f1c7973a75fa134c371b94281b9 Mon Sep 17 00:00:00 2001 From: TheLazieR Yip Date: Fri, 25 Nov 2016 07:14:09 -0500 Subject: [PATCH 01/10] Add header_hash and header_prevhash class methods --- lib/coins.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/coins.py b/lib/coins.py index 71129b6..44ded03 100644 --- a/lib/coins.py +++ b/lib/coins.py @@ -214,6 +214,16 @@ class Coin(object): payload.append(0x01) return Base58.encode_check(payload) + @classmethod + def header_hash(cls, header): + '''Given a header return hash''' + return double_sha256(header) + + @classmethod + def header_prevhash(cls, header): + '''Given a header return previous hash''' + return header[4:36] + @classmethod def header_hashes(cls, header): '''Given a header return the previous and current block hashes.''' @@ -367,6 +377,12 @@ class Dash(Coin): IRC_PREFIX = "D_" IRC_CHANNEL = "#electrum-dash" + @classmethod + def header_hash(cls, header): + '''Given a header return the hash.''' + import x11_hash + return x11_hash.getPoWHash(header) + @classmethod def header_hashes(cls, header): '''Given a header return the previous and current block hashes.''' From b3623f5455af680750dfdcf391b5f3be238fe378 Mon Sep 17 00:00:00 2001 From: TheLazieR Yip Date: Fri, 25 Nov 2016 07:15:18 -0500 Subject: [PATCH 02/10] replace header_hashes with header_prevhash , header_hash --- server/block_processor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/block_processor.py b/server/block_processor.py index 144fb6a..697b603 100644 --- a/server/block_processor.py +++ b/server/block_processor.py @@ -568,7 +568,7 @@ class BlockProcessor(server.db.DB): # the UTXO cache uses the FS cache via get_tx_hash() to # resolve compressed key collisions header, tx_hashes, txs = self.coin.read_block(block) - prev_hash, header_hash = self.coin.header_hashes(header) + prev_hash, header_hash = self.coin.header_prevhash(header), self.coin.header_hash(header) if prev_hash != self.tip: raise ChainReorg @@ -636,7 +636,7 @@ class BlockProcessor(server.db.DB): touched = set() for block in blocks: header, tx_hashes, txs = self.coin.read_block(block) - prev_hash, header_hash = self.coin.header_hashes(header) + prev_hash, header_hash = self.coin.header_prevhash(header), self.coin.header_hash(header) if header_hash != self.tip: raise ChainError('backup block {} is not tip {} at height {:,d}' .format(hash_to_str(header_hash), From e987510432f66c146caa34d239a8788146fa02ac Mon Sep 17 00:00:00 2001 From: TheLazieR Yip Date: Fri, 25 Nov 2016 07:20:25 -0500 Subject: [PATCH 03/10] Replace double_sha256 with header_hash from coin --- server/db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/db.py b/server/db.py index 58c7d9e..08a08e6 100644 --- a/server/db.py +++ b/server/db.py @@ -16,7 +16,7 @@ from bisect import bisect_right from collections import namedtuple from lib.util import chunks, formatted_time, LoggedClass -from lib.hash import double_sha256, hash_to_str +from lib.hash import hash_to_str from server.storage import open_db from server.version import VERSION @@ -175,7 +175,7 @@ class DB(LoggedClass): headers = self.fs_read_headers(height, count) # FIXME: move to coins.py hlen = self.coin.HEADER_LEN - return [double_sha256(header) for header in chunks(headers, hlen)] + return [self.coin.header_hash(header) for header in chunks(headers, hlen)] @staticmethod def _resolve_limit(limit): From dfaf36ae164b93221645d4c635bd6db14af39ba7 Mon Sep 17 00:00:00 2001 From: TheLazieR Yip Date: Fri, 25 Nov 2016 07:21:19 -0500 Subject: [PATCH 04/10] Remove header_hashes --- lib/coins.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/coins.py b/lib/coins.py index 44ded03..334299e 100644 --- a/lib/coins.py +++ b/lib/coins.py @@ -224,11 +224,6 @@ class Coin(object): '''Given a header return previous hash''' return header[4:36] - @classmethod - def header_hashes(cls, header): - '''Given a header return the previous and current block hashes.''' - return header[4:36], double_sha256(header) - @classmethod def read_block(cls, block): '''Return a tuple (header, tx_hashes, txs) given a raw block.''' @@ -383,12 +378,6 @@ class Dash(Coin): import x11_hash return x11_hash.getPoWHash(header) - @classmethod - def header_hashes(cls, header): - '''Given a header return the previous and current block hashes.''' - import x11_hash - return header[4:36], x11_hash.getPoWHash(header) - class DashTestnet(Dash): NAME = "Dash" SHORTNAME = "tDASH" From 828727d41a94ac0e685badc1a4edd7b2740f1aed Mon Sep 17 00:00:00 2001 From: TheLazieR Yip Date: Fri, 25 Nov 2016 07:44:19 -0500 Subject: [PATCH 05/10] Remove unneccessary local variables --- server/block_processor.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/block_processor.py b/server/block_processor.py index 697b603..eecc2eb 100644 --- a/server/block_processor.py +++ b/server/block_processor.py @@ -569,12 +569,12 @@ class BlockProcessor(server.db.DB): # resolve compressed key collisions header, tx_hashes, txs = self.coin.read_block(block) prev_hash, header_hash = self.coin.header_prevhash(header), self.coin.header_hash(header) - if prev_hash != self.tip: + if self.tip != self.coin.header_prevhash(header): raise ChainReorg touched = set() self.fs_advance_block(header, tx_hashes, txs) - self.tip = header_hash + self.tip = self.coin.header_hash(header) self.height += 1 undo_info = self.advance_txs(tx_hashes, txs, touched) if self.daemon.cached_height() - self.height <= self.reorg_limit: @@ -636,14 +636,14 @@ class BlockProcessor(server.db.DB): touched = set() for block in blocks: header, tx_hashes, txs = self.coin.read_block(block) - prev_hash, header_hash = self.coin.header_prevhash(header), self.coin.header_hash(header) + header_hash = self.coin.header_hash(header) if header_hash != self.tip: raise ChainError('backup block {} is not tip {} at height {:,d}' .format(hash_to_str(header_hash), hash_to_str(self.tip), self.height)) self.backup_txs(tx_hashes, txs, touched) - self.tip = prev_hash + self.tip = self.coin.header_prevhash(header) assert self.height >= 0 self.height -= 1 self.tx_counts.pop() From f3ecfe00e2cda976990a1d36b4a7e172171b2b06 Mon Sep 17 00:00:00 2001 From: TheLazieR Yip Date: Fri, 25 Nov 2016 07:45:45 -0500 Subject: [PATCH 06/10] Remove unused variables --- server/block_processor.py | 1 - 1 file changed, 1 deletion(-) diff --git a/server/block_processor.py b/server/block_processor.py index eecc2eb..583dad4 100644 --- a/server/block_processor.py +++ b/server/block_processor.py @@ -568,7 +568,6 @@ class BlockProcessor(server.db.DB): # the UTXO cache uses the FS cache via get_tx_hash() to # resolve compressed key collisions header, tx_hashes, txs = self.coin.read_block(block) - prev_hash, header_hash = self.coin.header_prevhash(header), self.coin.header_hash(header) if self.tip != self.coin.header_prevhash(header): raise ChainReorg From 198fe298b7f34a0250840db5a14c3f695d169752 Mon Sep 17 00:00:00 2001 From: Neil Booth Date: Sat, 26 Nov 2016 07:14:38 +0900 Subject: [PATCH 07/10] Bump timeout to 15s. Show timeout if timed out. --- electrumx_rpc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/electrumx_rpc.py b/electrumx_rpc.py index fb6e854..052ba7c 100755 --- a/electrumx_rpc.py +++ b/electrumx_rpc.py @@ -30,7 +30,7 @@ class RPCClient(JSONRPC): message = await f except asyncio.TimeoutError: future.cancel() - print('request timed out') + print('request timed out after {}s'.format(timeout)) else: await self.handle_message(message) @@ -82,7 +82,7 @@ def main(): coro = loop.create_connection(RPCClient, 'localhost', args.port) try: transport, protocol = loop.run_until_complete(coro) - coro = protocol.send_and_wait(args.command[0], args.param, timeout=5) + coro = protocol.send_and_wait(args.command[0], args.param, timeout=15) loop.run_until_complete(coro) except OSError: print('error connecting - is ElectrumX catching up or not running?') From 59733e4609712574e0571f7cf7676b0ae973b73a Mon Sep 17 00:00:00 2001 From: Neil Booth Date: Sat, 26 Nov 2016 08:32:30 +0900 Subject: [PATCH 08/10] Move bitcoin-specific coin defaults to Bitcoin --- lib/coins.py | 19 ++++++++----------- server/db.py | 3 ++- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/coins.py b/lib/coins.py index 334299e..bc58947 100644 --- a/lib/coins.py +++ b/lib/coins.py @@ -34,14 +34,10 @@ class Coin(object): REORG_LIMIT=200 # Not sure if these are coin-specific HEADER_LEN = 80 - DEFAULT_RPC_PORT = 8332 RPC_URL_REGEX = re.compile('.+@[^:]+(:[0-9]+)?') VALUE_PER_COIN = 100000000 CHUNK_SIZE=2016 STRANGE_VERBYTE = 0xff - # IRC Defaults - IRC_PREFIX = "E_" - IRC_CHANNEL = "#electrum" IRC_SERVER = "irc.freenode.net" IRC_PORT = 6667 @@ -65,7 +61,7 @@ class Coin(object): if not match: raise CoinError('invalid daemon URL: "{}"'.format(url)) if match.groups()[0] is None: - url += ':{:d}'.format(cls.DEFAULT_RPC_PORT) + url += ':{:d}'.format(cls.RPC_PORT) if not url.startswith('http://'): url = 'http://' + url return url + '/' @@ -269,18 +265,20 @@ class Bitcoin(Coin): TX_COUNT = 142791895 TX_COUNT_HEIGHT = 420976 TX_PER_BLOCK = 1600 + IRC_PREFIX = "E_" + IRC_CHANNEL = "#electrum" + RPC_PORT = 8332 -class BitcoinTestnet(Coin): - NAME = "Bitcoin" +class BitcoinTestnet(Bitcoin): SHORTNAME = "XTN" - REORG_LIMIT = 2000 NET = "testnet" XPUB_VERBYTES = bytes.fromhex("043587cf") XPRV_VERBYTES = bytes.fromhex("04358394") P2PKH_VERBYTE = 0x6f P2SH_VERBYTE = 0xc4 WIF_BYTE = 0xef + REORG_LIMIT = 2000 # Source: pycoin and others @@ -368,7 +366,7 @@ class Dash(Coin): TX_COUNT_HEIGHT = 569399 TX_COUNT = 2157510 TX_PER_BLOCK = 4 - DEFAULT_RPC_PORT = 9998 + RPC_PORT = 9998 IRC_PREFIX = "D_" IRC_CHANNEL = "#electrum-dash" @@ -379,7 +377,6 @@ class Dash(Coin): return x11_hash.getPoWHash(header) class DashTestnet(Dash): - NAME = "Dash" SHORTNAME = "tDASH" NET = "testnet" XPUB_VERBYTES = bytes.fromhex("3a805837") @@ -392,5 +389,5 @@ class DashTestnet(Dash): TX_COUNT_HEIGHT = 101619 TX_COUNT = 132681 TX_PER_BLOCK = 1 - DEFAULT_RPC_PORT = 19998 + RPC_PORT = 19998 IRC_PREFIX = "d_" diff --git a/server/db.py b/server/db.py index 08a08e6..609537e 100644 --- a/server/db.py +++ b/server/db.py @@ -175,7 +175,8 @@ class DB(LoggedClass): headers = self.fs_read_headers(height, count) # FIXME: move to coins.py hlen = self.coin.HEADER_LEN - return [self.coin.header_hash(header) for header in chunks(headers, hlen)] + return [self.coin.header_hash(header) + for header in chunks(headers, hlen)] @staticmethod def _resolve_limit(limit): From 292073f2c7ab65a9507f25964951cb0018053afe Mon Sep 17 00:00:00 2001 From: Neil Booth Date: Sat, 26 Nov 2016 09:32:29 +0900 Subject: [PATCH 09/10] Log large requests and reject them --- docs/ENV-NOTES | 23 ++++++++++++----------- lib/jsonrpc.py | 35 +++++++++++++++++++++++++++++++---- server/env.py | 2 +- server/protocol.py | 19 +++++++++---------- 4 files changed, 53 insertions(+), 26 deletions(-) diff --git a/docs/ENV-NOTES b/docs/ENV-NOTES index 1a30a2c..c93d15e 100644 --- a/docs/ENV-NOTES +++ b/docs/ENV-NOTES @@ -44,17 +44,18 @@ in ElectrumX are very cheap - they consume about 100 bytes of memory each and are processed efficiently. I feel the defaults are low and encourage you to raise them. -MAX_HIST - maximum number of historical transactions to serve for - a single address. The current Electrum protocol requires - address histories be served en-masse or not at all, - an obvious avenue for abuse. This limit is a - stop-gap until the protocol is improved to admit - incremental history requests. The default value is - 2,000 which should be ample for most legitimate - users. Increasing to around 10,000 is likely fine - but bear in mind one client can request multiple - addresses. I welcome your experiences and suggestions - for an appropriate value. +MAX_SEND - maximum size of a response message to send over the wire, + in bytes. Defaults to 250,000. The current Electrum + protocol has a flaw in that address histories must be + served all at once or not at all, an obvious avenue for + abuse. This limit is a stop-gap until the protocol is + improved to admit incremental history requests. + Each history entry is appoximately 100 bytes so the + default is equivalent to a history limit of around 2,500 + entries, which should be ample for most legitimate + users. Increasing by a single-digit factor is likely fine + but bear in mind one client can request history for + multiple addresses. MAX_SUBS - maximum number of address subscriptions across all sessions. Defaults to 250,000. MAX_SESSION_SUBS - maximum number of address subscriptions permitted to a diff --git a/lib/jsonrpc.py b/lib/jsonrpc.py index fda25f4..ee416e0 100644 --- a/lib/jsonrpc.py +++ b/lib/jsonrpc.py @@ -72,6 +72,10 @@ class JSONRPC(asyncio.Protocol, LoggedClass): self.msg = msg self.code = code + class LargeRequestError(Exception): + '''Raised if a large request was prevented from being sent.''' + + def __init__(self): super().__init__() self.start = time.time() @@ -87,6 +91,20 @@ class JSONRPC(asyncio.Protocol, LoggedClass): self.error_count = 0 self.peer_info = None self.messages = asyncio.Queue() + # Sends longer than max_send are prevented, instead returning + # an oversized request error to other end of the network + # connection. The request causing it is logged. Values under + # 1000 are treated as 1000. + self.max_send = 0 + self.anon_logs = False + + def peername(self, *, for_log=True): + '''Return the peer name of this connection.''' + if not self.peer_info: + return 'unknown' + if for_log and self.anon_logs: + return 'xx.xx.xx.xx:xx' + return '{}:{}'.format(self.peer_info[0], self.peer_info[1]) def connection_made(self, transport): '''Handle an incoming client connection.''' @@ -175,9 +193,14 @@ class JSONRPC(asyncio.Protocol, LoggedClass): self.logger.error(msg) self.send_json_error(msg, self.INTERNAL_ERROR, payload.get('id')) else: - self.send_count += 1 - self.send_size += len(data) - self.transport.write(data) + if len(data) > max(1000, self.max_send): + self.send_json_error('request too large', self.INVALID_REQUEST, + payload.get('id')) + raise self.LargeRequestError + else: + self.send_count += 1 + self.send_size += len(data) + self.transport.write(data) async def handle_message(self, message): '''Asynchronously handle a JSON request or response. @@ -190,7 +213,11 @@ class JSONRPC(asyncio.Protocol, LoggedClass): payload = await self.single_payload(message) if payload: - self.send_json(payload) + try: + self.send_json(payload) + except self.LargeRequestError: + self.logger.warning('blocked large request from {}: {}' + .format(self.peername(), message)) async def batch_payload(self, batch): '''Return the JSON payload corresponding to a batch JSON request.''' diff --git a/server/env.py b/server/env.py index b62b8fa..4c6716e 100644 --- a/server/env.py +++ b/server/env.py @@ -45,7 +45,7 @@ class Env(LoggedClass): self.donation_address = self.default('DONATION_ADDRESS', '') self.db_engine = self.default('DB_ENGINE', 'leveldb') # Server limits to help prevent DoS - self.max_hist = self.integer('MAX_HIST', 2000) + self.max_send = self.integer('MAX_SEND', 250000) self.max_subs = self.integer('MAX_SUBS', 250000) self.max_session_subs = self.integer('MAX_SESSION_SUBS', 50000) # IRC diff --git a/server/protocol.py b/server/protocol.py index 9af9e06..0e5bbf8 100644 --- a/server/protocol.py +++ b/server/protocol.py @@ -227,6 +227,8 @@ class ServerManager(util.LoggedClass): self.max_subs = env.max_subs self.subscription_count = 0 self.futures = [] + env.max_send = max(1000, env.max_send) + self.logger.info('max response size {:,d} bytes'.format(env.max_send)) self.logger.info('max subscriptions across all sessions: {:,d}' .format(self.max_subs)) self.logger.info('max subscriptions per session: {:,d}' @@ -421,6 +423,8 @@ class Session(JSONRPC): self.coin = bp.coin self.kind = kind self.client = 'unknown' + self.anon_logs = env.anon_logs + self.max_send = env.max_send def connection_made(self, transport): '''Handle an incoming client connection.''' @@ -463,14 +467,6 @@ class Session(JSONRPC): self.logger.error('error handling request {}'.format(message)) traceback.print_exc() - def peername(self, *, for_log=True): - if not self.peer_info: - return 'unknown' - # Anonymize IP addresses that will be logged - if for_log and self.env.anon_logs: - return 'xx.xx.xx.xx:xx' - return '{}:{}'.format(self.peer_info[0], self.peer_info[1]) - def sub_count(self): return 0 @@ -674,8 +670,11 @@ class ElectrumX(Session): return self.bp.read_headers(start_height, count).hex() async def async_get_history(self, hash168): - # Apply DoS limit - limit = self.env.max_hist + # History DoS limit. Each element of history is about 99 + # bytes when encoded as JSON. This limits resource usage on + # bloated history requests, and uses a smaller divisor so + # large requests are logged before refusing them. + limit = self.max_send // 97 # Python 3.6: use async generators; update callers history = [] for item in self.bp.get_history(hash168, limit=limit): From 9544170c55d94f31ac5ecc2936eff65bed284d33 Mon Sep 17 00:00:00 2001 From: Neil Booth Date: Sat, 26 Nov 2016 09:53:47 +0900 Subject: [PATCH 10/10] Prepare release-0.7.10 --- docs/RELEASE-NOTES => RELEASE-NOTES | 7 +++++++ ACKNOWLEDGEMENTS => docs/ACKNOWLEDGEMENTS | 0 AUTHORS => docs/AUTHORS | 0 docs/ENV-NOTES | 23 +++++++++++++---------- server/protocol.py | 2 +- server/version.py | 2 +- 6 files changed, 22 insertions(+), 12 deletions(-) rename docs/RELEASE-NOTES => RELEASE-NOTES (97%) rename ACKNOWLEDGEMENTS => docs/ACKNOWLEDGEMENTS (100%) rename AUTHORS => docs/AUTHORS (100%) diff --git a/docs/RELEASE-NOTES b/RELEASE-NOTES similarity index 97% rename from docs/RELEASE-NOTES rename to RELEASE-NOTES index a793349..b1ff9c0 100644 --- a/docs/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -1,3 +1,10 @@ +version 0.7.10 +-------------- + +- replaced MAX_HIST environment variable with MAX_SEND, see docs/ENV-NOTES. + Large requests are blocked and logged. The logs should help you determine + if the requests are genuine (perhaps requiring a higher MAX_SEND) or abuse. + version 0.7.9 ------------- diff --git a/ACKNOWLEDGEMENTS b/docs/ACKNOWLEDGEMENTS similarity index 100% rename from ACKNOWLEDGEMENTS rename to docs/ACKNOWLEDGEMENTS diff --git a/AUTHORS b/docs/AUTHORS similarity index 100% rename from AUTHORS rename to docs/AUTHORS diff --git a/docs/ENV-NOTES b/docs/ENV-NOTES index c93d15e..c12bb11 100644 --- a/docs/ENV-NOTES +++ b/docs/ENV-NOTES @@ -45,17 +45,20 @@ each and are processed efficiently. I feel the defaults are low and encourage you to raise them. MAX_SEND - maximum size of a response message to send over the wire, - in bytes. Defaults to 250,000. The current Electrum - protocol has a flaw in that address histories must be - served all at once or not at all, an obvious avenue for - abuse. This limit is a stop-gap until the protocol is - improved to admit incremental history requests. - Each history entry is appoximately 100 bytes so the - default is equivalent to a history limit of around 2,500 + in bytes. Defaults to 350,000 and will treat smaller + values as the same because standard Electrum protocol + header chunk requests are nearly that large. + The Electrum protocol has a flaw in that address + histories must be served all at once or not at all, + an obvious avenue for abuse. MAX_SEND is a + stop-gap until the protocol is improved to admit + incremental history requests. Each history entry + is appoximately 100 bytes so the default is + equivalent to a history limit of around 3,500 entries, which should be ample for most legitimate - users. Increasing by a single-digit factor is likely fine - but bear in mind one client can request history for - multiple addresses. + users. Increasing by a single-digit factor is + likely fine but bear in mind one client can request + history for multiple addresses. MAX_SUBS - maximum number of address subscriptions across all sessions. Defaults to 250,000. MAX_SESSION_SUBS - maximum number of address subscriptions permitted to a diff --git a/server/protocol.py b/server/protocol.py index 0e5bbf8..b9f1884 100644 --- a/server/protocol.py +++ b/server/protocol.py @@ -227,7 +227,7 @@ class ServerManager(util.LoggedClass): self.max_subs = env.max_subs self.subscription_count = 0 self.futures = [] - env.max_send = max(1000, env.max_send) + env.max_send = max(350000, env.max_send) self.logger.info('max response size {:,d} bytes'.format(env.max_send)) self.logger.info('max subscriptions across all sessions: {:,d}' .format(self.max_subs)) diff --git a/server/version.py b/server/version.py index e2b598b..8b4837f 100644 --- a/server/version.py +++ b/server/version.py @@ -1 +1 @@ -VERSION = "ElectrumX 0.7.9" +VERSION = "ElectrumX 0.7.10"