Skip to content

Restrict stun-client to IPv4 - #301

Open
hsdfat wants to merge 2 commits into
pion:mainfrom
hsdfat:stun-client-ipv4-default-211
Open

hsdfat wants to merge 2 commits into
pion:mainfrom
hsdfat:stun-client-ipv4-default-211

Conversation

@hsdfat

@hsdfat hsdfat commented Sep 26, 2026 •

Copy link
Copy Markdown

Description

stun-client previously used a dual-stack UDP connection despite its intended IPv4 default. On a host with IPv6 connectivity it can therefore report an IPv6 address instead of the NAT-mapped IPv4 address.

Add WithIPVersion(4) / WithIPVersion(6) to DialURI, using the functional options introduced in #304. Omitting the option, or passing 0, retains dual-stack address selection. Unsupported versions return ErrUnsupportedIPVersion.

The CLI now calls DialURI(uri, WithIPVersion(4)). The URI still selects UDP, TCP, TLS, or DTLS; the option only restricts the IP family. TLS configuration and DTLS options remain available.

Validation:

  • GOMAXPROCS=4 go test -race -p 2 ./...
  • GOMAXPROCS=4 go vet ./...
  • golangci-lint run --timeout=5m --new-from-rev=origin/main (0 new issues)
  • Tests cover default/IPv4/IPv6 routing for all six URI/transport combinations, rejection of opposite-family literal addresses, successful IPv4 and IPv6 UDP loopback connections, TLS with an IPv4 restriction, and invalid option values.

Reference issue

Fixes #211

@Sean-Der

Copy link
Copy Markdown
Member

Thanks @hsdfat!

Why not just change it to udp4? Why the if/else ?

@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.29%. Comparing base (6f4664e) to head (12ca224).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/stun-client/stun_client.go 0.00% 4 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
- Coverage   65.40%   65.29%   -0.11%     
==========================================
  Files          27       27              
  Lines        1798     1801       +3     
==========================================
  Hits         1176     1176              
- Misses        622      625       +3     
Flag Coverage Δ
go 65.29% <0.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Commit 1b14963 switched stun-client to stun.DialURI, which dials "udp"
and so may pick an IPv6 address. A mapped IPv6 address is not useful
for NAT discovery. Dial stun: URIs over udp4 again, as was done in pion#72,
and keep DialURI for the other schemes.

Fixes pion#211
@hsdfat
hsdfat force-pushed the stun-client-ipv4-default-211 branch from 12ca224 to 2413ade Compare October 1, 2026 01:53
@hsdfat

hsdfat commented Oct 2, 2026

Copy link
Copy Markdown
Author

The if/else keeps the CLI's existing URI transport support: stun: uses UDP, turn: can use TCP, and stuns:/turns: use TLS or DTLS. Dialing udp4 unconditionally would lose those choices. Changing the UDP default in the library's DialURI would also restrict IPv6 for other callers, beyond this CLI fix.

If you meant making this tool plain STUN over IPv4 only, I can simplify it that way. Is that the scope you prefer?

@JoTurk

JoTurk commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

@hsdfat I added options to DialURI, we can add a family / network type option now. #304

Signed-off-by: phatlc <phatle.hsd@gmail.com>
@hsdfat

hsdfat commented Oct 2, 2026

Copy link
Copy Markdown
Author

Thanks, I updated this to use the functional options from #304. The new WithIPVersion(4) / WithIPVersion(6) option restricts the IP family while the URI continues to select UDP, TCP, TLS, or DTLS. The default remains dual-stack for library callers, and the CLI now uses DialURI(uri, WithIPVersion(4)) without a scheme-specific bypass.

The update is in 9950834. The full race-enabled test suite and go vet pass; lint reports no new issues relative to main. Tests cover all URI transports, both IP families, the default, opposite-family rejection, and real loopback connections.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stun-client uses IPv6 by default

3 participants