Summary
In the AutoNAT v2 server, handle_request_internal selects the address to dial-back with addrs.pop() (the last address in the request) but hardcodes the reported index to 0:
|
all_addrs.clone_from(&addrs); |
|
let idx = 0; |
|
let addr = addrs.pop().ok_or(HandleFail::DialRefused)?; |
|
*tested_addrs = Some(addr.clone()); |
idx is never reassigned, so the DialResponse always carries addr_idx == 0 regardless of which address was actually tested.
The client maps the outcome back to an address using that index:
|
let tested_address = req |
|
.addrs |
|
.get(res.addr_idx) |
|
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "address index out of bounds"))? |
|
.clone(); |
So when a DialRequest carries more than one address, the server dials the last one while the client attributes the result to the address at index 0.
Impact
DialRequest.addrs is a repeated field and addr_idx exists specifically to disambiguate which address was tested, so the wire format permits multi-address requests. The rust-libp2p client currently sends a single address per request (addrs: vec![addr]), so rust-to-rust is unaffected (the popped element is the only one, index 0). A spec-conformant client that sends multiple addresses would receive a reachability result attributed to the wrong address.
Separately, pop() selects the last (lowest-priority) address rather than the first.
Question
Is handling multi-address DialRequests in scope for the v2 server? If so the fix is small — either test addrs[0] and keep addr_idx = 0, or report the index of whichever address is tested. Happy to open a PR with a focused regression test once the preferred direction is confirmed.
Version
master (6a6814bb698b01c40d6ce08eb0f61281c909124f)
Summary
In the AutoNAT v2 server,
handle_request_internalselects the address to dial-back withaddrs.pop()(the last address in the request) but hardcodes the reported index to0:rust-libp2p/protocols/autonat/src/v2/server/handler/dial_request.rs
Lines 275 to 278 in 6a6814b
idxis never reassigned, so theDialResponsealways carriesaddr_idx == 0regardless of which address was actually tested.The client maps the outcome back to an address using that index:
rust-libp2p/protocols/autonat/src/v2/client/handler/dial_request.rs
Lines 285 to 289 in 6a6814b
So when a
DialRequestcarries more than one address, the server dials the last one while the client attributes the result to the address at index 0.Impact
DialRequest.addrsis arepeatedfield andaddr_idxexists specifically to disambiguate which address was tested, so the wire format permits multi-address requests. The rust-libp2p client currently sends a single address per request (addrs: vec![addr]), so rust-to-rust is unaffected (the popped element is the only one, index 0). A spec-conformant client that sends multiple addresses would receive a reachability result attributed to the wrong address.Separately,
pop()selects the last (lowest-priority) address rather than the first.Question
Is handling multi-address
DialRequests in scope for the v2 server? If so the fix is small — either testaddrs[0]and keepaddr_idx = 0, or report the index of whichever address is tested. Happy to open a PR with a focused regression test once the preferred direction is confirmed.Version
master(6a6814bb698b01c40d6ce08eb0f61281c909124f)