Merge "Fix runtime permissinos toggling and relax XML parsing."

4 files changed, 161 insertions, 165 deletions

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 1b8ed22..c7dc74f 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -1349,7 +1349,7 @@ public class PackageManagerService extends IPackageManager.Stub {
         mOnlyCore = onlyCore;
         mLazyDexOpt = "eng".equals(SystemProperties.get("ro.build.type"));
         mMetrics = new DisplayMetrics();
-        mSettings = new Settings(mContext, mPackages);
+        mSettings = new Settings(mPackages);
         mSettings.addSharedUserLPw("android.uid.system", Process.SYSTEM_UID,
                 ApplicationInfo.FLAG_SYSTEM, ApplicationInfo.PRIVATE_FLAG_PRIVILEGED);
         mSettings.addSharedUserLPw("android.uid.phone", RADIO_UID,
@@ -1812,23 +1812,11 @@ public class PackageManagerService extends IPackageManager.Stub {
                     + "; regranting permissions for internal storage");
             mSettings.mInternalSdkPlatform = mSdkVersion;
 
-
-            // We keep track for which users we granted permissions to be able
-            // to grant runtime permissions to system apps for newly appeared
-            // users. If we supported runtime permissions during the previous
-            // boot, then we already granted permissions for all device users.
-            // In such a case we set the users for which we granted permissions
-            // to avoid clobbering of runtime permissions we granted to system
-            // apps but the user revoked later.
-            if (PackageManagerService.RUNTIME_PERMISSIONS_ENABLED &&
-                    mSettings.mRuntimePermissionEnabled) {
-                final int[] userIds = UserManagerService.getInstance().getUserIds();
-                for (PackageSetting ps : mSettings.mPackages.values()) {
-                    ps.setPermissionsUpdatedForUserIds(userIds);
-                }
-                for (SharedUserSetting sus : mSettings.mSharedUsers.values()) {
-                    sus.setPermissionsUpdatedForUserIds(userIds);
-                }
+            // For now runtime permissions are toggled via a system property.
+            if (!RUNTIME_PERMISSIONS_ENABLED) {
+                // Remove the runtime permissions state if the feature
+                // was disabled by flipping the system property.
+                mSettings.deleteRuntimePermissionsFiles();
             }
 
             updatePermissionsLPw(null, null, UPDATE_PERMISSIONS_ALL
@@ -2714,6 +2702,10 @@ public class PackageManagerService extends IPackageManager.Stub {
 
     @Override
     public boolean grantPermission(String packageName, String name, int userId) {
+        if (!RUNTIME_PERMISSIONS_ENABLED) {
+            return false;
+        }
+
         if (!sUserManager.exists(userId)) {
             return false;
         }
@@ -2725,6 +2717,9 @@ public class PackageManagerService extends IPackageManager.Stub {
         enforceCrossUserPermission(Binder.getCallingUid(), userId, true, false,
                 "grantPermission");
 
+        boolean gidsChanged = false;
+        final SettingBase sb;
+
         synchronized (mPackages) {
             final PackageParser.Package pkg = mPackages.get(packageName);
             if (pkg == null) {
@@ -2738,7 +2733,7 @@ public class PackageManagerService extends IPackageManager.Stub {
 
             enforceDeclaredAsUsedAndRuntimePermission(pkg, bp);
 
-            final SettingBase sb = (SettingBase) pkg.mExtras;
+            sb = (SettingBase) pkg.mExtras;
             if (sb == null) {
                 throw new IllegalArgumentException("Unknown package: " + packageName);
             }
@@ -2752,19 +2747,27 @@ public class PackageManagerService extends IPackageManager.Stub {
                 }
 
                 case PermissionsState.PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED: {
-                    killSettingPackagesForUser(sb, userId, KILL_APP_REASON_GIDS_CHANGED);
+                    gidsChanged = true;
                 } break;
             }
 
             // Not critical if that is lost - app has to request again.
             mSettings.writeRuntimePermissionsForUserLPr(userId, false);
+        }
 
-            return true;
+        if (gidsChanged) {
+            killSettingPackagesForUser(sb, userId, KILL_APP_REASON_GIDS_CHANGED);
         }
+
+        return true;
     }
 
     @Override
     public boolean revokePermission(String packageName, String name, int userId) {
+        if (!RUNTIME_PERMISSIONS_ENABLED) {
+            return false;
+        }
+
         if (!sUserManager.exists(userId)) {
             return false;
         }
@@ -2776,6 +2779,8 @@ public class PackageManagerService extends IPackageManager.Stub {
         enforceCrossUserPermission(Binder.getCallingUid(), userId, true, false,
                 "revokePermission");
 
+        final SettingBase sb;
+
         synchronized (mPackages) {
             final PackageParser.Package pkg = mPackages.get(packageName);
             if (pkg == null) {
@@ -2789,7 +2794,7 @@ public class PackageManagerService extends IPackageManager.Stub {
 
             enforceDeclaredAsUsedAndRuntimePermission(pkg, bp);
 
-            final SettingBase sb = (SettingBase) pkg.mExtras;
+            sb = (SettingBase) pkg.mExtras;
             if (sb == null) {
                 throw new IllegalArgumentException("Unknown package: " + packageName);
             }
@@ -2801,13 +2806,13 @@ public class PackageManagerService extends IPackageManager.Stub {
                 return false;
             }
 
-            killSettingPackagesForUser(sb, userId, KILL_APP_REASON_PERMISSIONS_REVOKED);
-
             // Critical, after this call all should never have the permission.
             mSettings.writeRuntimePermissionsForUserLPr(userId, true);
-
-            return true;
         }
+
+        killSettingPackagesForUser(sb, userId, KILL_APP_REASON_PERMISSIONS_REVOKED);
+
+        return true;
     }
 
     @Override
@@ -7067,7 +7072,7 @@ public class PackageManagerService extends IPackageManager.Stub {
                                     <= Build.VERSION_CODES.LOLLIPOP_MR1) {
                         // For legacy apps dangerous permissions are install time ones.
                         grant = GRANT_INSTALL;
-                    } else if ((pkg.applicationInfo.flags & ApplicationInfo.FLAG_SYSTEM) != 0) {
+                    } else if (ps.isSystem()) {
                         final int[] updatedUserIds = ps.getPermissionsUpdatedForUserIds();
                         if (origPermissions.hasInstallPermission(bp.name)) {
                             // If a system app had an install permission, then the app was
@@ -7200,11 +7205,13 @@ public class PackageManagerService extends IPackageManager.Stub {
             ps.installPermissionsFixed = true;
         }
 
-        ps.setPermissionsUpdatedForUserIds(changedRuntimePermissionUserIds);
+        ps.setPermissionsUpdatedForUserIds(currentUserIds);
 
         // Persist the runtime permissions state for users with changes.
-        for (int userId : changedRuntimePermissionUserIds) {
-           mSettings.writeRuntimePermissionsForUserLPr(userId, true);
+        if (RUNTIME_PERMISSIONS_ENABLED) {
+            for (int userId : changedRuntimePermissionUserIds) {
+                mSettings.writeRuntimePermissionsForUserLPr(userId, true);
+            }
         }
     }
 
@@ -11043,15 +11050,18 @@ public class PackageManagerService extends IPackageManager.Stub {
                         for (int userId : UserManagerService.getInstance().getUserIds()) {
                             final int userIdToKill = mSettings.updateSharedUserPermsLPw(deletedPs,
                                     userId);
-                            if (userIdToKill == userId) {
+                            if (userIdToKill == UserHandle.USER_ALL
+                                    || userIdToKill >= UserHandle.USER_OWNER) {
                                 // If gids changed for this user, kill all affected packages.
-                                killSettingPackagesForUser(deletedPs, userIdToKill,
-                                        KILL_APP_REASON_GIDS_CHANGED);
-                            } else if (userIdToKill == UserHandle.USER_ALL) {
-                                // If gids changed for all users, kill them all - done.
-                                killSettingPackagesForUser(deletedPs, userIdToKill,
-                                        KILL_APP_REASON_GIDS_CHANGED);
-                                break;
+                                mHandler.post(new Runnable() {
+                                    @Override
+                                    public void run() {
+                                        // This has to happen with no lock held.
+                                        killSettingPackagesForUser(deletedPs, userIdToKill,
+                                                KILL_APP_REASON_GIDS_CHANGED);
+                                    }
+                                });
+                            break;
                             }
                         }
                     }
diff --git a/services/core/java/com/android/server/pm/PackageSetting.java b/services/core/java/com/android/server/pm/PackageSetting.java
index 889164c..a3f4c0b 100644
--- a/services/core/java/com/android/server/pm/PackageSetting.java
+++ b/services/core/java/com/android/server/pm/PackageSetting.java
@@ -70,4 +70,8 @@ final class PackageSetting extends PackageSettingBase {
     public boolean isForwardLocked() {
         return (pkgPrivateFlags & ApplicationInfo.PRIVATE_FLAG_FORWARD_LOCK) != 0;
     }
+
+    public boolean isSystem() {
+        return (pkgFlags & ApplicationInfo.FLAG_SYSTEM) != 0;
+    }
 }
diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java
index 0a2389f..8f185ec 100644
--- a/services/core/java/com/android/server/pm/Settings.java
+++ b/services/core/java/com/android/server/pm/Settings.java
@@ -25,7 +25,6 @@ import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
 import static android.os.Process.SYSTEM_UID;
 import static android.os.Process.PACKAGE_INFO_GID;
 
-import android.content.Context;
 import android.content.IntentFilter;
 import android.content.pm.ActivityInfo;
 import android.content.pm.ResolveInfo;
@@ -175,10 +174,8 @@ final class Settings {
     private static final String ATTR_HIDDEN = "hidden";
     private static final String ATTR_INSTALLED = "inst";
     private static final String ATTR_BLOCK_UNINSTALL = "blockUninstall";
-    private static final String ATTR_RUNTIME_PERMSISSIONS_ENABLED = "runtime-permissions-enabled";
 
     private final Object mLock;
-    private final Context mContext;
 
     private final RuntimePermissionPersistence mRuntimePermissionsPersistence;
 
@@ -202,10 +199,6 @@ final class Settings {
     int mInternalSdkPlatform;
     int mExternalSdkPlatform;
 
-
-    // Whether runtime permissions are enabled.
-    boolean mRuntimePermissionEnabled;
-
     /**
      * The current database version for apps on internal storage. This is
      * used to upgrade the format of the packages.xml database not necessarily
@@ -282,12 +275,11 @@ final class Settings {
 
     public final KeySetManagerService mKeySetManagerService = new KeySetManagerService(mPackages);
 
-    Settings(Context context, Object lock) {
-        this(context, Environment.getDataDirectory(), lock);
+    Settings(Object lock) {
+        this(Environment.getDataDirectory(), lock);
     }
 
-    Settings(Context context, File dataDir, Object lock) {
-        mContext = context;
+    Settings(File dataDir, Object lock) {
         mLock = lock;
 
         mRuntimePermissionsPersistence = new RuntimePermissionPersistence(mLock);
@@ -953,6 +945,17 @@ final class Settings {
         return new File(userDir, RUNTIME_PERMISSIONS_FILE_NAME);
     }
 
+    boolean isFirstRuntimePermissionsBoot() {
+        return !getUserRuntimePermissionsFile(UserHandle.USER_OWNER).exists();
+    }
+
+    void deleteRuntimePermissionsFiles() {
+        for (int userId : UserManagerService.getInstance().getUserIds()) {
+            File file = getUserRuntimePermissionsFile(userId);
+            file.delete();
+        }
+    }
+
     private File getUserPackagesStateBackupFile(int userId) {
         return new File(Environment.getUserSystemDirectory(userId),
                 "package-restrictions-backup.xml");
@@ -1650,8 +1653,6 @@ final class Settings {
             serializer.attribute(null, "internal", Integer.toString(mInternalSdkPlatform));
             serializer.attribute(null, "external", Integer.toString(mExternalSdkPlatform));
             serializer.attribute(null, "fingerprint", mFingerprint);
-            serializer.attribute(null, ATTR_RUNTIME_PERMSISSIONS_ENABLED,
-                    String.valueOf(PackageManagerService.RUNTIME_PERMISSIONS_ENABLED));
             serializer.endTag(null, "last-platform-version");
 
             serializer.startTag(null, "database-version");
@@ -2148,8 +2149,6 @@ final class Settings {
                     } catch (NumberFormatException e) {
                     }
                     mFingerprint = parser.getAttributeValue(null, "fingerprint");
-                    mRuntimePermissionEnabled = XmlUtils.readBooleanAttribute(parser,
-                            ATTR_RUNTIME_PERMSISSIONS_ENABLED);
                 } else if (tagName.equals("database-version")) {
                     mInternalDatabaseVersion = mExternalDatabaseVersion = 0;
                     try {
@@ -2739,6 +2738,18 @@ final class Settings {
             }
         }
 
+        // We keep track for which users we granted permissions to be able
+        // to grant runtime permissions to system apps for newly appeared
+        // users or newly appeared system apps. If we supported runtime
+        // permissions during the previous boot, then we already granted
+        // permissions for all device users. In such a case we set the users
+        // for which we granted permissions to avoid clobbering of runtime
+        // permissions we granted to system apps but the user revoked later.
+        if (!isFirstRuntimePermissionsBoot()) {
+            final int[] userIds = UserManagerService.getInstance().getUserIds();
+            ps.setPermissionsUpdatedForUserIds(userIds);
+        }
+
         mDisabledSysPackages.put(name, ps);
     }
 
@@ -3019,6 +3030,18 @@ final class Settings {
                     XmlUtils.skipCurrentTag(parser);
                 }
             }
+
+            // We keep track for which users we granted permissions to be able
+            // to grant runtime permissions to system apps for newly appeared
+            // users or newly appeared system apps. If we supported runtime
+            // permissions during the previous boot, then we already granted
+            // permissions for all device users. In such a case we set the users
+            // for which we granted permissions to avoid clobbering of runtime
+            // permissions we granted to system apps but the user revoked later.
+            if (!isFirstRuntimePermissionsBoot()) {
+                final int[] userIds = UserManagerService.getInstance().getUserIds();
+                packageSetting.setPermissionsUpdatedForUserIds(userIds);
+            }
         } else {
             XmlUtils.skipCurrentTag(parser);
         }
@@ -3136,6 +3159,18 @@ final class Settings {
                     XmlUtils.skipCurrentTag(parser);
                 }
             }
+
+            // We keep track for which users we granted permissions to be able
+            // to grant runtime permissions to system apps for newly appeared
+            // users or newly appeared system apps. If we supported runtime
+            // permissions during the previous boot, then we already granted
+            // permissions for all device users. In such a case we set the users
+            // for which we granted permissions to avoid clobbering of runtime
+            // permissions we granted to system apps but the user revoked later.
+            if (!isFirstRuntimePermissionsBoot()) {
+                final int[] userIds = UserManagerService.getInstance().getUserIds();
+                su.setPermissionsUpdatedForUserIds(userIds);
+            }
         } else {
             XmlUtils.skipCurrentTag(parser);
         }
@@ -3850,11 +3885,19 @@ final class Settings {
         }
 
         public void writePermissionsForUserSyncLPr(int userId) {
+            if (!PackageManagerService.RUNTIME_PERMISSIONS_ENABLED) {
+                return;
+            }
+
             mHandler.removeMessages(userId);
             writePermissionsSync(userId);
         }
 
         public void writePermissionsForUserAsyncLPr(int userId) {
+            if (!PackageManagerService.RUNTIME_PERMISSIONS_ENABLED) {
+                return;
+            }
+
             final long currentTimeMillis = SystemClock.uptimeMillis();
 
             if (mWriteScheduled.get(userId)) {
@@ -4014,128 +4057,67 @@ final class Settings {
 
         private void parseRuntimePermissionsLPr(XmlPullParser parser, int userId)
                 throws IOException, XmlPullParserException {
-            parser.next();
-            skipEmptyTextTags(parser);
-            if (!accept(parser, XmlPullParser.START_TAG, TAG_RUNTIME_PERMISSIONS)) {
-                return;
-            }
-
-            parser.next();
-
-            while (parsePackageLPr(parser, userId)
-                    || parseSharedUserLPr(parser, userId)) {
-                parser.next();
-            }
-
-            skipEmptyTextTags(parser);
-            expect(parser, XmlPullParser.END_TAG, TAG_RUNTIME_PERMISSIONS);
-        }
-
-        private boolean parsePackageLPr(XmlPullParser parser, int userId)
-                throws IOException, XmlPullParserException {
-            skipEmptyTextTags(parser);
-            if (!accept(parser, XmlPullParser.START_TAG, TAG_PACKAGE)) {
-                return false;
-            }
-
-            String name = parser.getAttributeValue(null, ATTR_NAME);
-
-            parser.next();
-
-            PackageSetting ps = mPackages.get(name);
-            if (ps != null) {
-                while (parsePermissionLPr(parser, ps.getPermissionsState(), userId)) {
-                    parser.next();
+            final int outerDepth = parser.getDepth();
+            int type;
+            while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
+                    && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
+                if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+                    continue;
                 }
-            }
-
-            skipEmptyTextTags(parser);
-            expect(parser, XmlPullParser.END_TAG, TAG_PACKAGE);
-
-            return true;
-        }
-
-        private boolean parseSharedUserLPr(XmlPullParser parser, int userId)
-                throws IOException, XmlPullParserException {
-            skipEmptyTextTags(parser);
-            if (!accept(parser, XmlPullParser.START_TAG, TAG_SHARED_USER)) {
-                return false;
-            }
 
-            String name = parser.getAttributeValue(null, ATTR_NAME);
-
-            parser.next();
-
-            SharedUserSetting sus = mSharedUsers.get(name);
-            if (sus != null) {
-                while (parsePermissionLPr(parser, sus.getPermissionsState(), userId)) {
-                    parser.next();
+                switch (parser.getName()) {
+                    case TAG_PACKAGE: {
+                        String name = parser.getAttributeValue(null, ATTR_NAME);
+                        PackageSetting ps = mPackages.get(name);
+                        if (ps == null) {
+                            Slog.w(PackageManagerService.TAG, "Unknown package:" + name);
+                            XmlUtils.skipCurrentTag(parser);
+                            continue;
+                        }
+                        parsePermissionsLPr(parser, ps.getPermissionsState(), userId);
+                    } break;
+
+                    case TAG_SHARED_USER: {
+                        String name = parser.getAttributeValue(null, ATTR_NAME);
+                        SharedUserSetting sus = mSharedUsers.get(name);
+                        if (sus == null) {
+                            Slog.w(PackageManagerService.TAG, "Unknown shared user:" + name);
+                            XmlUtils.skipCurrentTag(parser);
+                            continue;
+                        }
+                        parsePermissionsLPr(parser, sus.getPermissionsState(), userId);
+                    } break;
                 }
             }
-
-            skipEmptyTextTags(parser);
-            expect(parser, XmlPullParser.END_TAG, TAG_SHARED_USER);
-
-            return true;
         }
 
-        private boolean parsePermissionLPr(XmlPullParser parser, PermissionsState permissionsState,
+        private void parsePermissionsLPr(XmlPullParser parser, PermissionsState permissionsState,
                 int userId) throws IOException, XmlPullParserException {
-            skipEmptyTextTags(parser);
-            if (!accept(parser, XmlPullParser.START_TAG, TAG_ITEM)) {
-                return false;
-            }
-
-            String name = parser.getAttributeValue(null, ATTR_NAME);
-
-            parser.next();
-
-            BasePermission bp = mPermissions.get(name);
-            if (bp != null) {
-                if (permissionsState.grantRuntimePermission(bp, userId) ==
-                        PermissionsState.PERMISSION_OPERATION_FAILURE) {
-                    Slog.w(PackageManagerService.TAG, "Duplicate permission:" + name);
+            final int outerDepth = parser.getDepth();
+            int type;
+            while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
+                    && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) {
+                if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+                    continue;
                 }
-            } else {
-                Slog.w(PackageManagerService.TAG, "Unknown permission:" + name);
-            }
-
-            skipEmptyTextTags(parser);
-            expect(parser, XmlPullParser.END_TAG, TAG_ITEM);
-
-            return true;
-        }
 
-        private void expect(XmlPullParser parser, int type, String tag)
-                throws IOException, XmlPullParserException {
-            if (!accept(parser, type, tag)) {
-                throw new XmlPullParserException("Expected event: " + type
-                        + " and tag: " + tag + " but got event: " + parser.getEventType()
-                        + " and tag:" + parser.getName());
-            }
-        }
-
-        private void skipEmptyTextTags(XmlPullParser parser)
-                throws IOException, XmlPullParserException {
-            while (accept(parser, XmlPullParser.TEXT, null)
-                    && parser.isWhitespace()) {
-                parser.next();
-            }
-        }
+                switch (parser.getName()) {
+                    case TAG_ITEM: {
+                        String name = parser.getAttributeValue(null, ATTR_NAME);
+                        BasePermission bp = mPermissions.get(name);
+                        if (bp == null) {
+                            Slog.w(PackageManagerService.TAG, "Unknown permission:" + name);
+                            XmlUtils.skipCurrentTag(parser);
+                            continue;
+                        }
 
-        private boolean accept(XmlPullParser parser, int type, String tag)
-                throws IOException, XmlPullParserException {
-            if (parser.getEventType() != type) {
-                return false;
-            }
-            if (tag != null) {
-                if (!tag.equals(parser.getName())) {
-                    return false;
+                        if (permissionsState.grantRuntimePermission(bp, userId) ==
+                                PermissionsState.PERMISSION_OPERATION_FAILURE) {
+                            Slog.w(PackageManagerService.TAG, "Duplicate permission:" + name);
+                        }
+                    } break;
                 }
-            } else if (parser.getName() != null) {
-                return false;
             }
-            return true;
         }
 
         private void writePermissions(XmlSerializer serializer, Set<String> permissions)
diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java b/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java
index 4dc1131..6a471fe 100644
--- a/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java
+++ b/services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java
@@ -134,7 +134,7 @@ public class PackageManagerSettingsTests extends AndroidTestCase {
     public void testSettingsReadOld() {
         // Write the package files and make sure they're parsed properly the first time
         writeOldFiles();
-        Settings settings = new Settings(getContext(), getContext().getFilesDir(), new Object());
+        Settings settings = new Settings(getContext().getFilesDir(), new Object());
         assertEquals(true, settings.readLPw(null, null, 0, false));
         assertNotNull(settings.peekPackageLPr(PACKAGE_NAME_3));
         assertNotNull(settings.peekPackageLPr(PACKAGE_NAME_1));
@@ -152,7 +152,7 @@ public class PackageManagerSettingsTests extends AndroidTestCase {
     public void testNewPackageRestrictionsFile() {
         // Write the package files and make sure they're parsed properly the first time
         writeOldFiles();
-        Settings settings = new Settings(getContext(), getContext().getFilesDir(), new Object());
+        Settings settings = new Settings(getContext().getFilesDir(), new Object());
         assertEquals(true, settings.readLPw(null, null, 0, false));
         settings.writeLPr();