From e5f3866f9e187102c96349a0e289508f25c5dfe2 Mon Sep 17 00:00:00 2001 From: Kevin Froman Date: Fri, 20 Dec 2019 01:24:38 -0600 Subject: [PATCH] linting refactoring communicator(utils) and reduced TOCTOU issus with online peer picking --- .../onlinepeers/clearofflinepeer.py | 28 ++++++++++------- src/communicator/onlinepeers/onlinepeers.py | 26 ++++++++-------- .../onlinepeers/pickonlinepeers.py | 30 ++++++++++++------- src/communicatorutils/announcenode.py | 13 ++++++-- .../downloadblocks/__init__.py | 11 +++++-- src/communicatorutils/lookupadders.py | 8 +++-- src/communicatorutils/lookupblocks.py | 10 ++++++- .../uploadblocks/__init__.py | 7 +++-- 8 files changed, 88 insertions(+), 45 deletions(-) diff --git a/src/communicator/onlinepeers/clearofflinepeer.py b/src/communicator/onlinepeers/clearofflinepeer.py index 31507878..eb38c4b2 100644 --- a/src/communicator/onlinepeers/clearofflinepeer.py +++ b/src/communicator/onlinepeers/clearofflinepeer.py @@ -1,9 +1,13 @@ -''' - Onionr - Private P2P Communication +"""Onionr - Private P2P Communication. - clear offline peer in a communicator instance -''' -''' +clear offline peer in a communicator instance +""" +from typing import TYPE_CHECKING + +import logger +if TYPE_CHECKING: + from communicator import OnionrCommunicatorDaemon +""" This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or @@ -16,14 +20,16 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -''' -import logger -def clear_offline_peer(comm_inst): - '''Removes the longest offline peer to retry later''' +""" + + +def clear_offline_peer(comm_inst: 'OnionrCommunicatorDaemon'): + """Remove the longest offline peer to retry later.""" try: removed = comm_inst.offlinePeers.pop(0) except IndexError: pass else: - logger.debug('Removed ' + removed + ' from offline list, will try them again.') - comm_inst.decrementThreadCount('clear_offline_peer') \ No newline at end of file + logger.debug('Removed ' + removed + + ' from offline list, will try them again.') + comm_inst.decrementThreadCount('clear_offline_peer') diff --git a/src/communicator/onlinepeers/onlinepeers.py b/src/communicator/onlinepeers/onlinepeers.py index 0dbdb271..6a786fe6 100644 --- a/src/communicator/onlinepeers/onlinepeers.py +++ b/src/communicator/onlinepeers/onlinepeers.py @@ -1,11 +1,14 @@ -""" - Onionr - Private P2P Communication +"""Onionr - Private P2P Communication. - get online peers in a communicator instance +get online peers in a communicator instance """ import time +from typing import TYPE_CHECKING + from etc import humanreadabletime import logger +if TYPE_CHECKING: + from communicator import OnionrCommunicatorDaemon """ This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -22,25 +25,25 @@ import logger """ -def get_online_peers(comm_inst): - """ - Manages the comm_inst.onlinePeers attribute list, - ]connects to more peers if we have none connected +def get_online_peers(comm_inst: 'OnionrCommunicatorDaemon'): + """Manage the comm_inst.onlinePeers attribute list. + + Connect to more peers if we have none connected """ config = comm_inst.config if config.get('general.offline_mode', False): comm_inst.decrementThreadCount('get_online_peers') return logger.debug('Refreshing peer pool...') - maxPeers = int(config.get('peers.max_connect', 10)) - needed = maxPeers - len(comm_inst.onlinePeers) + max_peers = int(config.get('peers.max_connect', 10)) + needed = max_peers - len(comm_inst.onlinePeers) last_seen = 'never' if not isinstance(comm_inst.lastNodeSeen, type(None)): last_seen = humanreadabletime.human_readable_time( comm_inst.lastNodeSeen) - for i in range(needed): + for _ in range(needed): if len(comm_inst.onlinePeers) == 0: comm_inst.connectNewPeer(useBootstrap=True) else: @@ -50,8 +53,7 @@ def get_online_peers(comm_inst): break else: if len(comm_inst.onlinePeers) == 0: - logger.debug - ('Couldn\'t connect to any peers.' + + logger.debug('Couldn\'t connect to any peers.' + f' Last node seen {last_seen} ago.') else: comm_inst.lastNodeSeen = time.time() diff --git a/src/communicator/onlinepeers/pickonlinepeers.py b/src/communicator/onlinepeers/pickonlinepeers.py index 5fdcc08e..d6f83514 100644 --- a/src/communicator/onlinepeers/pickonlinepeers.py +++ b/src/communicator/onlinepeers/pickonlinepeers.py @@ -1,9 +1,12 @@ -''' - Onionr - Private P2P Communication +""" +Onionr - Private P2P Communication. - pick online peers in a communicator instance -''' -''' +pick online peers in a communicator instance +""" +import secrets + +import onionrexceptions +""" This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or @@ -16,17 +19,22 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -''' -import secrets +""" + + def pick_online_peer(comm_inst): - '''randomly picks peer from pool without bias (using secrets module)''' + """Randomly picks peer from pool without bias (using secrets module).""" ret_data = '' + peer_length = len(comm_inst.onlinePeers) + if peer_length <= 0: + raise onionrexceptions.OnlinePeerNeeded + while True: peer_length = len(comm_inst.onlinePeers) - if peer_length <= 0: - break + try: - # get a random online peer, securely. May get stuck in loop if network is lost or if all peers in pool magically disconnect at once + # Get a random online peer, securely. + # May get stuck in loop if network is lost or if all peers in pool magically disconnect at once ret_data = comm_inst.onlinePeers[secrets.randbelow(peer_length)] except IndexError: pass diff --git a/src/communicatorutils/announcenode.py b/src/communicatorutils/announcenode.py index c4475d52..4667afc7 100755 --- a/src/communicatorutils/announcenode.py +++ b/src/communicatorutils/announcenode.py @@ -25,6 +25,8 @@ from utils import gettransports from netcontroller import NetController from communicator import onlinepeers from coredb import keydb +import onionrexceptions + def announce_node(daemon): '''Announce our node to our peers''' ret_data = False @@ -41,12 +43,17 @@ def announce_node(daemon): peer = i break else: - peer = onlinepeers.pick_online_peer(daemon) + try: + peer = onlinepeers.pick_online_peer(daemon) + except onionrexceptions.OnlinePeerNeeded: + peer = "" - for x in range(1): + for _ in range(1): try: ourID = gettransports.get()[0] - except IndexError: + if not peer: + raise onionrexceptions.OnlinePeerNeeded + except (IndexError, onionrexceptions.OnlinePeerNeeded): break url = 'http://' + peer + '/announce' diff --git a/src/communicatorutils/downloadblocks/__init__.py b/src/communicatorutils/downloadblocks/__init__.py index 1291d988..86c427d1 100755 --- a/src/communicatorutils/downloadblocks/__init__.py +++ b/src/communicatorutils/downloadblocks/__init__.py @@ -43,8 +43,7 @@ def download_blocks_from_communicator(comm_inst: "OnionrCommunicatorDaemon"): # Iterate the block queue in the communicator for blockHash in list(comm_inst.blockQueue): count += 1 - if len(comm_inst.onlinePeers) == 0: - break + triedQueuePeers = [] # List of peers we've tried for a block try: blockPeers = list(comm_inst.blockQueue[blockHash]) @@ -62,9 +61,15 @@ def download_blocks_from_communicator(comm_inst: "OnionrCommunicatorDaemon"): if blockHash in comm_inst.currentDownloading: continue + if len(comm_inst.onlinePeers) == 0: + break + comm_inst.currentDownloading.append(blockHash) # So we can avoid concurrent downloading in other threads of same block if len(blockPeers) == 0: - peerUsed = onlinepeers.pick_online_peer(comm_inst) + try: + peerUsed = onlinepeers.pick_online_peer(comm_inst) + except onionrexceptions.OnlinePeerNeeded: + continue else: blockPeers = onionrcrypto.cryptoutils.random_shuffle(blockPeers) peerUsed = blockPeers.pop(0) diff --git a/src/communicatorutils/lookupadders.py b/src/communicatorutils/lookupadders.py index 55060ea6..1f042b0b 100755 --- a/src/communicatorutils/lookupadders.py +++ b/src/communicatorutils/lookupadders.py @@ -21,6 +21,7 @@ import logger from onionrutils import stringvalidators from communicator import peeraction, onlinepeers from utils import gettransports +import onionrexceptions def lookup_new_peer_transports_with_communicator(comm_inst): logger.info('Looking up new addresses...') tryAmount = 1 @@ -32,8 +33,11 @@ def lookup_new_peer_transports_with_communicator(comm_inst): if len(newPeers) > 10000: # Don't get new peers if we have too many queued up break - peer = onlinepeers.pick_online_peer(comm_inst) - newAdders = peeraction.peer_action(comm_inst, peer, action='pex') + try: + peer = onlinepeers.pick_online_peer(comm_inst) + newAdders = peeraction.peer_action(comm_inst, peer, action='pex') + except onionrexceptions.OnlinePeerNeeded: + continue try: newPeers = newAdders.split(',') except AttributeError: diff --git a/src/communicatorutils/lookupblocks.py b/src/communicatorutils/lookupblocks.py index 57acfecf..b2e2a35c 100755 --- a/src/communicatorutils/lookupblocks.py +++ b/src/communicatorutils/lookupblocks.py @@ -17,12 +17,15 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . ''' +from gevent import time + import logger, onionrproofs from onionrutils import stringvalidators, epoch from communicator import peeraction, onlinepeers from coredb import blockmetadb from utils import reconstructhash from onionrblocks import onionrblacklist +import onionrexceptions blacklist = onionrblacklist.OnionrBlackList() def lookup_blocks_from_communicator(comm_inst): logger.info('Looking up new blocks') @@ -43,7 +46,12 @@ def lookup_blocks_from_communicator(comm_inst): if comm_inst.storage_counter.is_full(): logger.debug('Not looking up new blocks due to maximum amount of allowed disk space used') break - peer = onlinepeers.pick_online_peer(comm_inst) # select random online peer + try: + # select random online peer + peer = onlinepeers.pick_online_peer(comm_inst) + except onionrexceptions.OnlinePeerNeeded: + time.sleep(1) + continue # if we've already tried all the online peers this time around, stop if peer in triedPeers: if len(comm_inst.onlinePeers) == len(triedPeers): diff --git a/src/communicatorutils/uploadblocks/__init__.py b/src/communicatorutils/uploadblocks/__init__.py index 7d4648ac..baff0ae2 100755 --- a/src/communicatorutils/uploadblocks/__init__.py +++ b/src/communicatorutils/uploadblocks/__init__.py @@ -45,8 +45,11 @@ def upload_blocks_from_communicator(comm_inst: OnionrCommunicatorDaemon): comm_inst.decrementThreadCount(TIMER_NAME) return session = session_manager.add_session(bl) - for i in range(min(len(comm_inst.onlinePeers), 6)): - peer = onlinepeers.pick_online_peer(comm_inst) + for _ in range(min(len(comm_inst.onlinePeers), 6)): + try: + peer = onlinepeers.pick_online_peer(comm_inst) + except onionrexceptions.OnlinePeerNeeded: + continue try: session.peer_exists[peer] continue