diff --git a/jmclient/jmclient/configure.py b/jmclient/jmclient/configure.py index 9f1f9f8..fbc6413 100644 --- a/jmclient/jmclient/configure.py +++ b/jmclient/jmclient/configure.py @@ -251,6 +251,15 @@ tx_broadcast = self # after a timeout. minimum_makers = 4 +# Threshold number of satoshis below which an incoming utxo +# to a reused address in the wallet will be AUTOMATICALLY frozen. +# This avoids forced address reuse attacks; see: +# https://en.bitcoin.it/wiki/Privacy#Forced_address_reuse +# +# The default is to ALWAYS freeze a utxo to an already used address, +# whatever the value of it, and this is set with the value -1. +max_sats_freeze_reuse = -1 + ############################## #THE FOLLOWING SETTINGS ARE REQUIRED TO DEFEND AGAINST SNOOPERS. #DON'T ALTER THEM UNLESS YOU UNDERSTAND THE IMPLICATIONS. diff --git a/jmclient/jmclient/wallet_service.py b/jmclient/jmclient/wallet_service.py index 3667a1b..f24b197 100644 --- a/jmclient/jmclient/wallet_service.py +++ b/jmclient/jmclient/wallet_service.py @@ -59,6 +59,8 @@ class WalletService(Service): # to ensure transactions are only logged once: self.logged_txids = [] + self.set_autofreeze_warning_cb() + def update_blockheight(self): """ Can be called manually (on startup, or for tests) but will be called as part of main monitoring @@ -173,6 +175,54 @@ class WalletService(Service): self.bci.import_addresses([address], self.EXTERNAL_WALLET_LABEL, restart_cb=self.restart_callback) + def default_autofreeze_warning_cb(self, utxostr): + jlog.warning("WARNING: new utxo has been automatically " + "frozen to prevent forced address reuse: ") + jlog.warning(utxostr) + jlog.warning("You can unfreeze this utxo with the method " + "'freeze' of wallet-tool.py or the Coins tab " + "of Joinmarket-Qt.") + + def set_autofreeze_warning_cb(self, cb=None): + """ This callback takes a single argument, the + string representation of a utxo in form txid:index, + and informs the user that the utxo has been frozen. + It returns nothing (the user is not deciding in this case, + as the decision was already taken by the configuration). + """ + if cb is None: + self.autofreeze_warning_cb = self.default_autofreeze_warning_cb + else: + self.autofreeze_warning_cb = cb + + def check_for_reuse(self, added_utxos): + """ (a) Check if addresses in new utxos are already in + used address list, (b) record new addresses as now used + (c) disable the new utxo if it returned as true for (a), + and it passes the filter set in the configuration. + """ + to_be_frozen = set() + for au in added_utxos: + if added_utxos[au]["address"] in self.used_addresses: + to_be_frozen.add(au) + # any utxos actually added must have their destination address + # added to the used address list for this program run: + for au in added_utxos.values(): + self.used_addresses.add(au["address"]) + + # disable those that passed the first check, before the addition, + # if they satisfy configured logic + for utxo in to_be_frozen: + freeze_threshold = jm_single().config.getint("POLICY", + "max_sats_freeze_reuse") + if freeze_threshold == -1 or added_utxos[ + utxo]["value"] <= freeze_threshold: + # freezing of coins must be communicated to user: + self.autofreeze_warning_cb(utxo) + # process_new_tx returns added utxos in str format: + txidstr, idx = utxo.split(":") + self.disable_utxo(binascii.unhexlify(txidstr), int(idx)) + def transaction_monitor(self): """Keeps track of any changes in the wallet (new transactions). Intended to be run as a twisted task.LoopingCall so that this @@ -216,6 +266,10 @@ class WalletService(Service): if txd is None: continue removed_utxos, added_utxos = self.wallet.process_new_tx(txd, txid, height) + + # apply checks to disable/freeze utxos to reused addrs if needed: + self.check_for_reuse(added_utxos) + # TODO note that this log message will be missed if confirmation # is absurdly fast, this is considered acceptable compared with # additional complexity. @@ -350,17 +404,22 @@ class WalletService(Service): can just list all used addresses to find the right index values. """ - self.get_address_usages() + self.sync_addresses_fast() self.sync_unspent() + def has_address_been_used(self, address): + """ Once wallet has been synced, the set of used + addresses includes those identified at sync time, + plus any used during operation. This is stored in + the WalletService object as self.used_addresses. + """ + return address in self.used_addresses + def get_address_usages(self): - """Use rpc `listaddressgroupings` to locate all used - addresses in the account (whether spent or unspent outputs). - This will not result in a full sync if working with a new - Bitcoin Core instance, in which case "fast" should have been - specifically disabled by the user. + """ sets, at time of sync, the list of addresses that + have been used in our Core wallet with the specific label + for our JM wallet. This operation is generally immediate. """ - wallet_name = self.get_wallet_name() agd = self.bci.rpc('listaddressgroupings', []) # flatten all groups into a single list; then, remove duplicates fagd = (tuple(item) for sublist in agd for item in sublist) @@ -368,10 +427,23 @@ class WalletService(Service): dfagd = set(fagd) used_addresses = set() for addr_info in dfagd: - if len(addr_info) < 3 or addr_info[2] != wallet_name: + if len(addr_info) < 3 or addr_info[2] != self.get_wallet_name(): continue used_addresses.add(addr_info[0]) + self.used_addresses = used_addresses + def sync_addresses_fast(self): + """Locates all used addresses in the account (whether spent or + unspent outputs), and then, assuming that all such usages must be + related to our wallet, calculates the correct wallet indices and + does any needed imports. + + This will not result in a full sync if working with a new + Bitcoin Core instance, in which case "recoversync" should have + been specifically chosen by the user. + """ + wallet_name = self.get_wallet_name() + used_addresses = self.get_address_usages() # for a first run, import first chunk if not used_addresses: jlog.info("Detected new wallet, performing initial import") diff --git a/jmclient/test/test_walletservice.py b/jmclient/test/test_walletservice.py new file mode 100644 index 0000000..8858568 --- /dev/null +++ b/jmclient/test/test_walletservice.py @@ -0,0 +1,73 @@ +from __future__ import (absolute_import, division, + print_function, unicode_literals) +from builtins import * # noqa: F401 +'''Tests of functionality at walletservice layer.''' + +import os +import pytest +from jmbase import get_log +from jmclient import load_program_config, jm_single, \ + WalletService +from test_blockchaininterface import sync_test_wallet +from test_wallet import fund_wallet_addr, get_populated_wallet +testdir = os.path.dirname(os.path.realpath(__file__)) +log = get_log() + +def set_freeze_reuse_config(x): + jm_single().config.set('POLICY', 'max_sats_freeze_reuse', str(x)) + +def try_address_reuse(wallet_service, idx, funding_amt, config_threshold, + expected_final_balance): + set_freeze_reuse_config(config_threshold) + # check that below the threshold on the same address is not allowed: + fund_wallet_addr(wallet_service.wallet, wallet_service.get_addr(0, 0, idx), + value_btc=funding_amt) + wallet_service.transaction_monitor() + balances = wallet_service.get_balance_by_mixdepth() + assert balances[0] == expected_final_balance + +def test_address_reuse_freezing(setup_walletservice): + """ Creates a WalletService on a pre-populated wallet, + and sets different values of the config var + 'max_sats_freeze_reuse' then adds utxos to different + already used addresses to check that they are frozen or + not as appropriate. + Note that to avoid a twisted main loop the WalletService is + not actually started, but the transaction_monitor is triggered + manually (which executes the same code). + A custom test version of the reuse warning callback is added + and to check correct function, we check that this callback is + called, and that the balance available in the mixdepth correctly + reflects the usage pattern and freeze policy. + """ + amount = 10**8 + num_tx = 3 + cb_called = 0 + def reuse_callback(utxostr): + nonlocal cb_called + print("Address reuse freezing callback on utxo: ", utxostr) + cb_called += 1 + wallet = get_populated_wallet(amount, num_tx) + wallet_service = WalletService(wallet) + wallet_service.set_autofreeze_warning_cb(reuse_callback) + sync_test_wallet(True, wallet_service) + wallet_service.transaction_monitor() + try_address_reuse(wallet_service, 0, 1, -1, 3 * 10**8) + assert cb_called == 1, "Failed to trigger freeze callback" + # check that above the threshold is allowed (1 sat less than funding) + try_address_reuse(wallet_service, 1, 1, 99999999, 4 * 10**8) + assert cb_called == 1, "Incorrectly triggered freeze callback" + # check that below the threshold on the same address is not allowed: + try_address_reuse(wallet_service, 1, 0.99999998, 99999999, 4 * 10**8) + # note can be more than 1 extra call here, somewhat suboptimal: + assert cb_called > 1, "Failed to trigger freeze callback" + + +@pytest.fixture(scope='module') +def setup_walletservice(request): + load_program_config() + old_reuse_freeze_val = jm_single().config.getint("POLICY", + "max_sats_freeze_reuse") + def reset_config(): + set_freeze_reuse_config(old_reuse_freeze_val) + request.addfinalizer(reset_config) diff --git a/scripts/joinmarket-qt.py b/scripts/joinmarket-qt.py index 4c03faf..516cb57 100644 --- a/scripts/joinmarket-qt.py +++ b/scripts/joinmarket-qt.py @@ -1513,6 +1513,18 @@ class JMMainWindow(QMainWindow): mn_extension = pp_field.text() return message_e.toPlainText(), mn_extension + def autofreeze_warning_cb(self, utxostr): + """ Handles coins sent to reused addresses, + preventing forced address reuse, according to value of + POLICY setting `max_sats_freeze_reuse` (see + WalletService.check_for_reuse()). + """ + msg = "New utxo has been automatically " +\ + "frozen to prevent forced address reuse:\n" + utxostr +\ + "\n You can unfreeze this utxo via the Coins tab." + JMQtMessageBox(self, msg, mbtype='info', + title="New utxo frozen") + def restartWithMsg(self, msg): JMQtMessageBox(self, msg, mbtype='info', title="Restart") @@ -1603,7 +1615,10 @@ class JMMainWindow(QMainWindow): self.walletRefresh.stop() self.wallet_service = WalletService(wallet) + # add information callbacks: self.wallet_service.add_restart_callback(self.restartWithMsg) + self.wallet_service.autofreeze_warning_cb = self.autofreeze_warning_cb + self.wallet_service.startService() self.walletRefresh = task.LoopingCall(self.updateWalletInfo) self.walletRefresh.start(5.0) diff --git a/test/regtest_joinmarket.cfg b/test/regtest_joinmarket.cfg index 543fac5..366b5af 100644 --- a/test/regtest_joinmarket.cfg +++ b/test/regtest_joinmarket.cfg @@ -50,3 +50,5 @@ accept_commitment_broadcasts = 1 taker_utxo_retries = 3 minimum_makers = 1 listunspent_args = [0] +max_sats_freeze_reuse = -1 +