From 253b3fb6da7e5403571586e424e8b5f0f79cea47 Mon Sep 17 00:00:00 2001 From: Handa Wang Date: Fri, 19 Jan 2024 07:29:00 +0000 Subject: [PATCH] Make ThreadNetworkController.setTestNetworkAsUpstream require NETWORK_SETTINGS permission This CL contains following enhancements: - Enforce NETWORK_SETTINGS permission for ThreadNetworkController.setTestNetworkAsUpstream. - Move the setTestNetworkAsUpstream in ThreadNetworkControllerService to make the order of methods more logical. - Update the test case name in BorderRoutingTest. Bug: 295843010 Bug: 320842152 Change-Id: I82edac6c819dd637f4c98f31368a1b520338ce24 --- .../net/thread/ThreadNetworkController.java | 3 +- .../ThreadNetworkControllerService.java | 65 ++++++++++--------- thread/tests/integration/AndroidManifest.xml | 1 + .../android/net/thread/BorderRoutingTest.java | 5 +- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/thread/framework/java/android/net/thread/ThreadNetworkController.java b/thread/framework/java/android/net/thread/ThreadNetworkController.java index b5699a90e4..7242ed78d5 100644 --- a/thread/framework/java/android/net/thread/ThreadNetworkController.java +++ b/thread/framework/java/android/net/thread/ThreadNetworkController.java @@ -510,7 +510,8 @@ public final class ThreadNetworkController { * @hide */ @VisibleForTesting - @RequiresPermission("android.permission.THREAD_NETWORK_PRIVILEGED") + @RequiresPermission( + allOf = {"android.permission.THREAD_NETWORK_PRIVILEGED", permission.NETWORK_SETTINGS}) public void setTestNetworkAsUpstream( @Nullable String testNetworkInterfaceName, @NonNull @CallbackExecutor Executor executor, diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java index cd59e4e94a..1c51c42aba 100644 --- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java +++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java @@ -14,6 +14,7 @@ package com.android.server.thread; +import static android.Manifest.permission.NETWORK_SETTINGS; import static android.net.MulticastRoutingConfig.CONFIG_FORWARD_NONE; import static android.net.MulticastRoutingConfig.FORWARD_NONE; import static android.net.MulticastRoutingConfig.FORWARD_SELECTED; @@ -253,38 +254,6 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub .build(); } - @Override - public void setTestNetworkAsUpstream( - @Nullable String testNetworkInterfaceName, @NonNull IOperationReceiver receiver) { - enforceAllPermissionsGranted(PERMISSION_THREAD_NETWORK_PRIVILEGED); - - Log.i(TAG, "setTestNetworkAsUpstream: " + testNetworkInterfaceName); - mHandler.post(() -> setTestNetworkAsUpstreamInternal(testNetworkInterfaceName, receiver)); - } - - private void setTestNetworkAsUpstreamInternal( - @Nullable String testNetworkInterfaceName, @NonNull IOperationReceiver receiver) { - checkOnHandlerThread(); - - TestNetworkSpecifier testNetworkSpecifier = null; - if (testNetworkInterfaceName != null) { - testNetworkSpecifier = new TestNetworkSpecifier(testNetworkInterfaceName); - } - - if (!Objects.equals(mUpstreamTestNetworkSpecifier, testNetworkSpecifier)) { - cancelRequestUpstreamNetwork(); - mUpstreamTestNetworkSpecifier = testNetworkSpecifier; - mUpstreamNetworkRequest = newUpstreamNetworkRequest(); - requestUpstreamNetwork(); - sendLocalNetworkConfig(); - } - try { - receiver.onSuccess(); - } catch (RemoteException ignored) { - // do nothing if the client is dead - } - } - private void initializeOtDaemon() { try { getOtDaemon(); @@ -786,6 +755,38 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub } } + @Override + public void setTestNetworkAsUpstream( + @Nullable String testNetworkInterfaceName, @NonNull IOperationReceiver receiver) { + enforceAllPermissionsGranted(PERMISSION_THREAD_NETWORK_PRIVILEGED, NETWORK_SETTINGS); + + Log.i(TAG, "setTestNetworkAsUpstream: " + testNetworkInterfaceName); + mHandler.post(() -> setTestNetworkAsUpstreamInternal(testNetworkInterfaceName, receiver)); + } + + private void setTestNetworkAsUpstreamInternal( + @Nullable String testNetworkInterfaceName, @NonNull IOperationReceiver receiver) { + checkOnHandlerThread(); + + TestNetworkSpecifier testNetworkSpecifier = null; + if (testNetworkInterfaceName != null) { + testNetworkSpecifier = new TestNetworkSpecifier(testNetworkInterfaceName); + } + + if (!Objects.equals(mUpstreamTestNetworkSpecifier, testNetworkSpecifier)) { + cancelRequestUpstreamNetwork(); + mUpstreamTestNetworkSpecifier = testNetworkSpecifier; + mUpstreamNetworkRequest = newUpstreamNetworkRequest(); + requestUpstreamNetwork(); + sendLocalNetworkConfig(); + } + try { + receiver.onSuccess(); + } catch (RemoteException ignored) { + // do nothing if the client is dead + } + } + private void enableBorderRouting(String infraIfName) { if (mBorderRouterConfig.isBorderRoutingEnabled && infraIfName.equals(mBorderRouterConfig.infraInterfaceName)) { diff --git a/thread/tests/integration/AndroidManifest.xml b/thread/tests/integration/AndroidManifest.xml index a347654857..a049184d6d 100644 --- a/thread/tests/integration/AndroidManifest.xml +++ b/thread/tests/integration/AndroidManifest.xml @@ -23,6 +23,7 @@ obtain CHANGE_NETWORK_STATE for testing once R device is no longer supported. --> + diff --git a/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java b/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java index ba7e4b8f23..25f5bd3bd1 100644 --- a/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java +++ b/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java @@ -17,6 +17,7 @@ package android.net.thread; import static android.Manifest.permission.MANAGE_TEST_NETWORKS; +import static android.Manifest.permission.NETWORK_SETTINGS; import static android.net.thread.IntegrationTestUtils.isExpectedIcmpv6Packet; import static android.net.thread.IntegrationTestUtils.isSimulatedThreadRadioSupported; import static android.net.thread.IntegrationTestUtils.newPacketReader; @@ -101,6 +102,7 @@ public class BorderRoutingTest { mContext, new LinkProperties(), 5000 /* timeoutMs */)); runAsShell( PERMISSION_THREAD_NETWORK_PRIVILEGED, + NETWORK_SETTINGS, () -> { CountDownLatch latch = new CountDownLatch(1); mThreadNetworkController.setTestNetworkAsUpstream( @@ -117,6 +119,7 @@ public class BorderRoutingTest { public void tearDown() throws Exception { runAsShell( PERMISSION_THREAD_NETWORK_PRIVILEGED, + NETWORK_SETTINGS, () -> { CountDownLatch latch = new CountDownLatch(2); mThreadNetworkController.setTestNetworkAsUpstream( @@ -132,7 +135,7 @@ public class BorderRoutingTest { } @Test - public void infraDevicePingTheadDeviceOmr_Succeeds() throws Exception { + public void unicastRouting_infraDevicePingTheadDeviceOmr_replyReceived() throws Exception { assumeTrue(isSimulatedThreadRadioSupported()); /*