Merge pull request #3199 from mvanhorn/fix/3197-vpn-control-intent-identity-fallback

fix: restore external intent control when sender identity is absent

Author: hussainmohd-a

app/src/main/java/com/celzero/bravedns/receiver/VPNControlReceiver.kt

@@ -44,16 +44,17 @@ import org.koin.core.component.inject
  *  - [ACTION_WG_RESUME] - resume live wireguard proxy(es)
  *
  * Caller identity:
- *  - Android 14 (API 34) and above: resolved exclusively from the OS-provided
- *    [BroadcastReceiver.getSentFromPackage] / [BroadcastReceiver.getSentFromUid]; intent extras
- *    are never trusted, even when the OS identity is absent.
+ *  - Android 14 (API 34) and above: prefers the OS-provided
+ *    [BroadcastReceiver.getSentFromPackage] / [BroadcastReceiver.getSentFromUid]. When the OS
+ *    identity is unavailable, [EXTRA_SENDER] is used as a compatibility fallback. That extra is
+ *    sender-controlled and spoofable, so it is accepted only when it is allowlisted below.
  *  - Below Android 14: no OS-provided broadcast sender identity exists, so the legacy
- *    [EXTRA_SENDER] extra convention is honored instead. NOTE: that extra is sender-controlled
- *    and therefore spoofable on this path; it is kept solely to preserve automation support on
- *    older Android versions.
+ *    [EXTRA_SENDER] extra convention is honored instead. That extra is sender-controlled and
+ *    therefore spoofable on this path; it is kept solely to preserve automation support on older
+ *    Android versions.
  *
  * Extras:
- *  - [EXTRA_SENDER] (String, legacy, only honored below Android 14): calling package name
+ *  - [EXTRA_SENDER] (String, legacy or Android 14+ fallback): calling package name
  *  - [EXTRA_WG_IDS] (String, required for the WG_* actions): comma separated wireguard config
  *    ids, e.g. "1,3"
  *
