From 9bedbd28b8c34feeed0ee373fed5ee7d7d3cc1aa Mon Sep 17 00:00:00 2001 From: Alex Johnston Date: Tue, 27 Apr 2021 10:33:48 +0100 Subject: [PATCH] DPM Check for unaffiliated secondary users * Add authorization check in DPMS for API methods to set the private DNS mode and the recommended global proxy. * When there is a unaffiliated user, it is assumed that it is managed separately. * These are device-wide policies that affect all users, so should only be set when it is the same managing entity and all users on the device are affiliated. Bug: 177926343 Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest atest com.android.cts.devicepolicy.DeviceOwnerTest#testProxyPacProxyTest atest com.android.cts.devicepolicy.DeviceOwnerTest#testProxyStaticProxyTest Change-Id: I119a84988ad3cda42b709dbc6d63acf76b0031d6 --- .../app/admin/DevicePolicyManager.java | 14 ++++++ .../DevicePolicyManagerService.java | 2 + .../devicepolicy/DevicePolicyManagerTest.java | 43 +++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index c93a88f9f312c..30607517adaa8 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -5504,6 +5504,10 @@ public class DevicePolicyManager { * This method requires the caller to be the device owner. *

* This proxy is only a recommendation and it is possible that some apps will ignore it. + *

+ * Note: The device owner won't be able to set a global HTTP proxy if there are unaffiliated + * secondary users or profiles on the device. It's recommended that affiliation ids are set for + * new users as soon as possible after provisioning via {@link #setAffiliationIds}. * * @see ProxyInfo * @param admin Which {@link DeviceAdminReceiver} this request is associated with. @@ -12770,6 +12774,11 @@ public class DevicePolicyManager { *

In this mode, the DNS subsystem will attempt a TLS handshake to the network-supplied * resolver prior to attempting name resolution in cleartext. * + *

Note: The device owner won't be able to set the global private DNS mode if there are + * unaffiliated secondary users or profiles on the device. It's recommended that affiliation + * ids are set for new users as soon as possible after provisioning via + * {@link #setAffiliationIds}. + * * @param admin which {@link DeviceAdminReceiver} this request is associated with. * * @return {@code PRIVATE_DNS_SET_NO_ERROR} if the mode was set successfully, or @@ -12805,6 +12814,11 @@ public class DevicePolicyManager { * the ability to resolve hostnames as system traffic to the resolver may not go through the * VPN. * + *

Note: The device owner won't be able to set the global private DNS mode if there are + * unaffiliated secondary users or profiles on the device. It's recommended that affiliation + * ids are set for new users as soon as possible after provisioning via + * {@link #setAffiliationIds}. + * * @param admin which {@link DeviceAdminReceiver} this request is associated with. * @param privateDnsHost The hostname of a server that implements DNS over TLS (RFC7858). * diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 64cdfa65f9c56..8026f1b96e57d 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -7232,6 +7232,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Objects.requireNonNull(who, "ComponentName is null"); final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(isDeviceOwner(caller)); + checkAllUsersAreAffiliatedWithDevice(); mInjector.binderWithCleanCallingIdentity( () -> mInjector.getConnectivityManager().setGlobalProxy(proxyInfo)); } @@ -15709,6 +15710,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Objects.requireNonNull(who, "ComponentName is null"); final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(isDeviceOwner(caller)); + checkAllUsersAreAffiliatedWithDevice(); checkCanExecuteOrThrowUnsafe(DevicePolicyManager.OPERATION_SET_GLOBAL_PRIVATE_DNS); switch (mode) { diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 1b42dfa0712eb..c54dffc3c431c 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -30,6 +30,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_HIGH; import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_LOW; import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_MEDIUM; import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_NONE; +import static android.app.admin.DevicePolicyManager.PRIVATE_DNS_SET_NO_ERROR; import static android.app.admin.DevicePolicyManager.WIPE_EUICC; import static android.app.admin.PasswordMetrics.computeForPasswordOrPin; import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_DIRECT_BOOT_AWARE; @@ -7333,6 +7334,48 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertThat(dpm.getPolicyExemptApps()).containsExactly("4", "8", "15", "16", "23", "42"); } + @Test + public void testSetGlobalPrivateDnsModeOpportunistic_asDeviceOwner() throws Exception { + setDeviceOwner(); + // setUp() adds a secondary user for CALLER_USER_HANDLE. Remove it as otherwise the + // feature is disabled because there are non-affiliated secondary users. + getServices().removeUser(CALLER_USER_HANDLE); + clearInvocations(getServices().settings); + + int result = dpm.setGlobalPrivateDnsModeOpportunistic(admin1); + + assertThat(result).isEqualTo(PRIVATE_DNS_SET_NO_ERROR); + } + + @Test + public void testSetGlobalPrivateDnsModeOpportunistic_hasUnaffiliatedUsers() throws Exception { + setDeviceOwner(); + setAsProfileOwner(admin2); + + assertThrows(SecurityException.class, + () -> dpm.setGlobalPrivateDnsModeOpportunistic(admin1)); + } + + @Test + public void testSetRecommendedGlobalProxy_asDeviceOwner() throws Exception { + setDeviceOwner(); + // setUp() adds a secondary user for CALLER_USER_HANDLE. Remove it as otherwise the + // feature is disabled because there are non-affiliated secondary users. + getServices().removeUser(CALLER_USER_HANDLE); + + dpm.setRecommendedGlobalProxy(admin1, null); + + verify(getServices().connectivityManager).setGlobalProxy(null); + } + + @Test + public void testSetRecommendedGlobalProxy_hasUnaffiliatedUsers() throws Exception { + setDeviceOwner(); + setAsProfileOwner(admin2); + + assertThrows(SecurityException.class, () -> dpm.setRecommendedGlobalProxy(admin1, null)); + } + private void setUserUnlocked(int userHandle, boolean unlocked) { when(getServices().userManager.isUserUnlocked(eq(userHandle))).thenReturn(unlocked); }