syncthing-android icon indicating copy to clipboard operation
syncthing-android copied to clipboard

Feature request: add VPN as a run condition

Open jgillula opened this issue 5 years ago • 10 comments

Hi there! I have a use case for Syncthing for Android where in addition to all the current run conditions, I also want it to run if a VPN is active. I'm half-decent at coding, so I'd be willing to try to put together a pull request. Before I do, though, I have three questions for the devs:

  1. Would such a feature be welcomed? If not, no worries: I'll stop bugging you.
  2. If a PR would be welcomed, the solution for detecting active VPNs in recent Android versions is pretty clean. However, based on my research the solution for older Android versions is kind of hacky (detecting if an interface name starts with "tun", "ppp", or "pptp"). If that's a non-starter, then I understand, and I'll stop bugging you.
  3. If both of those are OK, then before I start writing code I want to make sure we agree on how, exactly, the new run condition should work. I can see two major design choices we need to make, depending on the use cases we want to support, which I'll describe below.

The first design choice is: will people only want the following choices (option "a"):

  • Ignore VPN status (just use the other run conditions) <-- default
  • Always run when VPN is on (regardless of other run conditions)

Or should we also support running only when a VPN is not connected? I.e. should the choices be (option "b")

  • Ignore VPN status <-- default
  • Always run when VPN is on
  • Never run when VPN is on (overriding other run conditions)

The second design choice is: should the VPN setting apply regardless of whether the underlying connection is mobile data or wifi (option "x")?

Or do we want to give users a choice for VPN run conditions on mobile data and a separate choice for VPN run conditions on wifi (option "y")?


Personally, my vote would be options (a) and (y).

I think (a) is simpler and more straightforward, and I'm struggling to come up with use cases for why a person would only want to run Syncthing when a VPN is off. But if you disagree, I'm happy to implement (b).

Between (x) and (y), I think we should still give users a choice with (y), since users may not want to incur higher data use over mobile data, regardless of whether a VPN is active. But if you disagree and prefer (x), I'll defer to you.


Let me know what you think. Once we agree on the design, I'll mock up how the run conditions would actually look in the settings screen/behave in pseudo code, and if that still looks OK, I'll write actual code. :smiley:

jgillula avatar Mar 12 '21 16:03 jgillula

Sounds reasonable.

AudriusButkevicius avatar Mar 12 '21 18:03 AudriusButkevicius

Great, I miss this feature too!

I am for options (a) and (y) as well. My syncthing peers are in a wireguard network and I want syncthing to just run if there is an actual chance of talking to other peers - that is, if my phone is connected to the wg VPN.

sunkup avatar Mar 17 '21 09:03 sunkup

OK, then here's how I'd change the settings (bold is new):

  • Run on Wi-Fi
  • Run on metered Wi-Fi
  • Run on specified Wi-Fi networks
  • VPN over Wi-Fi options --> Disabled unless "Run on Wi-Fi" is enabled. Popup with three options:
    1. Ignore VPN status when connected to Wi-Fi (default)
    2. Run on Wi-Fi only if VPN is active
    3. Run on Wi-Fi if VPN is active or connected to specified Wi-Fi networks
  • Run on mobile data
  • Run on mobile data only if VPN is active --> Disabled unless "Run on mobile data" is enabled. Checkbox that is unchecked by default.
  • Run when device is powered by
  • ...

WDYT?

jgillula avatar Mar 21 '21 18:03 jgillula

Generally looks good. Two comments:

Isn't iii. redundant with the combination of having both "run on specified..." and ii set?

Also before you wrote about an option to not run on wi-fi. I don't see that option in the latest proposal - on purpose?

imsodin avatar Mar 21 '21 19:03 imsodin

Isn't iii. redundant with the combination of having both "run on specified..." and ii set?

I didn't mean for iii (Run on Wi-Fi if VPN is active or connected to specified Wi-Fi networks) to be redundant, but I may not have made it very clear. iii is meant to allow users to run Syncthing either only on specified Wi-Fi networks, OR on any Wi-Fi network as long as the VPN is on. The use case is that you trust some Wi-Fi networks, but you also trust your VPN, and you want Syncthing to run in either case.

With ii (Run on Wi-Fi only if VPN is active) and "Run on specified Wi-Fi networks" both set, Syncthing would only run if you were on a trusted network AND the VPN was on.

Also I should have explicitly mentioned before that the popup with i, ii, and iii are mutually exclusive options (a set of radio toggles, not checkboxes).

With that explanation, do you think it's still redundant? I agree it's confusing, and I've been struggling with how to make it clearer. One possibility I've been toying with is to instead add two new preferences for VPN/Wi-Fi instead of the one I suggested above:

  1. A simple checkbox "Run on Wi-Fi only if VPN is active" (default unchecked)
  2. Another checkbox that would show up in the "Run on specified Wi-Fi networks" pop-up were a user selects the specified networks, saying something like "Also run on VPN connections over Wi-Fi regardless of network"

