From 1f04060be06545c86c0d377b8dd81e6f4cbb1c9f Mon Sep 17 00:00:00 2001
From: Adam Bookatz <bookatz@google.com>
Date: Thu, 14 Jan 2021 13:50:29 -0800
Subject: [PATCH] UserSystemPackageInstaller auto-treats static overlays

The UserSystemPackageInstaller installs packages on new users
based on their configuration in an allowlist. An exception was
made for auto-generated RROs, which were instead installed based
on their overlay target package's configuration (thus avoiding
the need for manually allowlisting each such overlay).

This is now expanded to all static overlays (which should
include auto-generated RROs, according to the bug). That is:
Static overlays no longer need to be mentioned in the list,
but will instead be installed based on the listing for their
overlay target package.

Test: atest UserSystemPackageInstallerTest
Fixes: 172956245
Change-Id: I6b974bba20ee059a8f744092db7c3441580d327f
---
 data/etc/preinstalled-packages-platform.xml   |  5 +-
 .../server/pm/UserSystemPackageInstaller.java | 47 ++++++-------------
 .../pm/UserSystemPackageInstallerTest.java    | 46 +++++++++---------
 3 files changed, 38 insertions(+), 60 deletions(-)

diff --git a/data/etc/preinstalled-packages-platform.xml b/data/etc/preinstalled-packages-platform.xml
index 625558406b9c0..ff8d96dd23f27 100644
--- a/data/etc/preinstalled-packages-platform.xml
+++ b/data/etc/preinstalled-packages-platform.xml
@@ -17,9 +17,8 @@
 <!--
 This XML file declares which system packages should be initially installed for new users based on
 their user type. All system packages on the device should ideally have an entry in an xml file
-(keyed by its manifest name), except auto-generated rro packages. Auto-generated RRO packages 
-(package name ends with ".auto_generated_rro_product__" or ".auto_generated_rro_vendor__")
-will be installed for new users according to corresponding overlay target packages.
+(keyed by its manifest name), except for static overlays which are instead treated automatically
+according to the entry for their corresponding overlay target package.
 
 Base user-types (every user will be at least one of these types) are:
   SYSTEM    (user 0)
diff --git a/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java b/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java
index 252ba6061b7a4..6e6d7c3f4fabc 100644
--- a/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java
+++ b/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java
@@ -327,17 +327,17 @@ class UserSystemPackageInstaller {
         final PackageManagerInternal pmInt = LocalServices.getService(PackageManagerInternal.class);
 
         // Check whether all allowlisted packages are indeed on the system.
-        final String notPresentFmt = "%s is whitelisted but not present.";
-        final String notSystemFmt = "%s is whitelisted and present but not a system package.";
-        final String overlayPackageFmt = "%s is whitelisted but it's auto-generated RRO package.";
+        final String notPresentFmt = "%s is allowlisted but not present.";
+        final String notSystemFmt = "%s is allowlisted and present but not a system package.";
+        final String overlayFmt = "%s is allowlisted unnecessarily since it's a static overlay.";
         for (String pkgName : allWhitelistedPackages) {
             final AndroidPackage pkg = pmInt.getPackage(pkgName);
             if (pkg == null) {
                 warnings.add(String.format(notPresentFmt, pkgName));
             } else if (!pkg.isSystem()) {
                 warnings.add(String.format(notSystemFmt, pkgName));
-            } else if (isAutoGeneratedRRO(pkg)) {
-                warnings.add(String.format(overlayPackageFmt, pkgName));
+            } else if (shouldUseOverlayTargetName(pkg)) {
+                warnings.add(String.format(overlayFmt, pkgName));
             }
         }
         return warnings;
@@ -363,7 +363,7 @@ class UserSystemPackageInstaller {
             if (!pkg.isSystem()) return;
             final String pkgName = pkg.getManifestPackageName();
             if (!allWhitelistedPackages.contains(pkgName)
-                    && !isAutoGeneratedRRO(pmInt.getPackage(pkgName))) {
+                    && !shouldUseOverlayTargetName(pmInt.getPackage(pkgName))) {
                 errors.add(String.format(logMessageFmt, pkgName));
             }
         });
@@ -413,22 +413,13 @@ class UserSystemPackageInstaller {
     }
 
     /**
-     * Whether package name has auto-generated RRO package name suffix.
+     * Returns whether the package is a static overlay, whose installation should depend on the
+     * allowlisting of the overlay's target's package name, rather than of its own package name.
+     *
+     * @param pkg A package (which need not be an overlay)
      */
