feat(p2p): blacklist and whitelist #883
Conversation
This commit add a feature to block incoming connections according to the blacklist or whitelist IP addresses. The following topics needs to be discussed - [ ] Persistence for such blacklist or whitelist - [ ] Add IP addresses into blacklist or whitelist dynamically through RPC calls Resolve #458 Resolve #179
ghost
assigned 
There was a problem hiding this comment.
blacklist: [],
whitelist: [],
I don't think that's the right way to go in the config. If I insert the same PubKey into both arrays, how will xud behave - connect or not? ;) I believe the config should assume blacklist as default mode and whitelist mode with a true/false - like this both modes are mutually exclusive and xud either cares about the whitelist: string[]; OR the blacklist: string[];.
|
@kilrau This PR dose not extend original |
|
|
||
| /** | ||
| * An array of IP addresses or host names which can be used as a whitelist to limit connecting peers. | ||
| * Is will bypass all peers when it is empty. |
There was a problem hiding this comment.
"It" instead of "Is". Also I think it's a bit more clear to say "It will allow all IP addresses when it is empty."
| /** | ||
| * An array of IP addresses or host names which can be used as a blacklist to limit connecting peers. | ||
| */ | ||
| blacklist: string[]; |
There was a problem hiding this comment.
Conceptually, I wonder whether we need a configurable blacklist of IP addresses within xud. For one thing, I believe it would be much more efficient to block IPs before they even get to this point, banning them within the OS rather than within our app. When I wrote #458, I was thinking of banning IPs dynamically while xud is running if we get a lot of bad requests from them.
But in general, I don't think any sort of fixed list will be all that effective since attackers can easily change IP addresses. My thinking is we should drop this config option altogether. Thoughts @kilrau ?
| this.blacklist.push(addressString); | ||
| }); | ||
| config.whitelist.forEach((addressString) => { | ||
| this.whitelist.push(addressString); |
There was a problem hiding this comment.
No need to loop through and push every element into a new array, you can just do this.whitelist = config.whitelist;.
|
I agree that we should not simultaneously have a blacklist and whitelist active, but I think actually it doesn't make sense to have a blacklist period. See my comment above. |
Agree - I believe we can drop the blacklist altogether. I don't see a great use case for a manual ip/domain based blacklist. Please make this PR about a white-list only.
Agree too. This should be handled automatically in the background at run-time, totally differend from what this PR intends to do. Please remove
|
|
As discussed with @reliveyy , there will be a major revision of the PR after agreeing on these basics. |
This commit add a feature to block incoming connections according to the blacklist or whitelist IP addresses.
The following topics needs to be discussed
xucli ban <pubkey>withxucli ban <ip>and persist that IP address in blacklist.Resolve #458
Resolve #179