From 88f3d4dbe1dbc08316d8a9e25edce1ad16fe2eab Mon Sep 17 00:00:00 2001 From: Geoffrey Pitsch Date: Wed, 22 Nov 2017 13:31:11 -0500 Subject: [PATCH] Security model for moving sharesheet to systemui ResolverActivity (still in frameworks) now requests a "permission token" that it hands to a stubbed system ui activity ChooserActivity. This permission token allows an app (SysUI) with the signed permission "START_ACTIVITY_AS_CALLER" to call ActivityManagerService#startActivityAsCaller. Permission tokens are a one-time use, limited-time offer. Test: runtest systemui && manual testing Bug: 69850752 Change-Id: I3600e1a8ff9eea7397f5f59853423c79b6401f98 --- core/java/android/app/Activity.java | 6 +- core/java/android/app/ActivityManager.java | 25 ++++ core/java/android/app/IActivityManager.aidl | 3 +- core/java/android/app/Instrumentation.java | 7 +- .../android/internal/app/ChooserActivity.java | 5 +- .../internal/app/IntentForwarderActivity.java | 2 +- .../internal/app/ResolverActivity.java | 38 ++++- core/res/AndroidManifest.xml | 6 + core/res/res/values/config.xml | 5 +- core/res/res/values/symbols.xml | 1 + .../app/IntentForwarderActivityTest.java | 7 +- data/etc/privapp-permissions-platform.xml | 1 + packages/SystemUI/AndroidManifest.xml | 17 +++ .../systemui/chooser/ChooserActivity.java | 41 ++++++ .../systemui/chooser/ChooserHelper.java | 45 ++++++ .../systemui/chooser/ChooserHelperTest.java | 63 ++++++++ .../server/am/ActivityManagerService.java | 135 ++++++++++++++++-- 17 files changed, 376 insertions(+), 31 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/chooser/ChooserActivity.java create mode 100644 packages/SystemUI/src/com/android/systemui/chooser/ChooserHelper.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/chooser/ChooserHelperTest.java diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java index 0a5b848e62208..73fbb19339435 100644 --- a/core/java/android/app/Activity.java +++ b/core/java/android/app/Activity.java @@ -17,6 +17,7 @@ package android.app; import static android.Manifest.permission.CONTROL_REMOTE_APP_TRANSITION_ANIMATIONS; + import static java.lang.Character.MIN_VALUE; import android.annotation.CallSuper; @@ -4671,6 +4672,7 @@ public class Activity extends ContextThemeWrapper * their launch had come from the original activity. * @param intent The Intent to start. * @param options ActivityOptions or null. + * @param permissionToken Token received from the system that permits this call to be made. * @param ignoreTargetSecurity If true, the activity manager will not check whether the * caller it is doing the start is, is actually allowed to start the target activity. * If you set this to true, you must set an explicit component in the Intent and do any @@ -4679,7 +4681,7 @@ public class Activity extends ContextThemeWrapper * @hide */ public void startActivityAsCaller(Intent intent, @Nullable Bundle options, - boolean ignoreTargetSecurity, int userId) { + IBinder permissionToken, boolean ignoreTargetSecurity, int userId) { if (mParent != null) { throw new RuntimeException("Can't be called from a child"); } @@ -4687,7 +4689,7 @@ public class Activity extends ContextThemeWrapper Instrumentation.ActivityResult ar = mInstrumentation.execStartActivityAsCaller( this, mMainThread.getApplicationThread(), mToken, this, - intent, -1, options, ignoreTargetSecurity, userId); + intent, -1, options, permissionToken, ignoreTargetSecurity, userId); if (ar != null) { mMainThread.sendActivityResult( mToken, mEmbeddedID, -1, ar.getResultCode(), diff --git a/core/java/android/app/ActivityManager.java b/core/java/android/app/ActivityManager.java index 455458436c2f2..b5a9412831849 100644 --- a/core/java/android/app/ActivityManager.java +++ b/core/java/android/app/ActivityManager.java @@ -443,6 +443,31 @@ public class ActivityManager { */ public static final int INTENT_SENDER_FOREGROUND_SERVICE = 5; + /** + * Extra included on intents that are delegating the call to + * ActivityManager#startActivityAsCaller to another app. This token is necessary for that call + * to succeed. Type is IBinder. + * @hide + */ + public static final String EXTRA_PERMISSION_TOKEN = "android.app.extra.PERMISSION_TOKEN"; + + /** + * Extra included on intents that contain an EXTRA_INTENT, with options that the contained + * intent may want to be started with. Type is Bundle. + * TODO: remove once the ChooserActivity moves to systemui + * @hide + */ + public static final String EXTRA_OPTIONS = "android.app.extra.OPTIONS"; + + /** + * Extra included on intents that contain an EXTRA_INTENT, use this boolean value for the + * parameter of the same name when starting the contained intent. + * TODO: remove once the ChooserActivity moves to systemui + * @hide + */ + public static final String EXTRA_IGNORE_TARGET_SECURITY = + "android.app.extra.EXTRA_IGNORE_TARGET_SECURITY"; + /** @hide User operation call: success! */ public static final int USER_OP_SUCCESS = 0; diff --git a/core/java/android/app/IActivityManager.aidl b/core/java/android/app/IActivityManager.aidl index 04ee77d764aa9..5f5d834425b63 100644 --- a/core/java/android/app/IActivityManager.aidl +++ b/core/java/android/app/IActivityManager.aidl @@ -438,10 +438,11 @@ interface IActivityManager { boolean isTopOfTask(in IBinder token); void notifyLaunchTaskBehindComplete(in IBinder token); void notifyEnterAnimationComplete(in IBinder token); + IBinder requestStartActivityPermissionToken(in IBinder delegatorToken); int startActivityAsCaller(in IApplicationThread caller, in String callingPackage, in Intent intent, in String resolvedType, in IBinder resultTo, in String resultWho, int requestCode, int flags, in ProfilerInfo profilerInfo, in Bundle options, - boolean ignoreTargetSecurity, int userId); + in IBinder permissionToken, boolean ignoreTargetSecurity, int userId); int addAppTask(in IBinder activityToken, in Intent intent, in ActivityManager.TaskDescription description, in Bitmap thumbnail); Point getAppTaskThumbnailSize(); diff --git a/core/java/android/app/Instrumentation.java b/core/java/android/app/Instrumentation.java index b469de56d857c..98dacccc5d28e 100644 --- a/core/java/android/app/Instrumentation.java +++ b/core/java/android/app/Instrumentation.java @@ -1872,8 +1872,8 @@ public class Instrumentation { */ public ActivityResult execStartActivityAsCaller( Context who, IBinder contextThread, IBinder token, Activity target, - Intent intent, int requestCode, Bundle options, boolean ignoreTargetSecurity, - int userId) { + Intent intent, int requestCode, Bundle options, IBinder permissionToken, + boolean ignoreTargetSecurity, int userId) { IApplicationThread whoThread = (IApplicationThread) contextThread; if (mActivityMonitors != null) { synchronized (mSync) { @@ -1904,7 +1904,8 @@ public class Instrumentation { .startActivityAsCaller(whoThread, who.getBasePackageName(), intent, intent.resolveTypeIfNeeded(who.getContentResolver()), token, target != null ? target.mEmbeddedID : null, - requestCode, 0, null, options, ignoreTargetSecurity, userId); + requestCode, 0, null, options, permissionToken, + ignoreTargetSecurity, userId); checkStartActivityResult(result, intent); } catch (RemoteException e) { throw new RuntimeException("Failure from system", e); diff --git a/core/java/com/android/internal/app/ChooserActivity.java b/core/java/com/android/internal/app/ChooserActivity.java index 6e0ba3413e8ca..997d47fe8cf0c 100644 --- a/core/java/com/android/internal/app/ChooserActivity.java +++ b/core/java/com/android/internal/app/ChooserActivity.java @@ -841,7 +841,7 @@ public class ChooserActivity extends ResolverActivity { } @Override - public boolean startAsCaller(Activity activity, Bundle options, int userId) { + public boolean startAsCaller(ResolverActivity activity, Bundle options, int userId) { final Intent intent = getBaseIntentToSend(); if (intent == null) { return false; @@ -860,8 +860,7 @@ public class ChooserActivity extends ResolverActivity { final boolean ignoreTargetSecurity = mSourceInfo != null && mSourceInfo.getResolvedComponentName().getPackageName() .equals(mChooserTarget.getComponentName().getPackageName()); - activity.startActivityAsCaller(intent, options, ignoreTargetSecurity, userId); - return true; + return activity.startAsCallerImpl(intent, options, ignoreTargetSecurity, userId); } @Override diff --git a/core/java/com/android/internal/app/IntentForwarderActivity.java b/core/java/com/android/internal/app/IntentForwarderActivity.java index 398d08791b5c8..86731bcb4bf61 100644 --- a/core/java/com/android/internal/app/IntentForwarderActivity.java +++ b/core/java/com/android/internal/app/IntentForwarderActivity.java @@ -107,7 +107,7 @@ public class IntentForwarderActivity extends Activity { || ChooserActivity.class.getName().equals(ri.activityInfo.name)); try { - startActivityAsCaller(newIntent, null, false, targetUserId); + startActivityAsCaller(newIntent, null, null, false, targetUserId); } catch (RuntimeException e) { int launchedFromUid = -1; String launchedFromPackage = "?"; diff --git a/core/java/com/android/internal/app/ResolverActivity.java b/core/java/com/android/internal/app/ResolverActivity.java index ceb06f5111087..d6d44908a15bb 100644 --- a/core/java/com/android/internal/app/ResolverActivity.java +++ b/core/java/com/android/internal/app/ResolverActivity.java @@ -43,6 +43,7 @@ import android.net.Uri; import android.os.AsyncTask; import android.os.Build; import android.os.Bundle; +import android.os.IBinder; import android.os.PatternMatcher; import android.os.RemoteException; import android.os.StrictMode; @@ -857,6 +858,36 @@ public class ResolverActivity extends Activity { } } + public boolean startAsCallerImpl(Intent intent, Bundle options, boolean ignoreTargetSecurity, + int userId) { + // Pass intent to delegate chooser activity with permission token. + // TODO: This should move to a trampoline Activity in the system when the ChooserActivity + // moves into systemui + try { + // TODO: Once this is a small springboard activity, it can move off the UI process + // and we can move the request method to ActivityManagerInternal. + IBinder permissionToken = ActivityManager.getService() + .requestStartActivityPermissionToken(getActivityToken()); + final Intent chooserIntent = new Intent(); + final ComponentName delegateActivity = ComponentName.unflattenFromString( + Resources.getSystem().getString(R.string.config_chooserActivity)); + chooserIntent.setClassName(delegateActivity.getPackageName(), + delegateActivity.getClassName()); + chooserIntent.putExtra(ActivityManager.EXTRA_PERMISSION_TOKEN, permissionToken); + + // TODO: These extras will change as chooser activity moves into systemui + chooserIntent.putExtra(Intent.EXTRA_INTENT, intent); + chooserIntent.putExtra(ActivityManager.EXTRA_OPTIONS, options); + chooserIntent.putExtra(ActivityManager.EXTRA_IGNORE_TARGET_SECURITY, + ignoreTargetSecurity); + chooserIntent.putExtra(Intent.EXTRA_USER_ID, userId); + startActivity(chooserIntent); + } catch (RemoteException e) { + Log.e(TAG, e.toString()); + } + return true; + } + public void onActivityStarted(TargetInfo cti) { // Do nothing } @@ -1181,9 +1212,8 @@ public class ResolverActivity extends Activity { } @Override - public boolean startAsCaller(Activity activity, Bundle options, int userId) { - activity.startActivityAsCaller(mResolvedIntent, options, false, userId); - return true; + public boolean startAsCaller(ResolverActivity activity, Bundle options, int userId) { + return activity.startAsCallerImpl(mResolvedIntent, options, false, userId); } @Override @@ -1242,7 +1272,7 @@ public class ResolverActivity extends Activity { * @param userId userId to start as or {@link UserHandle#USER_NULL} for activity's caller * @return true if the start completed successfully */ - boolean startAsCaller(Activity activity, Bundle options, int userId); + boolean startAsCaller(ResolverActivity activity, Bundle options, int userId); /** * Start the activity referenced by this target as a given user. diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index ba30981e63778..c3647f36f71d9 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -1929,6 +1929,12 @@ + + + android/android.accounts.ChooseTypeAndAccountActivity - + + com.android.systemui/com.android.systemui.chooser.ChooserActivity diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 03a800d8bd6f4..843b33ab82efe 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -1071,6 +1071,7 @@ + diff --git a/core/tests/coretests/src/com/android/internal/app/IntentForwarderActivityTest.java b/core/tests/coretests/src/com/android/internal/app/IntentForwarderActivityTest.java index b18fa747557d2..c0bc3a8eeb9e7 100644 --- a/core/tests/coretests/src/com/android/internal/app/IntentForwarderActivityTest.java +++ b/core/tests/coretests/src/com/android/internal/app/IntentForwarderActivityTest.java @@ -24,6 +24,7 @@ import android.content.pm.IPackageManager; import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.os.Bundle; +import android.os.IBinder; import android.os.UserHandle; import android.os.UserManager; import android.support.test.InstrumentationRegistry; @@ -269,8 +270,8 @@ public class IntentForwarderActivityTest { } @Override - public void startActivityAsCaller(Intent intent, @Nullable Bundle options, boolean - ignoreTargetSecurity, int userId) { + public void startActivityAsCaller(Intent intent, @Nullable Bundle options, + IBinder permissionToken, boolean ignoreTargetSecurity, int userId) { mStartActivityIntent = intent; mUserIdActivityLaunchedIn = userId; } @@ -293,4 +294,4 @@ public class IntentForwarderActivityTest { return mPm; } } -} \ No newline at end of file +} diff --git a/data/etc/privapp-permissions-platform.xml b/data/etc/privapp-permissions-platform.xml index c0958cd6cdd7d..0949a90877e1a 100644 --- a/data/etc/privapp-permissions-platform.xml +++ b/data/etc/privapp-permissions-platform.xml @@ -368,6 +368,7 @@ applications that come with the platform + diff --git a/packages/SystemUI/AndroidManifest.xml b/packages/SystemUI/AndroidManifest.xml index 80ac82576d130..b20aa7af67251 100644 --- a/packages/SystemUI/AndroidManifest.xml +++ b/packages/SystemUI/AndroidManifest.xml @@ -89,6 +89,7 @@ + @@ -555,6 +556,22 @@ + + + + + + + + mActiveInstrumentation = new ArrayList<>(); + // Activity tokens of system activities that are delegating their call to + // #startActivityByCaller, keyed by the permissionToken granted to the delegate. + final HashMap mStartActivitySources = new HashMap<>(); + + // Permission tokens that have expired, but we remember for error reporting. + final ArrayList mExpiredStartAsCallerTokens = new ArrayList<>(); + public final IntentFirewall mIntentFirewall; // Whether we should show our dialogs (ANR, crash, etc) or just perform their @@ -1781,6 +1806,8 @@ public class ActivityManagerService extends IActivityManager.Stub static final int PUSH_TEMP_WHITELIST_UI_MSG = 68; static final int SERVICE_FOREGROUND_CRASH_MSG = 69; static final int DISPATCH_OOM_ADJ_OBSERVER_MSG = 70; + static final int EXPIRE_START_AS_CALLER_TOKEN_MSG = 75; + static final int FORGET_START_AS_CALLER_TOKEN_MSG = 76; static final int FIRST_ACTIVITY_STACK_MSG = 100; static final int FIRST_BROADCAST_QUEUE_MSG = 200; @@ -2445,6 +2472,19 @@ public class ActivityManagerService extends IActivityManager.Stub } } } break; + case EXPIRE_START_AS_CALLER_TOKEN_MSG: { + synchronized (ActivityManagerService.this) { + final IBinder permissionToken = (IBinder)msg.obj; + mStartActivitySources.remove(permissionToken); + mExpiredStartAsCallerTokens.add(permissionToken); + } + } break; + case FORGET_START_AS_CALLER_TOKEN_MSG: { + synchronized (ActivityManagerService.this) { + final IBinder permissionToken = (IBinder)msg.obj; + mExpiredStartAsCallerTokens.remove(permissionToken); + } + } break; } } }; @@ -4712,16 +4752,54 @@ public class ActivityManagerService extends IActivityManager.Stub } + /** + * Only callable from the system. This token grants a temporary permission to call + * #startActivityAsCallerWithToken. The token will time out after + * START_AS_CALLER_TOKEN_TIMEOUT if it is not used. + * + * @param delegatorToken The Binder token referencing the system Activity that wants to delegate + * the #startActivityAsCaller to another app. The "caller" will be the caller of this + * activity's token, not the delegate's caller (which is probably the delegator itself). + * + * @return Returns a token that can be given to a "delegate" app that may call + * #startActivityAsCaller + */ @Override - public final int startActivityAsCaller(IApplicationThread caller, String callingPackage, - Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode, - int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, boolean ignoreTargetSecurity, - int userId) { + public IBinder requestStartActivityPermissionToken(IBinder delegatorToken) { + int callingUid = Binder.getCallingUid(); + if (UserHandle.getAppId(callingUid) != SYSTEM_UID) { + throw new SecurityException("Only the system process can request a permission token, " + + "received request from uid: " + callingUid); + } + IBinder permissionToken = new Binder(); + synchronized (this) { + mStartActivitySources.put(permissionToken, delegatorToken); + } + Message expireMsg = mHandler.obtainMessage(EXPIRE_START_AS_CALLER_TOKEN_MSG, + permissionToken); + mHandler.sendMessageDelayed(expireMsg, START_AS_CALLER_TOKEN_TIMEOUT_IMPL); + + Message forgetMsg = mHandler.obtainMessage(FORGET_START_AS_CALLER_TOKEN_MSG, + permissionToken); + mHandler.sendMessageDelayed(forgetMsg, START_AS_CALLER_TOKEN_EXPIRED_TIMEOUT); + + return permissionToken; + } + + @Override + public final int startActivityAsCaller(IApplicationThread caller, + String callingPackage, Intent intent, String resolvedType, IBinder resultTo, + String resultWho, int requestCode, int startFlags, ProfilerInfo profilerInfo, + Bundle bOptions, IBinder permissionToken, boolean ignoreTargetSecurity, int userId) { // This is very dangerous -- it allows you to perform a start activity (including - // permission grants) as any app that may launch one of your own activities. So - // we will only allow this to be done from activities that are part of the core framework, - // and then only when they are running as the system. + // permission grants) as any app that may launch one of your own activities. So we only + // allow this in two cases: + // 1) The caller is an activity that is part of the core framework, and then only when it + // is running as the system. + // 2) The caller provides a valid permissionToken. Permission tokens are one-time use and + // can only be requested by a system activity, which may then delegate this call to + // another app. final ActivityRecord sourceRecord; final int targetUid; final String targetPackage; @@ -4729,17 +4807,47 @@ public class ActivityManagerService extends IActivityManager.Stub if (resultTo == null) { throw new SecurityException("Must be called from an activity"); } - sourceRecord = mStackSupervisor.isInAnyStackLocked(resultTo); - if (sourceRecord == null) { - throw new SecurityException("Called with bad activity token: " + resultTo); + + final IBinder sourceToken; + if (permissionToken != null) { + // To even attempt to use a permissionToken, an app must also have this signature + // permission. + enforceCallingPermission(android.Manifest.permission.START_ACTIVITY_AS_CALLER, + "startActivityAsCaller"); + // If called with a permissionToken, we want the sourceRecord from the delegator + // activity that requested this token. + sourceToken = + mStartActivitySources.remove(permissionToken); + if (sourceToken == null) { + // Invalid permissionToken, check if it recently expired. + if (mExpiredStartAsCallerTokens.contains(permissionToken)) { + throw new SecurityException("Called with expired permission token: " + + permissionToken); + } else { + throw new SecurityException("Called with invalid permission token: " + + permissionToken); + } + } + } else { + // This method was called directly by the source. + sourceToken = resultTo; } - if (!sourceRecord.info.packageName.equals("android")) { - throw new SecurityException( - "Must be called from an activity that is declared in the android package"); + + sourceRecord = mStackSupervisor.isInAnyStackLocked(sourceToken); + if (sourceRecord == null) { + throw new SecurityException("Called with bad activity token: " + sourceToken); } if (sourceRecord.app == null) { throw new SecurityException("Called without a process attached to activity"); } + + // Whether called directly or from a delegate, the source activity must be from the + // android package. + if (!sourceRecord.info.packageName.equals("android")) { + throw new SecurityException("Must be called from an activity that is " + + "declared in the android package"); + } + if (UserHandle.getAppId(sourceRecord.app.uid) != SYSTEM_UID) { // This is still okay, as long as this activity is running under the // uid of the original calling activity. @@ -4750,6 +4858,7 @@ public class ActivityManagerService extends IActivityManager.Stub + sourceRecord.launchedFromUid); } } + if (ignoreTargetSecurity) { if (intent.getComponent() == null) { throw new SecurityException(