Android detects VPN (wireguard) connection as WI-FI enabled

[Galunid]

• KOReader version: v2020.09
• Device: Xiaomi MI A2 Lite/Onyx Boox Nova 2 (should be device independent)

Issue

When wifi is disabled, and device has VPN (in my case wireguard) enabled KOReader detects wifi as enabled. It's problematic because KOSync plugin/Calibre plugin have to timeout if triggered in this state.

Steps to reproduce

Connect to wireguard -> Disconnect wifi -> KOReader still detects wifi enabled (because VPN connection is still active)

[pazos]

See https://github.com/koreader/android-luajit-launcher/blob/e3d58c6432ba06d43bdd609c38e6315367825d7e/app/src/org/koreader/launcher/BaseActivity.kt#L338-L402

It is expected. VPN will prevail over wifi (ie: if both are active). Maybe if connection type is VPN do aditional online check?.

I wouldn't want to remove VPN as a valid network type. I use gnirehtet a lot (which is recongized as VPN network).

[Galunid]

Perhaps it's enough to just not return true or return connected in those snippets https://github.com/koreader/android-luajit-launcher/blob/e3d58c6432ba06d43bdd609c38e6315367825d7e/app/src/org/koreader/launcher/BaseActivity.kt#L364-L367 https://github.com/koreader/android-luajit-launcher/blob/e3d58c6432ba06d43bdd609c38e6315367825d7e/app/src/org/koreader/launcher/BaseActivity.kt#L392-L395 I'm not actually too clear on how the when works when multiple conditions are true, but my idea is to not change "connected" state with VPN connection, because user being connected to VPN doesn't imply internet connection. Hope it makes sense

[pazos]

@Galunid: the real problem is android != all the other platforms.

On embedded linux devices network == wifi On desktop we don't care about network

On android network means a lot of things and VPN is a valid kind of network. It's usually used over WiFi, over mobile and over Ethernet.

You might want to try

--- a/frontend/device/android/device.lua
+++ b/frontend/device/android/device.lua
@@ -259,9 +259,10 @@ function Device:initNetworkManager(NetworkMgr)
     end
 
     function NetworkMgr:isWifiOn()
-        local ok = android.getNetworkInfo()
+        local ok, transport = android.getNetworkInfo()
         ok = tonumber(ok)
         if not ok then return false end
+        if transport == C.ANETWORK_VPN then return false end
         return ok == 1
     end
 end

A snippet like that is cool for own usage, but I don't like it as is even more broken than before :)

Hence my comment

I wouldn't want to remove VPN as a valid network type. I use gnirehtet a lot (which is recongized as VPN network).

[pazos]

Of course the proper implementation would be

function NetworkMgr:isWifiOn()
        local ok, transport = android.getNetworkInfo()
        ok = tonumber(ok)
        if not ok then return false end
        return ok == 1 and transport == C.ANETWORK_WIFI
end

But that will break wikipedia over mobile network, among other things.

[Galunid]

@pazos What I had in mind is something more like this (this targets android only devices), basically extra check if WIFI is up when we detect VPN. https://github.com/Galunid/android-luajit-launcher/blob/eb5198eac8136a34c824fd4d8bc347685fb9e377/app/src/org/koreader/launcher/BaseActivity.kt#L363-L371 It's pretty crude, but it works for me. If you want I can clean it up a bit and submit PR, if not I can keep it to myself ;-) For older devices, I don't really know how to fix that.

EDIT: Still doesn't work with KOSync... EDIT2: KOSync doesn't even check if network is online

[NiLuJe]

There's a good reason for that: an online check will, by definition, be blocking.

(Remember gitter? ;p)

IIRC, it also doesn't actually check anything most of the time, because it can be triggered approximately seven billion different ways.

The one thing it tries to rely on (the NetworkConnected/NetworkDisconnected events), Android doesn't/can't support (c.f., when the Wi-Fi cleanup was implemented, we discussed that with @pazos).

[Galunid]

It makes sense, except it doesn't. Since if network is offline, KOSync starts doing network stuff that's blocking (at least on android and kobo) and hangs device for ~30 seconds (till request timeouts). Am I missing something?

[NiLuJe]

IIRC, it shouldn't happen on Kobo, at least in most cases. But, like I said, this thing is full of corner cases, so, who knows ;).

