Merge "Fix vulnerability in AttributionSource due to incorrect Binder call" into sc-dev
diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java
index bccc52f..fd73e8c 100644
--- a/services/core/java/com/android/server/notification/ManagedServices.java
+++ b/services/core/java/com/android/server/notification/ManagedServices.java
@@ -155,7 +155,9 @@
// List of approved packages or components (by user, then by primary/secondary) that are
// allowed to be bound as managed services. A package or component appearing in this list does
// not mean that we are currently bound to said package/component.
- protected ArrayMap<Integer, ArrayMap<Boolean, ArraySet<String>>> mApproved = new ArrayMap<>();
+ @GuardedBy("mApproved")
+ protected final ArrayMap<Integer, ArrayMap<Boolean, ArraySet<String>>> mApproved =
+ new ArrayMap<>();
// List of packages or components (by user) that are configured to be enabled/disabled
// explicitly by the user
@@ -871,6 +873,23 @@
return false;
}
+ protected boolean isPackageOrComponentAllowedWithPermission(ComponentName component,
+ int userId) {
+ if (!(isPackageOrComponentAllowed(component.flattenToString(), userId)
+ || isPackageOrComponentAllowed(component.getPackageName(), userId))) {
+ return false;
+ }
+ return componentHasBindPermission(component, userId);
+ }
+
+ private boolean componentHasBindPermission(ComponentName component, int userId) {
+ ServiceInfo info = getServiceInfo(component, userId);
+ if (info == null) {
+ return false;
+ }
+ return mConfig.bindPermission.equals(info.permission);
+ }
+
boolean isPackageOrComponentUserSet(String pkgOrComponent, int userId) {
synchronized (mApproved) {
ArraySet<String> services = mUserSetServices.get(userId);
@@ -928,6 +947,7 @@
for (int uid : uidList) {
if (isPackageAllowed(pkgName, UserHandle.getUserId(uid))) {
anyServicesInvolved = true;
+ trimApprovedListsForInvalidServices(pkgName, UserHandle.getUserId(uid));
}
}
}
@@ -1057,8 +1077,7 @@
for (int i = 0; i < userIds.size(); i++) {
final int userId = userIds.get(i);
if (enabled) {
- if (isPackageOrComponentAllowed(component.flattenToString(), userId)
- || isPackageOrComponentAllowed(component.getPackageName(), userId)) {
+ if (isPackageOrComponentAllowedWithPermission(component, userId)) {
registerServiceLocked(component, userId);
} else {
Slog.d(TAG, component + " no longer has permission to be bound");
@@ -1197,6 +1216,33 @@
return removed;
}
+ private void trimApprovedListsForInvalidServices(String packageName, int userId) {
+ synchronized (mApproved) {
+ final ArrayMap<Boolean, ArraySet<String>> approvedByType = mApproved.get(userId);
+ if (approvedByType == null) {
+ return;
+ }
+ for (int i = 0; i < approvedByType.size(); i++) {
+ final ArraySet<String> approved = approvedByType.valueAt(i);
+ for (int j = approved.size() - 1; j >= 0; j--) {
+ final String approvedPackageOrComponent = approved.valueAt(j);
+ if (TextUtils.equals(getPackageName(approvedPackageOrComponent), packageName)) {
+ final ComponentName component = ComponentName.unflattenFromString(
+ approvedPackageOrComponent);
+ if (component != null && !componentHasBindPermission(component, userId)) {
+ approved.removeAt(j);
+ if (DEBUG) {
+ Slog.v(TAG, "Removing " + approvedPackageOrComponent
+ + " from approved list; no bind permission found "
+ + mConfig.bindPermission);
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
protected String getPackageName(String packageOrComponent) {
final ComponentName component = ComponentName.unflattenFromString(packageOrComponent);
if (component != null) {
@@ -1380,28 +1426,20 @@
final int userId = componentsToBind.keyAt(i);
final Set<ComponentName> add = componentsToBind.get(userId);
for (ComponentName component : add) {
- try {
- ServiceInfo info = mPm.getServiceInfo(component,
- PackageManager.GET_META_DATA
- | PackageManager.MATCH_DIRECT_BOOT_AWARE
- | PackageManager.MATCH_DIRECT_BOOT_UNAWARE,
- userId);
- if (info == null) {
- Slog.w(TAG, "Not binding " + getCaption() + " service " + component
- + ": service not found");
- continue;
- }
- if (!mConfig.bindPermission.equals(info.permission)) {
- Slog.w(TAG, "Not binding " + getCaption() + " service " + component
- + ": it does not require the permission " + mConfig.bindPermission);
- continue;
- }
- Slog.v(TAG,
- "enabling " + getCaption() + " for " + userId + ": " + component);
- registerService(info, userId);
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
+ ServiceInfo info = getServiceInfo(component, userId);
+ if (info == null) {
+ Slog.w(TAG, "Not binding " + getCaption() + " service " + component
+ + ": service not found");
+ continue;
}
+ if (!mConfig.bindPermission.equals(info.permission)) {
+ Slog.w(TAG, "Not binding " + getCaption() + " service " + component
+ + ": it does not require the permission " + mConfig.bindPermission);
+ continue;
+ }
+ Slog.v(TAG,
+ "enabling " + getCaption() + " for " + userId + ": " + component);
+ registerService(info, userId);
}
}
}
@@ -1422,6 +1460,15 @@
}
}
+ @VisibleForTesting
+ void reregisterService(final ComponentName cn, final int userId) {
+ // If rebinding a package that died, ensure it still has permission
+ // after the rebind delay
+ if (isPackageOrComponentAllowedWithPermission(cn, userId)) {
+ registerService(cn, userId);
+ }
+ }
+
/**
* Inject a system service into the management list.
*/
@@ -1523,7 +1570,7 @@
mHandler.postDelayed(new Runnable() {
@Override
public void run() {
- registerService(name, userid);
+ reregisterService(name, userid);
}
}, ON_BINDING_DIED_REBIND_DELAY_MS);
} else {
@@ -1655,6 +1702,19 @@
}
}
+ private ServiceInfo getServiceInfo(ComponentName component, int userId) {
+ try {
+ return mPm.getServiceInfo(component,
+ PackageManager.GET_META_DATA
+ | PackageManager.MATCH_DIRECT_BOOT_AWARE
+ | PackageManager.MATCH_DIRECT_BOOT_UNAWARE,
+ userId);
+ } catch (RemoteException e) {
+ e.rethrowFromSystemServer();
+ }
+ return null;
+ }
+
public class ManagedServiceInfo implements IBinder.DeathRecipient {
public IInterface service;
public ComponentName component;
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
index dd8b96e..31babe0 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -691,6 +691,7 @@
// TODO: switch this back to SecurityException
Slog.wtf(TAG, "Not allowed to modify non-dynamic permission "
+ permName);
+ return;
}
mRegistry.removePermission(permName);
}
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index 6c3d543..8c4f104 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -3239,8 +3239,10 @@
return false;
}
if (doAnimation) {
- mWinAnimator.applyAnimationLocked(TRANSIT_EXIT, false);
- if (!isAnimating(TRANSITION | PARENTS)) {
+ // If a hide animation is applied, then let onAnimationFinished
+ // -> checkPolicyVisibilityChange hide the window. Otherwise make doAnimation false
+ // to commit invisible immediately.
+ if (!mWinAnimator.applyAnimationLocked(TRANSIT_EXIT, false /* isEntrance */)) {
doAnimation = false;
}
}
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java
index f9663f2..f818725 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java
@@ -27,8 +27,10 @@
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -956,6 +958,58 @@
}
@Test
+ public void testUpgradeAppNoPermissionNoRebind() throws Exception {
+ Context context = spy(getContext());
+ doReturn(true).when(context).bindServiceAsUser(any(), any(), anyInt(), any());
+
+ ManagedServices service = new TestManagedServices(context, mLock, mUserProfiles,
+ mIpm,
+ APPROVAL_BY_COMPONENT);
+
+ List<String> packages = new ArrayList<>();
+ packages.add("package");
+ addExpectedServices(service, packages, 0);
+
+ final ComponentName unapprovedComponent = ComponentName.unflattenFromString("package/C1");
+ final ComponentName approvedComponent = ComponentName.unflattenFromString("package/C2");
+
+ // Both components are approved initially
+ mExpectedPrimaryComponentNames.clear();
+ mExpectedPrimaryPackages.clear();
+ mExpectedPrimaryComponentNames.put(0, "package/C1:package/C2");
+ mExpectedSecondaryComponentNames.clear();
+ mExpectedSecondaryPackages.clear();
+
+ loadXml(service);
+
+ //Component package/C1 loses bind permission
+ when(mIpm.getServiceInfo(any(), anyInt(), anyInt())).thenAnswer(
+ (Answer<ServiceInfo>) invocation -> {
+ ComponentName invocationCn = invocation.getArgument(0);
+ if (invocationCn != null) {
+ ServiceInfo serviceInfo = new ServiceInfo();
+ serviceInfo.packageName = invocationCn.getPackageName();
+ serviceInfo.name = invocationCn.getClassName();
+ if (invocationCn.equals(unapprovedComponent)) {
+ serviceInfo.permission = "none";
+ } else {
+ serviceInfo.permission = service.getConfig().bindPermission;
+ }
+ serviceInfo.metaData = null;
+ return serviceInfo;
+ }
+ return null;
+ }
+ );
+
+ // Trigger package update
+ service.onPackagesChanged(false, new String[]{"package"}, new int[]{0});
+
+ assertFalse(service.isComponentEnabledForCurrentProfiles(unapprovedComponent));
+ assertTrue(service.isComponentEnabledForCurrentProfiles(approvedComponent));
+ }
+
+ @Test
public void testSetPackageOrComponentEnabled() throws Exception {
for (int approvalLevel : new int[] {APPROVAL_BY_COMPONENT, APPROVAL_BY_PACKAGE}) {
ManagedServices service = new TestManagedServices(getContext(), mLock, mUserProfiles,
diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java
index d88ac25..6fcdcde 100644
--- a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java
@@ -215,6 +215,26 @@
assertTrue(window.isOnScreen());
window.hide(false /* doAnimation */, false /* requestAnim */);
assertFalse(window.isOnScreen());
+
+ // Verifies that a window without animation can be hidden even if its parent is animating.
+ window.show(false /* doAnimation */, false /* requestAnim */);
+ assertTrue(window.isVisibleByPolicy());
+ window.getParent().startAnimation(mTransaction, mock(AnimationAdapter.class),
+ false /* hidden */, SurfaceAnimator.ANIMATION_TYPE_WINDOW_ANIMATION);
+ window.mAttrs.windowAnimations = 0;
+ window.hide(true /* doAnimation */, true /* requestAnim */);
+ assertFalse(window.isSelfAnimating(0, SurfaceAnimator.ANIMATION_TYPE_WINDOW_ANIMATION));
+ assertFalse(window.isVisibleByPolicy());
+ assertFalse(window.isOnScreen());
+
+ // Verifies that a window with animation can be hidden after the hide animation is finished.
+ window.show(false /* doAnimation */, false /* requestAnim */);
+ window.mAttrs.windowAnimations = android.R.style.Animation_Dialog;
+ window.hide(true /* doAnimation */, true /* requestAnim */);
+ assertTrue(window.isSelfAnimating(0, SurfaceAnimator.ANIMATION_TYPE_WINDOW_ANIMATION));
+ assertTrue(window.isVisibleByPolicy());
+ window.cancelAnimation();
+ assertFalse(window.isVisibleByPolicy());
}
@Test
diff --git a/tests/BootImageProfileTest/AndroidTest.xml b/tests/BootImageProfileTest/AndroidTest.xml
index 7e97fa3..50f323c 100644
--- a/tests/BootImageProfileTest/AndroidTest.xml
+++ b/tests/BootImageProfileTest/AndroidTest.xml
@@ -14,6 +14,7 @@
limitations under the License.
-->
<configuration description="Config for BootImageProfileTest">
+ <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer" />
<!-- do not use DeviceSetup#set-property because it reboots the device b/136200738.
furthermore the changes in /data/local.prop don't actually seem to get picked up.
-->