From c3736bc10da63d6a351d3f8e7781ff1d67ecc9a6 Mon Sep 17 00:00:00 2001 From: Robin Lee Date: Fri, 10 Mar 2017 16:19:54 +0000 Subject: [PATCH] Use Vpn rules (not firewall) for always-on VPN Firewall rules don't work on 464xlat because they were created under an assumption that there's only one address for the server and it's ipv4, which doesn't go so well when we're on an ipv6-only network. Bug: 33159037 Test: runtest -x net/java/com/android/server/connectivity/VpnTest.java Change-Id: Id331526367fe13838874961da194b07bd50d4c97 --- .../android/server/ConnectivityService.java | 14 +--- .../server/NetworkManagementService.java | 3 +- .../com/android/server/connectivity/Vpn.java | 62 ++++++++++++----- .../server/net/LockdownVpnTracker.java | 67 +------------------ .../android/server/connectivity/VpnTest.java | 57 ++++++++++++++++ 5 files changed, 110 insertions(+), 93 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 5b029d1cc007a..5467a341463fb 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3702,17 +3702,9 @@ public class ConnectivityService extends IConnectivityManager.Stub existing.shutdown(); } - try { - if (tracker != null) { - mNetd.setFirewallEnabled(true); - mNetd.setFirewallInterfaceRule("lo", true); - mLockdownTracker = tracker; - mLockdownTracker.init(); - } else { - mNetd.setFirewallEnabled(false); - } - } catch (RemoteException e) { - // ignored; NMS lives inside system_server + if (tracker != null) { + mLockdownTracker = tracker; + mLockdownTracker.init(); } } diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index 87938cb2c8cfd..f4e11c792ed7e 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -95,7 +95,6 @@ import com.android.internal.util.HexDump; import com.android.internal.util.Preconditions; import com.android.server.NativeDaemonConnector.Command; import com.android.server.NativeDaemonConnector.SensitiveArg; -import com.android.server.net.LockdownVpnTracker; import com.google.android.collect.Maps; import java.io.BufferedReader; @@ -628,7 +627,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub } } - setFirewallEnabled(mFirewallEnabled || LockdownVpnTracker.isEnabled()); + setFirewallEnabled(mFirewallEnabled); syncFirewallChainLocked(FIREWALL_CHAIN_NONE, mUidFirewallRules, ""); syncFirewallChainLocked(FIREWALL_CHAIN_STANDBY, mUidFirewallStandbyRules, "standby "); diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 584994a785ec0..5dc8640ff41f8 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -263,6 +263,30 @@ public class Vpn { updateAlwaysOnNotification(detailedState); } + /** + * Chooses whether to force all connections to go though VPN. + * + * Used to enable/disable legacy VPN lockdown. + * + * This uses the same ip rule mechanism as {@link #setAlwaysOnPackage(String, boolean)}; + * previous settings from calling that function will be replaced and saved with the + * always-on state. + * + * @param lockdown whether to prevent all traffic outside of a VPN. + */ + public synchronized void setLockdown(boolean lockdown) { + enforceControlPermissionOrInternalCaller(); + + setVpnForcedLocked(lockdown); + mLockdown = lockdown; + + // Update app lockdown setting if it changed. Legacy VPN lockdown status is controlled by + // LockdownVpnTracker.isEnabled() which keeps track of its own state. + if (mAlwaysOn) { + saveAlwaysOnPackage(); + } + } + /** * Configures an always-on VPN connection through a specific application. * This connection is automatically granted and persisted after a reboot. @@ -376,7 +400,7 @@ public class Vpn { mSystemServices.settingsSecurePutStringForUser(Settings.Secure.ALWAYS_ON_VPN_APP, getAlwaysOnPackage(), mUserHandle); mSystemServices.settingsSecurePutIntForUser(Settings.Secure.ALWAYS_ON_VPN_LOCKDOWN, - (mLockdown ? 1 : 0), mUserHandle); + (mAlwaysOn && mLockdown ? 1 : 0), mUserHandle); } finally { Binder.restoreCallingIdentity(token); } @@ -557,6 +581,7 @@ public class Vpn { mConfig = null; updateState(DetailedState.IDLE, "prepare"); + setVpnForcedLocked(mLockdown); } finally { Binder.restoreCallingIdentity(token); } @@ -1003,9 +1028,7 @@ public class Vpn { Log.wtf(TAG, "Failed to add restricted user to owner", e); } } - if (mAlwaysOn) { - setVpnForcedLocked(mLockdown); - } + setVpnForcedLocked(mLockdown); } } } @@ -1022,9 +1045,7 @@ public class Vpn { Log.wtf(TAG, "Failed to remove restricted user to owner", e); } } - if (mAlwaysOn) { - setVpnForcedLocked(mLockdown); - } + setVpnForcedLocked(mLockdown); } } } @@ -1034,7 +1055,7 @@ public class Vpn { */ public synchronized void onUserStopped() { // Switch off networking lockdown (if it was enabled) - setVpnForcedLocked(false); + setLockdown(false); mAlwaysOn = false; unregisterPackageChangeReceiverLocked(); @@ -1061,20 +1082,31 @@ public class Vpn { */ @GuardedBy("this") private void setVpnForcedLocked(boolean enforce) { + final List exemptedPackages = + isNullOrLegacyVpn(mPackage) ? null : Collections.singletonList(mPackage); + setVpnForcedWithExemptionsLocked(enforce, exemptedPackages); + } + + /** + * @see #setVpnForcedLocked + */ + @GuardedBy("this") + private void setVpnForcedWithExemptionsLocked(boolean enforce, + @Nullable List exemptedPackages) { final Set removedRanges = new ArraySet<>(mBlockedUsers); + + Set addedRanges = Collections.emptySet(); if (enforce) { - final Set addedRanges = createUserAndRestrictedProfilesRanges(mUserHandle, + addedRanges = createUserAndRestrictedProfilesRanges(mUserHandle, /* allowedApplications */ null, - /* disallowedApplications */ Collections.singletonList(mPackage)); + /* disallowedApplications */ exemptedPackages); removedRanges.removeAll(addedRanges); addedRanges.removeAll(mBlockedUsers); - - setAllowOnlyVpnForUids(false, removedRanges); - setAllowOnlyVpnForUids(true, addedRanges); - } else { - setAllowOnlyVpnForUids(false, removedRanges); } + + setAllowOnlyVpnForUids(false, removedRanges); + setAllowOnlyVpnForUids(true, addedRanges); } /** diff --git a/services/core/java/com/android/server/net/LockdownVpnTracker.java b/services/core/java/com/android/server/net/LockdownVpnTracker.java index 4a8539aa32827..b40a7e5619785 100644 --- a/services/core/java/com/android/server/net/LockdownVpnTracker.java +++ b/services/core/java/com/android/server/net/LockdownVpnTracker.java @@ -139,7 +139,6 @@ public class LockdownVpnTracker { " " + mAcceptedEgressIface + "->" + egressIface); if (egressDisconnected || egressChanged) { - clearSourceRulesLocked(); mAcceptedEgressIface = null; mVpn.stopLegacyVpnPrivileged(); } @@ -191,24 +190,6 @@ public class LockdownVpnTracker { EventLogTags.writeLockdownVpnConnected(egressType); showNotification(R.string.vpn_lockdown_connected, R.drawable.vpn_connected); - try { - clearSourceRulesLocked(); - - mNetService.setFirewallInterfaceRule(iface, true); - for (LinkAddress addr : sourceAddrs) { - setFirewallEgressSourceRule(addr, true); - } - - mNetService.setFirewallUidRule(FIREWALL_CHAIN_NONE, ROOT_UID, FIREWALL_RULE_ALLOW); - mNetService.setFirewallUidRule(FIREWALL_CHAIN_NONE, Os.getuid(), FIREWALL_RULE_ALLOW); - - mErrorCount = 0; - mAcceptedIface = iface; - mAcceptedSourceAddr = sourceAddrs; - } catch (RemoteException e) { - throw new RuntimeException("Problem setting firewall rules", e); - } - final NetworkInfo clone = new NetworkInfo(egressInfo); augmentNetworkInfo(clone); mConnService.sendConnectedBroadcast(clone); @@ -225,19 +206,11 @@ public class LockdownVpnTracker { Slog.d(TAG, "initLocked()"); mVpn.setEnableTeardown(false); + mVpn.setLockdown(true); final IntentFilter resetFilter = new IntentFilter(ACTION_LOCKDOWN_RESET); mContext.registerReceiver(mResetReceiver, resetFilter, CONNECTIVITY_INTERNAL, null); - try { - // TODO: support non-standard port numbers - mNetService.setFirewallEgressDestRule(mProfile.server, 500, true); - mNetService.setFirewallEgressDestRule(mProfile.server, 4500, true); - mNetService.setFirewallEgressDestRule(mProfile.server, 1701, true); - } catch (RemoteException e) { - throw new RuntimeException("Problem setting firewall rules", e); - } - handleStateChangedLocked(); } @@ -254,14 +227,7 @@ public class LockdownVpnTracker { mErrorCount = 0; mVpn.stopLegacyVpnPrivileged(); - try { - mNetService.setFirewallEgressDestRule(mProfile.server, 500, false); - mNetService.setFirewallEgressDestRule(mProfile.server, 4500, false); - mNetService.setFirewallEgressDestRule(mProfile.server, 1701, false); - } catch (RemoteException e) { - throw new RuntimeException("Problem setting firewall rules", e); - } - clearSourceRulesLocked(); + mVpn.setLockdown(false); hideNotification(); mContext.unregisterReceiver(mResetReceiver); @@ -278,35 +244,6 @@ public class LockdownVpnTracker { } } - private void clearSourceRulesLocked() { - try { - if (mAcceptedIface != null) { - mNetService.setFirewallInterfaceRule(mAcceptedIface, false); - mAcceptedIface = null; - } - if (mAcceptedSourceAddr != null) { - for (LinkAddress addr : mAcceptedSourceAddr) { - setFirewallEgressSourceRule(addr, false); - } - - mNetService.setFirewallUidRule(FIREWALL_CHAIN_NONE, ROOT_UID, FIREWALL_RULE_DEFAULT); - mNetService.setFirewallUidRule(FIREWALL_CHAIN_NONE,Os.getuid(), FIREWALL_RULE_DEFAULT); - - mAcceptedSourceAddr = null; - } - } catch (RemoteException e) { - throw new RuntimeException("Problem setting firewall rules", e); - } - } - - private void setFirewallEgressSourceRule( - LinkAddress address, boolean allow) throws RemoteException { - // Our source address based firewall rules must only cover our own source address, not the - // whole subnet - final String addrString = address.getAddress().getHostAddress(); - mNetService.setFirewallEgressSourceRule(addrString, allow); - } - public void onNetworkInfoChanged() { synchronized (mStateLock) { handleStateChangedLocked(); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index efe6fec6fc7d2..506d9e5043f4b 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -27,6 +27,7 @@ import android.annotation.UserIdInt; import android.app.AppOpsManager; import android.app.NotificationManager; import android.content.Context; +import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.UserInfo; @@ -42,6 +43,8 @@ import android.test.suitebuilder.annotation.SmallTest; import android.util.ArrayMap; import android.util.ArraySet; +import com.android.internal.net.VpnConfig; + import java.util.ArrayList; import java.util.Arrays; import java.util.Map; @@ -101,8 +104,10 @@ public class VpnTest extends AndroidTestCase { @Override public void setUp() throws Exception { MockitoAnnotations.initMocks(this); + when(mContext.getPackageManager()).thenReturn(mPackageManager); setMockedPackages(mPackages); + when(mContext.getPackageName()).thenReturn(Vpn.class.getPackage().getName()); when(mContext.getSystemService(eq(Context.USER_SERVICE))).thenReturn(mUserManager); when(mContext.getSystemService(eq(Context.APP_OPS_SERVICE))).thenReturn(mAppOps); @@ -257,6 +262,58 @@ public class VpnTest extends AndroidTestCase { })); } + @SmallTest + public void testLockdownRuleRepeatability() throws Exception { + final Vpn vpn = createVpn(primaryUser.id); + + // Given legacy lockdown is already enabled, + vpn.setLockdown(true); + verify(mNetService, times(1)).setAllowOnlyVpnForUids( + eq(true), aryEq(new UidRange[] {UidRange.createForUser(primaryUser.id)})); + + // Enabling legacy lockdown twice should do nothing. + vpn.setLockdown(true); + verify(mNetService, times(1)).setAllowOnlyVpnForUids(anyBoolean(), any(UidRange[].class)); + + // And disabling should remove the rules exactly once. + vpn.setLockdown(false); + verify(mNetService, times(1)).setAllowOnlyVpnForUids( + eq(false), aryEq(new UidRange[] {UidRange.createForUser(primaryUser.id)})); + + // Removing the lockdown again should have no effect. + vpn.setLockdown(false); + verify(mNetService, times(2)).setAllowOnlyVpnForUids(anyBoolean(), any(UidRange[].class)); + } + + @SmallTest + public void testLockdownRuleReversibility() throws Exception { + final Vpn vpn = createVpn(primaryUser.id); + + final UidRange[] entireUser = { + UidRange.createForUser(primaryUser.id) + }; + final UidRange[] exceptPkg0 = { + new UidRange(entireUser[0].start, entireUser[0].start + PKG_UIDS[0] - 1), + new UidRange(entireUser[0].start + PKG_UIDS[0] + 1, entireUser[0].stop) + }; + + final InOrder order = inOrder(mNetService); + + // Given lockdown is enabled with no package (legacy VPN), + vpn.setLockdown(true); + order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); + + // When a new VPN package is set the rules should change to cover that package. + vpn.prepare(null, PKGS[0]); + order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(entireUser)); + order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(exceptPkg0)); + + // When that VPN package is unset, everything should be undone again in reverse. + vpn.prepare(null, VpnConfig.LEGACY_VPN); + order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(exceptPkg0)); + order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); + } + @SmallTest public void testNotificationShownForAlwaysOnApp() { final UserHandle userHandle = UserHandle.of(primaryUser.id);