diff --git a/Tethering/Android.bp b/Tethering/Android.bp index 4eafc2afa9..79c2b9dd17 100644 --- a/Tethering/Android.bp +++ b/Tethering/Android.bp @@ -30,6 +30,7 @@ java_defaults { ":services-tethering-shared-srcs", ], static_libs: [ + "NetworkStackApiStableShims", "androidx.annotation_annotation", "modules-utils-build", "netlink-client", diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index df9e12a25c..0e8b2b5e71 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -2040,8 +2040,7 @@ public class Tethering { } private void startTrackDefaultNetwork() { - mUpstreamNetworkMonitor.startTrackDefaultNetwork(mDeps.getDefaultNetworkRequest(), - mEntitlementMgr); + mUpstreamNetworkMonitor.startTrackDefaultNetwork(mEntitlementMgr); } /** Get the latest value of the tethering entitlement check. */ diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java index 45b914178e..7df9475153 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java @@ -20,7 +20,6 @@ import android.app.usage.NetworkStatsManager; import android.bluetooth.BluetoothAdapter; import android.content.Context; import android.net.INetd; -import android.net.NetworkRequest; import android.net.ip.IpServer; import android.net.util.SharedLog; import android.os.Handler; @@ -98,11 +97,6 @@ public abstract class TetheringDependencies { return true; } - /** - * Get the NetworkRequest that should be fulfilled by the default network. - */ - public abstract NetworkRequest getDefaultNetworkRequest(); - /** * Get a reference to the EntitlementManager to be used by tethering. */ diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringService.java b/Tethering/src/com/android/networkstack/tethering/TetheringService.java index 745ebd0762..5ab34015a9 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringService.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringService.java @@ -35,8 +35,6 @@ import android.net.IIntResultListener; import android.net.INetworkStackConnector; import android.net.ITetheringConnector; import android.net.ITetheringEventCallback; -import android.net.NetworkCapabilities; -import android.net.NetworkRequest; import android.net.NetworkStack; import android.net.TetheringRequestParcel; import android.net.dhcp.DhcpServerCallbacks; @@ -306,19 +304,6 @@ public class TetheringService extends Service { @VisibleForTesting public TetheringDependencies makeTetheringDependencies() { return new TetheringDependencies() { - @Override - public NetworkRequest getDefaultNetworkRequest() { - // TODO: b/147280869, add a proper system API to replace this. - final NetworkRequest trackDefaultRequest = new NetworkRequest.Builder() - .clearCapabilities() - .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) - .addCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) - .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .build(); - return trackDefaultRequest; - } - @Override public Looper getTetheringLooper() { final HandlerThread tetherThread = new HandlerThread("android.tethering"); diff --git a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java index 9de4c8799f..e615334bbb 100644 --- a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java +++ b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java @@ -47,6 +47,9 @@ import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.StateMachine; +import com.android.networkstack.apishim.ConnectivityManagerShimImpl; +import com.android.networkstack.apishim.common.ConnectivityManagerShim; +import com.android.networkstack.apishim.common.UnsupportedApiLevelException; import java.util.HashMap; import java.util.HashSet; @@ -142,33 +145,28 @@ public class UpstreamNetworkMonitor { mWhat = what; mLocalPrefixes = new HashSet<>(); mIsDefaultCellularUpstream = false; - } - - @VisibleForTesting - public UpstreamNetworkMonitor( - ConnectivityManager cm, StateMachine tgt, SharedLog log, int what) { - this((Context) null, tgt, log, what); - mCM = cm; + mCM = (ConnectivityManager) ctx.getSystemService(Context.CONNECTIVITY_SERVICE); } /** - * Tracking the system default network. This method should be called when system is ready. + * Tracking the system default network. This method should be only called once when system is + * ready, and the callback is never unregistered. * - * @param defaultNetworkRequest should be the same as ConnectivityService default request * @param entitle a EntitlementManager object to communicate between EntitlementManager and * UpstreamNetworkMonitor */ - public void startTrackDefaultNetwork(NetworkRequest defaultNetworkRequest, - EntitlementManager entitle) { - - // defaultNetworkRequest is not really a "request", just a way of tracking the system - // default network. It's guaranteed not to actually bring up any networks because it's - // the should be the same request as the ConnectivityService default request, and thus - // shares fate with it. We can't use registerDefaultNetworkCallback because it will not - // track the system default network if there is a VPN that applies to our UID. - if (mDefaultNetworkCallback == null) { - mDefaultNetworkCallback = new UpstreamNetworkCallback(CALLBACK_DEFAULT_INTERNET); - cm().requestNetwork(defaultNetworkRequest, mDefaultNetworkCallback, mHandler); + public void startTrackDefaultNetwork(EntitlementManager entitle) { + if (mDefaultNetworkCallback != null) { + Log.wtf(TAG, "default network callback is already registered"); + return; + } + ConnectivityManagerShim mCmShim = ConnectivityManagerShimImpl.newInstance(mContext); + mDefaultNetworkCallback = new UpstreamNetworkCallback(CALLBACK_DEFAULT_INTERNET); + try { + mCmShim.registerSystemDefaultNetworkCallback(mDefaultNetworkCallback, mHandler); + } catch (UnsupportedApiLevelException e) { + Log.wtf(TAG, "registerSystemDefaultNetworkCallback is not supported"); + return; } if (mEntitlementMgr == null) { mEntitlementMgr = entitle; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TestConnectivityManager.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TestConnectivityManager.java index df82d7cf22..6090213516 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TestConnectivityManager.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TestConnectivityManager.java @@ -16,6 +16,10 @@ package com.android.networkstack.tethering; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; @@ -64,12 +68,13 @@ public class TestConnectivityManager extends ConnectivityManager { public static final boolean CALLBACKS_FIRST = true; final Map mAllCallbacks = new ArrayMap<>(); + // This contains the callbacks tracking the system default network, whether it's registered + // with registerSystemDefaultNetworkCallback (S+) or with a custom request (R-). final Map mTrackingDefault = new ArrayMap<>(); final Map mListening = new ArrayMap<>(); final Map mRequested = new ArrayMap<>(); final Map mLegacyTypeMap = new ArrayMap<>(); - private final NetworkRequest mDefaultRequest; private final Context mContext; private int mNetworkId = 100; @@ -80,13 +85,10 @@ public class TestConnectivityManager extends ConnectivityManager { * @param ctx the context to use. Must be a fake or a mock because otherwise the test will * attempt to send real broadcasts and resulting in permission denials. * @param svc an IConnectivityManager. Should be a fake or a mock. - * @param defaultRequest the default NetworkRequest that will be used by Tethering. */ - public TestConnectivityManager(Context ctx, IConnectivityManager svc, - NetworkRequest defaultRequest) { + public TestConnectivityManager(Context ctx, IConnectivityManager svc) { super(ctx, svc); mContext = ctx; - mDefaultRequest = defaultRequest; } class NetworkRequestInfo { @@ -181,11 +183,19 @@ public class TestConnectivityManager extends ConnectivityManager { makeDefaultNetwork(agent, BROADCAST_FIRST, null /* inBetween */); } + static boolean looksLikeDefaultRequest(NetworkRequest req) { + return req.hasCapability(NET_CAPABILITY_INTERNET) + && !req.hasCapability(NET_CAPABILITY_DUN) + && !req.hasTransport(TRANSPORT_CELLULAR); + } + @Override public void requestNetwork(NetworkRequest req, NetworkCallback cb, Handler h) { assertFalse(mAllCallbacks.containsKey(cb)); mAllCallbacks.put(cb, new NetworkRequestInfo(req, h)); - if (mDefaultRequest.equals(req)) { + // For R- devices, Tethering will invoke this function in 2 cases, one is to request mobile + // network, the other is to track system default network. + if (looksLikeDefaultRequest(req)) { assertFalse(mTrackingDefault.containsKey(cb)); mTrackingDefault.put(cb, new NetworkRequestInfo(req, h)); } else { diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index fedeeeb609..2bc7fc4a53 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -26,7 +26,6 @@ import static android.net.ConnectivityManager.ACTION_RESTRICT_BACKGROUND_CHANGED import static android.net.ConnectivityManager.RESTRICT_BACKGROUND_STATUS_DISABLED; import static android.net.ConnectivityManager.RESTRICT_BACKGROUND_STATUS_ENABLED; import static android.net.ConnectivityManager.TYPE_MOBILE_DUN; -import static android.net.ConnectivityManager.TYPE_NONE; import static android.net.ConnectivityManager.TYPE_WIFI; import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; @@ -73,7 +72,6 @@ import static com.android.testutils.TestPermissionUtil.runAsShell; 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.argThat; @@ -274,8 +272,6 @@ public class TetheringTest { private UpstreamNetworkMonitor mUpstreamNetworkMonitor; private TestConnectivityManager mCm; - private NetworkRequest mNetworkRequest; - private NetworkCallback mDefaultNetworkCallback; private class TestContext extends BroadcastInterceptingContext { TestContext(Context base) { @@ -449,11 +445,6 @@ public class TetheringTest { return mIpServerDependencies; } - @Override - public NetworkRequest getDefaultNetworkRequest() { - return mNetworkRequest; - } - @Override public EntitlementManager getEntitlementManager(Context ctx, Handler h, SharedLog log, Runnable callback) { @@ -645,15 +636,7 @@ public class TetheringTest { mServiceContext.registerReceiver(mBroadcastReceiver, new IntentFilter(ACTION_TETHER_STATE_CHANGED)); - // TODO: add NOT_VCN_MANAGED here, but more importantly in the production code. - // TODO: even better, change TetheringDependencies.getDefaultNetworkRequest() to use - // registerSystemDefaultNetworkCallback() on S and above. - NetworkCapabilities defaultCaps = new NetworkCapabilities() - .addCapability(NET_CAPABILITY_INTERNET); - mNetworkRequest = new NetworkRequest(defaultCaps, TYPE_NONE, 1 /* requestId */, - NetworkRequest.Type.REQUEST); - mCm = spy(new TestConnectivityManager(mServiceContext, mock(IConnectivityManager.class), - mNetworkRequest)); + mCm = spy(new TestConnectivityManager(mServiceContext, mock(IConnectivityManager.class))); mTethering = makeTethering(); verify(mStatsManager, times(1)).registerNetworkStatsProvider(anyString(), any()); @@ -793,15 +776,13 @@ public class TetheringTest { } private void verifyDefaultNetworkRequestFiled() { - ArgumentCaptor captor = ArgumentCaptor.forClass(NetworkCallback.class); - verify(mCm, times(1)).requestNetwork(eq(mNetworkRequest), - captor.capture(), any(Handler.class)); - mDefaultNetworkCallback = captor.getValue(); - assertNotNull(mDefaultNetworkCallback); - + ArgumentCaptor reqCaptor = ArgumentCaptor.forClass(NetworkRequest.class); + verify(mCm, times(1)).requestNetwork(reqCaptor.capture(), + any(NetworkCallback.class), any(Handler.class)); + assertTrue(TestConnectivityManager.looksLikeDefaultRequest(reqCaptor.getValue())); // The default network request is only ever filed once. verifyNoMoreInteractions(mCm); - mUpstreamNetworkMonitor.startTrackDefaultNetwork(mNetworkRequest, mEntitleMgr); + mUpstreamNetworkMonitor.startTrackDefaultNetwork(mEntitleMgr); verifyNoMoreInteractions(mCm); } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java index cee03655db..ce4ba85565 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java @@ -29,10 +29,10 @@ import static com.android.networkstack.tethering.UpstreamNetworkMonitor.TYPE_NON import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyString; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -66,6 +66,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -83,10 +84,6 @@ public class UpstreamNetworkMonitorTest { private static final boolean INCLUDES = true; private static final boolean EXCLUDES = false; - // Actual contents of the request don't matter for this test. The lack of - // any specific TRANSPORT_* is sufficient to identify this request. - private static final NetworkRequest sDefaultRequest = new NetworkRequest.Builder().build(); - private static final NetworkCapabilities CELL_CAPABILITIES = new NetworkCapabilities.Builder() .addTransportType(TRANSPORT_CELLULAR).addCapability(NET_CAPABILITY_INTERNET).build(); private static final NetworkCapabilities DUN_CAPABILITIES = new NetworkCapabilities.Builder() @@ -113,9 +110,10 @@ public class UpstreamNetworkMonitorTest { when(mLog.forSubComponent(anyString())).thenReturn(mLog); when(mEntitleMgr.isCellularUpstreamPermitted()).thenReturn(true); - mCM = spy(new TestConnectivityManager(mContext, mCS, sDefaultRequest)); + mCM = spy(new TestConnectivityManager(mContext, mCS)); + when(mContext.getSystemService(eq(Context.CONNECTIVITY_SERVICE))).thenReturn(mCM); mSM = new TestStateMachine(mLooper.getLooper()); - mUNM = new UpstreamNetworkMonitor(mCM, mSM, mLog, EVENT_UNM_UPDATE); + mUNM = new UpstreamNetworkMonitor(mContext, mSM, mLog, EVENT_UNM_UPDATE); } @After public void tearDown() throws Exception { @@ -146,7 +144,7 @@ public class UpstreamNetworkMonitorTest { @Test public void testDefaultNetworkIsTracked() throws Exception { assertTrue(mCM.hasNoCallbacks()); - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); mUNM.startObserveAllNetworks(); assertEquals(1, mCM.mTrackingDefault.size()); @@ -159,7 +157,7 @@ public class UpstreamNetworkMonitorTest { public void testListensForAllNetworks() throws Exception { assertTrue(mCM.mListening.isEmpty()); - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); mUNM.startObserveAllNetworks(); assertFalse(mCM.mListening.isEmpty()); assertTrue(mCM.isListeningForAll()); @@ -170,9 +168,17 @@ public class UpstreamNetworkMonitorTest { @Test public void testCallbacksRegistered() { - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); + // Verify the fired default request matches expectation. + final ArgumentCaptor requestCaptor = + ArgumentCaptor.forClass(NetworkRequest.class); verify(mCM, times(1)).requestNetwork( - eq(sDefaultRequest), any(NetworkCallback.class), any(Handler.class)); + requestCaptor.capture(), any(NetworkCallback.class), any(Handler.class)); + // For R- devices, Tethering will invoke this function in 2 cases, one is to + // request mobile network, the other is to track system default network. Verify + // the request is the one tracks default network. + assertTrue(TestConnectivityManager.looksLikeDefaultRequest(requestCaptor.getValue())); + mUNM.startObserveAllNetworks(); verify(mCM, times(1)).registerNetworkCallback( any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class)); @@ -293,7 +299,7 @@ public class UpstreamNetworkMonitorTest { final Collection preferredTypes = new ArrayList<>(); preferredTypes.add(TYPE_WIFI); - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); mUNM.startObserveAllNetworks(); // There are no networks, so there is nothing to select. assertSatisfiesLegacyType(TYPE_NONE, mUNM.selectPreferredUpstreamType(preferredTypes)); @@ -366,7 +372,7 @@ public class UpstreamNetworkMonitorTest { @Test public void testGetCurrentPreferredUpstream() throws Exception { - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); mUNM.startObserveAllNetworks(); mUNM.setUpstreamConfig(true /* autoUpstream */, false /* dunRequired */); mUNM.setTryCell(true); @@ -438,7 +444,7 @@ public class UpstreamNetworkMonitorTest { @Test public void testLocalPrefixes() throws Exception { - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); mUNM.startObserveAllNetworks(); // [0] Test minimum set of local prefixes. @@ -549,7 +555,7 @@ public class UpstreamNetworkMonitorTest { // Mobile has higher pirority than wifi. preferredTypes.add(TYPE_MOBILE_HIPRI); preferredTypes.add(TYPE_WIFI); - mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr); + mUNM.startTrackDefaultNetwork(mEntitleMgr); mUNM.startObserveAllNetworks(); // Setup wifi and make wifi as default network. final TestNetworkAgent wifiAgent = new TestNetworkAgent(mCM, WIFI_CAPABILITIES);