-    @VisibleForTesting
-    static boolean hasAutoGeneratedRROSuffix(String name) {
-        return name.endsWith(".auto_generated_rro_product__")
-                // TODO(b/172956245): temporary workaround until OEMs can customize name
-                || name.endsWith("carui.rro") || name.endsWith("carui.overlayable.rro")
-                || name.endsWith(".auto_generated_rro_vendor__");
-    }
-
-    /**
-     * Whether the package is auto-generated RRO package.
-     */
-    private static boolean isAutoGeneratedRRO(AndroidPackage pkg) {
-        return pkg.isOverlay()
-                && (hasAutoGeneratedRROSuffix(pkg.getManifestPackageName()));
+    private static boolean shouldUseOverlayTargetName(AndroidPackage pkg) {
+        return pkg.isOverlayIsStatic();
     }
 
     /** See {@link #isEnforceMode()}. */
@@ -542,18 +533,8 @@ class UserSystemPackageInstaller {
     static boolean shouldInstallPackage(AndroidPackage sysPkg,
             @NonNull ArrayMap<String, Long> userTypeWhitelist,
             @NonNull Set<String> userWhitelist, boolean implicitlyWhitelist) {
-        final String pkgName;
-        if (isAutoGeneratedRRO(sysPkg)) {
-            pkgName = sysPkg.getOverlayTarget();
-            if (DEBUG) {
-                Slog.i(TAG, "shouldInstallPackage(): " + sysPkg.getManifestPackageName()
-                        + " is auto-generated RRO package, will look for overlay system package: "
-                        + pkgName);
-            }
-        } else {
-            pkgName = sysPkg.getManifestPackageName();
-        }
-
+        final String pkgName = shouldUseOverlayTargetName(sysPkg) ?
+                sysPkg.getOverlayTarget() : sysPkg.getManifestPackageName();
         return (implicitlyWhitelist && !userTypeWhitelist.containsKey(pkgName))
                 || userWhitelist.contains(pkgName);
     }
diff --git a/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java
index 0b44c59a3e15f..b11bb85e82783 100644
--- a/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java
@@ -366,36 +366,19 @@ public class UserSystemPackageInstallerTest {
             actualPackages.add(p.packageName);
         }
 
-        // Add auto-generated RRO package to expectedPackages since they are not (supposed to be)
-        // in the allowlist but they should be installed.
+        // Add static overlays to expectedPackages since they are not (supposed to be)
+        // in the allowlist but they should be installed if their target is.
         for (PackageInfo p : packageInfos) {
-            if (p.isOverlayPackage()
-                        && UserSystemPackageInstaller.hasAutoGeneratedRROSuffix(p.packageName)
-                        && expectedPackages.contains(p.overlayTarget)) {
+            if (p.isStaticOverlayPackage() && expectedPackages.contains(p.overlayTarget)) {
                 expectedPackages.add(p.packageName);
             }
         }
         checkPackageDifferences(expectedPackages, actualPackages);
     }
 
-    @Test
-    public void testAutoGeneratedRROMatchesSuffix() {
-        final List<PackageInfo> packageInfos = mContext.getPackageManager()
-                .getInstalledPackages(PackageManager.GET_UNINSTALLED_PACKAGES);
-
-        Log.v(TAG, "Found total packages: " + packageInfos.size());
-
-        for (PackageInfo p : packageInfos) {
-            if (p.packageName.contains(".auto_generated_rro_")) {
-                assertTrue("Auto-generated RRO package name does not match the suffix: "
-                        + p.packageName,
-                        UserSystemPackageInstaller.hasAutoGeneratedRROSuffix(p.packageName));
-            }
-        }
-    }
-
     /**
-     * Test that overlay package not in allowlist should be installed for all user at Explicit mode.
+     * Test that static overlay package not in allowlist should be installed for all users
+     * based on their targets, in Explicit mode.
      */
     @Test
     public void testInstallOverlayPackagesExplicitMode() {
@@ -408,19 +391,32 @@ public class UserSystemPackageInstallerTest {
         final String packageName2 = "nonWhitelistedPkg";
         final String overlayName1 = String.format("%s.auto_generated_rro_product__", packageName1);
         final String overlayName2 = String.format("%s.auto_generated_rro_product__", packageName2);
+        final String overlayName3 = String.format("%s.non_static_overlay", packageName1);
 
+        // Static overlay for allowlisted package1 -> should be installed, like package1
         final AndroidPackage overlayPackage1 = ((ParsedPackage) PackageImpl.forTesting(overlayName1)
                 .setOverlay(true)
+                .setOverlayIsStatic(true)
                 .setOverlayTarget(packageName1)
                 .hideAsParsed())
                 .hideAsFinal();
 
+        // Static overlay for non-allowlisted package2 -> should not be installed, like package2
         final AndroidPackage overlayPackage2 = ((ParsedPackage) PackageImpl.forTesting(overlayName2)
                 .setOverlay(true)
+                .setOverlayIsStatic(true)
                 .setOverlayTarget(packageName2)
                 .hideAsParsed())
                 .hideAsFinal();
 
+        // Non-static overlay for package1 -> not explicitly allowlisted, so shouldn't be installed
+        final AndroidPackage overlayPackage3 = ((ParsedPackage) PackageImpl.forTesting(overlayName3)
+                .setOverlay(true)
+                .setOverlayIsStatic(false) // non-static
+                .setOverlayTarget(packageName1)
+                .hideAsParsed())
+                .hideAsFinal();
+
         final ArrayMap<String, Long> userTypeWhitelist = new ArrayMap<>();
         userTypeWhitelist.put(packageName1, maskOfType);
 
@@ -428,10 +424,12 @@ public class UserSystemPackageInstallerTest {
         userWhitelist.add(packageName1);
 
         boolean implicit = false;
-        assertTrue("Overlay for package1 should be installed", UserSystemPackageInstaller
+        assertTrue("Should install static overlay for package1", UserSystemPackageInstaller
                 .shouldInstallPackage(overlayPackage1, userTypeWhitelist, userWhitelist, implicit));
-        assertFalse("Overlay for package2 should not be installed", UserSystemPackageInstaller
+        assertFalse("Should not install static overlay for package2", UserSystemPackageInstaller
                 .shouldInstallPackage(overlayPackage2, userTypeWhitelist, userWhitelist, implicit));
+        assertFalse("Should not install regular overlay for package1", UserSystemPackageInstaller
+                .shouldInstallPackage(overlayPackage3, userTypeWhitelist, userWhitelist, implicit));
     }
 
     /** Asserts that actual is a subset of expected. */