Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
SaadAhmedGit opened this issue Jun 14, 2024 · 6 comments
Closed

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

SaadAhmedGit opened this issue Jun 14, 2024 · 6 comments

Comments

@SaadAhmedGit
Copy link

SaadAhmedGit commented Jun 14, 2024 •
edited
Loading

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.
@SaadAhmedGit SaadAhmedGit changed the title [Wireguard Bug] - Listening port isn't released when another VPN app attempts to start a VPN on the same port [Wireguard Bug] - Listen port isn't released when another VPN app attempts to start a VPN on the same port Jun 14, 2024
@ignoramous
Copy link
Collaborator

ignoramous commented Jun 14, 2024 •
edited
Loading

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?

@ignoramous ignoramous reopened this Jun 14, 2024
@SaadAhmedGit
Copy link
Author

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
Copy link
Collaborator

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).

rethink-app/app/src/main/java/com/celzero/bravedns/service/BraveVPNService.kt

Lines 1926 to 1931 in 72fc90e

fun signalStopService(userInitiated: Boolean = true) {
if (!userInitiated) notifyUserOnVpnFailure()
stopVpnAdapter()
stopSelf()
Logger.i(LOG_TAG_VPN, "stopped vpn adapter and vpn service")
}

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

@SaadAhmedGit
Copy link
Author

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.

SaadAhmedGit pushed a commit to SaadAhmedGit/rethink-app that referenced this issue Jun 15, 2024
tunnel: stop vpn adapter when another app takes over the vpn
b917387 
fixes: celzero#1545
@SaadAhmedGit SaadAhmedGit mentioned this issue Jun 15, 2024
Closed
@ignoramous
Copy link
Collaborator

Where can I see the commit for that?

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

@hussainmohd-a
Copy link
Collaborator

Multiple improvements to the VPN service have altered the approach to fixing the issue. Now, the closure/stop of the VPN service is handled differently, ensuring the service is stopped in onRevoke() as well.

Commits:

41a7f3f
bdead32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tunnel: stop vpn adapter when another app takes over the vpn SaadAhmedGit/rethink-app
3 participants
@ignoramous @hussainmohd-a @SaadAhmedGit