Client: add --connectdirectory for directory-assisted hole punch on --connect by mcfnord · Pull Request #3815 · jamulussoftware/jamulus

mcfnord · 2026-07-21T05:30:00Z

Short description of changes

Adds an optional --connectdirectory <host:port> client option and gives CClient::Connect an optional directory argument. When a directory is supplied, the client first asks that directory for its server list — which makes the directory tell its registered servers to send an "empty message" toward the client's socket (the same directory-assisted UDP hole punch the GUI directory list relies on) — waits HOLE_PUNCH_CONNECT_DELAY_MS, then connects.

Today -c / --connect connects straight to the address and never contacts a directory, so a server behind a cloud firewall/NAT that the GUI reaches fine is silently unreachable via -c. The server address is connected to verbatim and need not appear in the directory's server list, so this also lets you reach a server that isn't in any built-in directory by naming the (possibly custom/private) directory it is registered with.

CHANGELOG: Client: Added --connectdirectory to hole-punch through a directory when connecting on startup with --connect

Context: Fixes an issue?

Relates to jamulussoftware/jamuluswebsite#1122, which documents this -c reachability limitation. This is the code side of that gap.

Stacked on #3805 — it uses the CClient::Connect refactor introduced there, so the diff currently includes #3805's commit; the single top commit is the change in this PR. Please review/merge #3805 first.

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

