Skip to content

WireGuard NAT hole punching - #1522

Merged
anjmao merged 9 commits into
masterfrom
wg-nat-punching
Jan 21, 2020
Merged

WireGuard NAT hole punching#1522
anjmao merged 9 commits into
masterfrom
wg-nat-punching

Conversation

@anjmao

@anjmao anjmao commented Jan 15, 2020

Copy link
Copy Markdown
Contributor

Implement NAT hole punching the same way as it is done for OpenVPN service by reusing NAT pinger and UDP Proxy. One difference is that for mobile consumser side separate UDP proxy is now not needed as because WireGuard does not change source port for outgoing packets.

TODO:

  • Add more tests including compatibility with old node versions.
  • Do more manual testing from desktop and mobile.
  • Fix e2e tests. Check way it's failing. Can be due to stability issues.

@anjmao
anjmao requested review from Waldz and tadaskay January 15, 2020 15:35
@anjmao anjmao changed the title Wg nat punching WireGuard NAT hole punching Jan 15, 2020
Comment thread services/wireguard/handshake.go Outdated
Comment thread bin/docker/dev/Dockerfile Outdated
Comment thread bin/docker/dev/README.md Outdated
Comment thread bin/docker/dev/README.md Outdated
Comment thread bin/docker/dev/README.md Outdated
Comment thread bin/docker/dev/README.md Outdated
Comment thread nat/traversal/pinger.go Outdated
Comment thread services/wireguard/connection/connection.go Outdated
Comment thread services/wireguard/connection/connection.go Outdated
Comment thread services/wireguard/endpoint/kernelspace/client.go
Comment thread services/wireguard/handshake.go Outdated
Comment thread ci/test/e2e.go Outdated
Comment thread mobile/mysterium/wireguard_connection_setup.go
Comment thread mobile/mysterium/wireguard_connection_setup.go Outdated
Comment thread mobile/mysterium/wireguard_connection_setup.go Outdated
Comment thread mobile/mysterium/wireguard_connection_setup.go Outdated
Comment thread mobile/mysterium/wireguard_connection_setup.go Outdated
Comment thread mobile/mysterium/wireguard_connection_setup.go Outdated
Comment thread mobile/mysterium/wireguard_connection_setup.go Outdated
Comment thread services/wireguard/connection/connection.go Outdated
Comment thread services/wireguard/connection/connection.go
@anjmao
anjmao force-pushed the wg-nat-punching branch 5 times, most recently from 898cdc3 to 8b9886c Compare January 20, 2020 13:56
@codecov-io

codecov-io commented Jan 20, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1522 into master will decrease coverage by 0.55%.
The diff coverage is 9.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1522      +/-   ##
==========================================
- Coverage   47.97%   47.42%   -0.56%     
==========================================
  Files         284      285       +1     
  Lines       10888    11007     +119     
==========================================
- Hits         5224     5220       -4     
- Misses       5282     5413     +131     
+ Partials      382      374       -8
Impacted Files Coverage Δ
session/manager.go 67.08% <ø> (ø) ⬆️
core/service/manager.go 67.16% <ø> (ø) ⬆️
services/openvpn/service/factory.go 0% <0%> (ø) ⬆️
mobile/mysterium/entrypoint.go 0% <0%> (ø) ⬆️
session/create_producer.go 48.57% <0%> (ø) ⬆️
services/wireguard/handshake.go 0% <0%> (ø)
services/openvpn/client.go 12.76% <0%> (+0.26%) ⬆️
mobile/mysterium/wireguard_connection_setup.go 0% <0%> (ø) ⬆️
mobile/mysterium/openvpn_connection_setup.go 0% <0%> (ø) ⬆️
session/dialog_handler.go 0% <0%> (ø) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b51e3d9...0fbd9dc. Read the comment docs.

}

func waitUDPPortReadyFor(port int, timeout time.Duration) error {
timeoutChan := time.After(timeout)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be smaller, not sure if it's better, but smaller. 😂

done := time.Now().Add(timeout)
for t := time.NewTicker(500 * time.Millisecond); time.Now().Before(done); <-t.C {
	p, err := net.ListenPacket("udp", fmt.Sprintf(":%d", port))
	if err == nil {
		p.Close()
		return nil
	} 
	log.Err(err).Msgf("Port %d is in use. Trying to check again...", port)
}
return fmt.Errorf("timeout waiting for UDP port %d", port)

@zolia zolia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!
Will need a lot of testing in various platforms / network envs.

Building node for mobile could take a lot of time when building in docker using xgo.
When openvpn is not needed it is possible to build it locally on MacOS. Need to look
if it is possible to cross compile OpenVPN C++ bridge on MacOS too.
Before port mapping was using service type as a key but now
WireGuard need to start new proxy instance for each new consumser
connection.
Remove unneeded traversal.Params which instance was created in different places
and modified in many strange ways. When provider provides traversal params
it already knows how to form them.
Port is assigned at provider side.
There is no need to start proxy for multiple client connections
as they will still reuse the same DNS proxy port.
Implement NAT hole punching the same way as it is done for OpenVPN service by reusing NAT pinger and UDP Proxy.
One difference is that for mobile consumser side separate UDP proxy is now not needed as because WireGuard
does not change source port for outgoing packets.
@anjmao
anjmao merged commit 1700d9c into master Jan 21, 2020
@anjmao
anjmao deleted the wg-nat-punching branch January 21, 2020 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants