Adopt and drop shell permission once per VCN test.

This CL updates VcnManagerTest to explicitly adopt and drop shell
permission identity once per test. This change ensures that no races
occur between adopting and dropping shell permission identity in
testing, which can cause test flake.

This CL also updates CarrierPrivilegeUtils to not manage its own
permissions (needed for adopting CarrierPrivileges). Instead, the caller
must manage these permissions before using the util.

Bug: 182291467
Test: atest CtsVcnTestCases
Change-Id: I3e82d3b2da3fe30b87a90751b0b996a42e6f15c3

2 files changed, 81 insertions, 98 deletions

diff --git a/tests/tests/vcn/src/android/net/vcn/cts/TestNetworkWrapper.java b/tests/tests/vcn/src/android/net/vcn/cts/TestNetworkWrapper.java
index 6d0a95743e4..52b6245a186 100644
--- a/tests/tests/vcn/src/android/net/vcn/cts/TestNetworkWrapper.java
+++ b/tests/tests/vcn/src/android/net/vcn/cts/TestNetworkWrapper.java
@@ -18,9 +18,6 @@ package android.net.vcn.cts;
 
 import static android.net.cts.util.CtsNetUtils.TestNetworkCallback;
 
-import static com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity;
-import static com.android.compatibility.common.util.SystemUtil.runWithShellPermissionIdentity;
-
 import android.annotation.NonNull;
 import android.content.Context;
 import android.net.ConnectivityManager;
@@ -106,9 +103,7 @@ public class TestNetworkWrapper implements AutoCloseable {
             throws Exception {
         mConnectivityManager = context.getSystemService(ConnectivityManager.class);
         mVcnManager = context.getSystemService(VcnManager.class);
-        mTestNetworkManager =
-                callWithShellPermissionIdentity(
-                        () -> context.getSystemService(TestNetworkManager.class));
+        mTestNetworkManager = context.getSystemService(TestNetworkManager.class);
 
         try {
             final LinkAddress linkAddress =
@@ -116,49 +111,31 @@ public class TestNetworkWrapper implements AutoCloseable {
                             localAddress,
                             localAddress instanceof Inet4Address ? IP4_PREFIX_LEN : IP6_PREFIX_LEN);
             final TestNetworkInterface tni =
-                    callWithShellPermissionIdentity(
-                            () -> {
-                                return mTestNetworkManager.createTunInterface(
-                                        Arrays.asList(linkAddress));
-                            });
+                    mTestNetworkManager.createTunInterface(Arrays.asList(linkAddress));
             tunFd = tni.getFileDescriptor();
             final String iface = tni.getInterfaceName();
 
-            vcnNetworkCallback =
-                    callWithShellPermissionIdentity(
-                            () -> {
-                                final NetworkRequest nr =
-                                        new NetworkRequest.Builder(TEST_NETWORK_REQUEST)
-                                                .setNetworkSpecifier(iface)
-                                                .build();
-                                final VcnTestNetworkCallback cb = new VcnTestNetworkCallback();
-                                mConnectivityManager.requestNetwork(nr, cb);
-                                return cb;
-                            });
+            final NetworkRequest nr =
+                    new NetworkRequest.Builder(TEST_NETWORK_REQUEST)
+                            .setNetworkSpecifier(iface)
+                            .build();
+            vcnNetworkCallback = new VcnTestNetworkCallback();
+            mConnectivityManager.requestNetwork(nr, vcnNetworkCallback);
 
             final NetworkCapabilities nc =
                     createNetworkCapabilitiesForIface(iface, isMetered, subIds);
             final LinkProperties lp = createLinkPropertiesForIface(iface, mtu);
 
+            final VcnNetworkPolicyResult policy = mVcnManager.applyVcnNetworkPolicy(nc, lp);
+            if (policy.isTeardownRequested()) {
+                throw new IllegalStateException("Restart requested in bringup");
+            }
+
             mTestNetworkAgent =
-                    callWithShellPermissionIdentity(
-                            () -> {
-                                final VcnNetworkPolicyResult policy =
-                                        mVcnManager.applyVcnNetworkPolicy(nc, lp);
-                                if (policy.isTeardownRequested()) {
-                                    throw new IllegalStateException("Restart requested in bringup");
-                                }
-
-                                final TestNetworkAgent agent =
-                                        new TestNetworkAgent(
-                                                context,
-                                                Looper.getMainLooper(),
-                                                policy.getNetworkCapabilities(),
-                                                lp);
-                                agent.register();
-                                agent.markConnected();
-                                return agent;
-                            });
+                    new TestNetworkAgent(
+                            context, Looper.getMainLooper(), policy.getNetworkCapabilities(), lp);
+            mTestNetworkAgent.register();
+            mTestNetworkAgent.markConnected();
 
             tunNetwork = vcnNetworkCallback.waitForAvailable();
             mCloseGuard.open(TAG);
@@ -232,42 +209,40 @@ public class TestNetworkWrapper implements AutoCloseable {
     @Override
     public void close() {
         mCloseGuard.close();
-        runWithShellPermissionIdentity(
-                () -> {
-                    if (vcnNetworkCallback != null) {
-                        try {
-                            mConnectivityManager.unregisterNetworkCallback(vcnNetworkCallback);
-                        } catch (Exception e) {
-                            Log.e(TAG, "Failed to unregister Network CB", e);
-                        }
-                    }
 
-                    if (mTestNetworkAgent != null) {
-                        synchronized (mTestNetworkAgent) {
-                            try {
-                                mTestNetworkAgent.teardown();
-                            } catch (Exception e) {
-                                Log.e(TAG, "Failed to unregister TestNetworkAgent", e);
-                            }
-                        }
-                    }
+        if (vcnNetworkCallback != null) {
+            try {
+                mConnectivityManager.unregisterNetworkCallback(vcnNetworkCallback);
+            } catch (Exception e) {
+                Log.e(TAG, "Failed to unregister Network CB", e);
+            }
+        }
 
-                    if (tunNetwork != null) {
-                        try {
-                            mTestNetworkManager.teardownTestNetwork(tunNetwork);
-                        } catch (Exception e) {
-                            Log.e(TAG, "Failed to tear down Test Network", e);
-                        }
-                    }
+        if (mTestNetworkAgent != null) {
+            synchronized (mTestNetworkAgent) {
+                try {
+                    mTestNetworkAgent.teardown();
+                } catch (Exception e) {
+                    Log.e(TAG, "Failed to unregister TestNetworkAgent", e);
+                }
+            }
+        }
 
-                    if (tunFd != null) {
-                        try {
-                            tunFd.close();
-                        } catch (Exception e) {
-                            Log.e(TAG, "Failed to close Test Network FD", e);
-                        }
-                    }
-                });
+        if (tunNetwork != null) {
+            try {
+                mTestNetworkManager.teardownTestNetwork(tunNetwork);
+            } catch (Exception e) {
+                Log.e(TAG, "Failed to tear down Test Network", e);
+            }
+        }
+
+        if (tunFd != null) {
+            try {
+                tunFd.close();
+            } catch (Exception e) {
+                Log.e(TAG, "Failed to close Test Network FD", e);
+            }
+        }
     }
 
     @Override
@@ -321,11 +296,9 @@ public class TestNetworkWrapper implements AutoCloseable {
 
         @Override
         public void onNetworkUnwanted() {
-            // Not guaranteed to be called from the same thread, so synchronize on this. The shell
-            // is needed for teardown because unregistering the VcnPolicyListener requires
-            // permission MANAGE_TEST_NETWORKS.
+            // Not guaranteed to be called from the same thread, so synchronize on this.
             synchronized (this) {
-                runWithShellPermissionIdentity(() -> teardown());
+                teardown();
             }
         }
 
@@ -353,24 +326,19 @@ public class TestNetworkWrapper implements AutoCloseable {
         private class TestVcnNetworkPolicyChangeListener implements VcnNetworkPolicyChangeListener {
             @Override
             public void onPolicyChanged() {
-                runWithShellPermissionIdentity(
-                        () -> {
-                            synchronized (TestNetworkAgent.this) {
-                                final VcnNetworkPolicyResult policy =
-                                        mVcnManager.applyVcnNetworkPolicy(
-                                                mTestNetworkAgent.getNetworkCapabilities(),
-                                                mTestNetworkAgent.getLinkProperties());
-                                if (policy.isTeardownRequested()) {
-                                    Log.w(
-                                            POLICY_LISTENER_TAG,
-                                            "network teardown requested on policy change");
-                                    teardown();
-                                    return;
-                                }
-
-                                updateNetworkCapabilities(policy.getNetworkCapabilities());
-                            }
-                        });
+                synchronized (TestNetworkAgent.this) {
+                    final VcnNetworkPolicyResult policy =
+                            mVcnManager.applyVcnNetworkPolicy(
+                                    mTestNetworkAgent.getNetworkCapabilities(),
+                                    mTestNetworkAgent.getLinkProperties());
+                    if (policy.isTeardownRequested()) {
+                        Log.w(POLICY_LISTENER_TAG, "network teardown requested on policy change");
+                        teardown();
+                        return;
+                    }
+
+                    updateNetworkCapabilities(policy.getNetworkCapabilities());
+                }
             }
         }
     }
diff --git a/tests/tests/vcn/src/android/net/vcn/cts/VcnManagerTest.java b/tests/tests/vcn/src/android/net/vcn/cts/VcnManagerTest.java
index 9bf818ddab3..7fec0011b09 100644
--- a/tests/tests/vcn/src/android/net/vcn/cts/VcnManagerTest.java
+++ b/tests/tests/vcn/src/android/net/vcn/cts/VcnManagerTest.java
@@ -23,6 +23,8 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID;
 
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+
 import static com.android.compatibility.common.util.SystemUtil.runWithShellPermissionIdentity;
 
 import static org.junit.Assert.assertEquals;
@@ -96,12 +98,19 @@ public class VcnManagerTest extends VcnTestBase {
     @Before
     public void setUp() throws Exception {
         assumeTrue(mContext.getPackageManager().hasSystemFeature(FEATURE_TELEPHONY));
+
+        getInstrumentation().getUiAutomation().adoptShellPermissionIdentity();
     }
 
     @After
     public void tearDown() throws Exception {
-        if (mTestNetworkWrapper != null) {
-            mTestNetworkWrapper.close();
+        try {
+            if (mTestNetworkWrapper != null) {
+                mTestNetworkWrapper.close();
+                mTestNetworkWrapper = null;
+            }
+        } finally {
+            getInstrumentation().getUiAutomation().dropShellPermissionIdentity();
         }
     }
 
@@ -172,6 +181,9 @@ public class VcnManagerTest extends VcnTestBase {
     @Test(expected = SecurityException.class)
     public void testAddVcnNetworkPolicyChangeListener_noNetworkFactoryPermission()
             throws Exception {
+        // Drop shell permission identity to test unpermissioned behavior.
+        getInstrumentation().getUiAutomation().dropShellPermissionIdentity();
+
         final TestVcnNetworkPolicyChangeListener listener =
                 new TestVcnNetworkPolicyChangeListener();
 
@@ -192,6 +204,9 @@ public class VcnManagerTest extends VcnTestBase {
 
     @Test(expected = SecurityException.class)
     public void testApplyVcnNetworkPolicy_noNetworkFactoryPermission() throws Exception {
+        // Drop shell permission identity to test unpermissioned behavior.
+        getInstrumentation().getUiAutomation().dropShellPermissionIdentity();
+
         final NetworkCapabilities nc = new NetworkCapabilities.Builder().build();
         final LinkProperties lp = new LinkProperties();
 
@@ -334,7 +349,7 @@ public class VcnManagerTest extends VcnTestBase {
                 true /* expectNotVcnManaged */,
                 false /* expectNotMetered */);
 
-        CarrierPrivilegeUtils.withCarrierPrivileges(mContext, subId, () -> {
+        CarrierPrivilegeUtils.withCarrierPrivilegesForShell(mContext, subId, () -> {
             SubscriptionGroupUtils.withEphemeralSubscriptionGroup(mContext, subId, (subGrp) -> {
                 mVcnManager.setVcnConfig(subGrp, buildVcnConfig());