Skip to content

Add IPv6 support for Directories (still incomplete) - #3809

Draft
softins wants to merge 5 commits into
jamulussoftware:mainfrom
softins:ipv6-directories
Draft

Add IPv6 support for Directories (still incomplete)#3809
softins wants to merge 5 commits into
jamulussoftware:mainfrom
softins:ipv6-directories

Conversation

@softins

@softins softins commented Jul 20, 2026

Copy link
Copy Markdown
Member

Short description of changes

When complete, this will allow servers with IPv6 addresses to register with a directory using both their IPv4 address and IPv6 address. It should also allow for a server that only has an IPv6 address to register, even though it will be inaccessible to clients that just have IPv4.

CHANGELOG: Server: Add IPv6 support to Directory operations.

Context: Fixes an issue?

No issue, but a long-standing discussion at #1950

Does this change need documentation? What needs to be documented and how?

It will do.

Status of this Pull Request

Incomplete work in progress, posted for visibility and comments

What is missing until this pull request can be merged?

To be completed

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@softins softins added this to the Release 4.0.0 milestone Jul 20, 2026
@softins softins self-assigned this Jul 20, 2026
@softins softins added this to Tracking Jul 20, 2026
@softins softins added feature request Feature request needs documentation PRs requiring documentation changes or additions labels Jul 20, 2026
@github-project-automation github-project-automation Bot moved this to Triage in Tracking Jul 20, 2026
@pljones pljones moved this from Triage to In Progress in Tracking Jul 20, 2026
@mcfnord mcfnord mentioned this pull request Jul 22, 2026
5 tasks
@mcfnord

mcfnord commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

MY LLM WROTE:

During #3636's TCP-fallback testing, we reproduced the exact gap this PR aims to close. A server registering from a genuine (non-v4-mapped) IPv6 address appears in the server list with IP = 0.0.0.0.

The reproduction:

  • Server registered from fd00::2 (with IPV6_V6ONLY=1, no dual-stack v4-mapping)
  • Server appeared in the list with correct port/name/city but ip=0.0.0.0
  • Root cause: CreateCLServerListMes / CreateCLRedServerListMes (protocol.cpp:2154, :2277) encode via HostAddr.InetAddr.toIPv4Address() — a hard 4-byte IPv4-only field. Qt's toIPv4Address() returns 0 for a true IPv6 address.

This confirms that TCP fallback (#3636) is necessary but not sufficient — even with reliable transport, the list's wire format itself can't currently describe an IPv6-only server. Your branch's protocol changes are the right lever; wanted to offer this concrete repro in case it helps the verification/test plan.


Edit: since the PR asks what's still missing, two more observations from reading the current diff. Both are "not yet done" rather than "wrong" — take or leave them for the eventual checklist.

1. MAX_LEN_IP_ADDRESS 15 → 39 also has a directory-side effect. We raised the client-side half of this in #1950 (comment) (old clients decode the list's string slot bound to 15 at protocol.cpp:2178, and GetStringFromStream rejects the entire message on overflow). The same guard sits on the registration path, read by the directory: protocol.cpp:1900 and :2020, the "server internal address" field of CLM_REGISTER_SERVER[_EX]. So if a server on this branch ever sends an IPv6 literal longer than 15 characters to a directory running a released version, the register message is discarded whole — silent total registration failure rather than a degraded listing.

The current diff avoids that, because ServerPublicIP6 is only passed when DirectoryAddress is itself IPv6, so an IPv4 directory still receives an IPv4 literal. But that invariant is implicit — nothing in the code states or enforces it, and it would be easy to lose in a later refactor. Might be worth pinning it down explicitly, and noting the upgrade ordering (directories before servers) in the release notes.

2. There's no --serverpublicip6 counterpart. The IPv4 path (serverlist.cpp:170-181) has a user override precisely because auto-detection is wrong behind NAT. The new v6 branch (:186-190) always uses NetworkUtil::GetLocalAddress6() with no escape hatch — fine for a host with a plain global address, but it leaves no answer for NAT66/prefix translation, and it will register whatever temporary/privacy address happens to be selected at the time, which then rotates.

Also minor: the // Allow IPv4 only for communicating with Directories comment still sits directly above all three changed ParseNetworkAddress calls (clientrpc.cpp:189, connectdlg.cpp:353, serverlist.cpp:1013).

Two things we checked that turned out to be fine, in case it saves a reviewer the trip: swapping ServerList[0].LHostAddr for ServerPublicIP at serverlist.cpp:1030 is equivalent (ServerList[0] is constructed at :215 with ServerPublicIP as its NLHAddr, and :640 only rewrites LHostAddr for other entries), and the serverlist.h bEnableIPv6bIPv6Available rename just aligns the declaration with the definition already at serverlist.cpp:87.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request Feature request needs documentation PRs requiring documentation changes or additions

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants