summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author TreeHugger Robot <treehugger-gerrit@google.com> 2017-12-08 18:16:24 +0000
committer Android (Google) Code Review <android-gerrit@google.com> 2017-12-08 18:16:24 +0000
commitbd355dbf58be7304c069bfa0b6e75cfe080e02b5 (patch)
treec4e586f865e17b5528db3ab89b3724dd05ddf280
parent2ff6320d773c0e0f7f06a9ae3d36924ab13d5658 (diff)
parent9ad29006302ca7262c0df9de7b7cbb3e35247137 (diff)
Merge "Make sure apps cannot forge package name on AssistStructure used for Autofill." into oc-dev
-rw-r--r--core/java/android/app/assist/AssistStructure.java10
-rw-r--r--core/java/android/view/autofill/AutofillManager.java26
-rw-r--r--core/java/android/view/autofill/IAutoFillManager.aidl6
-rw-r--r--proto/src/metrics_constants.proto13
-rw-r--r--services/autofill/java/com/android/server/autofill/AutofillManagerService.java14
-rw-r--r--services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java52
-rw-r--r--services/autofill/java/com/android/server/autofill/Session.java53
7 files changed, 142 insertions, 32 deletions
diff --git a/core/java/android/app/assist/AssistStructure.java b/core/java/android/app/assist/AssistStructure.java
index 4e8277c388de..8bd23136ed59 100644
--- a/core/java/android/app/assist/AssistStructure.java
+++ b/core/java/android/app/assist/AssistStructure.java
@@ -2054,6 +2054,16 @@ public class AssistStructure implements Parcelable {
return mActivityComponent;
}
+ /**
+ * Called by Autofill server when app forged a different value.
+ *
+ * @hide
+ */
+ public void setActivityComponent(ComponentName componentName) {
+ ensureData();
+ mActivityComponent = componentName;
+ }
+
/** @hide */
public int getFlags() {
return mFlags;
diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java
index c123a807800f..f2d2d5bb28da 100644
--- a/core/java/android/view/autofill/AutofillManager.java
+++ b/core/java/android/view/autofill/AutofillManager.java
@@ -24,6 +24,8 @@ import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.SystemService;
+import android.app.Activity;
+import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.content.IntentSender;
@@ -43,6 +45,7 @@ import android.view.View;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.logging.MetricsLogger;
import com.android.internal.logging.nano.MetricsProto;
+import com.android.internal.util.Preconditions;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -326,7 +329,7 @@ public final class AutofillManager {
* @hide
*/
public AutofillManager(Context context, IAutoFillManager service) {
- mContext = context;
+ mContext = Preconditions.checkNotNull(context, "context cannot be null");
mService = service;
}
@@ -832,6 +835,13 @@ public final class AutofillManager {
return null;
}
+ private ComponentName getComponentNameFromContext() {
+ if (mContext instanceof Activity) {
+ return ((Activity) mContext).getComponentName();
+ }
+ return null;
+ }
+
/** @hide */
public void onAuthenticationResult(int authenticationId, Intent data) {
if (!hasAutofillFeature()) {
@@ -876,9 +886,14 @@ public final class AutofillManager {
}
try {
+ final ComponentName componentName = getComponentNameFromContext();
+ if (componentName == null) {
+ Log.w(TAG, "startSessionLocked(): context is not activity: " + mContext);
+ return;
+ }
mSessionId = mService.startSession(mContext.getActivityToken(),
mServiceClient.asBinder(), id, bounds, value, mContext.getUserId(),
- mCallback != null, flags, mContext.getOpPackageName());
+ mCallback != null, flags, componentName);
final AutofillClient client = getClientLocked();
if (client != null) {
client.autofillCallbackResetableStateAvailable();
@@ -929,9 +944,14 @@ public final class AutofillManager {
try {
if (restartIfNecessary) {
+ final ComponentName componentName = getComponentNameFromContext();
+ if (componentName == null) {
+ Log.w(TAG, "startSessionLocked(): context is not activity: " + mContext);
+ return;
+ }
final int newId = mService.updateOrRestartSession(mContext.getActivityToken(),
mServiceClient.asBinder(), id, bounds, value, mContext.getUserId(),
- mCallback != null, flags, mContext.getOpPackageName(), mSessionId, action);
+ mCallback != null, flags, componentName, mSessionId, action);
if (newId != mSessionId) {
if (sDebug) Log.d(TAG, "Session restarted: " + mSessionId + "=>" + newId);
mSessionId = newId;
diff --git a/core/java/android/view/autofill/IAutoFillManager.aidl b/core/java/android/view/autofill/IAutoFillManager.aidl
index 627afa7f8364..92dd5b1df30d 100644
--- a/core/java/android/view/autofill/IAutoFillManager.aidl
+++ b/core/java/android/view/autofill/IAutoFillManager.aidl
@@ -16,6 +16,7 @@
package android.view.autofill;
+import android.content.ComponentName;
import android.graphics.Rect;
import android.os.Bundle;
import android.os.IBinder;
@@ -34,14 +35,15 @@ interface IAutoFillManager {
int addClient(in IAutoFillManagerClient client, int userId);
int startSession(IBinder activityToken, in IBinder appCallback, in AutofillId autoFillId,
in Rect bounds, in AutofillValue value, int userId, boolean hasCallback, int flags,
- String packageName);
+ in ComponentName componentName);
FillEventHistory getFillEventHistory();
boolean restoreSession(int sessionId, in IBinder activityToken, in IBinder appCallback);
void updateSession(int sessionId, in AutofillId id, in Rect bounds,
in AutofillValue value, int action, int flags, int userId);
int updateOrRestartSession(IBinder activityToken, in IBinder appCallback,
in AutofillId autoFillId, in Rect bounds, in AutofillValue value, int userId,
- boolean hasCallback, int flags, String packageName, int sessionId, int action);
+ boolean hasCallback, int flags, in ComponentName componentName, int sessionId,
+ int action);
void finishSession(int sessionId, int userId);
void cancelSession(int sessionId, int userId);
void setAuthenticationResult(in Bundle data, int sessionId, int authenticationId, int userId);
diff --git a/proto/src/metrics_constants.proto b/proto/src/metrics_constants.proto
index 3dd75bbe9bbb..7877e41eee47 100644
--- a/proto/src/metrics_constants.proto
+++ b/proto/src/metrics_constants.proto
@@ -3972,6 +3972,19 @@ message MetricsEvent {
// OS: O
FIELD_NOTIFICATION_GROUP_SUMMARY = 947;
+ // An app attempted to forge a different component name in the AssisStructure that would be
+ // passed to the autofill service.
+ // OS: O (security patch)
+ // Package: Real package of the app being autofilled
+ // Tag FIELD_AUTOFILL_SERVICE: Package of the autofill service that processed the request
+ // TAG FIELD_AUTOFILL_FORGED_COMPONENT_NAME: Component name being forged
+ AUTOFILL_FORGED_COMPONENT_ATTEMPT = 948;
+
+ // FIELD - The component that an app tried tro forged.
+ // Type: string
+ // OS: O (security patch)
+ FIELD_AUTOFILL_FORGED_COMPONENT_NAME = 949;
+
// ---- End O Constants, all O constants go above this line ----
// Add new aosp constants above this line.
diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java
index cb91f9308b03..9ae4396116c4 100644
--- a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java
+++ b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java
@@ -519,25 +519,26 @@ public final class AutofillManagerService extends SystemService {
@Override
public int startSession(IBinder activityToken, IBinder appCallback, AutofillId autofillId,
Rect bounds, AutofillValue value, int userId, boolean hasCallback, int flags,
- String packageName) {
+ ComponentName componentName) {
activityToken = Preconditions.checkNotNull(activityToken, "activityToken");
appCallback = Preconditions.checkNotNull(appCallback, "appCallback");
autofillId = Preconditions.checkNotNull(autofillId, "autoFillId");
- packageName = Preconditions.checkNotNull(packageName, "packageName");
+ componentName = Preconditions.checkNotNull(componentName, "componentName");
+ final String packageName = Preconditions.checkNotNull(componentName.getPackageName());
Preconditions.checkArgument(userId == UserHandle.getUserId(getCallingUid()), "userId");
try {
mContext.getPackageManager().getPackageInfoAsUser(packageName, 0, userId);
} catch (PackageManager.NameNotFoundException e) {
- throw new IllegalArgumentException(packageName + " is not a valid package", e);
+ throw new IllegalArgumentException(componentName + " is not a valid package", e);
}
synchronized (mLock) {
final AutofillManagerServiceImpl service = getServiceForUserLocked(userId);
return service.startSessionLocked(activityToken, getCallingUid(), appCallback,
- autofillId, bounds, value, hasCallback, flags, packageName);
+ autofillId, bounds, value, hasCallback, flags, componentName);
}
}
@@ -589,7 +590,8 @@ public final class AutofillManagerService extends SystemService {
@Override
public int updateOrRestartSession(IBinder activityToken, IBinder appCallback,
AutofillId autoFillId, Rect bounds, AutofillValue value, int userId,
- boolean hasCallback, int flags, String packageName, int sessionId, int action) {
+ boolean hasCallback, int flags, ComponentName componentName, int sessionId,
+ int action) {
boolean restart = false;
synchronized (mLock) {
final AutofillManagerServiceImpl service = peekServiceForUserLocked(userId);
@@ -600,7 +602,7 @@ public final class AutofillManagerService extends SystemService {
}
if (restart) {
return startSession(activityToken, appCallback, autoFillId, bounds, value, userId,
- hasCallback, flags, packageName);
+ hasCallback, flags, componentName);
}
// Nothing changed...
diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
index 751c0547afd6..5831209ce54f 100644
--- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
+++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
@@ -32,8 +32,10 @@ import android.content.ComponentName;
import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
+import android.content.pm.PackageManager.NameNotFoundException;
import android.content.pm.ServiceInfo;
import android.graphics.Rect;
+import android.metrics.LogMaker;
import android.os.AsyncTask;
import android.os.Binder;
import android.os.Bundle;
@@ -61,6 +63,8 @@ import android.view.autofill.IAutoFillManagerClient;
import com.android.internal.R;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.logging.MetricsLogger;
+import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
import com.android.internal.os.HandlerCaller;
import com.android.server.autofill.ui.AutoFillUI;
@@ -87,6 +91,7 @@ final class AutofillManagerServiceImpl {
private final Context mContext;
private final Object mLock;
private final AutoFillUI mUi;
+ private final MetricsLogger mMetricsLogger = new MetricsLogger();
private RemoteCallbackList<IAutoFillManagerClient> mClients;
private AutofillServiceInfo mInfo;
@@ -168,6 +173,15 @@ final class AutofillManagerServiceImpl {
return null;
}
+ @Nullable
+ String getServicePackageName() {
+ final ComponentName serviceComponent = getServiceComponentName();
+ if (serviceComponent != null) {
+ return serviceComponent.getPackageName();
+ }
+ return null;
+ }
+
ComponentName getServiceComponentName() {
synchronized (mLock) {
if (mInfo == null) {
@@ -286,7 +300,7 @@ final class AutofillManagerServiceImpl {
int startSessionLocked(@NonNull IBinder activityToken, int uid,
@NonNull IBinder appCallbackToken, @NonNull AutofillId autofillId,
@NonNull Rect virtualBounds, @Nullable AutofillValue value, boolean hasCallback,
- int flags, @NonNull String packageName) {
+ int flags, @NonNull ComponentName componentName) {
if (!isEnabled()) {
return 0;
}
@@ -295,7 +309,7 @@ final class AutofillManagerServiceImpl {
pruneAbandonedSessionsLocked();
final Session newSession = createSessionByTokenLocked(activityToken, uid, appCallbackToken,
- hasCallback, packageName);
+ hasCallback, componentName);
if (newSession == null) {
return NO_SESSION;
}
@@ -378,7 +392,8 @@ final class AutofillManagerServiceImpl {
}
private Session createSessionByTokenLocked(@NonNull IBinder activityToken, int uid,
- @NonNull IBinder appCallbackToken, boolean hasCallback, @NonNull String packageName) {
+ @NonNull IBinder appCallbackToken, boolean hasCallback,
+ @NonNull ComponentName componentName) {
// use random ids so that one app cannot know that another app creates sessions
int sessionId;
int tries = 0;
@@ -392,15 +407,44 @@ final class AutofillManagerServiceImpl {
sessionId = sRandom.nextInt();
} while (sessionId == NO_SESSION || mSessions.indexOfKey(sessionId) >= 0);
+ assertCallerLocked(componentName);
+
final Session newSession = new Session(this, mUi, mContext, mHandlerCaller, mUserId, mLock,
sessionId, uid, activityToken, appCallbackToken, hasCallback,
- mInfo.getServiceInfo().getComponentName(), packageName);
+ mInfo.getServiceInfo().getComponentName(), componentName);
mSessions.put(newSession.id, newSession);
return newSession;
}
/**
+ * Asserts the component is owned by the caller.
+ */
+ private void assertCallerLocked(@NonNull ComponentName componentName) {
+ final PackageManager pm = mContext.getPackageManager();
+ final int callingUid = Binder.getCallingUid();
+ final int packageUid;
+ try {
+ packageUid = pm.getPackageUidAsUser(componentName.getPackageName(),
+ UserHandle.getCallingUserId());
+ } catch (NameNotFoundException e) {
+ throw new SecurityException("Could not verify UID for " + componentName);
+ }
+ if (callingUid != packageUid) {
+ final String[] packages = pm.getPackagesForUid(callingUid);
+ final String callingPackage = packages != null ? packages[0] : "uid-" + callingUid;
+ Slog.w(TAG, "App (package=" + callingPackage + ", UID=" + callingUid
+ + ") passed component (" + componentName + ") owned by UID " + packageUid);
+ mMetricsLogger.write(new LogMaker(MetricsEvent.AUTOFILL_FORGED_COMPONENT_ATTEMPT)
+ .setPackageName(callingPackage)
+ .addTaggedData(MetricsEvent.FIELD_AUTOFILL_SERVICE, getServicePackageName())
+ .addTaggedData(MetricsEvent.FIELD_AUTOFILL_FORGED_COMPONENT_NAME,
+ componentName == null ? "null" : componentName.flattenToShortString()));
+ throw new SecurityException("Invalid component: " + componentName);
+ }
+ }
+
+ /**
* Restores a session after an activity was temporarily destroyed.
*
* @param sessionId The id of the session to restore
diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java
index 72ad752caf19..2027a8e1f49b 100644
--- a/services/autofill/java/com/android/server/autofill/Session.java
+++ b/services/autofill/java/com/android/server/autofill/Session.java
@@ -119,8 +119,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
@GuardedBy("mLock")
@NonNull private IBinder mActivityToken;
- /** Package name of the app that is auto-filled */
- @NonNull private final String mPackageName;
+ /** Component that's being auto-filled */
+ @NonNull private final ComponentName mComponentName;
@GuardedBy("mLock")
private final ArrayMap<AutofillId, ViewState> mViewStates = new ArrayMap<>();
@@ -200,6 +200,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
structure.ensureData();
// Sanitize structure before it's sent to service.
+ final ComponentName componentNameFromApp = structure.getActivityComponent();
+ if (!mComponentName.equals(componentNameFromApp)) {
+ Slog.w(TAG, "Activity " + mComponentName + " forged different component on "
+ + "AssistStructure: " + componentNameFromApp);
+ structure.setActivityComponent(mComponentName);
+ final LogMaker log = (new LogMaker(
+ MetricsEvent.AUTOFILL_FORGED_COMPONENT_ATTEMPT))
+ .setPackageName(mComponentName.getPackageName())
+ .addTaggedData(MetricsEvent.FIELD_AUTOFILL_SERVICE,
+ mService.getPackageName())
+ .addTaggedData(MetricsEvent.FIELD_AUTOFILL_FORGED_COMPONENT_NAME,
+ componentNameFromApp == null ? "null"
+ : componentNameFromApp.flattenToShortString());
+ mMetricsLogger.write(log);
+ }
structure.sanitizeForParceling(true);
// Flags used to start the session.
@@ -345,20 +360,21 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
@NonNull Context context, @NonNull HandlerCaller handlerCaller, int userId,
@NonNull Object lock, int sessionId, int uid, @NonNull IBinder activityToken,
@NonNull IBinder client, boolean hasCallback,
- @NonNull ComponentName componentName, @NonNull String packageName) {
+ @NonNull ComponentName serviceComponentName, @NonNull ComponentName appComponentName) {
id = sessionId;
this.uid = uid;
mService = service;
mLock = lock;
mUi = ui;
mHandlerCaller = handlerCaller;
- mRemoteFillService = new RemoteFillService(context, componentName, userId, this);
+ mRemoteFillService = new RemoteFillService(context, serviceComponentName, userId, this);
mActivityToken = activityToken;
mHasCallback = hasCallback;
- mPackageName = packageName;
+ mComponentName = appComponentName;
mClient = IAutoFillManagerClient.Stub.asInterface(client);
- mMetricsLogger.action(MetricsEvent.AUTOFILL_SESSION_STARTED, mPackageName);
+ mMetricsLogger.action(MetricsEvent.AUTOFILL_SESSION_STARTED,
+ mComponentName.getPackageName());
}
/**
@@ -427,7 +443,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
final LogMaker log = (new LogMaker(MetricsEvent.AUTOFILL_REQUEST))
.setType(MetricsEvent.TYPE_SUCCESS)
- .setPackageName(mPackageName)
+ .setPackageName(mComponentName.getPackageName())
.addTaggedData(MetricsEvent.FIELD_AUTOFILL_NUM_DATASETS,
response.getDatasets() == null ? 0 : response.getDatasets().size())
.addTaggedData(MetricsEvent.FIELD_AUTOFILL_SERVICE,
@@ -449,7 +465,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
}
LogMaker log = (new LogMaker(MetricsEvent.AUTOFILL_REQUEST))
.setType(MetricsEvent.TYPE_FAILURE)
- .setPackageName(mPackageName)
+ .setPackageName(mComponentName.getPackageName())
.addTaggedData(MetricsEvent.FIELD_AUTOFILL_SERVICE, servicePackageName);
mMetricsLogger.write(log);
@@ -472,7 +488,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
LogMaker log = (new LogMaker(
MetricsEvent.AUTOFILL_DATA_SAVE_REQUEST))
.setType(MetricsEvent.TYPE_SUCCESS)
- .setPackageName(mPackageName)
+ .setPackageName(mComponentName.getPackageName())
.addTaggedData(MetricsEvent.FIELD_AUTOFILL_SERVICE, servicePackageName);
mMetricsLogger.write(log);
@@ -496,7 +512,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
LogMaker log = (new LogMaker(
MetricsEvent.AUTOFILL_DATA_SAVE_REQUEST))
.setType(MetricsEvent.TYPE_FAILURE)
- .setPackageName(mPackageName)
+ .setPackageName(mComponentName.getPackageName())
.addTaggedData(MetricsEvent.FIELD_AUTOFILL_SERVICE, servicePackageName);
mMetricsLogger.write(log);
@@ -691,7 +707,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
final Parcelable result = data.getParcelable(AutofillManager.EXTRA_AUTHENTICATION_RESULT);
if (result instanceof FillResponse) {
final FillResponse response = (FillResponse) result;
- mMetricsLogger.action(MetricsEvent.AUTOFILL_AUTHENTICATED, mPackageName);
+ mMetricsLogger.action(MetricsEvent.AUTOFILL_AUTHENTICATED,
+ mComponentName.getPackageName());
replaceResponseLocked(authenticatedResponse, response);
} else if (result instanceof Dataset) {
if (datasetIdx != AutofillManager.AUTHENTICATION_ID_DATASET_ID_UNDEFINED) {
@@ -834,8 +851,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
if (atLeastOneChanged) {
if (sDebug) Slog.d(TAG, "at least one field changed - showing save UI");
mService.setSaveShown(id);
- getUiForShowing().showSaveUi(mService.getServiceLabel(), saveInfo, mPackageName,
- this);
+ getUiForShowing().showSaveUi(mService.getServiceLabel(), saveInfo,
+ mComponentName.getPackageName(), this);
mIsSaving = true;
return false;
@@ -1143,7 +1160,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
filterText = value.getTextValue().toString();
}
- getUiForShowing().showFillUi(filledId, response, filterText, mPackageName, this);
+ getUiForShowing().showFillUi(filledId, response, filterText,
+ mComponentName.getPackageName(), this);
}
boolean isDestroyed() {
@@ -1408,14 +1426,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
@Override
public String toString() {
- return "Session: [id=" + id + ", pkg=" + mPackageName + "]";
+ return "Session: [id=" + id + ", pkg=" + mComponentName.getPackageName() + "]";
}
void dumpLocked(String prefix, PrintWriter pw) {
final String prefix2 = prefix + " ";
pw.print(prefix); pw.print("id: "); pw.println(id);
pw.print(prefix); pw.print("uid: "); pw.println(uid);
- pw.print(prefix); pw.print("mPackagename: "); pw.println(mPackageName);
+ pw.print(prefix); pw.print("mComponentName: "); pw.println(mComponentName);
pw.print(prefix); pw.print("mActivityToken: "); pw.println(mActivityToken);
pw.print(prefix); pw.print("mResponses: ");
if (mResponses == null) {
@@ -1519,7 +1537,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
hideAllUiIfOwnedByMe();
mUi.clearCallback(this);
mDestroyed = true;
- mMetricsLogger.action(MetricsEvent.AUTOFILL_SESSION_FINISHED, mPackageName);
+ mMetricsLogger.action(MetricsEvent.AUTOFILL_SESSION_FINISHED,
+ mComponentName.getPackageName());
return mRemoteFillService;
}