Conversation
|
|
||
| ## AutoNAT V2 Protocol | ||
|
|
||
|  |
There was a problem hiding this comment.
These diagrams need to be updated. They use “DialStatuses” and reference a undefined error.
|
|
||
| `OK`: the server completed the request successfully. A request is considered | ||
| completed successfully when the server either completes a dial(successfully or | ||
| unsuccessfully) or rejects all addresses in the request as undialable. |
There was a problem hiding this comment.
If a server rejects all addresses as undialable, should it set OK or E_DIAL_REFUSED?
There was a problem hiding this comment.
good catch! it should be E_DIAL_REFUSED
| IP address, the server asks the client to send some non trivial amount of bytes | ||
| as a cost to dial a different IP address. To make amplification attacks | ||
| unattractive, servers SHOULD ask for 30k to 100k bytes. Since most handshakes | ||
| cost less than 10k bytes in bandwidth, 30kB is sufficient to make attacks |
There was a problem hiding this comment.
Linking to QUIC’s similar mitigation here would be helpful
| as the server is free to use a separate peerID for the dial backs. | ||
|
|
||
| Servers SHOULD determine whether they have IPv6 and IPv4 connectivity. IPv4 only servers SHOULD refuse requests for dialing IPv6 addresses and IPv6 only | ||
| servers SHOULD refuse requests for dialing IPv4 addresses. |
There was a problem hiding this comment.
Do we currently handle this in go-libp2p?
There was a problem hiding this comment.
Yes. We use the black hole detector to tell us if we have IPv6 connectivity. https://github.com/libp2p/go-libp2p/blob/master/p2p/protocol/autonatv2/server.go#L183
sukunrt
force-pushed
the
autonat-rename
branch
2 times, most recently
from
55ae737 to
d9f46e6
Compare
|
|
||
| == Amplification Attack Prevention == | ||
|
|
||
| Cli -> Srv: [conn1: stream: dial] DialRequest:{nonce: 0xabcd, addrs: (addr1, addr2, addr3)} |
There was a problem hiding this comment.
Very small nit, but can we 0-index these addrs too? addr0, addr1, addr2
MarcoPolo
left a comment
MarcoPolo
left a comment
There was a problem hiding this comment.
I pushed a couple small changes as well fyi
sukunrt
force-pushed
the
autonat-rename
branch
3 times, most recently
from
c0fdf37 to
92c9b64
Compare
--------- Co-authored-by: Marco Munizaga <git@marcopolo.io>
sukunrt
force-pushed
the
autonat-rename
branch
from
92c9b64 to
0a201f2
Compare
2 participants
This is the same as #538. That PR targeted
autonat-renamebranch which has a commit to move the autonat v1 spec to a separate v1 specific file.Note: When merging, make sure to use "Rebase and Merge" to ensure git blame works correctly for autonat v1 spec.