diff --git a/framework-t/api/system-current.txt b/framework-t/api/system-current.txt index b285d85256..05cf9e8463 100644 --- a/framework-t/api/system-current.txt +++ b/framework-t/api/system-current.txt @@ -500,8 +500,8 @@ package android.net.thread { method @RequiresPermission(allOf={android.Manifest.permission.ACCESS_NETWORK_STATE, "android.permission.THREAD_NETWORK_PRIVILEGED"}) public void registerOperationalDatasetCallback(@NonNull java.util.concurrent.Executor, @NonNull android.net.thread.ThreadNetworkController.OperationalDatasetCallback); method @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerStateCallback(@NonNull java.util.concurrent.Executor, @NonNull android.net.thread.ThreadNetworkController.StateCallback); method @RequiresPermission("android.permission.THREAD_NETWORK_PRIVILEGED") public void scheduleMigration(@NonNull android.net.thread.PendingOperationalDataset, @NonNull java.util.concurrent.Executor, @NonNull android.os.OutcomeReceiver); - method public void unregisterOperationalDatasetCallback(@NonNull android.net.thread.ThreadNetworkController.OperationalDatasetCallback); - method public void unregisterStateCallback(@NonNull android.net.thread.ThreadNetworkController.StateCallback); + method @RequiresPermission(allOf={android.Manifest.permission.ACCESS_NETWORK_STATE, "android.permission.THREAD_NETWORK_PRIVILEGED"}) public void unregisterOperationalDatasetCallback(@NonNull android.net.thread.ThreadNetworkController.OperationalDatasetCallback); + method @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void unregisterStateCallback(@NonNull android.net.thread.ThreadNetworkController.StateCallback); field public static final int DEVICE_ROLE_CHILD = 2; // 0x2 field public static final int DEVICE_ROLE_DETACHED = 1; // 0x1 field public static final int DEVICE_ROLE_LEADER = 4; // 0x4 diff --git a/thread/framework/java/android/net/thread/ThreadNetworkController.java b/thread/framework/java/android/net/thread/ThreadNetworkController.java index ec39db4260..5c5fda9a8a 100644 --- a/thread/framework/java/android/net/thread/ThreadNetworkController.java +++ b/thread/framework/java/android/net/thread/ThreadNetworkController.java @@ -226,15 +226,17 @@ public final class ThreadNetworkController { * @param callback the callback which has been registered with {@link #registerStateCallback} * @throws IllegalArgumentException if {@code callback} hasn't been registered */ + @RequiresPermission(permission.ACCESS_NETWORK_STATE) public void unregisterStateCallback(@NonNull StateCallback callback) { requireNonNull(callback, "callback cannot be null"); synchronized (mStateCallbackMapLock) { - StateCallbackProxy callbackProxy = mStateCallbackMap.remove(callback); + StateCallbackProxy callbackProxy = mStateCallbackMap.get(callback); if (callbackProxy == null) { throw new IllegalArgumentException("callback hasn't been registered"); } try { mControllerService.unregisterStateCallback(callbackProxy); + mStateCallbackMap.remove(callback); } catch (RemoteException e) { e.rethrowFromSystemServer(); } @@ -334,15 +336,21 @@ public final class ThreadNetworkController { * #registerOperationalDatasetCallback} * @throws IllegalArgumentException if {@code callback} hasn't been registered */ + @RequiresPermission( + allOf = { + permission.ACCESS_NETWORK_STATE, + "android.permission.THREAD_NETWORK_PRIVILEGED" + }) public void unregisterOperationalDatasetCallback(@NonNull OperationalDatasetCallback callback) { requireNonNull(callback, "callback cannot be null"); synchronized (mOpDatasetCallbackMapLock) { - OperationalDatasetCallbackProxy callbackProxy = mOpDatasetCallbackMap.remove(callback); + OperationalDatasetCallbackProxy callbackProxy = mOpDatasetCallbackMap.get(callback); if (callbackProxy == null) { throw new IllegalArgumentException("callback hasn't been registered"); } try { mControllerService.unregisterOperationalDatasetCallback(callbackProxy); + mOpDatasetCallbackMap.remove(callback); } catch (RemoteException e) { e.rethrowFromSystemServer(); } diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java index 84359121f6..33516aa351 100644 --- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java +++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java @@ -419,12 +419,12 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub @Override public void registerStateCallback(IStateCallback stateCallback) throws RemoteException { enforceAllCallingPermissionsGranted(permission.ACCESS_NETWORK_STATE); - mHandler.post(() -> mOtDaemonCallbackProxy.registerStateCallback(stateCallback)); } @Override public void unregisterStateCallback(IStateCallback stateCallback) throws RemoteException { + enforceAllCallingPermissionsGranted(permission.ACCESS_NETWORK_STATE); mHandler.post(() -> mOtDaemonCallbackProxy.unregisterStateCallback(stateCallback)); } @@ -439,6 +439,8 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub @Override public void unregisterOperationalDatasetCallback(IOperationalDatasetCallback callback) throws RemoteException { + enforceAllCallingPermissionsGranted( + permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED); mHandler.post(() -> mOtDaemonCallbackProxy.unregisterDatasetCallback(callback)); } diff --git a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java index e17dd02128..362ff3973c 100644 --- a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java +++ b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java @@ -101,7 +101,7 @@ public class ThreadNetworkControllerTest { public void tearDown() throws Exception { if (mManager != null) { leaveAndWait(); - dropPermissions(); + dropAllPermissions(); } } @@ -128,7 +128,7 @@ public class ThreadNetworkControllerTest { getInstrumentation().getUiAutomation().adoptShellPermissionIdentity(allPermissions); } - private static void dropPermissions() { + private static void dropAllPermissions() { getInstrumentation().getUiAutomation().dropShellPermissionIdentity(); } @@ -217,16 +217,21 @@ public class ThreadNetworkControllerTest { for (ThreadNetworkController controller : getAllControllers()) { SettableFuture deviceRole = SettableFuture.create(); + StateCallback callback = deviceRole::set; - controller.registerStateCallback(mExecutor, role -> deviceRole.set(role)); + try { + controller.registerStateCallback(mExecutor, callback); - assertThat(deviceRole.get()).isEqualTo(DEVICE_ROLE_STOPPED); + assertThat(deviceRole.get()).isEqualTo(DEVICE_ROLE_STOPPED); + } finally { + controller.unregisterStateCallback(callback); + } } } @Test public void registerStateCallback_noPermissions_throwsSecurityException() throws Exception { - dropPermissions(); + dropAllPermissions(); for (ThreadNetworkController controller : getAllControllers()) { assertThrows( @@ -251,6 +256,26 @@ public class ThreadNetworkControllerTest { } } + @Test + public void unregisterStateCallback_noPermissions_throwsSecurityException() throws Exception { + for (ThreadNetworkController controller : getAllControllers()) { + SettableFuture deviceRole = SettableFuture.create(); + StateCallback callback = role -> deviceRole.set(role); + grantPermissions(permission.ACCESS_NETWORK_STATE); + controller.registerStateCallback(mExecutor, callback); + + try { + dropAllPermissions(); + assertThrows( + SecurityException.class, + () -> controller.unregisterStateCallback(callback)); + } finally { + grantPermissions(permission.ACCESS_NETWORK_STATE); + controller.unregisterStateCallback(callback); + } + } + } + @Test public void unregisterStateCallback_callbackRegistered_success() throws Exception { grantPermissions(permission.ACCESS_NETWORK_STATE); @@ -282,7 +307,7 @@ public class ThreadNetworkControllerTest { grantPermissions(permission.ACCESS_NETWORK_STATE); for (ThreadNetworkController controller : getAllControllers()) { SettableFuture deviceRole = SettableFuture.create(); - StateCallback callback = role -> deviceRole.set(role); + StateCallback callback = deviceRole::set; controller.registerStateCallback(mExecutor, callback); controller.unregisterStateCallback(callback); @@ -300,12 +325,70 @@ public class ThreadNetworkControllerTest { for (ThreadNetworkController controller : getAllControllers()) { SettableFuture activeFuture = SettableFuture.create(); SettableFuture pendingFuture = SettableFuture.create(); + var callback = newDatasetCallback(activeFuture, pendingFuture); - controller.registerOperationalDatasetCallback( - mExecutor, newDatasetCallback(activeFuture, pendingFuture)); + try { + controller.registerOperationalDatasetCallback(mExecutor, callback); - assertThat(activeFuture.get()).isNull(); - assertThat(pendingFuture.get()).isNull(); + assertThat(activeFuture.get()).isNull(); + assertThat(pendingFuture.get()).isNull(); + } finally { + controller.unregisterOperationalDatasetCallback(callback); + } + } + } + + @Test + public void registerOperationalDatasetCallback_noPermissions_throwsSecurityException() + throws Exception { + dropAllPermissions(); + + for (ThreadNetworkController controller : getAllControllers()) { + SettableFuture activeFuture = SettableFuture.create(); + SettableFuture pendingFuture = SettableFuture.create(); + var callback = newDatasetCallback(activeFuture, pendingFuture); + + assertThrows( + SecurityException.class, + () -> controller.registerOperationalDatasetCallback(mExecutor, callback)); + } + } + + @Test + public void unregisterOperationalDatasetCallback_callbackRegistered_success() throws Exception { + grantPermissions(permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED); + for (ThreadNetworkController controller : getAllControllers()) { + SettableFuture activeFuture = SettableFuture.create(); + SettableFuture pendingFuture = SettableFuture.create(); + var callback = newDatasetCallback(activeFuture, pendingFuture); + controller.registerOperationalDatasetCallback(mExecutor, callback); + + controller.unregisterOperationalDatasetCallback(callback); + } + } + + @Test + public void unregisterOperationalDatasetCallback_noPermissions_throwsSecurityException() + throws Exception { + dropAllPermissions(); + + for (ThreadNetworkController controller : getAllControllers()) { + SettableFuture activeFuture = SettableFuture.create(); + SettableFuture pendingFuture = SettableFuture.create(); + var callback = newDatasetCallback(activeFuture, pendingFuture); + grantPermissions(permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED); + controller.registerOperationalDatasetCallback(mExecutor, callback); + + try { + dropAllPermissions(); + assertThrows( + SecurityException.class, + () -> controller.unregisterOperationalDatasetCallback(callback)); + } finally { + grantPermissions( + permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED); + controller.unregisterOperationalDatasetCallback(callback); + } } } @@ -343,7 +426,7 @@ public class ThreadNetworkControllerTest { @Test public void join_withoutPrivilegedPermission_throwsSecurityException() throws Exception { - dropPermissions(); + dropAllPermissions(); for (ThreadNetworkController controller : getAllControllers()) { ActiveOperationalDataset activeDataset = newRandomizedDataset("TestNet", controller); @@ -408,7 +491,7 @@ public class ThreadNetworkControllerTest { @Test public void leave_withoutPrivilegedPermission_throwsSecurityException() { - dropPermissions(); + dropAllPermissions(); for (ThreadNetworkController controller : getAllControllers()) { assertThrows(SecurityException.class, () -> controller.leave(mExecutor, v -> {}));