retrieve: block unsolicited chunk deliveries #1875
Conversation
acud
force-pushed
the
retrieve-block-unsolicited
branch
3 times, most recently
from
2dfbae7
to
b5c9e56
Compare
acud
changed the title
acud
force-pushed
the
retrieve-block-unsolicited
branch
from
b5c9e56
to
5959975
Compare
|
@acud some of network tests are failing, probably because of the new protocol version. |
|
tagging @janos for this question as well: is it impossible for a node to request 2 chunks from the same address parallelly enough so that both requests are sent out before a chunk delivery message arrives for any one of them? |
@mortelli yes, it is possible. Retrieval protocol is handling messages asynchronously. |
Thanks. While we're at it... what does |
Yes, that is correct, request (or retrieval) unique id. There is no check for uniqueness, but for short lived requests as we have here, random int32 would be fine, I suppose. |
why on earth do we have tests that depend on the protocol version......... |
There was a problem hiding this comment.
I left two comments about security.
- How random is the random number?
- Is there a possibility of sniffing on outgoing messages from a peer and thus figuring out his current outstanding requests.
It is important that this is absolutely watertight, as the rewards from hacking this is up to the total liquid balance currently outstanding in the network => high.
| err = protoPeer.Send(ctx, ret) | ||
| if err != nil { | ||
| protoPeer.logger.Error("error sending retrieve request to peer", "err", err) | ||
| protoPeer.logger.Error("error sending retrieve request to peer", "ruid", ret.Ruid, "err", err) |
There was a problem hiding this comment.
are you ok with logging and returning the error here?
There was a problem hiding this comment.
well, if i don't log the error here i lose the context of the peer address which is appended to the log line (since we are using the peer logger here)
There was a problem hiding this comment.
I know that I am a bit late, but we should use error annotations instead both logging them and returning to the caller.
This PR adds a
Ruidto theretrieveprotocol messages in order to block unsolicited chunk deliveries (which is an attack vector on several swarm properties)fixes #1800