From 44ab8453e1c4c46790f792a46d026fa1017d8cfe Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 16 Nov 2010 15:10:49 -0800 Subject: Permission fix: don't require BACKUP perm for self-restores The public API is not supposed to require the BACKUP permission in order for an application to restore its own last-known-good backup data. However, as currently implemented, BackupManager.requestRestore() [the public API in question] depends on private Backup Manager methods that *do* enforce that permission. The net result is that the method cannot be successfully used by third party applications: it will throw an exception if attempted. This CL restructures the permission checking involved. First, the underlying beginRestoreSession() operation can now be passed a 'null' transport name; if this is done, then the restore session is begun on whatever the currently-active transport is. Looking up the name of the active transport is one of the permission-guarded actions that was required with the initial implementation. Second, a package name can now be passed to beginRestoreSession(). If this is done, then the restore session can only be used to perform a single-package restore of that one application. The BACKUP permission is not required if the caller is tying the restore to its own package name. In combination, these changes permit BackupManager.requestRestore() to function without the calling app needing to hold any special permission. The no-permission case is intentionally quite narrow: the caller must hold the permission unless they both (a) pass 'null' for the transport name, thereby accepting whatever the currently active transport is, and (b) pass their own package name to restrict the restore session only to their own app. External bug http://code.google.com/p/android/issues/detail?id=10094 Internal bug 3197202 Change-Id: Ibc9d652323f2da03727d850f991b4096af6520d2 --- cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java | 9 ++-- core/java/android/app/backup/BackupManager.java | 8 +-- core/java/android/app/backup/IBackupManager.aidl | 20 +++++-- .../com/android/server/BackupManagerService.java | 61 ++++++++++++++++++---- 4 files changed, 75 insertions(+), 23 deletions(-) diff --git a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java index b5fddfa..ac0e410 100644 --- a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java +++ b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java @@ -217,8 +217,7 @@ public final class Bmgr { // The rest of the 'list' options work with a restore session on the current transport try { - String curTransport = mBmgr.getCurrentTransport(); - mRestore = mBmgr.beginRestoreSession(curTransport); + mRestore = mBmgr.beginRestoreSession(null, null); if (mRestore == null) { System.err.println(BMGR_NOT_RUNNING_ERR); return; @@ -349,8 +348,7 @@ public final class Bmgr { private void doRestorePackage(String pkg) { try { - String curTransport = mBmgr.getCurrentTransport(); - mRestore = mBmgr.beginRestoreSession(curTransport); + mRestore = mBmgr.beginRestoreSession(pkg, null); if (mRestore == null) { System.err.println(BMGR_NOT_RUNNING_ERR); return; @@ -378,8 +376,7 @@ public final class Bmgr { try { boolean didRestore = false; - String curTransport = mBmgr.getCurrentTransport(); - mRestore = mBmgr.beginRestoreSession(curTransport); + mRestore = mBmgr.beginRestoreSession(null, null); if (mRestore == null) { System.err.println(BMGR_NOT_RUNNING_ERR); return; diff --git a/core/java/android/app/backup/BackupManager.java b/core/java/android/app/backup/BackupManager.java index 52dd707..80656a1 100644 --- a/core/java/android/app/backup/BackupManager.java +++ b/core/java/android/app/backup/BackupManager.java @@ -138,8 +138,8 @@ public class BackupManager { if (sService != null) { RestoreSession session = null; try { - String transport = sService.getCurrentTransport(); - IRestoreSession binder = sService.beginRestoreSession(transport); + IRestoreSession binder = sService.beginRestoreSession(mContext.getPackageName(), + null); session = new RestoreSession(mContext, binder); result = session.restorePackage(mContext.getPackageName(), observer); } catch (RemoteException e) { @@ -163,8 +163,8 @@ public class BackupManager { checkServiceBinder(); if (sService != null) { try { - String transport = sService.getCurrentTransport(); - IRestoreSession binder = sService.beginRestoreSession(transport); + // All packages, current transport + IRestoreSession binder = sService.beginRestoreSession(null, null); session = new RestoreSession(mContext, binder); } catch (RemoteException e) { Log.w(TAG, "beginRestoreSession() couldn't connect"); diff --git a/core/java/android/app/backup/IBackupManager.aidl b/core/java/android/app/backup/IBackupManager.aidl index 23d6351..8af59df 100644 --- a/core/java/android/app/backup/IBackupManager.aidl +++ b/core/java/android/app/backup/IBackupManager.aidl @@ -144,13 +144,25 @@ interface IBackupManager { String selectBackupTransport(String transport); /** - * Begin a restore session with the given transport (which may differ from the - * currently-active backup transport). + * Begin a restore session. Either or both of packageName and transportID + * may be null. If packageName is non-null, then only the given package will be + * considered for restore. If transportID is null, then the restore will use + * the current active transport. + *

