search

celzero / rethink-app

DNS over HTTPS / DNS over Tor / DNSCrypt client, WireGuard proxifier, firewall, and connection tracker for Android.
https://rethinkfirewall.com/
Apache License 2.0
2.58k stars 129 forks source link

[Wireguard Bug] - Listen port isn't released when another VPN app attempts to start a VPN on the same port #1545

Open SaadAhmedGit opened 4 days ago

SaadAhmedGit commented 4 days ago

App version: v0.5.5n-14-g72fc90ec (fdroid) Commit: 72fc90ec492f8289c58f4ff1d247472815ede9dc

If another VPN app uses the same listen port for wireguard and attempts to start a VPN when rethink's proxy is running, rethink stops the vpn but the port isn't released. Then when you try to start rethink's proxy again, it gives the following error in a toast and logcat:

failed; err: IPC error -98: failed to set listen_port: listen udp4 :51820: bind: address already in use

Here are the detailed steps to reproduce this with wireguard official app as the other VPN client. (I couldn't reproduce it a 100% of the times but I am positive that it works most of the times)

  1. Generate a config (ideally, one that works) and make sure it has a specific listen port. I used 51820.
  2. Import the config in both rethink and wireguard app.
  3. Enable the config (simple mode) in rethink and start VPN.
  4. Switch to wireguard app and enable VPN using the same config. It won't enable and there will be an error.
  5. Go back to rethink and you'll find that VPN has stopped. Turn it back on and you'll see the error about failing to set listen port.
  6. The VPN will start anyways but if you try to access the internet, it won't work (that's why a working config was suggested).

Reopening the app fixes this issue.

I also observed that after step 4, logcat kept logging wireguard-go (GoLog) errors even though VPN was being shown as stopped in the rethink app.

ignoramous commented 3 days ago

You're running WireGuard in Advanced mode? Rethink does not respect port number in Advanced mode (chooses at random).

Go back to rethink and you'll find that VPN has stopped.

That's weird. Rethink auto-stops the WireGuard tunnel listening on the conflicting port? In the UI, is it shown as active or inactive?

SaadAhmedGit commented 3 days ago

You're running WireGuard in Advanced mode? Rethink does not respect port number in Advanced mode (chooses at random).

In simple mode (adding this to issue description now)

That's weird. Rethink auto-stops the WireGuard tunnel listening on the conflicting port? In the UI, is it shown as active or inactive?

It's showing as inactive

ignoramous commented 3 days ago

Okay, we found the bug and have fixed it. Thanks!

network engine (stopVpnAdapter) is closed iff signalStopService -> stopSelf is called (which it isn't when another app forcefully acquires the VPN service from underneath Rethink). https://github.com/celzero/rethink-app/blob/72fc90ec492f8289c58f4ff1d247472815ede9dc/app/src/main/java/com/celzero/bravedns/service/BraveVPNService.kt#L1926-L1931

Moving stopVpnAdapter to VpnService.onUnbind should do the trick (as it is called on both stopSelf and when another VPN app takes over.

SaadAhmedGit commented 3 days ago

Okay, we found the bug and have fixed it. Thanks!

Where can I see the commit for that? If there isn't, can I make the PR?

Moving stopVpnAdapter to VpnService.onUnbind should do the trick (as it is called on both stopSelf and when another VPN app takes over.

Yes it works. I have also fixed the issue by overriding VpnService.onRevoke to call stopVpnAdapter. Rethink now gracefully lets the VPN go when another app tries to take over which is the expected behavior.

ignoramous commented 1 day ago

Where can I see the commit for that?

We haven't upstreamed it yet. It is in our local fork.