ChainSafe / forest

🌲 Rust Filecoin Node Implementation
https://forest.chainsafe.io
Apache License 2.0
637 stars 157 forks source link

Improve dagcbor decoding flow in libp2p `Hello` and `ChainExchange` protocols #2366

Open hanabi1224 opened 1 year ago

hanabi1224 commented 1 year ago

Issue summary

Currently, we're not decoding from a stream, and due to current lotus implementation, the stream might not be signaled as fully sent (more specifically, requests of Hello and ChainExchange are sent without end-of-stream signal before we can decode), so we're not able to read the entire stream before sending it to the decoder. Current flow is

  1. create a buffer for decoding
  2. try reading 8*1024 bytes from the stream, append to the buffer in step 1
  3. try decode buffer in step 1, if success with no trailing data left, it's considered successful and will terminate the loop. otherwise go back to step 2

With this flow, there're a few issues.

  1. Vulnerable to attacks. If some malicious nodes send random bytes, current logic is not capable of detecting it early, it will try reading more data from the input stream and cause deadlock. While Lotus can detect this with the head bytes.
  2. Many unnecessary decoding operations as described above for detecting the end of the message, while Lotus can do stream decoding

Other information and links

https://github.com/filecoin-project/lotus/pull/9892 https://github.com/ipld/serde_ipld_dagcbor/issues/5

LesnyRumcajs commented 6 months ago

@hanabi1224 what do we do with this issue?