diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/com/android/server/NetworkStackService.java | 3 | ||||
| -rw-r--r-- | src/com/android/server/connectivity/NetworkMonitor.java | 23 | ||||
| -rw-r--r-- | src/com/android/server/util/PermissionUtil.java | 34 |
3 files changed, 49 insertions, 11 deletions
diff --git a/src/com/android/server/NetworkStackService.java b/src/com/android/server/NetworkStackService.java index c394d4c..91cc8c3 100644 --- a/src/com/android/server/NetworkStackService.java +++ b/src/com/android/server/NetworkStackService.java @@ -189,6 +189,7 @@ public class NetworkStackService extends Service { @Override public void makeNetworkMonitor(Network network, String name, INetworkMonitorCallbacks cb) throws RemoteException { + checkNetworkStackCallingPermission(); updateSystemAidlVersion(cb.getInterfaceVersion()); final SharedLog log = addValidationLogs(network, name); final NetworkMonitor nm = new NetworkMonitor(mContext, cb, network, log); @@ -197,6 +198,7 @@ public class NetworkStackService extends Service { @Override public void makeIpClient(String ifName, IIpClientCallbacks cb) throws RemoteException { + checkNetworkStackCallingPermission(); updateSystemAidlVersion(cb.getInterfaceVersion()); final IpClient ipClient = new IpClient(mContext, ifName, cb, mObserverRegistry, this); @@ -222,6 +224,7 @@ public class NetworkStackService extends Service { @Override public void fetchIpMemoryStore(@NonNull final IIpMemoryStoreCallbacks cb) throws RemoteException { + checkNetworkStackCallingPermission(); updateSystemAidlVersion(cb.getInterfaceVersion()); cb.onIpMemoryStoreFetched(mIpMemoryStoreService); } diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 4e40ba4..9a8b304 100644 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java @@ -1034,7 +1034,7 @@ public class NetworkMonitor extends StateMachine { mPrivateDnsConfig = null; validationLog("Strict mode hostname resolution failed: " + uhe.getMessage()); } - mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, + mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, (mPrivateDnsConfig != null) /* succeeded */); } @@ -1086,7 +1086,7 @@ public class NetworkMonitor extends StateMachine { String.format("%dms - Error: %s", time, uhe.getMessage())); } logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); - mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); + mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); return success; } } @@ -2067,11 +2067,21 @@ public class NetworkMonitor extends StateMachine { } // Class to keep state of evaluation results and probe results. - // The main purpose is to ensure NetworkMonitor can notify ConnectivityService of probe results + // + // The main purpose was to ensure NetworkMonitor can notify ConnectivityService of probe results // as soon as they happen, without triggering any other changes. This requires keeping state on - // the most recent evaluation result. Calling reportProbeResult will ensure that the results + // the most recent evaluation result. Calling noteProbeResult will ensure that the results // reported to ConnectivityService contain the previous evaluation result, and thus won't // trigger a validation or partial connectivity state change. + // + // Note that this class is not currently being used for this purpose. The reason is that some + // of the system behaviour triggered by reporting network validation - notably, NetworkAgent + // behaviour - depends not only on the value passed by notifyNetworkTested, but also on the + // fact that notifyNetworkTested was called. For example, telephony triggers network recovery + // any time it is told that validation failed, i.e., if the result does not contain + // NETWORK_VALIDATION_RESULT_VALID. But with this scheme, the first two or three validation + // reports are all failures, because they are "HTTP succeeded but validation not yet passed", + // "HTTP and HTTPS succeeded but validation not yet passed", etc. @VisibleForTesting protected class EvaluationState { // The latest validation result for this network. This is a bitmask of @@ -2088,13 +2098,12 @@ public class NetworkMonitor extends StateMachine { } // Probe result for http probe should be updated from reportHttpProbeResult(). - protected void reportProbeResult(int probeResult, boolean succeeded) { + protected void noteProbeResult(int probeResult, boolean succeeded) { if (succeeded) { mProbeResults |= probeResult; } else { mProbeResults &= ~probeResult; } - notifyNetworkTested(getNetworkTestResult(), mRedirectUrl); } protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { @@ -2138,6 +2147,6 @@ public class NetworkMonitor extends StateMachine { if (succeeded) { probeResult |= NETWORK_VALIDATION_PROBE_DNS; } - mEvaluationState.reportProbeResult(probeResult, succeeded); + mEvaluationState.noteProbeResult(probeResult, succeeded); } } diff --git a/src/com/android/server/util/PermissionUtil.java b/src/com/android/server/util/PermissionUtil.java index 6fbeead..6701384 100644 --- a/src/com/android/server/util/PermissionUtil.java +++ b/src/com/android/server/util/PermissionUtil.java @@ -16,30 +16,56 @@ package com.android.server.util; +import static android.os.Binder.getCallingPid; import static android.os.Binder.getCallingUid; import android.os.Process; import android.os.UserHandle; +import java.util.concurrent.atomic.AtomicInteger; + /** * Utility class to check calling permissions on the network stack. */ public final class PermissionUtil { + private static final AtomicInteger sSystemPid = new AtomicInteger(-1); /** * Check that the caller is allowed to communicate with the network stack. * @throws SecurityException The caller is not allowed to communicate with the network stack. */ public static void checkNetworkStackCallingPermission() { - // TODO: check that the calling PID is the system server. final int caller = getCallingUid(); - if (caller != Process.SYSTEM_UID - && UserHandle.getAppId(caller) != Process.BLUETOOTH_UID - && UserHandle.getAppId(caller) != Process.PHONE_UID) { + if (caller == Process.SYSTEM_UID) { + checkConsistentSystemPid(); + return; + } + + if (UserHandle.getAppId(caller) != Process.BLUETOOTH_UID) { throw new SecurityException("Invalid caller: " + caller); } } + private static void checkConsistentSystemPid() { + // Apart from the system server process, no process with a system UID should try to + // communicate with the network stack. This is to ensure that the network stack does not + // need to maintain behavior for clients it was not designed to work with. + // Checking that all calls from a system UID originate from the same PID loosely enforces + // this restriction as if another system process calls the network stack first, the system + // server would lose access to the network stack and cause obvious failures. If the system + // server calls the network stack first, other clients would lose access as expected. + final int systemPid = getCallingPid(); + if (sSystemPid.compareAndSet(-1, systemPid)) { + // sSystemPid was unset (-1): this was the first call + return; + } + + if (sSystemPid.get() != systemPid) { + throw new SecurityException("Invalid PID for the system server, expected " + + sSystemPid.get() + " but was called from " + systemPid); + } + } + /** * Check that the caller is allowed to dump the network stack, e.g. dumpsys. * @throws SecurityException The caller is not allowed to dump the network stack. |