102 lines
5.3 KiB
Diff
102 lines
5.3 KiB
Diff
![]() |
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||
|
From: Tommy Webb <tommy@calyxinstitute.org>
|
||
|
Date: Mon, 5 Dec 2022 14:42:38 +0100
|
||
|
Subject: [PATCH] Reland "Fix network leaks with split-tunnel VPNs"
|
||
|
|
||
|
This does two things:
|
||
|
1. Revert the portion of I48e08f34 "fw/b: Add support for allowing
|
||
|
/disallowing apps on cellular, vpn and wifi networks" that was
|
||
|
previously responsible for updating the restricted mode allowlist
|
||
|
based on changes to the default network.
|
||
|
2. Bring in Ib4bcf5ae "Fix network leaks with split-tunnel VPNs", which
|
||
|
meets the same goal of updating the allowlist, but in a wider range
|
||
|
of conditions. Retaining the prior implementation led to a race
|
||
|
condition which caused crashes and soft reboots, because the calls
|
||
|
to `updateRestrictedModeAllowlistUL()` were not being appropriately
|
||
|
guarded by `mUidRulesFirstLock`.
|
||
|
|
||
|
Ultimately, this patch should probably be squashed into I48e08f34.
|
||
|
|
||
|
Co-authored-by: Oliver Scott <olivercscott@gmail.com>
|
||
|
Issue: calyxos#1081
|
||
|
Change-Id: I84c7667824cc840724a07e7d0435f5ec59a67986
|
||
|
---
|
||
|
.../net/NetworkPolicyManagerService.java | 43 ++++++-------------
|
||
|
1 file changed, 12 insertions(+), 31 deletions(-)
|
||
|
|
||
|
diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
|
||
|
index 8102d892c2d7..7addf69a28af 100644
|
||
|
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
|
||
|
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
|
||
|
@@ -1105,14 +1105,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
|
||
|
ACTION_CARRIER_CONFIG_CHANGED);
|
||
|
mContext.registerReceiver(mCarrierConfigReceiver, carrierConfigFilter, null, mHandler);
|
||
|
|
||
|
- for (UserInfo userInfo : mUserManager.getAliveUsers()) {
|
||
|
- mConnManager.registerDefaultNetworkCallbackForUid(
|
||
|
- UserHandle.getUid(userInfo.id, Process.myUid()),
|
||
|
- mDefaultNetworkCallback,
|
||
|
- mUidEventHandler
|
||
|
- );
|
||
|
- }
|
||
|
-
|
||
|
// listen for meteredness changes
|
||
|
mConnManager.registerNetworkCallback(
|
||
|
new NetworkRequest.Builder().build(), mNetworkCallback);
|
||
|
@@ -1303,11 +1295,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
|
||
|
ConnectivitySettingsManager.getUidsAllowedOnRestrictedNetworks(
|
||
|
mContext);
|
||
|
if (action == ACTION_USER_ADDED) {
|
||
|
- mConnManager.registerDefaultNetworkCallbackForUid(
|
||
|
- UserHandle.getUid(userId, Process.myUid()),
|
||
|
- mDefaultNetworkCallback,
|
||
|
- mUidEventHandler
|
||
|
- );
|
||
|
// Add apps that are allowed by default.
|
||
|
addDefaultRestrictBackgroundAllowlistUidsUL(userId);
|
||
|
try {
|
||
|
@@ -1443,24 +1430,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
|
||
|
return changed;
|
||
|
}
|
||
|
|
||
|
- private final NetworkCallback mDefaultNetworkCallback = new NetworkCallback() {
|
||
|
- @Override
|
||
|
- public void onAvailable(@NonNull Network network) {
|
||
|
- updateRestrictedModeAllowlistUL();
|
||
|
- }
|
||
|
-
|
||
|
- @Override
|
||
|
- public void onCapabilitiesChanged(@NonNull Network network,
|
||
|
- @NonNull NetworkCapabilities networkCapabilities) {
|
||
|
- final int[] newTransports = networkCapabilities.getTransportTypes();
|
||
|
- final boolean transportsChanged = updateTransportChange(
|
||
|
- mNetworkTransports, newTransports, network);
|
||
|
- if (transportsChanged) {
|
||
|
- updateRestrictedModeAllowlistUL();
|
||
|
- }
|
||
|
- }
|
||
|
- };
|
||
|
-
|
||
|
private final NetworkCallback mNetworkCallback = new NetworkCallback() {
|
||
|
@Override
|
||
|
public void onCapabilitiesChanged(@NonNull Network network,
|
||
|
@@ -1888,6 +1857,18 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
|
||
|
updateSubscriptions();
|
||
|
|
||
|
synchronized (mUidRulesFirstLock) {
|
||
|
+ /* With split-tunnel VPNs (those that only include specific apps),
|
||
|
+ * the usual NetworkCallback handlers are never called, because the call to
|
||
|
+ * registerDefaultNetworkCallbackForUid only detects changes that affect this
|
||
|
+ * process; if this process is not covered by the VPN, it won't get callbacks.
|
||
|
+ * Ordinarily, updateRestrictedModeAllowlistUL() would be called from those.
|
||
|
+ * Firewall restrictions for apps will not be updated properly on VPN connect
|
||
|
+ * or disconnect if we don't call it from somewhere else, like here. */
|
||
|
+ // TODO: Come up with an appropriate callback that runs more promptly.
|
||
|
+ // updateNetworksInternal runs later than NetworkCallback handlers run, so
|
||
|
+ // this may present a window of opportunity for unauthorized network access.
|
||
|
+ updateRestrictedModeAllowlistUL();
|
||
|
+
|
||
|
synchronized (mNetworkPoliciesSecondLock) {
|
||
|
ensureActiveCarrierPolicyAL();
|
||
|
normalizePoliciesNL();
|