+ * This method requires the android.permission.BACKUP permission except + * when transportID is null and packageName is the name of the caller's own + * package. In that case, the restore session returned is suitable for supporting + * the BackupManager.requestRestore() functionality via RestoreSession.restorePackage() + * without requiring the app to hold any special permission. * - * @param transport The name of the transport to use for the restore operation. + * @param packageName The name of the single package for which a restore will + * be requested. May be null, in which case all packages in the restore + * set can be restored. + * @param transportID The name of the transport to use for the restore operation. + * May be null, in which case the current active transport is used. * @return An interface to the restore session, or null on error. */ - IRestoreSession beginRestoreSession(String transportID); + IRestoreSession beginRestoreSession(String packageName, String transportID); /** * Notify the backup manager that a BackupAgent has completed the operation diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java index 6a86076..bebd013 100644 --- a/services/java/com/android/server/BackupManagerService.java +++ b/services/java/com/android/server/BackupManagerService.java @@ -2399,15 +2399,45 @@ class BackupManagerService extends IBackupManager.Stub { } // Hand off a restore session - public IRestoreSession beginRestoreSession(String transport) { - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, "beginRestoreSession"); + public IRestoreSession beginRestoreSession(String packageName, String transport) { + if (DEBUG) Slog.v(TAG, "beginRestoreSession: pkg=" + packageName + + " transport=" + transport); + + boolean needPermission = true; + if (transport == null) { + transport = mCurrentTransport; + + if (packageName != null) { + PackageInfo app = null; + try { + app = mPackageManager.getPackageInfo(packageName, 0); + } catch (NameNotFoundException nnf) { + Slog.w(TAG, "Asked to restore nonexistent pkg " + packageName); + throw new IllegalArgumentException("Package " + packageName + " not found"); + } + + if (app.applicationInfo.uid == Binder.getCallingUid()) { + // So: using the current active transport, and the caller has asked + // that its own package will be restored. In this narrow use case + // we do not require the caller to hold the permission. + needPermission = false; + } + } + } + + if (needPermission) { + mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, + "beginRestoreSession"); + } else { + if (DEBUG) Slog.d(TAG, "restoring self on current transport; no permission needed"); + } synchronized(this) { if (mActiveRestoreSession != null) { Slog.d(TAG, "Restore session requested but one already active"); return null; } - mActiveRestoreSession = new ActiveRestoreSession(transport); + mActiveRestoreSession = new ActiveRestoreSession(packageName, transport); } return mActiveRestoreSession; } @@ -2427,10 +2457,12 @@ class BackupManagerService extends IBackupManager.Stub { class ActiveRestoreSession extends IRestoreSession.Stub { private static final String TAG = "RestoreSession"; + private String mPackageName; private IBackupTransport mRestoreTransport = null; RestoreSet[] mRestoreSets = null; - ActiveRestoreSession(String transport) { + ActiveRestoreSession(String packageName, String transport) { + mPackageName = packageName; mRestoreTransport = getTransport(transport); } @@ -2466,11 +2498,16 @@ class BackupManagerService extends IBackupManager.Stub { mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, "performRestore"); - if (DEBUG) Slog.d(TAG, "performRestore token=" + Long.toHexString(token) + if (DEBUG) Slog.d(TAG, "restoreAll token=" + Long.toHexString(token) + " observer=" + observer); if (mRestoreTransport == null || mRestoreSets == null) { - Slog.e(TAG, "Ignoring performRestore() with no restore set"); + Slog.e(TAG, "Ignoring restoreAll() with no restore set"); + return -1; + } + + if (mPackageName != null) { + Slog.e(TAG, "Ignoring restoreAll() on single-package session"); return -1; } @@ -2495,6 +2532,14 @@ class BackupManagerService extends IBackupManager.Stub { public synchronized int restorePackage(String packageName, IRestoreObserver observer) { if (DEBUG) Slog.v(TAG, "restorePackage pkg=" + packageName + " obs=" + observer); + if (mPackageName != null) { + if (! mPackageName.equals(packageName)) { + Slog.e(TAG, "Ignoring attempt to restore pkg=" + packageName + + " on session for package " + mPackageName); + return -1; + } + } + PackageInfo app = null; try { app = mPackageManager.getPackageInfo(packageName, 0); @@ -2529,6 +2574,7 @@ class BackupManagerService extends IBackupManager.Stub { // the app has never been backed up from this device -- there's nothing // to do but return failure. if (token == 0) { + if (DEBUG) Slog.w(TAG, "No data available for this package; not restoring"); return -1; } @@ -2543,9 +2589,6 @@ class BackupManagerService extends IBackupManager.Stub { } public synchronized void endRestoreSession() { - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, - "endRestoreSession"); - if (DEBUG) Slog.d(TAG, "endRestoreSession"); synchronized (this) { -- cgit v1.1