diff options
author | Jeff Davidson <jpd@google.com> | 2015-02-10 10:02:11 -0800 |
---|---|---|
committer | Jeff Davidson <jpd@google.com> | 2015-02-10 23:41:42 -0800 |
commit | b21298a686b04d55ff97223dd317497845713f4b (patch) | |
tree | f601ce3c9ba7082f91d21dcf4bd17e6cf2fa1b68 | |
parent | 175ddbcf2ed71fdcd44a9b64cdc5d479df496a4d (diff) | |
download | frameworks_base-b21298a686b04d55ff97223dd317497845713f4b.zip frameworks_base-b21298a686b04d55ff97223dd317497845713f4b.tar.gz frameworks_base-b21298a686b04d55ff97223dd317497845713f4b.tar.bz2 |
Do not enforce CONTROL_VPN for calls from lockdown VPN.
Clearly document which methods in Vpn.java are designed to be used to
service a Binder call, and which must therefore check permissions and
clear the calling identity, and which methods are designed for
internal use only and which therefore need not check permission.
Add a new startLegacyVpnPrivileged method which bypasses the
permission checks, to be used by lockdown VPN which is a trusted
system service. Ensure that the existing startLegacyVpn method checks
permissions as this is used whenever we respond to a binder call.
Bug: 19311172
Change-Id: I34f13258ee7481f1356bc523124cf5db068b4972
-rw-r--r-- | services/core/java/com/android/server/connectivity/Vpn.java | 29 | ||||
-rw-r--r-- | services/core/java/com/android/server/net/LockdownVpnTracker.java | 8 |
2 files changed, 30 insertions, 7 deletions
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index f08a652..8533f69 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -846,9 +846,29 @@ public class Vpn { /** * Start legacy VPN, controlling native daemons as needed. Creates a * secondary thread to perform connection work, returning quickly. + * + * Should only be called to respond to Binder requests as this enforces caller permission. Use + * {@link #startLegacyVpnPrivileged(VpnProfile, KeyStore, LinkProperties)} to skip the + * permission check only when the caller is trusted (or the call is initiated by the system). */ public void startLegacyVpn(VpnProfile profile, KeyStore keyStore, LinkProperties egress) { enforceControlPermission(); + long token = Binder.clearCallingIdentity(); + try { + startLegacyVpnPrivileged(profile, keyStore, egress); + } finally { + Binder.restoreCallingIdentity(token); + } + } + + /** + * Like {@link #startLegacyVpn(VpnProfile, KeyStore, LinkProperties)}, but does not check + * permissions under the assumption that the caller is the system. + * + * Callers are responsible for checking permissions if needed. + */ + public void startLegacyVpnPrivileged(VpnProfile profile, KeyStore keyStore, + LinkProperties egress) { if (!keyStore.isUnlocked()) { throw new IllegalStateException("KeyStore isn't unlocked"); } @@ -959,10 +979,10 @@ public class Vpn { } private synchronized void startLegacyVpn(VpnConfig config, String[] racoon, String[] mtpd) { - stopLegacyVpn(); + stopLegacyVpnPrivileged(); - // Prepare for the new request. This also checks the caller. - prepare(null, VpnConfig.LEGACY_VPN); + // Prepare for the new request. + prepareInternal(VpnConfig.LEGACY_VPN); updateState(DetailedState.CONNECTING, "startLegacyVpn"); // Start a new LegacyVpnRunner and we are done! @@ -970,7 +990,8 @@ public class Vpn { mLegacyVpnRunner.start(); } - public synchronized void stopLegacyVpn() { + /** Stop legacy VPN. Permissions must be checked by callers. */ + public synchronized void stopLegacyVpnPrivileged() { if (mLegacyVpnRunner != null) { mLegacyVpnRunner.exit(); mLegacyVpnRunner = null; diff --git a/services/core/java/com/android/server/net/LockdownVpnTracker.java b/services/core/java/com/android/server/net/LockdownVpnTracker.java index 3a1e4a4..752614f 100644 --- a/services/core/java/com/android/server/net/LockdownVpnTracker.java +++ b/services/core/java/com/android/server/net/LockdownVpnTracker.java @@ -140,7 +140,7 @@ public class LockdownVpnTracker { if (egressDisconnected || egressChanged) { clearSourceRulesLocked(); mAcceptedEgressIface = null; - mVpn.stopLegacyVpn(); + mVpn.stopLegacyVpnPrivileged(); } if (egressDisconnected) { hideNotification(); @@ -163,7 +163,9 @@ public class LockdownVpnTracker { mAcceptedEgressIface = egressProp.getInterfaceName(); try { - mVpn.startLegacyVpn(mProfile, KeyStore.getInstance(), egressProp); + // Use the privileged method because Lockdown VPN is initiated by the system, so + // no additional permission checks are necessary. + mVpn.startLegacyVpnPrivileged(mProfile, KeyStore.getInstance(), egressProp); } catch (IllegalStateException e) { mAcceptedEgressIface = null; Slog.e(TAG, "Failed to start VPN", e); @@ -250,7 +252,7 @@ public class LockdownVpnTracker { mAcceptedEgressIface = null; mErrorCount = 0; - mVpn.stopLegacyVpn(); + mVpn.stopLegacyVpnPrivileged(); try { mNetService.setFirewallEgressDestRule(mProfile.server, 500, false); mNetService.setFirewallEgressDestRule(mProfile.server, 4500, false); |