libp2p / libp2p/rust-libp2p

swarm: smart dialing classifies DNS-only multiaddrs as private addresses

Open
#6,623 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
5.6k
Forks
1.3k
Avg merge
8h 47m
Merged PRs (30d)
19

Description

I was reading through the smart dialing code that landed in #6229 (swarm/src/connection/pool/dial_ranker.rs) and I think the DNS branch of is_global_addr has its condition the wrong way round. The effect is that DNS-only multiaddrs get filed under "private" and localhost gets filed under "other", i.e. the two cases are swapped, and the "Other" tier that rank_dials documents never actually runs for the addresses it names.

What the code does

rank_dials buckets each dial like this:

for dial in dials {
    if dial.addr.iter().any(|p| matches!(p, Protocol::P2pCircuit)) {
        relay.push(dial);
    } else if !is_global_addr(&dial.addr) {
        private.push(dial);
    } else if dial
        .addr
        .iter()
        .any(|p| matches!(p, Protocol::Ip4(_) | Protocol::Ip6(_)))
    {
        public.push(dial);
    } else {
        other.push(dial);
    }
}

and the doc comment on the last bucket says:

Other - addresses without any IP component (no Ip4/Ip6), e.g. DNS-only multiaddrs like /dns/example.com/tcp/443

is_global_addr handles DNS-only addresses here:

if let Some(dns) = a.iter().find_map(|p| match p {
    Protocol::Dns(dns) | Protocol::Dns4(dns) | Protocol::Dns6(dns) => Some(dns),
    _ => None,
}) {
    return dns == "localhost" || dns.ends_with(".localhost");
}

For example.com this returns false, so !is_global_addr(..) is true and the address is pushed into private; it never reaches the else branch that was written for it. For localhost it returns true, so a localhost DNS address skips private and ends up in other (it has no Ip4/Ip6). That is the opposite of the comment directly above the function, which says link-local IPv6 and localhost DNS names are the ones that are not globally routable, i.e. everything else, including ordinary DNS names, is meant to count as routable.

/dnsaddr/... isn't matched by that find_map at all (only Dns/Dns4/Dns6 are), so it falls through to the final false and is classified as private the same way.

Why it matters

Smart dialing is meant to start the fast, likely addresses first and leave the awkward ones for later; a DNS name costs an extra resolution step, which is exactly why it gets its own tier and an extra PUBLIC_OTHER_DELAY offset. With this predicate a /dns4/... address is treated like a LAN address: it's dialed ahead of real public IP addresses and gets the short private-tier delays instead of being parked at the back. The "Other" tier effectively never runs for the addresses it was written for, so for a peer whose address book mixes DNS names and direct addresses the dial order and the pacing both come out wrong.

Evidence

The tests in this file only ever use /ip4/... and /ip6/... addresses, so the DNS path is never exercised. Ranking something like ["/dns4/bootstrap.example.com/tcp/4001", "/ip6/2606:2800:220:1:248:1893:25c8:1946/tcp/4001"] returns the DNS entry first with a zero delay, instead of last.

What I'd expect

A DNS-only multiaddr should land in the tier the doc comment describes, and a localhost DNS name should be treated as non-global (private). A small regression test with a DNS-only address next to a couple of IP addresses would keep this covered.

Possible direction

Inverting the condition (return !(dns == "localhost" || dns.ends_with(".localhost"));, plus a decision on /dnsaddr) is the smallest change, but I'm not sure that's quite the right place for it: is_global_addr is also being asked about addresses with no IP component at all, which is arguably outside what "global" means. Either way, the bucket documented in rank_dials should actually contain the DNS-only addresses it mentions. I don't have a strong view on which shape the fix should take.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in swarm/src/connection/pool/dial_ranker.rs, reading is_global_addr and rank_dials alongside the existing IP-address tests. Add a regression case covering DNS-only, localhost DNS, and IP addresses, including the /dnsaddr behavior decision. Done means DNS-only addresses follow the documented Other tier, localhost is treated as non-global, and the ranking delays match those tiers.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.