Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Readability improvements to the Vpn class with no logic changes"

Browse Source
This commit is contained in:
Chalard Jean
2018-01-12 05:11:29 +00:00
committed by Gerrit Code Review
parent 5edae415b0 dd59eced79
commit 2fef505e1e
1 changed files with 45 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

91
services/core/java/com/android/server/connectivity/Vpn.java
View File

@@ -128,7 +128,7 @@ public class Vpn {
// Length of time (in milliseconds) that an app hosting an always-on VPN is placed on // Length of time (in milliseconds) that an app hosting an always-on VPN is placed on
// the device idle whitelist during service launch and VPN bootstrap. // the device idle whitelist during service launch and VPN bootstrap.
private static final long VPN_LAUNCH_IDLE_WHITELIST_DURATION = 60 * 1000; private static final long VPN_LAUNCH_IDLE_WHITELIST_DURATION_MS = 60 * 1000;
// TODO: create separate trackers for each unique VPN to support // TODO: create separate trackers for each unique VPN to support
// automated reconnection // automated reconnection
@@ -183,10 +183,10 @@ public class Vpn {
@GuardedBy("this") @GuardedBy("this")
private Set<UidRange> mBlockedUsers = new ArraySet<>(); private Set<UidRange> mBlockedUsers = new ArraySet<>();
// Handle of user initiating VPN. // Handle of the user initiating VPN.
private final int mUserHandle; private final int mUserHandle;
// Listen to package remove and change event in this user // Listen to package removal and change events (update/uninstall) for this user
private final BroadcastReceiver mPackageIntentReceiver = new BroadcastReceiver() { private final BroadcastReceiver mPackageIntentReceiver = new BroadcastReceiver() {
@Override @Override
public void onReceive(Context context, Intent intent) { public void onReceive(Context context, Intent intent) {
@@ -197,14 +197,14 @@ public class Vpn {
} }
synchronized (Vpn.this) { synchronized (Vpn.this) {
// Avoid race that always-on package has been unset // Avoid race where always-on package has been unset
if (!packageName.equals(getAlwaysOnPackage())) { if (!packageName.equals(getAlwaysOnPackage())) {
return; return;
} }
final String action = intent.getAction(); final String action = intent.getAction();
Log.i(TAG, "Received broadcast " + action + " for always-on package " + packageName Log.i(TAG, "Received broadcast " + action + " for always-on VPN package "
+ " in user " + mUserHandle); + packageName + " in user " + mUserHandle);
switch(action) { switch(action) {
case Intent.ACTION_PACKAGE_REPLACED: case Intent.ACTION_PACKAGE_REPLACED:
@@ -248,7 +248,8 @@ public class Vpn {
Log.wtf(TAG, "Problem registering observer", e); Log.wtf(TAG, "Problem registering observer", e);
} }
mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_VPN, 0, NETWORKTYPE, ""); mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_VPN, 0 /* subtype */, NETWORKTYPE,
"" /* subtypeName */);
mNetworkCapabilities = new NetworkCapabilities(); mNetworkCapabilities = new NetworkCapabilities();
mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN); mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN);
mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN);
@@ -258,7 +259,7 @@ public class Vpn {
} }
/** /**
* Set if this object is responsible for watching for {@link NetworkInfo} * Set whether this object is responsible for watching for {@link NetworkInfo}
* teardown. When {@code false}, teardown is handled externally by someone * teardown. When {@code false}, teardown is handled externally by someone
* else. * else.
*/ */
@@ -481,7 +482,6 @@ public class Vpn {
} }
private void unregisterPackageChangeReceiverLocked() { private void unregisterPackageChangeReceiverLocked() {
// register previous intent filter
if (mIsPackageIntentReceiverRegistered) { if (mIsPackageIntentReceiverRegistered) {
mContext.unregisterReceiver(mPackageIntentReceiver); mContext.unregisterReceiver(mPackageIntentReceiver);
mIsPackageIntentReceiverRegistered = false; mIsPackageIntentReceiverRegistered = false;
@@ -582,7 +582,7 @@ public class Vpn {
DeviceIdleController.LocalService idleController = DeviceIdleController.LocalService idleController =
LocalServices.getService(DeviceIdleController.LocalService.class); LocalServices.getService(DeviceIdleController.LocalService.class);
idleController.addPowerSaveTempWhitelistApp(Process.myUid(), alwaysOnPackage, idleController.addPowerSaveTempWhitelistApp(Process.myUid(), alwaysOnPackage,
VPN_LAUNCH_IDLE_WHITELIST_DURATION, mUserHandle, false, "vpn"); VPN_LAUNCH_IDLE_WHITELIST_DURATION_MS, mUserHandle, false, "vpn");
// Start the VPN service declared in the app's manifest. // Start the VPN service declared in the app's manifest.
Intent serviceIntent = new Intent(VpnConfig.SERVICE_INTERFACE); Intent serviceIntent = new Intent(VpnConfig.SERVICE_INTERFACE);
@@ -612,9 +612,10 @@ public class Vpn {
* It uses {@link VpnConfig#LEGACY_VPN} as its package name, and * It uses {@link VpnConfig#LEGACY_VPN} as its package name, and
* it can be revoked by itself. * it can be revoked by itself.
* *
* Note: when we added VPN pre-consent in http://ag/522961 the names oldPackage * Note: when we added VPN pre-consent in
* and newPackage become misleading, because when an app is pre-consented, we * https://android.googlesource.com/platform/frameworks/base/+/0554260
* actually prepare oldPackage, not newPackage. * the names oldPackage and newPackage became misleading, because when
* an app is pre-consented, we actually prepare oldPackage, not newPackage.
* *
* Their meanings actually are: * Their meanings actually are:
* *
@@ -630,7 +631,7 @@ public class Vpn {
* @param oldPackage The package name of the old VPN application * @param oldPackage The package name of the old VPN application
* @param newPackage The package name of the new VPN application * @param newPackage The package name of the new VPN application
* *
* @return true if the operation is succeeded. * @return true if the operation succeeded.
*/ */
public synchronized boolean prepare(String oldPackage, String newPackage) { public synchronized boolean prepare(String oldPackage, String newPackage) {
if (oldPackage != null) { if (oldPackage != null) {
@@ -639,7 +640,7 @@ public class Vpn {
return false; return false;
} }
// Package is not same or old package was reinstalled. // Package is not the same or old package was reinstalled.
if (!isCurrentPreparedPackage(oldPackage)) { if (!isCurrentPreparedPackage(oldPackage)) {
// The package doesn't match. We return false (to obtain user consent) unless the // The package doesn't match. We return false (to obtain user consent) unless the
// user has already consented to that VPN package. // user has already consented to that VPN package.
@@ -861,8 +862,8 @@ public class Vpn {
long token = Binder.clearCallingIdentity(); long token = Binder.clearCallingIdentity();
try { try {
mNetworkAgent = new NetworkAgent(mLooper, mContext, NETWORKTYPE, mNetworkAgent = new NetworkAgent(mLooper, mContext, NETWORKTYPE /* logtag */,
mNetworkInfo, mNetworkCapabilities, lp, 0, networkMisc) { mNetworkInfo, mNetworkCapabilities, lp, 0 /* score */, networkMisc) {
@Override @Override
public void unwanted() { public void unwanted() {
// We are user controlled, not driven by NetworkRequest. // We are user controlled, not driven by NetworkRequest.
@@ -936,7 +937,7 @@ public class Vpn {
} }
ResolveInfo info = AppGlobals.getPackageManager().resolveService(intent, ResolveInfo info = AppGlobals.getPackageManager().resolveService(intent,
null, 0, mUserHandle); null, 0, mUserHandle);
if (info == null) { if (info == null) {
throw new SecurityException("Cannot find " + config.user); throw new SecurityException("Cannot find " + config.user);
} }
@@ -944,7 +945,7 @@ public class Vpn {
throw new SecurityException(config.user + " does not require " + BIND_VPN_SERVICE); throw new SecurityException(config.user + " does not require " + BIND_VPN_SERVICE);
} }
} catch (RemoteException e) { } catch (RemoteException e) {
throw new SecurityException("Cannot find " + config.user); throw new SecurityException("Cannot find " + config.user);
} finally { } finally {
Binder.restoreCallingIdentity(token); Binder.restoreCallingIdentity(token);
} }
@@ -1337,7 +1338,7 @@ public class Vpn {
} }
private void enforceControlPermissionOrInternalCaller() { private void enforceControlPermissionOrInternalCaller() {
// Require caller to be either an application with CONTROL_VPN permission or a process // Require the caller to be either an application with CONTROL_VPN permission or a process
// in the system server. // in the system server.
mContext.enforceCallingOrSelfPermission(Manifest.permission.CONTROL_VPN, mContext.enforceCallingOrSelfPermission(Manifest.permission.CONTROL_VPN,
"Unauthorized Caller"); "Unauthorized Caller");
@@ -1417,7 +1418,7 @@ public class Vpn {
} }
/** /**
* This method should only be called by ConnectivityService. Because it doesn't * This method should only be called by ConnectivityService because it doesn't
* have enough data to fill VpnInfo.primaryUnderlyingIface field. * have enough data to fill VpnInfo.primaryUnderlyingIface field.
*/ */
public synchronized VpnInfo getVpnInfo() { public synchronized VpnInfo getVpnInfo() {
@@ -1768,7 +1769,7 @@ public class Vpn {
* Bringing up a VPN connection takes time, and that is all this thread * Bringing up a VPN connection takes time, and that is all this thread
* does. Here we have plenty of time. The only thing we need to take * does. Here we have plenty of time. The only thing we need to take
* care of is responding to interruptions as soon as possible. Otherwise * care of is responding to interruptions as soon as possible. Otherwise
* requests will be piled up. This can be done in a Handler as a state * requests will pile up. This could be done in a Handler as a state
* machine, but it is much easier to read in the current form. * machine, but it is much easier to read in the current form.
*/ */
private class LegacyVpnRunner extends Thread { private class LegacyVpnRunner extends Thread {
@@ -1781,7 +1782,7 @@ public class Vpn {
private final AtomicInteger mOuterConnection = private final AtomicInteger mOuterConnection =
new AtomicInteger(ConnectivityManager.TYPE_NONE); new AtomicInteger(ConnectivityManager.TYPE_NONE);
private long mTimer = -1; private long mBringupStartTime = -1;
/** /**
* Watch for the outer connection (passing in the constructor) going away. * Watch for the outer connection (passing in the constructor) going away.
@@ -1861,8 +1862,8 @@ public class Vpn {
synchronized (TAG) { synchronized (TAG) {
Log.v(TAG, "Executing"); Log.v(TAG, "Executing");
try { try {
execute(); bringup();
monitorDaemons(); waitForDaemonsToStop();
interrupted(); // Clear interrupt flag if execute called exit. interrupted(); // Clear interrupt flag if execute called exit.
} catch (InterruptedException e) { } catch (InterruptedException e) {
} finally { } finally {
@@ -1883,30 +1884,27 @@ public class Vpn {
} }
} }
private void checkpoint(boolean yield) throws InterruptedException { private void checkInterruptAndDelay(boolean sleepLonger) throws InterruptedException {
long now = SystemClock.elapsedRealtime(); long now = SystemClock.elapsedRealtime();
if (mTimer == -1) { if (now - mBringupStartTime <= 60000) {
mTimer = now; Thread.sleep(sleepLonger ? 200 : 1);
Thread.sleep(1);
} else if (now - mTimer <= 60000) {
Thread.sleep(yield ? 200 : 1);
} else { } else {
updateState(DetailedState.FAILED, "checkpoint"); updateState(DetailedState.FAILED, "checkpoint");
throw new IllegalStateException("Time is up"); throw new IllegalStateException("VPN bringup took too long");
} }
} }
private void execute() { private void bringup() {
// Catch all exceptions so we can clean up few things. // Catch all exceptions so we can clean up a few things.
boolean initFinished = false; boolean initFinished = false;
try { try {
// Initialize the timer. // Initialize the timer.
checkpoint(false); mBringupStartTime = SystemClock.elapsedRealtime();
// Wait for the daemons to stop. // Wait for the daemons to stop.
for (String daemon : mDaemons) { for (String daemon : mDaemons) {
while (!SystemService.isStopped(daemon)) { while (!SystemService.isStopped(daemon)) {
checkpoint(true); checkInterruptAndDelay(true);
} }
} }
@@ -1943,7 +1941,7 @@ public class Vpn {
// Wait for the daemon to start. // Wait for the daemon to start.
while (!SystemService.isRunning(daemon)) { while (!SystemService.isRunning(daemon)) {
checkpoint(true); checkInterruptAndDelay(true);
} }
// Create the control socket. // Create the control socket.
@@ -1959,7 +1957,7 @@ public class Vpn {
} catch (Exception e) { } catch (Exception e) {
// ignore // ignore
} }
checkpoint(true); checkInterruptAndDelay(true);
} }
mSockets[i].setSoTimeout(500); mSockets[i].setSoTimeout(500);
@@ -1973,7 +1971,7 @@ public class Vpn {
out.write(bytes.length >> 8); out.write(bytes.length >> 8);
out.write(bytes.length); out.write(bytes.length);
out.write(bytes); out.write(bytes);
checkpoint(false); checkInterruptAndDelay(false);
} }
out.write(0xFF); out.write(0xFF);
out.write(0xFF); out.write(0xFF);
@@ -1989,7 +1987,7 @@ public class Vpn {
} catch (Exception e) { } catch (Exception e) {
// ignore // ignore
} }
checkpoint(true); checkInterruptAndDelay(true);
} }
} }
@@ -2002,7 +2000,7 @@ public class Vpn {
throw new IllegalStateException(daemon + " is dead"); throw new IllegalStateException(daemon + " is dead");
} }
} }
checkpoint(true); checkInterruptAndDelay(true);
} }
// Now we are connected. Read and parse the new state. // Now we are connected. Read and parse the new state.
@@ -2058,8 +2056,8 @@ public class Vpn {
// Set the start time // Set the start time
mConfig.startTime = SystemClock.elapsedRealtime(); mConfig.startTime = SystemClock.elapsedRealtime();
// Check if the thread is interrupted while we are waiting. // Check if the thread was interrupted while we were waiting on the lock.
checkpoint(false); checkInterruptAndDelay(false);
// Check if the interface is gone while we are waiting. // Check if the interface is gone while we are waiting.
if (jniCheck(mConfig.interfaze) == 0) { if (jniCheck(mConfig.interfaze) == 0) {
@@ -2082,10 +2080,11 @@ public class Vpn {
} }
/** /**
* Monitor the daemons we started, moving to disconnected state if the * Check all daemons every two seconds. Return when one of them is stopped.
* underlying services fail. * The caller will move to the disconnected state when this function returns,
* which can happen if a daemon failed or if the VPN was torn down.
*/ */
private void monitorDaemons() throws InterruptedException{ private void waitForDaemonsToStop() throws InterruptedException {
if (!mNetworkInfo.isConnected()) { if (!mNetworkInfo.isConnected()) {
return; return;
} }