(Also, I don't actually use it, so I have no first hand experience with it).

[Galunid]

It happens on Kobo when autosync is enabled and it's pretty random I didn't really have trouble when I triggered it manually. On android it causes ANR (doesn't matter if triggered manually or automatically).

[pazos]

What I had in mind is something more like this (this targets android only devices), basically extra check if WIFI is up when we detect VPN.

Partially cool. If active network is VPN then see in all networks if has a different transport (wifi, ethernet, bluetooth) and return that as the active network. In any cases that function should return true if an active network is found. Anything that changes the logic belongs to the frontend isWifiOn and can be setup based on user preferences.

The one thing it tries to rely on (the NetworkConnected/NetworkDisconnected events), Android doesn't/can't support (c.f., when the Wi-Fi cleanup was implemented, we discussed that with @pazos).

Still unimplemented. See https://github.com/koreader/koreader/pull/6438#issuecomment-664534275 and https://github.com/koreader/koreader/pull/6438#issuecomment-664585012

On android it causes ANR (doesn't matter if triggered manually or automatically).

What causes ANR is a input event not consumed in 5 seconds. There's a workaround for that but I just implemented it for ANRs I found. See https://github.com/koreader/koreader/pull/6266

[NiLuJe]

@Galunid:

Something like that might help, but it'll probably also break stuff in fun and interesting ways in some cases? (I have a vague memory of someone trying something like that and failing horribly, but I don't recall the details...).

diff --git a/plugins/kosync.koplugin/main.lua b/plugins/kosync.koplugin/main.lua
index 6fcf4744..b56c0d59 100644
--- a/plugins/kosync.koplugin/main.lua
+++ b/plugins/kosync.koplugin/main.lua
@@ -494,6 +494,10 @@ function KOSync:updateProgress(manual)
         return
     end
 
+    if NetworkMgr:willRerunWhenOnline(function() self:updateProgress(manual) end) then
+        return
+    end
+
     local KOSyncClient = require("KOSyncClient")
     local client = KOSyncClient:new{
         custom_url = self.kosync_custom_server,
@@ -538,6 +542,10 @@ function KOSync:getProgress(manual)
         return
     end
 
+    if NetworkMgr:willRerunWhenOnline(function() self:getProgress(manual) end) then
+        return
+    end
+
     local KOSyncClient = require("KOSyncClient")
     local client = KOSyncClient:new{
         custom_url = self.kosync_custom_server,

[Galunid]

@NiLuJe It was me in koreader/koreader#6489, which resulted in koreader/koreader#6535 ;-) Sidenote, it we checked for manual variable, it should work (but only for updates triggered by user)

diff --git a/plugins/kosync.koplugin/main.lua b/plugins/kosync.koplugin/main.lua
index 6fcf4744..b56c0d59 100644
--- a/plugins/kosync.koplugin/main.lua
+++ b/plugins/kosync.koplugin/main.lua
@@ -494,6 +494,10 @@ function KOSync:updateProgress(manual)
         return
     end
 
+    if manual and NetworkMgr:willRerunWhenOnline(function() self:updateProgress(manual) end) then
+        return
+    end
+
     local KOSyncClient = require("KOSyncClient")
     local client = KOSyncClient:new{
         custom_url = self.kosync_custom_server,
@@ -538,6 +542,10 @@ function KOSync:getProgress(manual)
         return
     end
 
+    if manual and NetworkMgr:willRerunWhenOnline(function() self:getProgress(manual) end) then
+        return
+    end
+
     local KOSyncClient = require("KOSyncClient")
     local client = KOSyncClient:new{
         custom_url = self.kosync_custom_server,

[NiLuJe]

Right, that'd be a start ;).

That, and allowing to tweak the period like the TODO says ;p.

[Galunid]

@pazos ANR is still present in KOSync, not sure if you're interested in fixing that, but what koreader/koreader#6266 did was fix ANR in login/register functions, the more important ones getProgress/updateProgress, don't have your safeguard. I'm assume that's intentional to prevent reader from suddenly becoming unresponsive.

In any cases that function should return true if an active network is found. Anything that changes the logic belongs to the frontend isWifiOn and can be setup based on user preferences.

I don't really think it's possible to do this on frontend, since android will return the network type VPN, and it's not possible to check if there actually is an internet connection, so I'd be stuck with detecting VPN as either connected or disconnected, which is pretty much out of question, since I have VPN set as Always on in android settings. I could modify https://github.com/koreader/android-luajit-launcher/blob/e3d58c6432ba06d43bdd609c38e6315367825d7e/app/src/org/koreader/launcher/BaseActivity.kt#L401 to return different value if connected to VPN and online, vs connected to VPN offline, but that's probably less than ideal. Anyway, let me know your thoughts, feel free to close this issue, if you don't want to bother with this, since it probably doesn't affect too many people.

[pazos]

ANR is still present in KOSync, not sure if you're interested in fixing that, but what koreader/koreader#6266 did was fix ANR in login/register functions, the more important ones getProgress/updateProgress, don't have your safeguard. I'm assume that's intentional to prevent reader from suddenly becoming unresponsive.

Mmm. Nobody faced that issue. The safeguards were added to prevent https://github.com/koreader/koreader/issues/6237. I would suggest to focus now on your original issue with VPNs here.

Other issues with KoSync can be fixed later.

don't really think it's possible to do this on frontend, since android will return the network type VPN, and it's not possible to check if there actually is an internet connection

Use a g_setting called "vpn_alone_is_a_valid_network" paired with https://github.com/koreader/koreader/issues/6726#issuecomment-700340138. So the backend network info will report true, VPN and the frontend will decide if isWifiOn based on Gsettings. If you make the changes in the backend I can fix the frontend, if you want.

So, the backend will return:

on wifi (with or without VPN): true, WIFI on ethernet (""): true, ETHERNET on bluetooth(""): true, BLUETOOTH on VPN alone: true, VPN on no network: false, nil

and the frontend will do:

if vpn_is_valid_network then return ok == 1 else return ok == 1 and transport ~= VPN end