Yes — a short website entry for --connectdirectory next to the -c text (a companion to the -c note added in jamuluswebsite#1156). A draft on the website repo will follow.

Status of this Pull Request

Working implementation. Verified with a UDP capture: with --connectdirectory the client sends CLM_REQ_SERVER_LIST to the directory and connects ~400 ms later; without it there is no directory traffic and behaviour is unchanged. Clean SIGTERM shutdown, including when interrupted during the hole-punch delay.

What is missing until this pull request can be merged?

Depends on #3805 merging first. Otherwise ready for review.

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

This is an extract from jamulussoftware#2550 Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>

Introduce EConnectionState (disconnected / connecting / connected) owned by CClient as the single source of truth. A connection is 'requested' when the audio stream starts (CS_CONNECTING) and 'established' once the server assigns our channel ID (CS_CONNECTED). Every transition emits ConnectionStateChanged. Rename the Connected(name) signal emitted from Start() to Connecting(name), since at that point the connection is only requested, not established. CClient::Connect() now terminates any current connection first, so connecting while connected behaves as a reconnect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- SIGTERM/SIGINT: route through Disconnect() instead of a raw Stop(), and guard Disconnect() on the connection state rather than IsRunning() (which tracks the audio device). IsRunning() is false while connecting and in headless mode, so the old guard could skip notifying the server on shutdown; the raw Stop() worked around that but also fired a spurious disconnect when idle. Now the server is notified iff a connection is pending or established, via the single Disconnect() path. - Move EConnectionState from util.h to client.h (it is client specific). - Add a doc-comment to SetConnectionState; reword two member comments. Addresses review feedback on jamulussoftware#3805. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

-c / --connect connects straight to the given address and, unlike selecting a server from the GUI directory list, never asks a directory to open the target's firewall. Servers behind a cloud firewall/NAT that the GUI reaches fine are therefore silently unreachable via -c. Give CClient::Connect an optional directory argument: when set it sends a CLM_REQ_SERVER_LIST to that directory (which makes the directory poke its registered servers towards our socket -- the same UDP hole punch the GUI relies on), waits HOLE_PUNCH_CONNECT_DELAY_MS, then connects. The server address is used verbatim and need not be listed by the directory. Expose it on the command line as --connectdirectory <host:port>, wired through both the GUI and headless connect-on-startup paths in main.cpp. Relates to jamulussoftware/jamuluswebsite#1122 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

ann0see · 2026-07-21T07:50:17Z

Best case -c would fall back automatically to this after some timeout imo. This doesn't exclude an explicit option but would be more user friendly

mcfnord · 2026-07-21T15:21:18Z

Best case -c would fall back automatically to this after some timeout imo. This doesn't exclude an explicit option but would be more user friendly

The client would need to know which directory to use, and that will have to be specified by the CLI parameters. It would be too sloppy and nuts to guess all 7, and even if we do, it might be some other directory entirely.

dingodoppelt · 2026-08-08T12:42:56Z

Best case -c would fall back automatically to this after some timeout imo. This doesn't exclude an explicit option but would be more user friendly

We should definitely not make this a default. The current --connect behavior can used to mitigate the correlation attack as stated in this coment.

dingodoppelt · 2026-08-08T12:57:47Z

We don't actually need this because server admins can always create the server as a private directory which takes care of hole-punching without leaking the IP.

softins · 2026-08-08T16:59:10Z

Best case -c would fall back automatically to this after some timeout imo. This doesn't exclude an explicit option but would be more user friendly

We should definitely not make this a default. The current --connect behavior can used to mitigate the correlation attack as stated in this coment.

It's not the lack of hole-punching in --connect that mitigates the correlation behaviour (I hesitate to call it an "attack"), but rather the lack of regular pings (to an unrelated server such as Duet) up until the time of connecting to the desired server.

So the option in this PR would not actually open the client to that correlation behaviour.

But having said that, I don't feel there is much usefulness in adding the feature in this PR.

softins · 2026-08-08T17:02:30Z

We don't actually need this because server admins can always create the server as a private directory which takes care of hole-punching without leaking the IP.

Yes, if creating a private server that is not intended to register with a directory, it is actually a very useful idea to make it its own directory with -e localhost, and for the private clients to add it as a custom directory.

dingodoppelt · 2026-08-09T06:51:56Z

It's not the lack of hole-punching in --connect that mitigates the correlation behaviour (I hesitate to call it an "attack"), but rather the lack of regular pings (to an unrelated server such as Duet) up until the time of connecting to the desired server.

I still think we should let the user decide if he wants his IP leaked by CLM_EMPTY_MESSAGE, which is sent to every Server in the list. The pings can be easily blocked locally, CLM_EMPTY_MESSAGE sent by the Directory can't.
Comment

dingodoppelt

I'd rather not pull this in at all because of the security implications.

dingodoppelt · 2026-08-08T12:47:57Z

+    // tells each registered server to send us an "empty message", which opens
+    // the server's firewall for our socket (UDP hole punching).
+    CreateCLReqServerListMes ( haDirectoryAddress );
+


This breaks --connect's current behaviour by leaking the user's IP.

pgScorpio and others added 2 commits July 19, 2026 01:01

Refactor Connect out to CClient

43b5eb5

This is an extract from jamulussoftware#2550 Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>

mcfnord mentioned this pull request Jul 21, 2026

Add JSON-RPC methods to connect, disconnect and query connection state #3806

Draft

5 tasks

jrd and others added 2 commits July 21, 2026 05:55

mcfnord force-pushed the connect-directory-holepunch branch from 1339edc to 1e9a6b3 Compare July 21, 2026 06:01

ann0see added the AI AI generated or potentially AI generated label Jul 21, 2026

pljones added this to Tracking Jul 21, 2026

github-project-automation Bot moved this to Triage in Tracking Jul 21, 2026

pljones added feature request Feature request needs documentation PRs requiring documentation changes or additions labels Jul 21, 2026

pljones added this to the Release 4.1.0 milestone Jul 21, 2026

dingodoppelt requested changes Aug 10, 2026

View reviewed changes

github-project-automation Bot moved this from Triage to Waiting externally in Tracking Aug 10, 2026

Navigation Menu

Uh oh!

Client: add --connectdirectory for directory-assisted hole punch on --connect - #3815

Client: add --connectdirectory for directory-assisted hole punch on --connect#3815
mcfnord wants to merge 4 commits into
jamulussoftware:mainfrom
mcfnord:connect-directory-holepunch

mcfnord commented Jul 21, 2026

Uh oh!

ann0see commented Jul 21, 2026

Uh oh!

mcfnord commented Jul 21, 2026

Uh oh!

dingodoppelt commented Aug 8, 2026

Uh oh!

dingodoppelt commented Aug 8, 2026

Uh oh!

softins commented Aug 8, 2026

Uh oh!

softins commented Aug 8, 2026 •

edited

Loading

Uh oh!

dingodoppelt commented Aug 9, 2026

Uh oh!

dingodoppelt left a comment

Uh oh!

dingodoppelt Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

mcfnord commented Jul 21, 2026

Checklist

Uh oh!

ann0see commented Jul 21, 2026

Uh oh!

mcfnord commented Jul 21, 2026

Uh oh!

dingodoppelt commented Aug 8, 2026

Uh oh!

dingodoppelt commented Aug 8, 2026

Uh oh!

softins commented Aug 8, 2026

Uh oh!

softins commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dingodoppelt commented Aug 9, 2026

Uh oh!

dingodoppelt left a comment

Choose a reason for hiding this comment

Uh oh!

dingodoppelt Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

softins commented Aug 8, 2026 •

edited

Loading