@@ -66,7 +67,7 @@ class VpnControlReceiver: BroadcastReceiver(), KoinComponent {
     // seam for tests; executes validated wg commands
     internal var wgCommands: WgCommands = WgCommands()
 
-    // seam for tests; resolves the caller identity (never from untrusted extras on API 34+)
+    // seam for tests; resolves the caller identity, preferring the OS-provided identity
     internal var callerResolver: (Context, Intent) -> String? =
         { context, intent -> defaultCallerResolver(context, intent) }
 
@@ -214,26 +215,36 @@ class VpnControlReceiver: BroadcastReceiver(), KoinComponent {
     internal fun defaultCallerResolver(context: Context, intent: Intent): String? {
         if (DEBUG) dumpIntent(intent)
 
-        // Android 14 (API 34) and above: the OS stamps the sender identity on every broadcast;
-        // only trust that, never sender-controlled extras
+        val senderExtra = intent.getStringExtra(EXTRA_SENDER)
+
+        // Android 14 (API 34) and above: prefer the OS-provided sender identity. Some external
+        // automation broadcasts do not carry it, so retain the sender extra as a compatibility
+        // fallback for the allowlist check in onReceive (the extra is sender-controlled).
         if (isAtleastU()) {
             val pkg = sentFromPackage
                 ?: context.packageManager.getPackagesForUid(sentFromUid)?.firstOrNull()
             if (pkg != null) {
                 Logger.i(LOG_TAG_VPN, "$TAG caller from OS-provided identity: $pkg")
                 return pkg
             }
-            Logger.w(LOG_TAG_VPN, "$TAG no OS-provided sender identity, rejecting intent")
-            return null
+            if (senderExtra != null) {
+                Logger.w(
+                    LOG_TAG_VPN,
+                    "$TAG no OS-provided sender identity, using sender extra fallback " +
+                        "(sender-controlled and spoofable): $senderExtra"
+                )
+            } else {
+                Logger.w(LOG_TAG_VPN, "$TAG no OS-provided sender identity or sender extra")
+            }
+            return senderExtra
         }
 
         // below Android 14 there is no OS-provided broadcast sender identity; fall back to the
         // legacy "sender" extra convention (spoofable on this path, accepted for automation)
-        val legacy = intent.getStringExtra(EXTRA_SENDER)
-        if (legacy != null) {
-            Logger.i(LOG_TAG_VPN, "$TAG caller from legacy extra: $legacy")
+        if (senderExtra != null) {
+            Logger.i(LOG_TAG_VPN, "$TAG caller from legacy extra: $senderExtra")
         }
-        return legacy
+        return senderExtra
     }
 
     fun dumpIntent(intent: Intent) {

app/src/test/java/com/celzero/bravedns/receiver/VpnControlReceiverTest.kt

@@ -6,6 +6,7 @@ import com.celzero.bravedns.service.PersistentState
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
+import io.mockk.spyk
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import org.junit.Assert.assertEquals
@@ -200,28 +201,72 @@ class VpnControlReceiverTest {
     // ── caller identity resolution ───────────────────────────────────────────
 
     @Test
-    fun `default resolver ignores spoofed sender extra and rejects when os identity is absent`() {
+    @Config(sdk = [34])
+    fun `default resolver uses allowlisted sender extra when os identity is absent`() {
         every { persistentState.appTriggerPackages } returns "com.trusted.app"
         every { persistentState.wgTaskerAutomationEnabled } returns true
         receiver.callerResolver = { c, i -> receiver.defaultCallerResolver(c, i) }
         val intent = Intent(VpnControlReceiver.ACTION_WG_PAUSE).apply {
-            // spoofed extra naming an allowlisted package must not grant trust
             putExtra(VpnControlReceiver.EXTRA_SENDER, "com.trusted.app")
             putExtra(VpnControlReceiver.EXTRA_WG_IDS, "1")
         }
         receiver.onReceive(context, intent)
+        coVerify(exactly = 1) { wgCommands.pause(listOf(1)) }
+    }
+
+    @Test
+    @Config(sdk = [34])
+    fun `default resolver rejects missing malformed and untrusted sender extras`() {
+        every { persistentState.appTriggerPackages } returns "com.trusted.app"
+        every { persistentState.wgTaskerAutomationEnabled } returns true
+        receiver.callerResolver = { c, i -> receiver.defaultCallerResolver(c, i) }
+
+        listOf<String?>(null, "", "com.untrusted.app").forEach { sender ->
+            val intent = Intent(VpnControlReceiver.ACTION_WG_PAUSE).apply {
+                if (sender != null) {
+                    putExtra(VpnControlReceiver.EXTRA_SENDER, sender)
+                }
+                putExtra(VpnControlReceiver.EXTRA_WG_IDS, "1")
+            }
+            receiver.onReceive(context, intent)
+        }
+
         coVerify(exactly = 0) { wgCommands.pause(any()) }
     }
 
     @Test
-    fun `default resolver returns null on spoofed extra when no os identity is stamped`() {
+    @Config(sdk = [34])
+    fun `default resolver gives os package identity precedence over sender extra`() {
+        val osIdentityReceiver = spyk(receiver)
+        osIdentityReceiver.wgCommands = wgCommands
+        osIdentityReceiver.callerResolver = { c, i ->
+            osIdentityReceiver.defaultCallerResolver(c, i)
+        }
+        every { osIdentityReceiver.sentFromPackage } returns "com.os.app"
+        every { persistentState.appTriggerPackages } returns "com.trusted.app"
+        every { persistentState.wgTaskerAutomationEnabled } returns true
+        val intent = Intent(VpnControlReceiver.ACTION_WG_PAUSE).apply {
+            putExtra(VpnControlReceiver.EXTRA_SENDER, "com.trusted.app")
+            putExtra(VpnControlReceiver.EXTRA_WG_IDS, "1")
+        }
+
+        osIdentityReceiver.onReceive(context, intent)
+
+        coVerify(exactly = 0) { wgCommands.pause(any()) }
+    }
+
+    @Test
+    @Config(sdk = [34])
+    fun `default resolver uses package resolved from os uid before sender extra`() {
+        val osIdentityReceiver = spyk(receiver)
+        every { osIdentityReceiver.sentFromPackage } returns null
+        every { osIdentityReceiver.sentFromUid } returns 1234
+        every { context.packageManager.getPackagesForUid(1234) } returns arrayOf("com.os.app")
         val intent = Intent(VpnControlReceiver.ACTION_STOP).apply {
             putExtra(VpnControlReceiver.EXTRA_SENDER, "com.trusted.app")
         }
-        // on the Robolectric SDK no OS sender identity is stamped; the spoofed extra must not
-        // be honored even though it names an allowlisted package
-        val resolved = receiver.defaultCallerResolver(context, intent)
-        assertEquals(null, resolved)
+
+        assertEquals("com.os.app", osIdentityReceiver.defaultCallerResolver(context, intent))
     }
 
     @Test