From 64d1f3efd759b70462aecb6cf1d8c733872a8911 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Wed, 26 Sep 2012 15:25:59 -0700 Subject: [PATCH] DO NOT MERGE - Full (local) restore security changes (1) Prevent full restore from creating files/directories that are accessible by other applications (2) Don't restore filesets from "system" packages; i.e. any that runs as a special uid, unless they define their own agent for handling the restore process. Bug 7168284 This is a cherry-pick from the originating tree. Change-Id: I9f39ada3c4c3b7ee63330b015e62745e84ccb58f --- core/java/android/app/backup/FullBackup.java | 13 +++-- .../com/android/server/BackupManagerService.java | 63 +++++++++++++++------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/core/java/android/app/backup/FullBackup.java b/core/java/android/app/backup/FullBackup.java index d7f1c9f0fab4..f859599e18ce 100644 --- a/core/java/android/app/backup/FullBackup.java +++ b/core/java/android/app/backup/FullBackup.java @@ -64,7 +64,9 @@ public class FullBackup { /** * Copy data from a socket to the given File location on permanent storage. The - * modification time and access mode of the resulting file will be set if desired. + * modification time and access mode of the resulting file will be set if desired, + * although group/all rwx modes will be stripped: the restored file will not be + * accessible from outside the target application even if the original file was. * If the {@code type} parameter indicates that the result should be a directory, * the socket parameter may be {@code null}; even if it is valid, no data will be * read from it in this case. @@ -79,8 +81,9 @@ public class FullBackup { * @param type Must be either {@link BackupAgent#TYPE_FILE} for ordinary file data * or {@link BackupAgent#TYPE_DIRECTORY} for a directory. * @param mode Unix-style file mode (as used by the chmod(2) syscall) to be set on - * the output file or directory. If this parameter is negative then neither - * the mode nor the mtime parameters will be used. + * the output file or directory. group/all rwx modes are stripped even if set + * in this parameter. If this parameter is negative then neither + * the mode nor the mtime values will be applied to the restored file. * @param mtime A timestamp in the standard Unix epoch that will be imposed as the * last modification time of the output file. if the {@code mode} parameter is * negative then this parameter will be ignored. @@ -105,8 +108,6 @@ public class FullBackup { if (!parent.exists()) { // in practice this will only be for the default semantic directories, // and using the default mode for those is appropriate. - // TODO: support the edge case of apps that have adjusted the - // permissions on these core directories parent.mkdirs(); } out = new FileOutputStream(outFile); @@ -146,6 +147,8 @@ public class FullBackup { // Now twiddle the state to match the backup, assuming all went well if (mode >= 0 && outFile != null) { try { + // explicitly prevent emplacement of files accessible by outside apps + mode &= 0700; Libcore.os.chmod(outFile.getPath(), (int)mode); } catch (ErrnoException e) { e.rethrowAsIOException(); diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java index 2167c492e5a9..1f3f172bf28f 100644 --- a/services/java/com/android/server/BackupManagerService.java +++ b/services/java/com/android/server/BackupManagerService.java @@ -2450,6 +2450,21 @@ class BackupManagerService extends IBackupManager.Stub { } } + // Cull any packages that run as system-domain uids but do not define their + // own backup agents + for (int i = 0; i < packagesToBackup.size(); ) { + PackageInfo pkg = packagesToBackup.get(i); + if ((pkg.applicationInfo.uid < Process.FIRST_APPLICATION_UID) + && (pkg.applicationInfo.backupAgentName == null)) { + if (MORE_DEBUG) { + Slog.i(TAG, "... ignoring non-agent system package " + pkg.packageName); + } + packagesToBackup.remove(i); + } else { + i++; + } + } + FileOutputStream ofstream = new FileOutputStream(mOutputFile.getFileDescriptor()); OutputStream out = null; @@ -3664,29 +3679,37 @@ class BackupManagerService extends IBackupManager.Stub { // Fall through to IGNORE if the app explicitly disallows backup final int flags = pkgInfo.applicationInfo.flags; if ((flags & ApplicationInfo.FLAG_ALLOW_BACKUP) != 0) { - // Verify signatures against any installed version; if they - // don't match, then we fall though and ignore the data. The - // signatureMatch() method explicitly ignores the signature - // check for packages installed on the system partition, because - // such packages are signed with the platform cert instead of - // the app developer's cert, so they're different on every - // device. - if (signaturesMatch(sigs, pkgInfo)) { - if (pkgInfo.versionCode >= version) { - Slog.i(TAG, "Sig + version match; taking data"); - policy = RestorePolicy.ACCEPT; + // Restore system-uid-space packages only if they have + // defined a custom backup agent + if ((pkgInfo.applicationInfo.uid >= Process.FIRST_APPLICATION_UID) + || (pkgInfo.applicationInfo.backupAgentName != null)) { + // Verify signatures against any installed version; if they + // don't match, then we fall though and ignore the data. The + // signatureMatch() method explicitly ignores the signature + // check for packages installed on the system partition, because + // such packages are signed with the platform cert instead of + // the app developer's cert, so they're different on every + // device. + if (signaturesMatch(sigs, pkgInfo)) { + if (pkgInfo.versionCode >= version) { + Slog.i(TAG, "Sig + version match; taking data"); + policy = RestorePolicy.ACCEPT; + } else { + // The data is from a newer version of the app than + // is presently installed. That means we can only + // use it if the matching apk is also supplied. + Slog.d(TAG, "Data version " + version + + " is newer than installed version " + + pkgInfo.versionCode + " - requiring apk"); + policy = RestorePolicy.ACCEPT_IF_APK; + } } else { - // The data is from a newer version of the app than - // is presently installed. That means we can only - // use it if the matching apk is also supplied. - Slog.d(TAG, "Data version " + version - + " is newer than installed version " - + pkgInfo.versionCode + " - requiring apk"); - policy = RestorePolicy.ACCEPT_IF_APK; + Slog.w(TAG, "Restore manifest signatures do not match " + + "installed application for " + info.packageName); } } else { - Slog.w(TAG, "Restore manifest signatures do not match " - + "installed application for " + info.packageName); + Slog.w(TAG, "Package " + info.packageName + + " is system level with no agent"); } } else { if (DEBUG) Slog.i(TAG, "Restore manifest from " -- 2.11.0