Skip to content

Commit 6ec9173

Browse files
author
Chris Tate
committed
DO NOT MERGE : 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 (Cherrypick from master to gingerbread) Change-Id: Ie20b0bd2420345ce6eda178f854680b558f6372a
1 parent 433863b commit 6ec9173

File tree

4 files changed

+75
-23
lines changed

4 files changed

+75
-23
lines changed

cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,7 @@ private void doList() {
217217

218218
// The rest of the 'list' options work with a restore session on the current transport
219219
try {
220-
String curTransport = mBmgr.getCurrentTransport();
221-
mRestore = mBmgr.beginRestoreSession(curTransport);
220+
mRestore = mBmgr.beginRestoreSession(null, null);
222221
if (mRestore == null) {
223222
System.err.println(BMGR_NOT_RUNNING_ERR);
224223
return;
@@ -349,8 +348,7 @@ private void doRestore() {
349348

350349
private void doRestorePackage(String pkg) {
351350
try {
352-
String curTransport = mBmgr.getCurrentTransport();
353-
mRestore = mBmgr.beginRestoreSession(curTransport);
351+
mRestore = mBmgr.beginRestoreSession(pkg, null);
354352
if (mRestore == null) {
355353
System.err.println(BMGR_NOT_RUNNING_ERR);
356354
return;
@@ -378,8 +376,7 @@ private void doRestoreAll(long token) {
378376

379377
try {
380378
boolean didRestore = false;
381-
String curTransport = mBmgr.getCurrentTransport();
382-
mRestore = mBmgr.beginRestoreSession(curTransport);
379+
mRestore = mBmgr.beginRestoreSession(null, null);
383380
if (mRestore == null) {
384381
System.err.println(BMGR_NOT_RUNNING_ERR);
385382
return;

core/java/android/app/backup/BackupManager.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ public int requestRestore(RestoreObserver observer) {
138138
if (sService != null) {
139139
RestoreSession session = null;
140140
try {
141-
String transport = sService.getCurrentTransport();
142-
IRestoreSession binder = sService.beginRestoreSession(transport);
141+
IRestoreSession binder = sService.beginRestoreSession(mContext.getPackageName(),
142+
null);
143143
session = new RestoreSession(mContext, binder);
144144
result = session.restorePackage(mContext.getPackageName(), observer);
145145
} catch (RemoteException e) {
@@ -163,8 +163,8 @@ public RestoreSession beginRestoreSession() {
163163
checkServiceBinder();
164164
if (sService != null) {
165165
try {
166-
String transport = sService.getCurrentTransport();
167-
IRestoreSession binder = sService.beginRestoreSession(transport);
166+
// All packages, current transport
167+
IRestoreSession binder = sService.beginRestoreSession(null, null);
168168
session = new RestoreSession(mContext, binder);
169169
} catch (RemoteException e) {
170170
Log.w(TAG, "beginRestoreSession() couldn't connect");

core/java/android/app/backup/IBackupManager.aidl

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,25 @@ interface IBackupManager {
144144
String selectBackupTransport(String transport);
145145

146146
/**
147-
* Begin a restore session with the given transport (which may differ from the
148-
* currently-active backup transport).
147+
* Begin a restore session. Either or both of packageName and transportID
148+
* may be null. If packageName is non-null, then only the given package will be
149+
* considered for restore. If transportID is null, then the restore will use
150+
* the current active transport.
151+
* <p>
152+
* This method requires the android.permission.BACKUP permission <i>except</i>
153+
* when transportID is null and packageName is the name of the caller's own
154+
* package. In that case, the restore session returned is suitable for supporting
155+
* the BackupManager.requestRestore() functionality via RestoreSession.restorePackage()
156+
* without requiring the app to hold any special permission.
149157
*
150-
* @param transport The name of the transport to use for the restore operation.
158+
* @param packageName The name of the single package for which a restore will
159+
* be requested. May be null, in which case all packages in the restore
160+
* set can be restored.
161+
* @param transportID The name of the transport to use for the restore operation.
162+
* May be null, in which case the current active transport is used.
151163
* @return An interface to the restore session, or null on error.
152164
*/
153-
IRestoreSession beginRestoreSession(String transportID);
165+
IRestoreSession beginRestoreSession(String packageName, String transportID);
154166

155167
/**
156168
* Notify the backup manager that a BackupAgent has completed the operation

services/java/com/android/server/BackupManagerService.java

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2398,15 +2398,45 @@ public void restoreAtInstall(String packageName, int token) {
23982398
}
23992399

24002400
// Hand off a restore session
2401-
public IRestoreSession beginRestoreSession(String transport) {
2402-
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, "beginRestoreSession");
2401+
public IRestoreSession beginRestoreSession(String packageName, String transport) {
2402+
if (DEBUG) Slog.v(TAG, "beginRestoreSession: pkg=" + packageName
2403+
+ " transport=" + transport);
2404+
2405+
boolean needPermission = true;
2406+
if (transport == null) {
2407+
transport = mCurrentTransport;
2408+
2409+
if (packageName != null) {
2410+
PackageInfo app = null;
2411+
try {
2412+
app = mPackageManager.getPackageInfo(packageName, 0);
2413+
} catch (NameNotFoundException nnf) {
2414+
Slog.w(TAG, "Asked to restore nonexistent pkg " + packageName);
2415+
throw new IllegalArgumentException("Package " + packageName + " not found");
2416+
}
2417+
2418+
if (app.applicationInfo.uid == Binder.getCallingUid()) {
2419+
// So: using the current active transport, and the caller has asked
2420+
// that its own package will be restored. In this narrow use case
2421+
// we do not require the caller to hold the permission.
2422+
needPermission = false;
2423+
}
2424+
}
2425+
}
2426+
2427+
if (needPermission) {
2428+
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP,
2429+
"beginRestoreSession");
2430+
} else {
2431+
if (DEBUG) Slog.d(TAG, "restoring self on current transport; no permission needed");
2432+
}
24032433

24042434
synchronized(this) {
24052435
if (mActiveRestoreSession != null) {
24062436
Slog.d(TAG, "Restore session requested but one already active");
24072437
return null;
24082438
}
2409-
mActiveRestoreSession = new ActiveRestoreSession(transport);
2439+
mActiveRestoreSession = new ActiveRestoreSession(packageName, transport);
24102440
}
24112441
return mActiveRestoreSession;
24122442
}
@@ -2426,10 +2456,12 @@ public void opComplete(int token) {
24262456
class ActiveRestoreSession extends IRestoreSession.Stub {
24272457
private static final String TAG = "RestoreSession";
24282458

2459+
private String mPackageName;
24292460
private IBackupTransport mRestoreTransport = null;
24302461
RestoreSet[] mRestoreSets = null;
24312462

2432-
ActiveRestoreSession(String transport) {
2463+
ActiveRestoreSession(String packageName, String transport) {
2464+
mPackageName = packageName;
24332465
mRestoreTransport = getTransport(transport);
24342466
}
24352467

@@ -2465,11 +2497,16 @@ public synchronized int restoreAll(long token, IRestoreObserver observer) {
24652497
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP,
24662498
"performRestore");
24672499

2468-
if (DEBUG) Slog.d(TAG, "performRestore token=" + Long.toHexString(token)
2500+
if (DEBUG) Slog.d(TAG, "restoreAll token=" + Long.toHexString(token)
24692501
+ " observer=" + observer);
24702502

24712503
if (mRestoreTransport == null || mRestoreSets == null) {
2472-
Slog.e(TAG, "Ignoring performRestore() with no restore set");
2504+
Slog.e(TAG, "Ignoring restoreAll() with no restore set");
2505+
return -1;
2506+
}
2507+
2508+
if (mPackageName != null) {
2509+
Slog.e(TAG, "Ignoring restoreAll() on single-package session");
24732510
return -1;
24742511
}
24752512

@@ -2494,6 +2531,14 @@ public synchronized int restoreAll(long token, IRestoreObserver observer) {
24942531
public synchronized int restorePackage(String packageName, IRestoreObserver observer) {
24952532
if (DEBUG) Slog.v(TAG, "restorePackage pkg=" + packageName + " obs=" + observer);
24962533

2534+
if (mPackageName != null) {
2535+
if (! mPackageName.equals(packageName)) {
2536+
Slog.e(TAG, "Ignoring attempt to restore pkg=" + packageName
2537+
+ " on session for package " + mPackageName);
2538+
return -1;
2539+
}
2540+
}
2541+
24972542
PackageInfo app = null;
24982543
try {
24992544
app = mPackageManager.getPackageInfo(packageName, 0);
@@ -2528,6 +2573,7 @@ public synchronized int restorePackage(String packageName, IRestoreObserver obse
25282573
// the app has never been backed up from this device -- there's nothing
25292574
// to do but return failure.
25302575
if (token == 0) {
2576+
if (DEBUG) Slog.w(TAG, "No data available for this package; not restoring");
25312577
return -1;
25322578
}
25332579

@@ -2542,9 +2588,6 @@ public synchronized int restorePackage(String packageName, IRestoreObserver obse
25422588
}
25432589

25442590
public synchronized void endRestoreSession() {
2545-
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP,
2546-
"endRestoreSession");
2547-
25482591
if (DEBUG) Slog.d(TAG, "endRestoreSession");
25492592

25502593
synchronized (this) {

0 commit comments

Comments
 (0)