Remove fallback DNS

README.md

@@ -57,8 +57,9 @@ Default settings are picked to suit general use cases and maximize compatibility
 
 * Disable IPv6 tethering: Turning this option on will disable IPv6 for system tethering. Useful for stopping IPv6 leaks
   as this app currently doesn't handle IPv6 VPN tethering (see [#6](https://github.com/Mygod/VPNHotspot/issues/6)).
-* Fallback DNS server[:port]: Only used when a DNS server isn't found on the upstream interface.
+* Enable DHCP workaround:
-* Enable DHCP workaround: Only used if your device isn't able to get your clients IP addresses with VPN on.
+    Only used if your device isn't able to get your clients IP addresses with VPN on.
+    This is a global setting, meaning it will only be applied once globally.
 
 ### Misc
 

mobile/src/main/java/be/mygod/vpnhotspot/LocalOnlyHotspotService.kt

@@ -38,7 +38,7 @@ class LocalOnlyHotspotService : IpNeighbourMonitoringService() {
 
     private val binder = Binder()
     private var reservation: WifiManager.LocalOnlyHotspotReservation? = null
-    private var routingManager: LocalOnlyInterfaceManager? = null
+    private var routingManager: RoutingManager? = null
     private var locked = false
     private var receiverRegistered = false
     private val receiver = broadcastReceiver { _, intent ->
@@ -54,7 +54,7 @@ class LocalOnlyHotspotService : IpNeighbourMonitoringService() {
         } else {
             val routingManager = routingManager
             if (routingManager == null) {
-                this.routingManager = LocalOnlyInterfaceManager(this, iface)
+                this.routingManager = RoutingManager.LocalOnly(this, iface).apply { initRouting() }
                 IpNeighbourMonitor.registerCallback(this)
             } else check(iface == routingManager.downstream)
         }

mobile/src/main/java/be/mygod/vpnhotspot/LocalOnlyInterfaceManager.kt

@@ -1,47 +0,0 @@
-package be.mygod.vpnhotspot
-
-import be.mygod.vpnhotspot.App.Companion.app
-import be.mygod.vpnhotspot.net.Routing
-import be.mygod.vpnhotspot.widget.SmartSnackbar
-import timber.log.Timber
-import java.net.InterfaceAddress
-
-class LocalOnlyInterfaceManager(private val caller: Any, val downstream: String) {
-    private var routing: Routing? = null
-
-    init {
-        app.onPreCleanRoutings[this] = { routing?.stop() }
-        app.onRoutingsCleaned[this] = this::clean
-        initRouting()
-    }
-
-    private fun clean() {
-        initRouting((routing ?: return).hostAddress)
-    }
-
-    private fun initRouting(owner: InterfaceAddress? = null) {
-        routing = try {
-            Routing(caller, downstream, owner).apply {
-                try {
-                    ipForward() // local only interfaces need to enable ip_forward
-                    forward()
-                    masquerade(Routing.masquerade)
-                    commit(true)
-                } catch (e: Exception) {
-                    revert()
-                    throw e
-                }   // otw nothing needs to be done
-            }
-        } catch (e: Exception) {
-            SmartSnackbar.make(e).show()
-            Timber.w(e)
-            null
-        }
-    }
-
-    fun stop() {
-        app.onPreCleanRoutings -= this
-        app.onRoutingsCleaned -= this
-        routing?.revert()
-    }
-}

mobile/src/main/java/be/mygod/vpnhotspot/RepeaterService.kt

@@ -132,7 +132,7 @@ class RepeaterService : Service(), WifiP2pManager.ChannelListener, SharedPrefere
             WifiP2pManagerHelper.WIFI_P2P_PERSISTENT_GROUPS_CHANGED_ACTION -> onPersistentGroupsChanged()
         }
     }
-    private var routingManager: LocalOnlyInterfaceManager? = null
+    private var routingManager: RoutingManager? = null
     private var locked = false
 
     var status = Status.IDLE
@@ -282,7 +282,7 @@ class RepeaterService : Service(), WifiP2pManager.ChannelListener, SharedPrefere
         locked = true
         binder.group = group
         check(routingManager == null)
