From 5efcb772838e404ca5757818d5548efcb872724b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 5 Jun 2019 11:02:35 -0400 Subject: [PATCH 1/3] Disable bloom filtering by default. BIP 37 bloom filters have been well-known to be a significant DoS target for some time. However, in order to provide continuity for SPV clients relying on it, the NODE_BLOOM service flag was added, and left as a default, to ensure sufficient nodes exist with such a flag. NODE_BLOOM is, at this point, well-established and, as long as there exist 0.18 nodes with default config (which I'd anticipate will be true for many years), will be available from some peers. By that time, the continued slowdown of BIP 37-based filtering will likely have rendered it useless (though this is already largely the case). Further, BIP 37 was deliberately never updated to support witness-based filtering as newer wallets are expected to migrate to some yet-to-be-network-exposed filters. --- src/validation.h | 2 +- test/functional/p2p_node_network_limited.py | 6 +++--- test/functional/test_framework/messages.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/validation.h b/src/validation.h index 963439d35..31233aac1 100644 --- a/src/validation.h +++ b/src/validation.h @@ -127,7 +127,7 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; /** Maximum number of unconnecting headers announcements before DoS score */ static const int MAX_UNCONNECTING_HEADERS = 10; -static const bool DEFAULT_PEERBLOOMFILTERS = true; +static const bool DEFAULT_PEERBLOOMFILTERS = false; /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index 573d5f5a5..a4650df8e 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -8,7 +8,7 @@ Tests that a node configured with -prune=550 signals NODE_NETWORK_LIMITED correc and that it responds to getdata requests for blocks correctly: - send a block within 288 + 2 of the tip - disconnect peers who request blocks older than that.""" -from test_framework.messages import CInv, msg_getdata, msg_verack, NODE_BLOOM, NODE_NETWORK_LIMITED, NODE_WITNESS +from test_framework.messages import CInv, msg_getdata, msg_verack, NODE_NETWORK_LIMITED, NODE_WITNESS from test_framework.mininode import P2PInterface, mininode_lock from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -55,7 +55,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): def run_test(self): node = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) - expected_services = NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED + expected_services = NODE_WITNESS | NODE_NETWORK_LIMITED self.log.info("Check that node has signalled expected services.") assert_equal(node.nServices, expected_services) @@ -83,7 +83,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): node1.wait_for_addr() #must relay address with NODE_NETWORK_LIMITED - assert_equal(node1.firstAddrnServices, 1036) + assert_equal(node1.firstAddrnServices, expected_services) self.nodes[0].disconnect_p2ps() node1.wait_for_disconnect() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 954ae3c4d..2ab9c3757 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -44,7 +44,7 @@ BIP125_SEQUENCE_NUMBER = 0xfffffffd # Sequence number that is BIP 125 opt-in an NODE_NETWORK = (1 << 0) # NODE_GETUTXO = (1 << 1) -NODE_BLOOM = (1 << 2) +# NODE_BLOOM = (1 << 2) NODE_WITNESS = (1 << 3) NODE_NETWORK_LIMITED = (1 << 10) From f27309f55c4fa2b115525d72abb280757a568709 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 6 Jun 2019 10:01:06 -0400 Subject: [PATCH 2/3] Move DEFAULT_PEERBLOOMFILTERS from validation.h to net_processing.h --- src/net_processing.h | 1 + src/validation.h | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net_processing.h b/src/net_processing.h index 39c22d711..dffc3f273 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -19,6 +19,7 @@ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; /** Default for BIP61 (sending reject messages) */ static constexpr bool DEFAULT_ENABLE_BIP61{false}; +static const bool DEFAULT_PEERBLOOMFILTERS = false; class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { private: diff --git a/src/validation.h b/src/validation.h index 31233aac1..0c19d9493 100644 --- a/src/validation.h +++ b/src/validation.h @@ -127,8 +127,6 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; /** Maximum number of unconnecting headers announcements before DoS score */ static const int MAX_UNCONNECTING_HEADERS = 10; -static const bool DEFAULT_PEERBLOOMFILTERS = false; - /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; From bead32e31e399090af30b2ee3539995d4105a66d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 8 Jun 2019 09:19:47 -0400 Subject: [PATCH 3/3] Add release notes for DEFAULT_BLOOM change --- doc/release-notes/release-notes-16152.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 doc/release-notes/release-notes-16152.md diff --git a/doc/release-notes/release-notes-16152.md b/doc/release-notes/release-notes-16152.md new file mode 100644 index 000000000..9c77cb9ae --- /dev/null +++ b/doc/release-notes/release-notes-16152.md @@ -0,0 +1,7 @@ +P2P Changes +----------- +- The default value for the -peerbloomfilters configuration option (and, thus, NODE_BLOOM support) has been changed to false. + This resolves well-known DoS vectors in Bitcoin Core, especially for nodes with spinning disks. It is not anticipated that + this will result in a significant lack of availability of NODE_BLOOM-enabled nodes in the coming years, however, clients + which rely on the availability of NODE_BLOOM-supporting nodes on the P2P network should consider the process of migrating + to a more modern (and less trustful and privacy-violating) alternative over the coming years.