bitshares-core
bitshares-core copied to clipboard
P2P network security
For better security,
- [ ] add option to only connect to trusted peers, so certain nodes won't be visible to other peers in the network
- [ ] add option to not advertise IP/port of certain peers or known peer list to other peers
Possible to port this feature from BitShares1-core.
More ideas are welcome.
I may not be understanding the problem this issue is trying to resolve, so please stop reading if I'm off in the weeds.
Short version: I see a quick fix as a command-line parameter that only allows connections from certain ip:port combinations. As well, another command-line parameter that prevents sharing of their peer list.
The longer (and more secure IMHO) fix would be protocol negotiation and authentication.
Long version: IPFS has the notion of protocol negotiation at connection time. Peer A can connect to B, and ask for the protocols that B speaks. At that point, it can choose to use one of those protocols or disconnect. Peer B can filter the list of protocols, so that if A selects a protocol that requires authentication, handshaking occurs, and possibly an encryption protocol is chosen. After that negotiation, either can query the other for available protocols with confidence that they know the other. Often a more liberal list of protocols is then shared, depending on the wishes of the peer.
All of the above was to get to this: If we truly want to know who's who, there should be some authentication beyond ip:port. That may be in the code somewhere, but I don't see it. I haven't looked hard, as some comments I've found indicate connections are not authenticated, so I stopped looking. I have found pieces, as node.cpp:L235 indicates peers have key pairs.
With authentication in place, peers know who they're talking to, and can discriminate. With protocol negotiation, peers can choose to degrade the channel to something they don't want, or disconnect (i.e. backward compatibility).
As for the not advertising part, it could be agreed upon during negotiation. But I can't imagine a way it could be guaranteed.
a command-line parameter that only allows connections from certain ip:port combinations.
This shoud be: ... that only allows connections to certain ip:port ...
This feature would enable a node to hide itself behind trusted peers. Although this can be done via a firewall.
another command-line parameter that prevents sharing of their peer list.
This is correct. The trusted nodes which are hiding a node behind should not let other peers know it.
After these are done, an attack wouldn't know where the real block producing node is, so would be harder if still able to attack.
This looks like something I'd like to tackle. Please let me know if someone else is already working on it.
Hey @jmjatlanta i think you can go ahead with the short version and the addition of the 2 commands. The longer version seems to require a lot of work and planning even if it seems to be a good idea we probably leave that for the future.
Here is a list of parameters that can be used in combination to hopefully achieve the desired result. I'll modify this post based on your comments:
Parameters that determine who your witness_node connects to:
- The parameter
seed-nodescan be used to specify nodes that are considered trusted. If not specified, only the internal addresses (see the 17 addresses at application.cpp:reset_p2p_node) are considered trusted. - The parameter
seed-nodecan be used to specify a node that is considered trusted. This parameter does not affect the decision made by theseed-nodesparameter mentioned above. - The (new) parameter
accept-incoming-connectionswill allow peers to request a connection to your node (Default is true. Set to false, your node will not listen for incoming connections). - The (new) parameter
disable-peer-advertisingwill respond to a request for your list of peers with an empty list.
The "accept-incoming-connections" is an existing field in the node configuration file, now accessible from the command line. The "disable-peer-advertising" is an existing field, now accessible from the command line. I am researching to see if expanding the definition of this field is a good idea, or if this is a bad idea and another should be used.
What I'm researching now: When a trusted peer drops, does the local node attempt to reconnect? Yes When the local node loses connectivity, does it starve, or attempt to reconnect to its list of nodes? Attempts reconnect Is there an existing way to tell the remote nodes "please do not include me in your advertised list of peers"? Should we worry about that? Not available. Perhaps a later, related feature What does peers.db do?
Is there an existing way to tell the remote nodes "please do not include me in your advertised list of peers"? Should we worry about that?
This is an advanced feature, which is "good to have" but not "must to have" for this issue.
Parameters that determine who can connect to your witness_node:
Should be who can connect your witness_node to. It's important that your node initializes a connection which then exposes your IP address.
- The parameter
seed-nodescan be used to specify nodes that are considered trusted. If not specified, only the internal addresses (see the 17 addresses at application.cpp:reset_p2p_node) are considered trusted.
Yes. But be aware that there is a peers.db file as well as a seed-node parameter which will be used by the node as well.
- The (new) parameter
accept-incoming-connectionswill allow peers to request a connection to your node (Default is true. Set to false, your node will not listen for incoming connections).
Although we can have this feature, I think a firewall can do the same job. Not sure which one is better though.
- The (new) parameter
disable-peer-advertisingwill not respond to a request for your list of peers.
This is needed.
Thank you for the clarifications @abitmore . I have changed the post above to include some of your clarifications.
A question: When you say "This is needed." Do you mean (a) the feature is needed, or (b) that a list of peers must be returned when a remote peer requests your list of peers? I believe you are saying (a), but wanted to be sure.
And another question: I've made the changes, and have tested it by running 3 nodes on my machine and making sure you can see when you're supposed to, and not see when you're not supposed to. Now I'd like to do this exercise in a repeatable test.
I've taken stabs at twisting the application and node classes, but with the implementation embedded in the class, I'm having difficulty calling just the methods I want and checking their results. Are there any suggestions here? I've looked at database_fixture to see if I can glean some ideas, but no success as yet.
... still digging and learning ...
Thank you.
A question: When you say "This is needed." Do you mean (a) the feature is needed, or (b) that a list of peers must be returned when a remote peer requests your list of peers? I believe you are saying (a), but wanted to be sure.
(a)
About testing, sorry I don't know if there are existing code.
@jmjatlanta I think disable-peer-advertising is not as ideal as selectively advertising. If a node disabled advertising entirely, it's easy to be detected by attackers that it's hiding something behind, so likely it will be easily targeted. On the other hand, if the node is acting as normal, aka advertising some peers but not all, then it's harder for attackers to identify if it's hiding something behind.
@abitmore What about only advertising a random number of the trusted nodes? That will exclude the node advertising itself, and give a possible attacker something to chew on.
Another question for everyone: I have made some changes that allow me to test the node_impl->on_message method. But it required some changes to node.cpp/node.hpp and some others. The changes aren't drastic, but the test is ugly. Rather than pollute the bitshares repository with a PR that has a high probability of drastically changing, I did it in my own repository. Please critique:
https://github.com/jmjatlanta/bitshares-core/pull/3
After some pondering and testing, I believe that separating the implementation from the declaration of things like node_impl (by placing the declaration in a header file) will make things easier to test. I am attempting to do so, and will propose a new PR that shows how it will work. I hope this will satisfy the requirements of (1) hide implementations and (2) make things testable.
Declarations that we don't want to typically "share" with others will be put in a header file with the extension .hxx. Such header files will exist in the source code directory instead of the include directory.
I think it's ok to use .hpp as extension, just need to put it in source directory.
@jmjatlanta Please take a look at this branch https://github.com/bitshares/bitshares-core/tree/network_params
@abitmore Thank you for the find. That certainly would open up configuring the p2p configuration from the command line.
The advertising question: What about another parameter that is advertise-peer-algorithm and let the user choose between nothing, random, list and all? Other algos could be added later.
- nothing would do what disable-peer-advertising was going to do.
- random would provide a random number of random nodes.
- list would send a command-line-provided list of nodes
- all would send everything, and be the default.
I see this solution as the hardest to implement, but the most comprehensive. Am I overengineering?
@jmjatlanta I think it's a good idea, so it's good to do it at the correct time (I don't know whether now is the best time).
A draft specification. Please comment with any questions, holes, opinions, etc.:
To help with configuring the security of BitShares nodes, the following features have been added to the reference implementation of witness_node:
Trusted Connections
Node operators may want control of which nodes they are connected to. Therefore, parameters have been added to modify when nodes interconnect.
accept-incoming-connections(default oftrue) will allow peers to request a connection to your node. Setting to false is the software equivalent to firewalling your incoming connection port.accept-connection-suggestions(default oftrue) will add peers suggested by others to the list of possible connections. Such suggestions could become connected nodes if the number of active connections falls below the number desired. If set to false, suggestions sent by peers are ignored.use-connection-database(default oftrue) will use the stored database to attempt peer connections if the number of connected peers falls below the desired threshold.seed-nodewill define the URL you pass in as a trusted node.seed-nodesworks the same asseed-node, but accepts a collection instead of a single URL.
The Peer Database
Some of the options above affect the existing peer database that is stored by the node.
- If
accept-incoming-connectionsis false, (obviously) any rejected connection will not be added to the database. - If
accept-connection-suggestionsis false, (obviously) any suggested connection will not be added to the database. - If
use-connection-databaseis false, any incoming or suggested connections will not be added to the database.
seed-node(s), use-connection-database, and Reconnections
If use-connection-database is set to false, and the number of connected peers fall below the desired threshold, reconnects only to the seed nodes will be attempted.
Advertising Connections
Node operators may want control of which nodes are advertised when a peer asks for their peer list. The following parameters have been added to provide control:
advertise-peer-algorithmdetermines how peers are selected to be advertised. The options are:all(the default) which will list all connected nodesnothingwhich will respond to the caller with an empty listlistwhich will respond with a previously defined list ( seeadvertise-peer-list)exclude-listwhich will respond with all connected nodes except those previously defined ( seeexclude-peer-list)
advertise-peer-listwill be the list returned when thelistalgorithm above is enabled, and peers ask for your list.exclude-peer-listwill be the peers removed from the returned list when theexclude-listalgorithm is enabled and peers ask for your list.
Done via #1764.