Clarify use of E_DIAL_REFUSED in autonat #430
Conversation
Adds a paragraph detailing when `E_DIAL_REFUSED` should be sent as a `DialResponse` status as it's not mentioned in the current spec. I think this is consistent with go-libp2p since libp2p/go-libp2p#1527
| If the receiver elects not to dial any addresses from the message - for example | ||
| because they are invalid, because they resolve to the same host as the receiver, | ||
| because they only include private addresses, because it does not support the | ||
| relevant transports, or because it has no capacity, is sends a `DialResponse` |
There was a problem hiding this comment.
| relevant transports, or because it has no capacity, is sends a `DialResponse` | |
| relevant transports, or because it has no capacity, it sends a `DialResponse` |
|
Thanks 🙏 @achingbrain! |
| because they are invalid, because they resolve to the same host as the receiver, | ||
| because they only include private addresses, because it does not support the |
There was a problem hiding this comment.
| because they are invalid, because they resolve to the same host as the receiver, | |
| because they only include private addresses, because it does not support the | |
| because they are invalid, because it does not support the |
because they only include private addresses
In rust-libp2p we don't reject requests in this case. Directly after starting to listen, a peer won't know any of their external addresses and instead just their listening addresses (which are all private). The server still attempts to dial them, based on the address it observes this peer on, i.e. it replaces the IP of each address in the message with the observed IP. We do this anyway for DDoS protection (see vacp2p/nim-libp2p#739 (comment); @Tanguy suggested that we also add a section on for this, which imo would make sense).
Only if the observed address is private we may reject the peer (depending on whether this was set in our config or not).
because they resolve to the same host as the receiver
Again, because we replace the IP with the observed one at least in rust-libp2p we don't really handle this case.
Not sure how I think about having an exemplary list of rejection reasons. They seem rather go-lib2p2p specific right now. I'd propose to either properly specify in which cases a DialRequest shall be rejected, or keep it on a higher level to leave it up to the implementation (see suggestion).
| If the receiver elects not to dial any addresses from the message - for example | ||
| because they are invalid, because they resolve to the same host as the receiver, | ||
| because they only include private addresses, because it does not support the | ||
| relevant transports, or because it has no capacity, is sends a `DialResponse` |
There was a problem hiding this comment.
| relevant transports, or because it has no capacity, is sends a `DialResponse` | |
| relevant transports, or because it has no capacity, it SHOULD send a `DialResponse` |
We should probably add that you can’t trust on your peer doing this correctly - the Go implementation has been sending the wrong error code for a long time (until Elena fixed it recently).
@marten-seemann What do you think about using the keyword SHOULD, in the sense that it is recommended but one can not trust that the other peer actually does it?
There was a problem hiding this comment.
Technically, this is a MUST. You should be able to trust your peer's reporting here.
In practice you can't. But that's just because the Go implementation was just not compliant with the spec, which brought us to the mess we're in right now (one more reason for Autonat v2!). I suggest adding a note here that this was the case.
|
@achingbrain and @p-shahi : this has been open for a long time. What are the next steps to get this landed? |
Adds a paragraph detailing when
E_DIAL_REFUSEDshould be sent as aDialResponsestatus as it's not mentioned in the current spec.I think this is consistent with go-libp2p since libp2p/go-libp2p#1527