-        routingManager = LocalOnlyInterfaceManager(this, group.`interface`!!)
+        routingManager = RoutingManager.LocalOnly(this, group.`interface`!!).apply { initRouting() }
         status = Status.ACTIVE
         showNotification(group)
     }

mobile/src/main/java/be/mygod/vpnhotspot/RoutingManager.kt

@@ -0,0 +1,60 @@
+package be.mygod.vpnhotspot
+
+import be.mygod.vpnhotspot.App.Companion.app
+import be.mygod.vpnhotspot.net.Routing
+import be.mygod.vpnhotspot.widget.SmartSnackbar
+import timber.log.Timber
+
+abstract class RoutingManager(private val caller: Any, val downstream: String) {
+    companion object {
+        private const val KEY_MASQUERADE_MODE = "service.masqueradeMode"
+        var masqueradeMode: Routing.MasqueradeMode
+            get() {
+                app.pref.getString(KEY_MASQUERADE_MODE, null)?.let { return Routing.MasqueradeMode.valueOf(it) }
+                return if (app.pref.getBoolean("service.masquerade", true)) // legacy settings
+                    Routing.MasqueradeMode.Simple else Routing.MasqueradeMode.None
+            }
+            set(value) = app.pref.edit().putString(KEY_MASQUERADE_MODE, value.name).apply()
+    }
+
+    class LocalOnly(caller: Any, downstream: String) : RoutingManager(caller, downstream) {
+        override fun Routing.configure() {
+            ipForward() // local only interfaces need to enable ip_forward
+            forward()
+            masquerade(masqueradeMode)
+            commit(true)
+        }
+    }
+
+    var routing: Routing? = null
+
+    init {
+        app.onPreCleanRoutings[this] = { routing?.stop() }
+        app.onRoutingsCleaned[this] = { initRouting() }
+    }
+
+    fun initRouting() = try {
+        routing = Routing(caller, downstream).apply {
+            try {
+                configure()
+            } catch (e: Exception) {
+                revert()
+                throw e
+            }
+        }
+        true
+    } catch (e: Exception) {
+        SmartSnackbar.make(e).show()
+        Timber.w(e)
+        routing = null
+        false
+    }
+
+    protected abstract fun Routing.configure()
+
+    fun stop() {
+        app.onPreCleanRoutings -= this
+        app.onRoutingsCleaned -= this
+        routing?.revert()
+    }
+}

mobile/src/main/java/be/mygod/vpnhotspot/SettingsPreferenceFragment.kt

@@ -15,7 +15,6 @@ import be.mygod.vpnhotspot.net.monitor.IpMonitor
 import be.mygod.vpnhotspot.net.monitor.UpstreamMonitor
 import be.mygod.vpnhotspot.preference.AlwaysAutoCompleteEditTextPreferenceDialogFragmentCompat
 import be.mygod.vpnhotspot.preference.SharedPreferenceDataStore
-import be.mygod.vpnhotspot.preference.SummaryFallbackProvider
 import be.mygod.vpnhotspot.util.RootSession
 import be.mygod.vpnhotspot.util.launchUrl
 import be.mygod.vpnhotspot.widget.SmartSnackbar
