From 20987c31752267ba4ebd87cc252d5bb57fd13a18 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 29 Jan 2021 20:14:04 +0900 Subject: [PATCH 1/3] Add a registerSystemDefaultNetworkCallback method. This method allows internal components to track the system default network. This differs from registerDefaultNetworkCallback because that method sends callbacks for the default network for the UID that called it. This may not be the system default network, for example, when a VPN is up and applies to the UID. Bug: 173331190 Test: new unit tests in ConnectivityServiceTest Test: new unit tests in ConnectivityManagerTest Test: new CTS tests in ConnectivityServiceTest Test: new CTS tests in HostsideVpnTests in other CL in this topic Change-Id: Id02748a2183f71b71ff2a53a580466b9dcecaa93 --- core/api/module-lib-current.txt | 1 + core/proto/android/net/networkrequest.proto | 3 + .../src/android/net/ConnectivityManager.java | 66 +++++++++++++++---- .../src/android/net/NetworkRequest.java | 22 +++---- .../android/server/ConnectivityService.java | 20 +++++- .../server/connectivity/NetworkAgentInfo.java | 3 +- .../android/net/ConnectivityManagerTest.java | 11 ++++ .../server/ConnectivityServiceTest.java | 39 +++++++++++ 8 files changed, 137 insertions(+), 28 deletions(-) diff --git a/core/api/module-lib-current.txt b/core/api/module-lib-current.txt index ab9799ff62724..c7d96effba11a 100644 --- a/core/api/module-lib-current.txt +++ b/core/api/module-lib-current.txt @@ -15,6 +15,7 @@ package android.net { } public class ConnectivityManager { + method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void registerSystemDefaultNetworkCallback(@NonNull android.net.ConnectivityManager.NetworkCallback, @NonNull android.os.Handler); method @RequiresPermission(anyOf={android.Manifest.permission.NETWORK_SETTINGS, android.Manifest.permission.NETWORK_STACK, android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK}) public void requestBackgroundNetwork(@NonNull android.net.NetworkRequest, @Nullable android.os.Handler, @NonNull android.net.ConnectivityManager.NetworkCallback); method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_TEST_NETWORKS, android.Manifest.permission.NETWORK_STACK}) public void simulateDataStall(int, long, @NonNull android.net.Network, @NonNull android.os.PersistableBundle); } diff --git a/core/proto/android/net/networkrequest.proto b/core/proto/android/net/networkrequest.proto index 6794c8cd8acb9..0041f199b4480 100644 --- a/core/proto/android/net/networkrequest.proto +++ b/core/proto/android/net/networkrequest.proto @@ -63,6 +63,9 @@ message NetworkRequestProto { // higher-scoring network will not go into the background immediately, // but will linger and go into the background after the linger timeout. TYPE_BACKGROUND_REQUEST = 5; + // Like TRACK_DEFAULT, but tracks the system default network, instead of + // the default network of the calling application. + TYPE_TRACK_SYSTEM_DEFAULT = 6; } // The type of the request. This is only used by the system and is always // NONE elsewhere. diff --git a/packages/Connectivity/framework/src/android/net/ConnectivityManager.java b/packages/Connectivity/framework/src/android/net/ConnectivityManager.java index d04a5bee51055..0976b753e674b 100644 --- a/packages/Connectivity/framework/src/android/net/ConnectivityManager.java +++ b/packages/Connectivity/framework/src/android/net/ConnectivityManager.java @@ -21,6 +21,7 @@ import static android.net.NetworkRequest.Type.BACKGROUND_REQUEST; import static android.net.NetworkRequest.Type.LISTEN; import static android.net.NetworkRequest.Type.REQUEST; import static android.net.NetworkRequest.Type.TRACK_DEFAULT; +import static android.net.NetworkRequest.Type.TRACK_SYSTEM_DEFAULT; import static android.net.QosCallback.QosCallbackRegistrationException; import android.annotation.CallbackExecutor; @@ -3721,7 +3722,8 @@ public class ConnectivityManager { printStackTrace(); checkCallbackNotNull(callback); Preconditions.checkArgument( - reqType == TRACK_DEFAULT || need != null, "null NetworkCapabilities"); + reqType == TRACK_DEFAULT || reqType == TRACK_SYSTEM_DEFAULT || need != null, + "null NetworkCapabilities"); final NetworkRequest request; final String callingPackageName = mContext.getOpPackageName(); try { @@ -4192,8 +4194,9 @@ public class ConnectivityManager { } /** - * Registers to receive notifications about changes in the system default network. The callbacks - * will continue to be called until either the application exits or + * Registers to receive notifications about changes in the application's default network. This + * may be a physical network or a virtual network, such as a VPN that applies to the + * application. The callbacks will continue to be called until either the application exits or * {@link #unregisterNetworkCallback(NetworkCallback)} is called. * *

To avoid performance issues due to apps leaking callbacks, the system will limit the @@ -4206,7 +4209,7 @@ public class ConnectivityManager { * {@link #unregisterNetworkCallback(NetworkCallback)}. * * @param networkCallback The {@link NetworkCallback} that the system will call as the - * system default network changes. + * application's default network changes. * The callback is invoked on the default internal Handler. * @throws RuntimeException if the app already has too many callbacks registered. */ @@ -4215,11 +4218,47 @@ public class ConnectivityManager { registerDefaultNetworkCallback(networkCallback, getDefaultHandler()); } + /** + * Registers to receive notifications about changes in the application's default network. This + * may be a physical network or a virtual network, such as a VPN that applies to the + * application. The callbacks will continue to be called until either the application exits or + * {@link #unregisterNetworkCallback(NetworkCallback)} is called. + * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #requestNetwork} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with + * {@link #unregisterNetworkCallback(NetworkCallback)}. + * + * @param networkCallback The {@link NetworkCallback} that the system will call as the + * application's default network changes. + * @param handler {@link Handler} to specify the thread upon which the callback will be invoked. + * @throws RuntimeException if the app already has too many callbacks registered. + */ + @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) + public void registerDefaultNetworkCallback(@NonNull NetworkCallback networkCallback, + @NonNull Handler handler) { + CallbackHandler cbHandler = new CallbackHandler(handler); + sendRequestForNetwork(null /* NetworkCapabilities need */, networkCallback, 0, + TRACK_DEFAULT, TYPE_NONE, cbHandler); + } + /** * Registers to receive notifications about changes in the system default network. The callbacks * will continue to be called until either the application exits or * {@link #unregisterNetworkCallback(NetworkCallback)} is called. * + * This method should not be used to determine networking state seen by applications, because in + * many cases, most or even all application traffic may not use the default network directly, + * and traffic from different applications may go on different networks by default. As an + * example, if a VPN is connected, traffic from all applications might be sent through the VPN + * and not onto the system default network. Applications or system components desiring to do + * determine network state as seen by applications should use other methods such as + * {@link #registerDefaultNetworkCallback(NetworkCallback, Handler)}. + * *

To avoid performance issues due to apps leaking callbacks, the system will limit the * number of outstanding requests to 100 per app (identified by their UID), shared with * all variants of this method, of {@link #requestNetwork} as well as @@ -4233,20 +4272,19 @@ public class ConnectivityManager { * system default network changes. * @param handler {@link Handler} to specify the thread upon which the callback will be invoked. * @throws RuntimeException if the app already has too many callbacks registered. + * + * @hide */ - @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) - public void registerDefaultNetworkCallback(@NonNull NetworkCallback networkCallback, + @SystemApi(client = MODULE_LIBRARIES) + @SuppressLint({"ExecutorRegistration", "PairedRegistration"}) + @RequiresPermission(anyOf = { + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + android.Manifest.permission.NETWORK_SETTINGS}) + public void registerSystemDefaultNetworkCallback(@NonNull NetworkCallback networkCallback, @NonNull Handler handler) { - // This works because if the NetworkCapabilities are null, - // ConnectivityService takes them from the default request. - // - // Since the capabilities are exactly the same as the default request's - // capabilities, this request is guaranteed, at all times, to be - // satisfied by the same network, if any, that satisfies the default - // request, i.e., the system default network. CallbackHandler cbHandler = new CallbackHandler(handler); sendRequestForNetwork(null /* NetworkCapabilities need */, networkCallback, 0, - TRACK_DEFAULT, TYPE_NONE, cbHandler); + TRACK_SYSTEM_DEFAULT, TYPE_NONE, cbHandler); } /** diff --git a/packages/Connectivity/framework/src/android/net/NetworkRequest.java b/packages/Connectivity/framework/src/android/net/NetworkRequest.java index 6540397d6200a..b4a651c0607e7 100644 --- a/packages/Connectivity/framework/src/android/net/NetworkRequest.java +++ b/packages/Connectivity/framework/src/android/net/NetworkRequest.java @@ -104,17 +104,14 @@ public class NetworkRequest implements Parcelable { * callbacks about the single, highest scoring current network * (if any) that matches the specified NetworkCapabilities, or * - * - TRACK_DEFAULT, a hybrid of the two designed such that the - * framework will issue callbacks for the single, highest scoring - * current network (if any) that matches the capabilities of the - * default Internet request (mDefaultRequest), but which cannot cause - * the framework to either create or retain the existence of any - * specific network. Note that from the point of view of the request - * matching code, TRACK_DEFAULT is identical to REQUEST: its special - * behaviour is not due to different semantics, but to the fact that - * the system will only ever create a TRACK_DEFAULT with capabilities - * that are identical to the default request's capabilities, thus - * causing it to share fate in every way with the default request. + * - TRACK_DEFAULT, which causes the framework to issue callbacks for + * the single, highest scoring current network (if any) that will + * be chosen for an app, but which cannot cause the framework to + * either create or retain the existence of any specific network. + * + * - TRACK_SYSTEM_DEFAULT, which causes the framework to send callbacks + * for the network (if any) that satisfies the default Internet + * request. * * - BACKGROUND_REQUEST, like REQUEST but does not cause any networks * to retain the NET_CAPABILITY_FOREGROUND capability. A network with @@ -137,6 +134,7 @@ public class NetworkRequest implements Parcelable { TRACK_DEFAULT, REQUEST, BACKGROUND_REQUEST, + TRACK_SYSTEM_DEFAULT, }; /** @@ -601,6 +599,8 @@ public class NetworkRequest implements Parcelable { return NetworkRequestProto.TYPE_REQUEST; case BACKGROUND_REQUEST: return NetworkRequestProto.TYPE_BACKGROUND_REQUEST; + case TRACK_SYSTEM_DEFAULT: + return NetworkRequestProto.TYPE_TRACK_SYSTEM_DEFAULT; default: return NetworkRequestProto.TYPE_UNKNOWN; } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 06f935833d0b3..5164437b68529 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5746,6 +5746,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new SecurityException("Insufficient permissions to specify legacy type"); } } + final NetworkCapabilities defaultNc = mDefaultRequest.mRequests.get(0).networkCapabilities; final int callingUid = mDeps.getCallingUid(); final NetworkRequest.Type reqType; try { @@ -5756,11 +5757,15 @@ public class ConnectivityService extends IConnectivityManager.Stub switch (reqType) { case TRACK_DEFAULT: // If the request type is TRACK_DEFAULT, the passed {@code networkCapabilities} - // is unused and will be replaced by the one from the default network request. - // This allows callers to keep track of the system default network. + // is unused and will be replaced by ones appropriate for the caller. + // This allows callers to keep track of the default network for their app. networkCapabilities = createDefaultNetworkCapabilitiesForUid(callingUid); enforceAccessPermission(); break; + case TRACK_SYSTEM_DEFAULT: + enforceSettingsPermission(); + networkCapabilities = new NetworkCapabilities(defaultNc); + break; case BACKGROUND_REQUEST: enforceNetworkStackOrSettingsPermission(); // Fall-through since other checks are the same with normal requests. @@ -5779,6 +5784,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, Binder.getCallingPid(), callingUid, callingPackageName); + // Set the UID range for this request to the single UID of the requester, or to an empty // set of UIDs if the caller has the appropriate permission and UIDs have not been set. // This will overwrite any allowed UIDs in the requested capabilities. Though there @@ -5798,6 +5804,16 @@ public class ConnectivityService extends IConnectivityManager.Stub new NetworkRequestInfo(messenger, networkRequest, binder, callingAttributionTag); if (DBG) log("requestNetwork for " + nri); + // For TRACK_SYSTEM_DEFAULT callbacks, the capabilities have been modified since they were + // copied from the default request above. (This is necessary to ensure, for example, that + // the callback does not leak sensitive information to unprivileged apps.) Check that the + // changes don't alter request matching. + if (reqType == NetworkRequest.Type.TRACK_SYSTEM_DEFAULT && + (!networkCapabilities.equalRequestableCapabilities(defaultNc))) { + Log.wtf(TAG, "TRACK_SYSTEM_DEFAULT capabilities don't match default request: " + + networkCapabilities + " vs. " + defaultNc); + } + mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_REQUEST, nri)); if (timeoutMs > 0) { mHandler.sendMessageDelayed(mHandler.obtainMessage(EVENT_TIMEOUT_NETWORK_REQUEST, diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index bff1a5c99dd21..c05e25367d033 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -717,8 +717,9 @@ public class NetworkAgentInfo implements Comparable { mNumBackgroundNetworkRequests += delta; break; - case TRACK_DEFAULT: case LISTEN: + case TRACK_DEFAULT: + case TRACK_SYSTEM_DEFAULT: break; case NONE: diff --git a/tests/net/java/android/net/ConnectivityManagerTest.java b/tests/net/java/android/net/ConnectivityManagerTest.java index fcfb4aa9b864b..6a09b0237a389 100644 --- a/tests/net/java/android/net/ConnectivityManagerTest.java +++ b/tests/net/java/android/net/ConnectivityManagerTest.java @@ -35,6 +35,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkRequest.Type.BACKGROUND_REQUEST; import static android.net.NetworkRequest.Type.REQUEST; import static android.net.NetworkRequest.Type.TRACK_DEFAULT; +import static android.net.NetworkRequest.Type.TRACK_SYSTEM_DEFAULT; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -329,6 +330,9 @@ public class ConnectivityManagerTest { mustFail(() -> { manager.registerDefaultNetworkCallback(null, handler); }); mustFail(() -> { manager.registerDefaultNetworkCallback(callback, null); }); + mustFail(() -> { manager.registerSystemDefaultNetworkCallback(null, handler); }); + mustFail(() -> { manager.registerSystemDefaultNetworkCallback(callback, null); }); + mustFail(() -> { manager.unregisterNetworkCallback(nullCallback); }); mustFail(() -> { manager.unregisterNetworkCallback(nullIntent); }); mustFail(() -> { manager.releaseNetworkRequest(nullIntent); }); @@ -377,6 +381,13 @@ public class ConnectivityManagerTest { eq(BACKGROUND_REQUEST.ordinal()), any(), anyInt(), any(), eq(TYPE_NONE), eq(testPkgName), eq(testAttributionTag)); reset(mService); + + Handler handler = new Handler(ConnectivityThread.getInstanceLooper()); + manager.registerSystemDefaultNetworkCallback(callback, handler); + verify(mService).requestNetwork(eq(null), + eq(TRACK_SYSTEM_DEFAULT.ordinal()), any(), anyInt(), any(), eq(TYPE_NONE), + eq(testPkgName), eq(testAttributionTag)); + reset(mService); } static Message makeMessage(NetworkRequest req, int messageType) { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index edc2c636e0ee2..ecdc62174ec01 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -3649,10 +3649,19 @@ public class ConnectivityServiceTest { @Test public void testRegisterDefaultNetworkCallback() throws Exception { + // NETWORK_SETTINGS is necessary to call registerSystemDefaultNetworkCallback. + mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, + PERMISSION_GRANTED); + final TestNetworkCallback defaultNetworkCallback = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(defaultNetworkCallback); defaultNetworkCallback.assertNoCallback(); + final Handler handler = new Handler(ConnectivityThread.getInstanceLooper()); + final TestNetworkCallback systemDefaultCallback = new TestNetworkCallback(); + mCm.registerSystemDefaultNetworkCallback(systemDefaultCallback, handler); + systemDefaultCallback.assertNoCallback(); + // Create a TRANSPORT_CELLULAR request to keep the mobile interface up // whenever Wi-Fi is up. Without this, the mobile network agent is // reaped before any other activity can take place. @@ -3667,27 +3676,35 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(true); cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); defaultNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + systemDefaultCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + assertEquals(systemDefaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up wifi and expect CALLBACK_AVAILABLE. mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(true); cellNetworkCallback.assertNoCallback(); defaultNetworkCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); + systemDefaultCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + assertEquals(systemDefaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring down cell. Expect no default network callback, since it wasn't the default. mCellNetworkAgent.disconnect(); cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); defaultNetworkCallback.assertNoCallback(); + systemDefaultCallback.assertNoCallback(); assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + assertEquals(systemDefaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up cell. Expect no default network callback, since it won't be the default. mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); mCellNetworkAgent.connect(true); cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); defaultNetworkCallback.assertNoCallback(); + systemDefaultCallback.assertNoCallback(); assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + assertEquals(systemDefaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring down wifi. Expect the default network callback to notified of LOST wifi // followed by AVAILABLE cell. @@ -3695,19 +3712,25 @@ public class ConnectivityServiceTest { cellNetworkCallback.assertNoCallback(); defaultNetworkCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); defaultNetworkCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); + systemDefaultCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); + systemDefaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); mCellNetworkAgent.disconnect(); cellNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); defaultNetworkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); + systemDefaultCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); waitForIdle(); assertEquals(null, mCm.getActiveNetwork()); mMockVpn.establishForMyUid(); assertUidRangesUpdatedForMyUid(true); defaultNetworkCallback.expectAvailableThenValidatedCallbacks(mMockVpn); + systemDefaultCallback.assertNoCallback(); assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + assertEquals(null, systemDefaultCallback.getLastAvailableNetwork()); mMockVpn.disconnect(); defaultNetworkCallback.expectCallback(CallbackEntry.LOST, mMockVpn); + systemDefaultCallback.assertNoCallback(); waitForIdle(); assertEquals(null, mCm.getActiveNetwork()); } @@ -6134,6 +6157,10 @@ public class ConnectivityServiceTest { @Test public void testVpnNetworkActive() throws Exception { + // NETWORK_SETTINGS is necessary to call registerSystemDefaultNetworkCallback. + mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, + PERMISSION_GRANTED); + final int uid = Process.myUid(); final TestNetworkCallback genericNetworkCallback = new TestNetworkCallback(); @@ -6141,6 +6168,7 @@ public class ConnectivityServiceTest { final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback(); final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback(); final TestNetworkCallback defaultCallback = new TestNetworkCallback(); + final TestNetworkCallback systemDefaultCallback = new TestNetworkCallback(); final NetworkRequest genericNotVpnRequest = new NetworkRequest.Builder().build(); final NetworkRequest genericRequest = new NetworkRequest.Builder() .removeCapability(NET_CAPABILITY_NOT_VPN).build(); @@ -6154,6 +6182,8 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback); mCm.registerDefaultNetworkCallback(defaultCallback); + mCm.registerSystemDefaultNetworkCallback(systemDefaultCallback, + new Handler(ConnectivityThread.getInstanceLooper())); defaultCallback.assertNoCallback(); mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); @@ -6163,6 +6193,7 @@ public class ConnectivityServiceTest { genericNotVpnNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + systemDefaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); vpnNetworkCallback.assertNoCallback(); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); @@ -6183,7 +6214,10 @@ public class ConnectivityServiceTest { wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectAvailableThenValidatedCallbacks(mMockVpn); defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); + systemDefaultCallback.assertNoCallback(); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + assertEquals(mWiFiNetworkAgent.getNetwork(), + systemDefaultCallback.getLastAvailableNetwork()); ranges.clear(); mMockVpn.setUids(ranges); @@ -6200,6 +6234,7 @@ public class ConnectivityServiceTest { // much, but that is the reason the test here has to check for an update to the // capabilities instead of the expected LOST then AVAILABLE. defaultCallback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, mMockVpn); + systemDefaultCallback.assertNoCallback(); ranges.add(new UidRange(uid, uid)); mMockVpn.setUids(ranges); @@ -6211,6 +6246,7 @@ public class ConnectivityServiceTest { // TODO : Here like above, AVAILABLE would be correct, but because this can't actually // happen outside of the test, ConnectivityService does not rematch callbacks. defaultCallback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, mMockVpn); + systemDefaultCallback.assertNoCallback(); mWiFiNetworkAgent.disconnect(); @@ -6219,6 +6255,7 @@ public class ConnectivityServiceTest { wifiNetworkCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); vpnNetworkCallback.assertNoCallback(); defaultCallback.assertNoCallback(); + systemDefaultCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); mMockVpn.disconnect(); @@ -6227,12 +6264,14 @@ public class ConnectivityServiceTest { wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCallback(CallbackEntry.LOST, mMockVpn); defaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn); + systemDefaultCallback.assertNoCallback(); assertEquals(null, mCm.getActiveNetwork()); mCm.unregisterNetworkCallback(genericNetworkCallback); mCm.unregisterNetworkCallback(wifiNetworkCallback); mCm.unregisterNetworkCallback(vpnNetworkCallback); mCm.unregisterNetworkCallback(defaultCallback); + mCm.unregisterNetworkCallback(systemDefaultCallback); } @Test From 76e8a43e680587b139026bc0ee69f4cdb1468c12 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 3 Feb 2021 03:12:15 +0900 Subject: [PATCH 2/3] Add a VpnTransportInfo object. This currently stores the VPN type and a session name, but can be extended in the future. Bug: 173331190 Test: added coverage in VpnTest Test: added coverage in ConnectivityServiceTest Test: added coverage in NetworkAgentTest Change-Id: I450858a9fa332c8d896dbdb4c14337d5ec23677f --- core/java/android/net/VpnTransportInfo.java | 79 +++++++++++++++++++ .../src/android/net/NetworkCapabilities.java | 2 + .../framework/src/android/net/VpnManager.java | 22 +++++- .../android/server/ConnectivityService.java | 2 +- .../com/android/server/connectivity/Vpn.java | 32 +++++--- .../android/net/VpnTransportInfoTest.java | 51 ++++++++++++ .../server/ConnectivityServiceTest.java | 9 ++- .../android/server/connectivity/VpnTest.java | 15 +++- 8 files changed, 193 insertions(+), 19 deletions(-) create mode 100644 core/java/android/net/VpnTransportInfo.java create mode 100644 tests/net/java/android/net/VpnTransportInfoTest.java diff --git a/core/java/android/net/VpnTransportInfo.java b/core/java/android/net/VpnTransportInfo.java new file mode 100644 index 0000000000000..082fa58f8ac28 --- /dev/null +++ b/core/java/android/net/VpnTransportInfo.java @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.net; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.os.Parcel; +import android.os.Parcelable; +import android.util.SparseArray; + +import com.android.internal.util.MessageUtils; + +import java.util.Objects; + +/** @hide */ +public final class VpnTransportInfo implements TransportInfo, Parcelable { + private static final SparseArray sTypeToString = + MessageUtils.findMessageNames(new Class[]{VpnManager.class}, new String[]{"TYPE_VPN_"}); + + /** Type of this VPN. */ + @VpnManager.VpnType public final int type; + + public VpnTransportInfo(@VpnManager.VpnType int type) { + this.type = type; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof VpnTransportInfo)) return false; + + VpnTransportInfo that = (VpnTransportInfo) o; + return this.type == that.type; + } + + @Override + public int hashCode() { + return Objects.hash(type); + } + + @Override + public String toString() { + final String typeString = sTypeToString.get(type, "VPN_TYPE_???"); + return String.format("VpnTransportInfo{%s}", typeString); + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(@NonNull Parcel dest, int flags) { + dest.writeInt(type); + } + + public static final @NonNull Creator CREATOR = + new Creator() { + public VpnTransportInfo createFromParcel(Parcel in) { + return new VpnTransportInfo(in.readInt()); + } + public VpnTransportInfo[] newArray(int size) { + return new VpnTransportInfo[size]; + } + }; +} diff --git a/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java b/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java index 55b2c3c9e11f6..9d67f0b84367c 100644 --- a/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java +++ b/packages/Connectivity/framework/src/android/net/NetworkCapabilities.java @@ -762,12 +762,14 @@ public final class NetworkCapabilities implements Parcelable { final int originalSignalStrength = mSignalStrength; final int originalOwnerUid = getOwnerUid(); final int[] originalAdministratorUids = getAdministratorUids(); + final TransportInfo originalTransportInfo = getTransportInfo(); clearAll(); mTransportTypes = (originalTransportTypes & TEST_NETWORKS_ALLOWED_TRANSPORTS) | (1 << TRANSPORT_TEST); mNetworkCapabilities = originalCapabilities & TEST_NETWORKS_ALLOWED_CAPABILITIES; mNetworkSpecifier = originalSpecifier; mSignalStrength = originalSignalStrength; + mTransportInfo = originalTransportInfo; // Only retain the owner and administrator UIDs if they match the app registering the remote // caller that registered the network. diff --git a/packages/Connectivity/framework/src/android/net/VpnManager.java b/packages/Connectivity/framework/src/android/net/VpnManager.java index 1812509ba6d2f..1e30283a9e6c8 100644 --- a/packages/Connectivity/framework/src/android/net/VpnManager.java +++ b/packages/Connectivity/framework/src/android/net/VpnManager.java @@ -55,13 +55,29 @@ import java.security.GeneralSecurityException; public class VpnManager { /** Type representing a lack of VPN @hide */ public static final int TYPE_VPN_NONE = -1; - /** VPN service type code @hide */ + + /** + * A VPN created by an app using the {@link VpnService} API. + * @hide + */ public static final int TYPE_VPN_SERVICE = 1; - /** Platform VPN type code @hide */ + + /** + * A VPN created using a {@link VpnManager} API such as {@link #startProvisionedVpnProfile}. + * @hide + */ public static final int TYPE_VPN_PLATFORM = 2; + /** + * An IPsec VPN created by the built-in LegacyVpnRunner. + * @deprecated new Android devices should use VPN_TYPE_PLATFORM instead. + * @hide + */ + @Deprecated + public static final int TYPE_VPN_LEGACY = 3; + /** @hide */ - @IntDef(value = {TYPE_VPN_NONE, TYPE_VPN_SERVICE, TYPE_VPN_PLATFORM}) + @IntDef(value = {TYPE_VPN_NONE, TYPE_VPN_SERVICE, TYPE_VPN_PLATFORM, TYPE_VPN_LEGACY}) @Retention(RetentionPolicy.SOURCE) public @interface VpnType {} diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 5164437b68529..f59b9fc4ef46d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -8505,7 +8505,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final Vpn vpn = enforceActiveVpnOrNetworkStackPermission(); // Only VpnService based VPNs should be able to get this information. - if (vpn != null && vpn.getActiveAppVpnType() != VpnManager.TYPE_VPN_SERVICE) { + if (vpn != null && vpn.getActiveVpnType() != VpnManager.TYPE_VPN_SERVICE) { throw new SecurityException( "getConnectionOwnerUid() not allowed for non-VpnService VPNs"); } diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index fc2c7e01efde5..33c19b1ca2b2c 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -74,6 +74,7 @@ import android.net.UidRangeParcel; import android.net.UnderlyingNetworkInfo; import android.net.VpnManager; import android.net.VpnService; +import android.net.VpnTransportInfo; import android.net.ipsec.ike.ChildSessionCallback; import android.net.ipsec.ike.ChildSessionConfiguration; import android.net.ipsec.ike.ChildSessionParams; @@ -435,6 +436,7 @@ public class Vpn { mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN); mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); mNetworkCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED); + mNetworkCapabilities.setTransportInfo(new VpnTransportInfo(VpnManager.TYPE_VPN_NONE)); loadAlwaysOnPackage(keyStore); } @@ -929,6 +931,7 @@ public class Vpn { jniReset(mInterface); mInterface = null; mNetworkCapabilities.setUids(null); + mNetworkCapabilities.setTransportInfo(null); } // Revoke the connection or stop the VpnRunner. @@ -999,6 +1002,8 @@ public class Vpn { case VpnManager.TYPE_VPN_SERVICE: toChange = new String[] {AppOpsManager.OPSTR_ACTIVATE_VPN}; break; + case VpnManager.TYPE_VPN_LEGACY: + return false; default: Log.wtf(TAG, "Unrecognized VPN type while granting authorization"); return false; @@ -1029,6 +1034,8 @@ public class Vpn { return isVpnServicePreConsented(context, packageName); case VpnManager.TYPE_VPN_PLATFORM: return isVpnProfilePreConsented(context, packageName); + case VpnManager.TYPE_VPN_LEGACY: + return VpnConfig.LEGACY_VPN.equals(packageName); default: return false; } @@ -1211,6 +1218,8 @@ public class Vpn { mNetworkCapabilities.setUids(createUserAndRestrictedProfilesRanges(mUserId, mConfig.allowedApplications, mConfig.disallowedApplications)); + mNetworkCapabilities.setTransportInfo(new VpnTransportInfo(getActiveVpnType())); + // Only apps targeting Q and above can explicitly declare themselves as metered. // These VPNs are assumed metered unless they state otherwise. if (mIsPackageTargetingAtLeastQ && mConfig.isMetered) { @@ -1736,6 +1745,7 @@ public class Vpn { private void cleanupVpnStateLocked() { mStatusIntent = null; mNetworkCapabilities.setUids(null); + mNetworkCapabilities.setTransportInfo(null); mConfig = null; mInterface = null; @@ -1846,22 +1856,18 @@ public class Vpn { } /** - * Gets the currently running App-based VPN type + * Gets the currently running VPN type * - * @return the {@link VpnManager.VpnType}. {@link VpnManager.TYPE_VPN_NONE} if not running an - * app-based VPN. While VpnService-based VPNs are always app VPNs and LegacyVpn is always + * @return the {@link VpnManager.VpnType}. {@link VpnManager.TYPE_VPN_NONE} if not running a + * VPN. While VpnService-based VPNs are always app VPNs and LegacyVpn is always * Settings-based, the Platform VPNs can be initiated by both apps and Settings. */ - public synchronized int getActiveAppVpnType() { - if (VpnConfig.LEGACY_VPN.equals(mPackage)) { - return VpnManager.TYPE_VPN_NONE; - } - - if (mVpnRunner != null && mVpnRunner instanceof IkeV2VpnRunner) { - return VpnManager.TYPE_VPN_PLATFORM; - } else { - return VpnManager.TYPE_VPN_SERVICE; - } + public synchronized int getActiveVpnType() { + if (!mNetworkInfo.isConnectedOrConnecting()) return VpnManager.TYPE_VPN_NONE; + if (mVpnRunner == null) return VpnManager.TYPE_VPN_SERVICE; + return mVpnRunner instanceof IkeV2VpnRunner + ? VpnManager.TYPE_VPN_PLATFORM + : VpnManager.TYPE_VPN_LEGACY; } private void updateAlwaysOnNotification(DetailedState networkState) { diff --git a/tests/net/java/android/net/VpnTransportInfoTest.java b/tests/net/java/android/net/VpnTransportInfoTest.java new file mode 100644 index 0000000000000..2fd5e3861cefd --- /dev/null +++ b/tests/net/java/android/net/VpnTransportInfoTest.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2010 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.net; + +import static com.android.testutils.ParcelUtils.assertParcelSane; +import static com.android.testutils.ParcelUtils.assertParcelingIsLossless; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +import androidx.test.filters.SmallTest; +import androidx.test.runner.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class VpnTransportInfoTest { + + @Test + public void testParceling() { + VpnTransportInfo v = new VpnTransportInfo(VpnManager.TYPE_VPN_PLATFORM); + assertParcelSane(v, 1 /* fieldCount */); + assertParcelingIsLossless(v); + } + + @Test + public void testEqualsAndHashCode() { + VpnTransportInfo v1 = new VpnTransportInfo(VpnManager.TYPE_VPN_PLATFORM); + VpnTransportInfo v2 = new VpnTransportInfo(VpnManager.TYPE_VPN_SERVICE); + VpnTransportInfo v3 = new VpnTransportInfo(VpnManager.TYPE_VPN_PLATFORM); + assertNotEquals(v1, v2); + assertEquals(v1, v3); + assertEquals(v1.hashCode(), v3.hashCode()); + } +} \ No newline at end of file diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index ecdc62174ec01..d5d7b475fdb5a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -205,6 +205,7 @@ import android.net.UidRangeParcel; import android.net.UnderlyingNetworkInfo; import android.net.Uri; import android.net.VpnManager; +import android.net.VpnTransportInfo; import android.net.metrics.IpConnectivityLog; import android.net.shared.NetworkMonitorUtils; import android.net.shared.PrivateDnsConfig; @@ -1110,7 +1111,7 @@ public class ConnectivityServiceTest { } @Override - public int getActiveAppVpnType() { + public int getActiveVpnType() { return mVpnType; } @@ -1123,10 +1124,12 @@ public class ConnectivityServiceTest { private void registerAgent(boolean isAlwaysMetered, Set uids, LinkProperties lp) throws Exception { if (mAgentRegistered) throw new IllegalStateException("already registered"); + updateState(NetworkInfo.DetailedState.CONNECTING, "registerAgent"); mConfig = new VpnConfig(); setUids(uids); if (!isAlwaysMetered) mNetworkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED); mInterface = VPN_IFNAME; + mNetworkCapabilities.setTransportInfo(new VpnTransportInfo(getActiveVpnType())); mMockNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_VPN, lp, mNetworkCapabilities); mMockNetworkAgent.waitForIdle(TIMEOUT_MS); @@ -7322,6 +7325,7 @@ public class ConnectivityServiceTest { } private void establishLegacyLockdownVpn() throws Exception { + mMockVpn.setVpnType(VpnManager.TYPE_VPN_LEGACY); // The legacy lockdown VPN only supports userId 0. final Set ranges = Collections.singleton(UidRange.createForUser(PRIMARY_USER)); mMockVpn.registerAgent(ranges); @@ -7434,6 +7438,9 @@ public class ConnectivityServiceTest { assertTrue(vpnNc.hasTransport(TRANSPORT_CELLULAR)); assertFalse(vpnNc.hasTransport(TRANSPORT_WIFI)); assertFalse(vpnNc.hasCapability(NET_CAPABILITY_NOT_METERED)); + VpnTransportInfo ti = (VpnTransportInfo) vpnNc.getTransportInfo(); + assertNotNull(ti); + assertEquals(VpnManager.TYPE_VPN_LEGACY, ti.type); // Switch default network from cell to wifi. Expect VPN to disconnect and reconnect. final LinkProperties wifiLp = new LinkProperties(); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 73cc9f129e79e..cffd2d1d428fd 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -25,6 +25,7 @@ import static android.net.ConnectivityManager.NetworkCallback; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; @@ -74,6 +75,7 @@ import android.net.UidRange; import android.net.UidRangeParcel; import android.net.VpnManager; import android.net.VpnService; +import android.net.VpnTransportInfo; import android.net.ipsec.ike.IkeSessionCallback; import android.net.ipsec.ike.exceptions.IkeProtocolException; import android.os.Build.VERSION_CODES; @@ -984,6 +986,13 @@ public class VpnTest { startRacoon("hostname", "5.6.7.8"); // address returned by deps.resolve } + private void assertTransportInfoMatches(NetworkCapabilities nc, int type) { + assertNotNull(nc); + VpnTransportInfo ti = (VpnTransportInfo) nc.getTransportInfo(); + assertNotNull(ti); + assertEquals(type, ti.type); + } + public void startRacoon(final String serverAddr, final String expectedAddr) throws Exception { final ConditionVariable legacyRunnerReady = new ConditionVariable(); @@ -1020,8 +1029,10 @@ public class VpnTest { // Now wait for the runner to be ready before testing for the route. ArgumentCaptor lpCaptor = ArgumentCaptor.forClass(LinkProperties.class); + ArgumentCaptor ncCaptor = + ArgumentCaptor.forClass(NetworkCapabilities.class); verify(mConnectivityManager, timeout(10_000)).registerNetworkAgent(any(), any(), - lpCaptor.capture(), any(), anyInt(), any(), anyInt()); + lpCaptor.capture(), ncCaptor.capture(), anyInt(), any(), anyInt()); // In this test the expected address is always v4 so /32. // Note that the interface needs to be specified because RouteInfo objects stored in @@ -1031,6 +1042,8 @@ public class VpnTest { final List actualRoutes = lpCaptor.getValue().getRoutes(); assertTrue("Expected throw route (" + expectedRoute + ") not found in " + actualRoutes, actualRoutes.contains(expectedRoute)); + + assertTransportInfoMatches(ncCaptor.getValue(), VpnManager.TYPE_VPN_LEGACY); } finally { // Now interrupt the thread, unblock the runner and clean up. vpn.mVpnRunner.exitVpnRunner(); From 9cc08edf1f31f615b8fc0dab653f484f9ca22b5d Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 4 Feb 2021 00:18:27 +0900 Subject: [PATCH 3/3] Stop using mVpns in getConnectionOwnerUid. Use data that is already available in ConnectivityService instead. The behaviour of the new implementation is slightly different from Q and R code when the permission check fails. - The old code would throw a SecurityException if an app that was not an active VPN called the method, and would return INVALID_UID if the connection belonged to a UID that was not subject to the VPN. - The new code returns INVALID_UID in both cases. This does not seem like a compatibility problem. The only case in which the code throws SecurityException is if the app is not a current VPN app, but the app already knows whether it is or not. The docs don't mention that the method SecurityException, either. Bug: 173331190 Test: atest FrameworksNetTests Test: atest HostsideVpnTests Change-Id: If3d031e74df33b5c97e12ebf02272faac6769d50 --- .../android/server/ConnectivityService.java | 42 +++++++------------ .../server/ConnectivityServiceTest.java | 12 +----- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f59b9fc4ef46d..6f0b8b7c0a48b 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -132,12 +132,14 @@ import android.net.RouteInfo; import android.net.RouteInfoParcel; import android.net.SocketKeepalive; import android.net.TetheringManager; +import android.net.TransportInfo; import android.net.UidRange; import android.net.UidRangeParcel; import android.net.UnderlyingNetworkInfo; import android.net.Uri; import android.net.VpnManager; import android.net.VpnService; +import android.net.VpnTransportInfo; import android.net.metrics.INetdEventListener; import android.net.metrics.IpConnectivityLog; import android.net.metrics.NetworkEvent; @@ -8477,22 +8479,11 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - /** - * Caller either needs to be an active VPN, or hold the NETWORK_STACK permission - * for testing. - */ - private Vpn enforceActiveVpnOrNetworkStackPermission() { - if (checkNetworkStackPermission()) { - return null; - } - synchronized (mVpns) { - Vpn vpn = getVpnIfOwner(); - if (vpn != null) { - return vpn; - } - } - throw new SecurityException("App must either be an active VPN or have the NETWORK_STACK " - + "permission"); + private @VpnManager.VpnType int getVpnType(@Nullable NetworkAgentInfo vpn) { + if (vpn == null) return VpnManager.TYPE_VPN_NONE; + final TransportInfo ti = vpn.networkCapabilities.getTransportInfo(); + if (!(ti instanceof VpnTransportInfo)) return VpnManager.TYPE_VPN_NONE; + return ((VpnTransportInfo) ti).type; } /** @@ -8502,14 +8493,6 @@ public class ConnectivityService extends IConnectivityManager.Stub * connection is not found. */ public int getConnectionOwnerUid(ConnectionInfo connectionInfo) { - final Vpn vpn = enforceActiveVpnOrNetworkStackPermission(); - - // Only VpnService based VPNs should be able to get this information. - if (vpn != null && vpn.getActiveVpnType() != VpnManager.TYPE_VPN_SERVICE) { - throw new SecurityException( - "getConnectionOwnerUid() not allowed for non-VpnService VPNs"); - } - if (connectionInfo.protocol != IPPROTO_TCP && connectionInfo.protocol != IPPROTO_UDP) { throw new IllegalArgumentException("Unsupported protocol " + connectionInfo.protocol); } @@ -8517,8 +8500,15 @@ public class ConnectivityService extends IConnectivityManager.Stub final int uid = mDeps.getConnectionOwnerUid(connectionInfo.protocol, connectionInfo.local, connectionInfo.remote); - /* Filter out Uids not associated with the VPN. */ - if (vpn != null && !vpn.appliesToUid(uid)) { + if (uid == INVALID_UID) return uid; // Not found. + + // Connection owner UIDs are visible only to the network stack and to the VpnService-based + // VPN, if any, that applies to the UID that owns the connection. + if (checkNetworkStackPermission()) return uid; + + final NetworkAgentInfo vpn = getVpnForUid(uid); + if (vpn == null || getVpnType(vpn) != VpnManager.TYPE_VPN_SERVICE + || vpn.networkCapabilities.getOwnerUid() != Binder.getCallingUid()) { return INVALID_UID; } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index d5d7b475fdb5a..0dce9e10e06cc 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -8567,11 +8567,7 @@ public class ConnectivityServiceTest { final int myUid = Process.myUid(); setupConnectionOwnerUidAsVpnApp(myUid, VpnManager.TYPE_VPN_PLATFORM); - try { - mService.getConnectionOwnerUid(getTestConnectionInfo()); - fail("Expected SecurityException for non-VpnService app"); - } catch (SecurityException expected) { - } + assertEquals(INVALID_UID, mService.getConnectionOwnerUid(getTestConnectionInfo())); } @Test @@ -8579,11 +8575,7 @@ public class ConnectivityServiceTest { final int myUid = Process.myUid(); setupConnectionOwnerUidAsVpnApp(myUid + 1, VpnManager.TYPE_VPN_SERVICE); - try { - mService.getConnectionOwnerUid(getTestConnectionInfo()); - fail("Expected SecurityException for non-VpnService app"); - } catch (SecurityException expected) { - } + assertEquals(INVALID_UID, mService.getConnectionOwnerUid(getTestConnectionInfo())); } @Test