amnezia-vpn / amnezia-client

Amnezia VPN Client (Desktop+Mobile)
https://amnezia.org
GNU General Public License v3.0
4.47k stars 291 forks source link

Android client doesn't support multiple comma-separated addresses per client in Wireguard and AmneziaWG protocols #499

Closed Moondarker closed 8 months ago

Moondarker commented 8 months ago

Describe the bug When I attempt to use imported configuration files with IPv6 networks in Address field on Android, it leads to connection error within the first second of connection attempt. I had to modify the code to acquire the stack trace attached in Logs section

To Reproduce Steps to reproduce the behavior:

  1. Add an IPv6 subnet in [Interface] -> Address section to any WireGuard or AmneziaWG config (in .conf format, not the .json one)
  2. Import the config
  3. Click "Connect"
  4. See error

Expected behavior It should establish connection without crashing

Logs

java.lang.IllegalArgumentException: Bad prefixLength
    at android.net.VpnService.check(VpnService.java:465)
    at android.net.VpnService.-$$Nest$smcheck(Unknown Source:0)
    at android.net.VpnService$Builder.addAddress(VpnService.java:561)
    at org.amnezia.vpn.protocol.ProtocolKt.addAddress(Protocol.kt:213)
    at org.amnezia.vpn.protocol.ProtocolKt.access$addAddress(Protocol.kt:1)
    at org.amnezia.vpn.protocol.Protocol.buildVpnInterface(Protocol.kt:98)
    at org.amnezia.vpn.protocol.wireguard.Wireguard.start(Wireguard.kt:144)
    at org.amnezia.vpn.protocol.wireguard.Wireguard.startVpn(Wireguard.kt:88)
    at org.amnezia.vpn.AmneziaVpnService$connect$1.invokeSuspend(AmneziaVpnService.kt:352)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
    at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
    at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)

Smartphone:

Additional context: The same config file works perfectly fine on Desktop Windows x64 client

Moondarker commented 8 months ago

[issue described in this particular comment was fixed in #513]

Instead of creating a separate issue, I'll throw in some off-topic here as a bonus (I can certainly create an issue if you think it's something you might want to fix):

WireGuard Android client can export configuration into a zip archive, but in process of doing so it changes PresharedKey to PreSharedKey in peer options.

Android Amnezia client seems to handle it fine

But Desktop client just hangs until a timeout unless it's fixed, without any errors at all:

2024-01-16 16:56:51 debug Set config data C:/Users/******/AppData/Local/Temp/AmneziaVPN.WireGuard0.conf
2024-01-16 16:56:51 debug Set config data 
2024-01-16 16:56:51 debug Amnezia "LocalSocketController" "Initializing"
2024-01-16 16:56:51 debug Amnezia "LocalSocketController" "Connecting to: \\\\.\\pipe\\amneziavpn"
2024-01-16 16:56:51 debug Amnezia "LocalSocketController" "Daemon connected"
2024-01-16 16:56:51 debug Amnezia "LocalSocketController" "Check status"
2024-01-16 16:56:51 debug ipRange  "0.0.0.0/0"
2024-01-16 16:56:51 debug Amnezia "LocalSocketController" "Reading"
2024-01-16 16:56:51 debug Amnezia "LocalSocketController" "Parse command: status"
// It just hangs here. Note: This comment is not present in actual logs.
2024-01-16 16:58:55 debug Amnezia "LocalSocketController" "Deactivating"
outspace commented 8 months ago

For the second part of this issue please check fix in #513. You can download installer with Amnezia client from github actions and approve fix.

albexk commented 8 months ago

@Moondarker, could you give an example of an IPv6 address that the error occurs with?

Moondarker commented 8 months ago

In my case, IPv6 address is fd42:42:42::9/128 - sorry, should've included it in the issue description, @albexk 😅

@outspace, yep, your PR did the trick, thanks 👍

Moondarker commented 8 months ago

~@albexk, I can't reproduce it on latest dev builds anymore, but there's another thing that bothers me~

~To be honest, it could even be that this issue was invalid all along, and the error I was getting here was caused by what I'm about to describe, my bad...~

Nevermind, I managed to reproduce the initial issue, but now I don't understand the pattern at all... I'll be back once I make sense of what is happening

Moondarker commented 8 months ago

Aight, so, the issue was not about IPv6 after all, it's just that client didn't expect multiple comma-separated addresses.

@albexk fixed it in #518 without mentioning this issue, so it took me some time to understand what has changed... Thanks for the fix! And please mention issues in such PRs even if you are not sure if your actions are going to fix the issue :)

I can't reproduce the other issue I mentioned earlier yet, but I'll describe it in short in case somebody encounters it later on:

On multiple occasions, my config files were imported cut to a certain length when I had a long list of AllowedIPs in the config (e.g. 0.0.0.0/1, 128.0.0.0/2, 192.0.0.0/9, 192.128.0.0/11, 192.160.0.0/13, 192.169.0.0/16, 192.170.0.0/15, 192.172.0.0/14, 192.176.0.0/12, 192.192.0.0/10, 193.0.0.0/8, 194.0.0.0/7, 196.0.0.0/6, 200.0.0.0/5, 208.0.0.0/4, 224.0.0.0/3).

When I looked at the imported config, it would end around the middle of AllowedIPs list and the remaining options (like PresharedKey and PublicKey) were not present at all.

But, again, can't reproduce, so could've been fixed by some other commit? Idk, anyways, good job, dear contributors!