@@ -29,7 +28,7 @@ import java.net.SocketException
 class SettingsPreferenceFragment : PreferenceFragmentCompat() {
     override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
         preferenceManager.preferenceDataStore = SharedPreferenceDataStore(app.pref)
-        Routing.masquerade = Routing.masquerade // flush default value
+        RoutingManager.masquerade = RoutingManager.masquerade   // flush default value
         addPreferencesFromResource(R.xml.pref_settings)
         val boot = findPreference("service.repeater.startOnBoot") as SwitchPreference
         if (RepeaterService.supported) {
@@ -52,7 +51,6 @@ class SettingsPreferenceFragment : PreferenceFragmentCompat() {
             if (cleaned) app.onRoutingsCleaned()
             true
         }
-        SummaryFallbackProvider(findPreference(Routing.KEY_DNS))
         findPreference<Preference>(IpMonitor.KEY).setOnPreferenceChangeListener { _, _ ->
             SmartSnackbar.make(R.string.settings_restart_required).show()
             true

mobile/src/main/java/be/mygod/vpnhotspot/TetheringService.kt

@@ -10,11 +10,9 @@ import be.mygod.vpnhotspot.net.monitor.IpNeighbourMonitor
 import be.mygod.vpnhotspot.net.wifi.WifiDoubleLock
 import be.mygod.vpnhotspot.util.Event0
 import be.mygod.vpnhotspot.util.broadcastReceiver
-import be.mygod.vpnhotspot.widget.SmartSnackbar
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.launch
-import timber.log.Timber
 
 class TetheringService : IpNeighbourMonitoringService() {
     companion object {
@@ -25,29 +23,38 @@ class TetheringService : IpNeighbourMonitoringService() {
     inner class Binder : android.os.Binder() {
         val routingsChanged = Event0()
 
-        fun isActive(iface: String): Boolean = synchronized(routings) { routings.containsKey(iface) }
+        fun isActive(iface: String): Boolean = synchronized(downstreams) { downstreams.containsKey(iface) }
+    }
+
+    inner class Downstream(caller: Any, downstream: String) : RoutingManager(caller, downstream) {
+        override fun Routing.configure() {
+            forward()
+            masquerade(RoutingManager.masqueradeMode)
+            if (app.pref.getBoolean("service.disableIpv6", true)) disableIpv6()
+            commit()
+        }
     }
 
     private val binder = Binder()
-    private val routings = HashMap<String, Routing?>()
+    private val downstreams = mutableMapOf<String, Downstream>()
     private var locked = false
     private var receiverRegistered = false
     private val receiver = broadcastReceiver { _, intent ->
         val extras = intent.extras ?: return@broadcastReceiver
-        synchronized(routings) {
+        synchronized(downstreams) {
-            for (iface in routings.keys - TetheringManager.getTetheredIfaces(extras))
+            for (iface in downstreams.keys - TetheringManager.getTetheredIfaces(extras))
-                routings.remove(iface)?.revert()
+                downstreams.remove(iface)?.stop()
             updateRoutingsLocked()
         }
     }
-    override val activeIfaces get() = synchronized(routings) { routings.keys.toList() }
+    override val activeIfaces get() = synchronized(downstreams) { downstreams.keys.toList() }
 
     private fun updateRoutingsLocked() {
-        if (locked && routings.keys.all { !TetherType.ofInterface(it).isWifi }) {
+        if (locked && downstreams.keys.all { !TetherType.ofInterface(it).isWifi }) {
             WifiDoubleLock.release()
             locked = false
         }
-        if (routings.isEmpty()) {
+        if (downstreams.isEmpty()) {
             unregisterReceiver()
             ServiceNotification.stopForeground(this)
             stopSelf()
@@ -55,35 +62,14 @@ class TetheringService : IpNeighbourMonitoringService() {
             if (!receiverRegistered) {
                 receiverRegistered = true
                 registerReceiver(receiver, IntentFilter(TetheringManager.ACTION_TETHER_STATE_CHANGED))
-                app.onPreCleanRoutings[this] = {
-                    synchronized(routings) { for (iface in routings.keys) routings.put(iface, null)?.stop() }
-                }
-                app.onRoutingsCleaned[this] = { synchronized(routings) { updateRoutingsLocked() } }
                 IpNeighbourMonitor.registerCallback(this)
             }
-            val disableIpv6 = app.pref.getBoolean("service.disableIpv6", true)
+            val iterator = downstreams.iterator()
-            val iterator = routings.iterator()
             while (iterator.hasNext()) {
-                val entry = iterator.next()
+                val downstream = iterator.next().value
-                if (entry.value == null) try {
+                if (downstream.routing == null && !downstream.initRouting()) iterator.remove()
-                    entry.setValue(Routing(this, entry.key).apply {
-                        try {
-                            forward()
-                            masquerade(Routing.masquerade)
-                            if (disableIpv6) disableIpv6()
-                            commit()
-                        } catch (e: Exception) {
-                            revert()
-                            throw e
             }
-                    })
+            if (downstreams.isEmpty()) {
-                } catch (e: Exception) {
-                    Timber.w(e)
-                    SmartSnackbar.make(e).show()
-                    iterator.remove()
-                }
-            }
-            if (routings.isEmpty()) {
                 updateRoutingsLocked()
                 return
             }
@@ -97,23 +83,26 @@ class TetheringService : IpNeighbourMonitoringService() {
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
         if (intent != null) {
             val ifaces = intent.getStringArrayExtra(EXTRA_ADD_INTERFACES) ?: emptyArray()
-            synchronized(routings) {
+            synchronized(downstreams) {
                 for (iface in ifaces) {
-                    routings[iface] = null
+                    Downstream(this, iface).apply {
+                        downstreams[iface] = this
+                        initRouting()
+                    }
                     if (TetherType.ofInterface(iface).isWifi && !locked) {
                         WifiDoubleLock.acquire()
                         locked = true
                     }
                 }
-                routings.remove(intent.getStringExtra(EXTRA_REMOVE_INTERFACE))?.revert()
+                downstreams.remove(intent.getStringExtra(EXTRA_REMOVE_INTERFACE))?.stop()
                 updateRoutingsLocked()
             }
-        } else if (routings.isEmpty()) stopSelf(startId)
+        } else if (downstreams.isEmpty()) stopSelf(startId)
         return START_NOT_STICKY
     }
 
     override fun onDestroy() {
-        routings.values.forEach { it?.revert() }    // force clean to prevent leakage
+        downstreams.values.forEach { it.stop() }    // force clean to prevent leakage
         unregisterReceiver()
         super.onDestroy()
     }

mobile/src/main/java/be/mygod/vpnhotspot/net/Routing.kt

@@ -20,8 +20,7 @@ import java.net.*
  *
  * Once revert is called, this object no longer serves any purpose.
  */
-class Routing(private val caller: Any, private val downstream: String, ownerAddress: InterfaceAddress? = null) :
+class Routing(private val caller: Any, private val downstream: String) : IpNeighbourMonitor.Callback {
-        IpNeighbourMonitor.Callback {
     companion object {
         /**
          * Since Android 5.0, RULE_PRIORITY_TETHERING = 18000.
@@ -41,16 +40,6 @@ class Routing(private val caller: Any, private val downstream: String, ownerAddr
          * Source: https://android.googlesource.com/platform/external/iptables/+/android-5.0.0_r1/iptables/iptables.c#1574
          */
         val IPTABLES = if (Build.VERSION.SDK_INT >= 26) "iptables -w 1" else "iptables -w"
-        const val KEY_DNS = "service.dns"
-        const val KEY_MASQUERADE = "service.masqueradeMode"
-
-        var masquerade: MasqueradeMode
-            get() {
-                app.pref.getString(KEY_MASQUERADE, null)?.let { return MasqueradeMode.valueOf(it) }
-                return if (app.pref.getBoolean("service.masquerade", true)) // legacy settings
-                    MasqueradeMode.Simple else MasqueradeMode.None
-            }
-            set(value) = app.pref.edit().putString(KEY_MASQUERADE, value.name).apply()
 
         fun clean() {
             TrafficRecorder.clean()
@@ -93,16 +82,15 @@ class Routing(private val caller: Any, private val downstream: String, ownerAddr
         override val message: String get() = app.getString(R.string.exception_interface_not_found)
     }
 
-    val hostAddress = try {
+    private val hostAddress = try {
-        ownerAddress ?: NetworkInterface.getByName(downstream)!!.interfaceAddresses!!
+        NetworkInterface.getByName(downstream)!!.interfaceAddresses!!.asSequence().single { it.address is Inet4Address }
-                .asSequence().single { it.address is Inet4Address }
     } catch (e: Exception) {
         throw InterfaceNotFoundException(e)
     }
-    val hostSubnet = "${hostAddress.address.hostAddress}/${hostAddress.networkPrefixLength}"
+    private val hostSubnet = "${hostAddress.address.hostAddress}/${hostAddress.networkPrefixLength}"
     private val transaction = RootSession.beginTransaction()
 
-    var masqueradeMode = MasqueradeMode.None
+    private var masqueradeMode = MasqueradeMode.None
 
     private val upstreams = HashSet<String>()
     private open inner class Upstream(val priority: Int) : UpstreamMonitor.Callback {
@@ -117,6 +105,7 @@ class Routing(private val caller: Any, private val downstream: String, ownerAddr
                             "ip rule del from all iif $downstream priority $priority")
                 }
                 when (masqueradeMode) {
+                    MasqueradeMode.None -> { }  // nothing to be done here
                     // note: specifying -i wouldn't work for POSTROUTING
                     MasqueradeMode.Simple -> {
                         iptablesAdd(if (upstream == null) "vpnhotspot_masquerade -s $hostSubnet -j MASQUERADE" else
@@ -278,11 +267,10 @@ class Routing(private val caller: Any, private val downstream: String, ownerAddr
     private var currentDns: DnsRoute? = null
     private fun updateDnsRoute() {
         var dns = (upstream.dns + fallbackUpstream.dns).firstOrNull { it is Inet4Address }?.hostAddress
-                ?: app.pref.getString(KEY_DNS, null)
+        if (dns.isNullOrBlank()) dns = null
-        if (dns.isNullOrBlank()) dns = "8.8.8.8"
         if (dns != currentDns?.dns) {
             currentDns?.transaction?.revert()
-            currentDns = try {
+            currentDns = if (dns == null) null else try {
                 DnsRoute(dns)
             } catch (e: RuntimeException) {
                 Timber.w(e)

mobile/src/main/res/drawable/ic_action_dns.xml

@@ -1,10 +0,0 @@
-<vector xmlns:android="http://schemas.android.com/apk/res/android"
-        android:width="24dp"
-        android:height="24dp"
-        android:viewportWidth="24.0"
-        android:viewportHeight="24.0"
-        android:tint="?attr/colorControlNormal">
-    <path
-        android:fillColor="#FF000000"
-        android:pathData="M20,13H4c-0.55,0 -1,0.45 -1,1v6c0,0.55 0.45,1 1,1h16c0.55,0 1,-0.45 1,-1v-6c0,-0.55 -0.45,-1 -1,-1zM7,19c-1.1,0 -2,-0.9 -2,-2s0.9,-2 2,-2 2,0.9 2,2 -0.9,2 -2,2zM20,3H4c-0.55,0 -1,0.45 -1,1v6c0,0.55 0.45,1 1,1h16c0.55,0 1,-0.45 1,-1V4c0,-0.55 -0.45,-1 -1,-1zM7,9c-1.1,0 -2,-0.9 -2,-2s0.9,-2 2,-2 2,0.9 2,2 -0.9,2 -2,2z"/>
-</vector>

mobile/src/main/res/values-zh-rCN/strings.xml

@@ -97,7 +97,6 @@
     <string name="settings_service_ip_monitor_monitor">Netlink 监听</string>
     <string name="settings_service_ip_monitor_monitor_root">Netlink 监听 (root)</string>
     <string name="settings_service_ip_monitor_poll">轮询</string>
-    <string name="settings_service_dns">备用 DNS 服务器[:端口]</string>
     <string name="settings_service_upstream">上游网络接口</string>
     <string name="settings_service_upstream_auto">自动检测系统 VPN</string>
     <string name="settings_service_clean">清理/重新应用路由规则</string>

mobile/src/main/res/values/strings.xml

@@ -103,7 +103,6 @@
     <string name="settings_service_ip_monitor_monitor">Netlink monitor</string>
     <string name="settings_service_ip_monitor_monitor_root">Netlink monitor with root</string>
     <string name="settings_service_ip_monitor_poll">Poll</string>
-    <string name="settings_service_dns">Fallback DNS server[:port]</string>
     <string name="settings_service_upstream">Upstream network interface</string>
     <string name="settings_service_upstream_auto">Auto detect system VPN</string>
     <string name="settings_service_dhcp_workaround">Enable DHCP workaround</string>

mobile/src/main/res/xml/pref_settings.xml

@@ -28,12 +28,6 @@
             app:title="@string/settings_service_disable_ipv6"
             app:summary="@string/settings_service_disable_ipv6_summary"
             app:defaultValue="true"/>
-        <EditTextPreference
-            app:key="service.dns"
-            app:icon="@drawable/ic_action_dns"
-            app:title="@string/settings_service_dns"
-            app:summary="8.8.8.8"
-            app:defaultValue="8.8.8.8"/>
         <SwitchPreference
             app:key="service.dhcpWorkaround"
             app:icon="@drawable/ic_action_build"