Venus: [bug] Flood of spam messages crashes nodes without limits on mpool size and content

Created on 4 Apr 2019  Â·  11Comments  Â·  Source: filecoin-project/venus

Describe the bug
All of the filecoin nodes on the user devnet are being flooded by very long messages, causing a restart loop.
The messages looks like:

{"timestamp":"2019-04-04T20:03:42.373777769Z","level":"DEBUG","system":"node","message":"Received new message from network: SignedMessage cid=[zDPWYqFCuh6nzD5Mstz3$9ybjScSz1zCkKzYyQEN2HNprhjAokz2]: {\n  \"meteredMessage\": {\n    \"message\": {\n      \"to\": \"t1aadt4eb5qsbkfgwpus65l2vau5yqrf3vxmanjba\",\n      \"from\": \"t$kkqtsilhxrgrpw6l5gk56ou6lw24t4bly2k4syq\",\n      \"nonce\": \"30062\",\n      \"value\": \"0\",\n      \"method\": \"HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJer$my:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:$HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJ$romy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJerom$:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)H$Jeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)HiJeromy:)
C-bug P0

Most helpful comment

So, the crux of the issue is that you don't have a limit on the message pool. Time limit is not enough, when transaction throughput picks up, you can get overwhelmed in any window. You also need to cater for SPAM transaction's whose sole purpose is to bloat the nodes. A special case here is the 0-price transaction that doesn't even cost anything and doesn't even require an existing account.

In Ethereum we have a multitude of protections on the txpool, ranging from limiting the size of a single transaction, to limiting the number of executable (correct nonce order) transactions, limiting the number of non-executable (nonce gap) transactions, limiting the number of senders. It gets even more complex when you want to ensure users have a minimum number of slots to cater for random network propagation. Also you want to special case local transactions that should never be flushed.

My 2c. HiJeromy:)

All 11 comments

@ZenGround0 we need an update to the spec to define a maximum for Params and Method. We should also identify other areas in the protocol that have unbounded values such as the number of messages in a block.

/cc @whyrusleeping

So, the crux of the issue is that you don't have a limit on the message pool. Time limit is not enough, when transaction throughput picks up, you can get overwhelmed in any window. You also need to cater for SPAM transaction's whose sole purpose is to bloat the nodes. A special case here is the 0-price transaction that doesn't even cost anything and doesn't even require an existing account.

In Ethereum we have a multitude of protections on the txpool, ranging from limiting the size of a single transaction, to limiting the number of executable (correct nonce order) transactions, limiting the number of non-executable (nonce gap) transactions, limiting the number of senders. It gets even more complex when you want to ensure users have a minimum number of slots to cater for random network propagation. Also you want to special case local transactions that should never be flushed.

My 2c. HiJeromy:)

@karalabe Thanks! Do you have links to relevant code in geth?

I propose we split our response to this into at least two parts:

  1. a limit on mpool size that will prevent nodes from crashing (even if it still allows a spammer to fill up the pool)
  2. more sophisticated limits on particular message properties to promote network progress in the
    face of spam

I think we can scope this specific issue to (1) to stop the crashes, and defer (2) to another issue, which will require deeper research and thought.

I have opened an issue to address 1.: https://github.com/filecoin-project/go-filecoin/issues/2517

@anorth

which will require deeper research and thought.

I don't think it does actually, we should follow the same algorithms used by ethereum. Our model is nearly identical to theirs, and the same DoS prevention mechanisms they use apply to us.

I meant deeper research and thought than a single limit on the number of
messages. I agree that that research is mostly just understanding what
Ethereum does.

On Tue, 9 Apr 2019 at 05:58, Whyrusleeping notifications@github.com wrote:

@anorth https://github.com/anorth

which will require deeper research and thought.

I don't think it does actually, we should follow the same algorithms used
by ethereum. Our model is nearly identical to theirs, and the same DoS
prevention mechanisms they use apply to us.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/filecoin-project/go-filecoin/issues/2500#issuecomment-480983570,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAbLetrO1AwWGELCy0bjRzeKrleLsLnSks5ve59JgaJpZM4cdnj2
.

@acruikshank could you comment on the status of this? I believe we have done lots of the low-hanging work here, but there may be some more detail required. If so, could we outline it here so that it can be prioritised?

@anortth Much of the pre-validation mentioned above is addressed in #2522 with some follow on work in #2517 and #2546. Since #2523 we require the gas price be greater than zero.

The biggest outstanding issue is #2525 to limit the size of messages entering the message pool. Since a considerable amount of processing occurs prior to the pre-validation, it would be best if long messages could be turned away before deserialization.

Thanks. Since we have an issue for that, I'll close this issue as it's no longer in progress.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

travisperson picture travisperson  Â·  9Comments

mishmosh picture mishmosh  Â·  9Comments

ZenGround0 picture ZenGround0  Â·  5Comments

rosalinekarr picture rosalinekarr  Â·  7Comments

Aboatlai picture Aboatlai  Â·  4Comments