Merge #471: Auto-freezing of address reuse below a threshold.

d719ff2 Auto-freezing of address reuse below a threshold. (Adam Gibson)

jmclient/jmclient/configure.py

@@ -252,6 +252,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.

jmclient/jmclient/wallet_service.py

@@ -61,6 +61,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
@@ -175,6 +177,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
@@ -218,6 +268,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.
@@ -354,17 +408,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
+        """ sets, at time of sync, the list of addresses that
-        addresses in the account (whether spent or unspent outputs).
+        have been used in our Core wallet with the specific label
-        This will not result in a full sync if working with a new
+        for our JM wallet. This operation is generally immediate.
-        Bitcoin Core instance, in which case "fast" should have been
-        specifically disabled by the user.
         """
-        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)
@@ -372,10 +431,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")

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)

scripts/joinmarket-qt.py

@@ -1515,6 +1515,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")
@@ -1605,7 +1617,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)

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
+