What do you think? I'm wide open to suggestions/more feedback.

Also before you wrote about an option to not run on wi-fi. I don't see that option in the latest proposal - on purpose?

Could you elaborate a little more on which part of the initial proposal is missing? Couldn't a user who didn't want to run on Wi-Fi just make sure the existing "Run on Wi-Fi" checkbox isn't checked? I don't think I'm missing anything, but I probably still need more coffee to clear my brain. :-)

jgillula avatar Mar 21 '21 19:03 jgillula

Yes, I do understand now. Your suggestion with the two checkboxes, one of them conditional below the "specified wi-fi" option sounds good - I think that's easier to grasp. And by default it's a single checkbox (run only on vpn).

I was referring to

  • Never run when VPN is on (overriding other run conditions)

which I understood as an option that means: Run when on wi-fi, but do not run when also connected to vpn. Use-case I was imagining was e.g. an organisation with annoyingly strict network policies, that doesn't allow (or complains about) "non-standard" traffic like Syncthing's. Not something very common in any case, so I am fine with not having it.

imsodin avatar Mar 21 '21 19:03 imsodin

I was referring to

  • Never run when VPN is on (overriding other run conditions)

which I understood as an option that means: Run when on wi-fi, but do not run when also connected to vpn. Use-case I was imagining was e.g. an organisation with annoyingly strict network policies, that doesn't allow (or complains about) "non-standard" traffic like Syncthing's. Not something very common in any case, so I am fine with not having it.

Gotcha! Yeah, I figured presenting that option would make things even more complicated/confusing, but maybe we could pull it off as follows:

  • Run on Wi-Fi
  • Run on metered Wi-Fi
  • Run on specified Wi-Fi networks
    • Also run on unselected Wi-Fi networks if VPN is connected <-- A checkbox, let's call it prefTreatVpnAsWhitelistedWifi, default off
  • VPN over Wi-Fi conditions <-- Enabled only if "Run on Wi-Fi" is enabled. Let's call it prefRunOnWifiVpn, pops up a toggle with the following mutually exclusive options:**
    • Run on Wi-Fi regardless of VPN status <-- Value: WIFI_IGNORE_VPN, Default
    • Run on Wi-Fi only if VPN is connected <-- Value: WIFI_ONLY_ON_VPN
    • Do not run on Wi-Fi if VPN is connected <-- Value: WIFI_NEVER_ON_VPN
  • Run on mobile data
  • VPN over mobile data conditions <-- Enabled only if "Run on mobile data" is enabled. Pops up a toggle with the following mutually exclusive options:
    • Run on mobile data regardless of VPN status <-- Default
    • Only run on mobile data if VPN is connected
    • Do not run on mobile data if VPN is connected
  • Run when device is powered by
  • ...

Then the table of run conditions for Wi-Fi looks like (assuming wifi is on, and ignoring metered wifi since the case is identical whether the wifi network is metered or not):

Screenshot from 2021-03-21 21-55-05 Cells where Syncthing would run are highlighted in green for clarity. Cells that represent default values (which should match the behavior as if the new VPN-specific settings didn't exist) have bold/italic/underlined text with thick borders. I've also collapsed down columns that had identical values (because the value of a given preference didn't matter).

Does this all sound right?

jgillula avatar Mar 22 '21 05:03 jgillula

Sounds right indeed, but as you said: Quite complex.
If the never_on_vpn options are included, I think it might be simpler if there was a checkbox on "vpn conditions" and then just a two-state selection between "run on vpn" and "do not run on vpn". Such if I don't care about vpn, it's obvious at a glance that when "vpn conditions" is not checked, there's no restrictions. I don't need to understand what the "default" means.

imsodin avatar Mar 22 '21 08:03 imsodin

If the never_on_vpn options are included, I think it might be simpler if there was a checkbox on "vpn conditions" and then just a two-state selection between "run on vpn" and "do not run on vpn". Such if I don't care about vpn, it's obvious at a glance that when "vpn conditions" is not checked, there's no restrictions. I don't need to understand what the "default" means.

Do you think that might clutter things too much? I was thinking we could still make it clear with the three-state option by having the descriptive text show the current state. I.e. it would look like


VPN over Wi-Fi conditions

Run on Wi-Fi regardless of VPN status


I was also thinking of doing the same thing for the descriptive text for "Run on specified Wi-Fi networks", so it might say "Run only on selected Wi-Fi networks: "Home Wi-Fi", "Work Wi-Fi", and also if VPN is connected"

WDYT?

Also, thank you so much for your patience and working through this with me--I really appreciate it!

jgillula avatar Mar 22 '21 16:03 jgillula

Sounds all good

imsodin avatar Mar 22 '21 17:03 imsodin