lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.15k stars 364 forks source link

DoS protection in PeerHandler #383

Open ariard opened 4 years ago

ariard commented 4 years ago

Was thinking we may need some DoS protection at the PeerHandler, like some noisy peer sending too much ping msgs or unknown message types. May need to take some Duration in do_read_event.

TheBlueMatt commented 4 years ago

Why? The Correct (tm) approach to DoS handling is:

Problem solved. Now, we could add something to disconnect useless peers (ie incoming connections which are not opening channels with us), as long as we have a configuration option for it, but I don’t see any material DoS concern atm?

On Oct 23, 2019, at 12:54, Antoine Riard notifications@github.com wrote:

 Was thinking we may need some DoS protection at the PeerHandler, like some noisy peer sending too much ping msgs or unknown message types. May need to take some Duration in do_read_event.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ariard commented 4 years ago
  • make sure all responses to trivial-to-generate messages are roughly smaller than the original message,

What's about me sending you a never-stopping flow of ping messages ? Beyond ensuring responses to be smaller, shouldn't we check also the frequency ? Do we already have check like that in PeerHandler ?

Seems to be an advice in BOLT 1 at least: "SHOULD fail the channels if it has received significantly in excess of one ping per 30 seconds."

TheBlueMatt commented 4 years ago

For mobile it may make sense to measure bytes received / utility gained (ie how many bytes are they sending us per htlc-addition/-removal/routing graph update) and disconnect if the peer is being too chatty, but, in general, trying to play whack-a-mole with "DoS" issues where someone is just sending you a lot of stuff is error-prone and also not really all that useful. If we want to go down that route, however, we should have some API to limit bandwidth consumption, because that's really what we're talking about (eg on mobile devices where you don't want to use more bandwidth than the user is willing to pay for).