From 85c6ad2363a90bb070f663057a89f206cde5d2ea Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Fri, 19 Mar 2021 00:25:53 -0700 Subject: [PATCH] Switch to using list of subIds for policy generation Bug: 183174340 Test: atest FrameworksVcnTests Change-Id: I2d30b75c282cf619d81b0f6dc06638566af61a33 --- .../android/server/VcnManagementService.java | 57 ++++++++++--------- .../server/VcnManagementServiceTest.java | 26 +++------ 2 files changed, 38 insertions(+), 45 deletions(-) diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index 4622e98bbbb25..8eefbfc606de0 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -36,7 +36,6 @@ import android.content.IntentFilter; import android.net.ConnectivityManager; import android.net.LinkProperties; import android.net.NetworkCapabilities; -import android.net.TelephonyNetworkSpecifier; import android.net.vcn.IVcnManagementService; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; @@ -717,19 +716,29 @@ public class VcnManagementService extends IVcnManagementService.Stub { }); } - private int getSubIdForNetworkCapabilities(@NonNull NetworkCapabilities networkCapabilities) { - if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) - && networkCapabilities.getNetworkSpecifier() instanceof TelephonyNetworkSpecifier) { - TelephonyNetworkSpecifier telephonyNetworkSpecifier = - (TelephonyNetworkSpecifier) networkCapabilities.getNetworkSpecifier(); - return telephonyNetworkSpecifier.getSubscriptionId(); - } else if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) - && networkCapabilities.getTransportInfo() instanceof WifiInfo) { - WifiInfo wifiInfo = (WifiInfo) networkCapabilities.getTransportInfo(); - return mDeps.getSubIdForWifiInfo(wifiInfo); + private ParcelUuid getSubGroupForNetworkCapabilities( + @NonNull NetworkCapabilities networkCapabilities) { + ParcelUuid subGrp = null; + final TelephonySubscriptionSnapshot snapshot; + + // Always access mLastSnapshot under lock. Technically this can be treated as a volatile + // but for consistency and safety, always access under lock. + synchronized (mLock) { + snapshot = mLastSnapshot; } - return SubscriptionManager.INVALID_SUBSCRIPTION_ID; + // If multiple subscription IDs exist, they MUST all point to the same subscription + // group. Otherwise undefined behavior may occur. + for (int subId : networkCapabilities.getSubIds()) { + // Verify that all subscriptions point to the same group + if (subGrp != null && !subGrp.equals(snapshot.getGroupForSubId(subId))) { + Slog.wtf(TAG, "Got multiple subscription groups for a single network"); + } + + subGrp = snapshot.getGroupForSubId(subId); + } + + return subGrp; } /** @@ -754,23 +763,19 @@ public class VcnManagementService extends IVcnManagementService.Stub { // mutates final NetworkCapabilities ncCopy = new NetworkCapabilities(networkCapabilities); - final int subId = getSubIdForNetworkCapabilities(ncCopy); + final ParcelUuid subGrp = getSubGroupForNetworkCapabilities(ncCopy); boolean isVcnManagedNetwork = false; boolean isRestrictedCarrierWifi = false; - if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) { - synchronized (mLock) { - ParcelUuid subGroup = mLastSnapshot.getGroupForSubId(subId); + synchronized (mLock) { + final Vcn vcn = mVcns.get(subGrp); + if (vcn != null) { + if (vcn.isActive()) { + isVcnManagedNetwork = true; + } - final Vcn vcn = mVcns.get(subGroup); - if (vcn != null) { - if (vcn.isActive()) { - isVcnManagedNetwork = true; - } - - if (ncCopy.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { - // Carrier WiFi always restricted if VCN exists (even in safe mode). - isRestrictedCarrierWifi = true; - } + if (ncCopy.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { + // Carrier WiFi always restricted if VCN exists (even in safe mode). + isRestrictedCarrierWifi = true; } } } diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index f15d4204d125e..c88b0c1547126 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -34,8 +34,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; @@ -45,7 +45,6 @@ import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import android.annotation.NonNull; import android.app.AppOpsManager; @@ -63,7 +62,6 @@ import android.net.vcn.VcnConfig; import android.net.vcn.VcnConfigTest; import android.net.vcn.VcnManager; import android.net.vcn.VcnUnderlyingNetworkPolicy; -import android.net.wifi.WifiInfo; import android.os.IBinder; import android.os.ParcelUuid; import android.os.PersistableBundle; @@ -708,22 +706,12 @@ public class VcnManagementServiceTest { int subId, ParcelUuid subGrp, boolean isVcnActive, int transport) { setupSubscriptionAndStartVcn(subId, subGrp, isVcnActive); - final NetworkCapabilities.Builder ncBuilder = new NetworkCapabilities.Builder(); - ncBuilder.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); - if (transport == TRANSPORT_CELLULAR) { - ncBuilder - .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) - .setNetworkSpecifier(new TelephonyNetworkSpecifier(TEST_SUBSCRIPTION_ID)); - } else if (transport == TRANSPORT_WIFI) { - WifiInfo wifiInfo = mock(WifiInfo.class); - when(wifiInfo.makeCopy(anyLong())).thenReturn(wifiInfo); - when(mMockDeps.getSubIdForWifiInfo(eq(wifiInfo))).thenReturn(TEST_SUBSCRIPTION_ID); - - ncBuilder - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) - .setTransportInfo(wifiInfo); - } else { - throw new IllegalArgumentException("Unknown transport"); + final NetworkCapabilities.Builder ncBuilder = + new NetworkCapabilities.Builder() + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .addTransportType(transport); + if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) { + ncBuilder.setSubIds(Collections.singleton(subId)); } return mVcnMgmtSvc.getUnderlyingNetworkPolicy(ncBuilder.build(), new LinkProperties());