Added new RPC method for explicitly banning nodes or removing a ban. #2906

pull CodeShark wants to merge 1 commits into bitcoin:master from CodeShark:bannode_rpc changing 5 files +63 −2

CodeShark commented at 8:34 PM on August 17, 2013: contributor

Makes it possible to test nodes against bitcoind and to test bitcoind's misbehavior detection without having to constantly restart bitcoind.
wtogami commented at 8:56 PM on August 20, 2013: contributor

This is highly desirable.
jgarzik commented at 2:39 AM on August 25, 2013: contributor

ACK. Please collapse last commit into previous one.
wtogami commented at 10:20 AM on August 25, 2013: contributor

ACK after the aforementioned squash.
in src/rpcnet.cpp:None in 4e07432936 outdated
```
 209 | +        throw runtime_error(
 210 | +            "bannode <node> <expiration>\n"
 211 | +            "Bans <node> until unix timestamp <expiration>. Set <expiration> to -1 to unban a node.");
 212 | +
 213 | +    std::vector<CNetAddr> addrs;
 214 | +    if (!LookupHost(params[0].get_str().c_str(), addrs, 50, true)) 
```
sipa commented at 3:10 PM on August 25, 2013:

I don't really like doing a network lookup for this, domain name changes during execution of the program are possible.

CNode already stores the name through which we connected to a peer (CNode::addrName) and the IP address as a fallback (CNode::addr); we should be doing lookups based on this.

CodeShark commented at 8:45 AM on August 28, 2013:

On the other hand, perhaps you want to ban or unban an entire domain (all entries presently in the DNS tables). It would still be an issue if an IP address had been reassigned to another domain and amazingly it also happens to be running a bitcoin node, but it seems this situation would be rather rare unless both domains are owned by the same entity. Then there's also the issue of IPv4 vs. IPv6 and, specifically, IPv4-mapped IPv6 addresses.

Anyhow, this pull request wasn't really intended to be a complete solution to what is ultimately a far deeper set of issues. I wouldn't want to rely on this feature for actual blacklisting of malicious nodes. It was mostly meant as a quick way to test code (both in bitcoind as well as other implementations) - and in these types of applications one generally would know what IP addresses are under consideration or would be aware of changes to DNS tables.

CodeShark commented at 8:51 AM on August 28, 2013:

I do see and acknowledge your point, though, sipa.
wtogami commented at 7:21 AM on September 10, 2013: contributor

What do we need for this to move forward?
gavinandresen commented at 11:14 PM on September 16, 2013: contributor

Either the help should make it clear that the banned list is cleared on restart, or the banned list should be saved at clean shutdown and read in at startup (either is OK with me, saving/restoring could be a future pull request).

Added new RPC method for explicitly banning nodes or removing a ban.

Added listbannednodes.

Return copy of banned node map instead of reference to avoid race conditions.

562a909aa6

BitcoinPullTester commented at 12:31 AM on September 17, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/562a909aa6cd909a67f58a47e3854039aa6a5408 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
wtogami commented at 6:10 AM on October 14, 2013: contributor

Is this even working? I see it add IP addresses to listbannednode, but I now realize that the IP address remains connected.
gavinandresen commented at 2:20 AM on October 21, 2013: contributor

Needs rebase, test plan, and somebody to run through the test plan to make sure it works for somebody other than CodeShark.
gavinandresen commented at 3:34 AM on December 10, 2013: contributor

Closing due to inactivity; it's been two months.
gavinandresen closed this on Dec 10, 2013
DrahtBot locked this on Sep 8, 2021

Contributors