From 761d4a0ccf9528df41c8b7854b454d43cd740676 Mon Sep 17 00:00:00 2001 From: Daniel Chapin Date: Tue, 8 Feb 2022 00:57:38 +0000 Subject: [PATCH] Revert "Revert "Revert "User consent for Logcat data access""" This reverts commit ec8c5511afb4a1f6e1f1b300ef7ac5263a990cc1. Reason for revert: Droidfood Blocking Bug: 218267535 Change-Id: I313f3fb7fdfbefb1c62b474b09f5b14e6ca0afd5 --- .../android/app/ActivityManagerInternal.java | 8 - .../os/logcat/ILogcatManagerService.aidl | 2 - core/res/AndroidManifest.xml | 8 - core/res/res/values/strings.xml | 14 - core/res/res/values/symbols.xml | 5 - .../server/am/ActivityManagerService.java | 17 - .../logcat/LogAccessConfirmationActivity.java | 130 -------- .../server/logcat/LogcatManagerService.java | 312 +----------------- 8 files changed, 9 insertions(+), 487 deletions(-) delete mode 100644 services/core/java/com/android/server/logcat/LogAccessConfirmationActivity.java diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java index a58ceaa990226..cce7dd338b3dc 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -214,14 +214,6 @@ public abstract class ActivityManagerInternal { */ public abstract boolean isSystemReady(); - /** - * Returns package name given pid. - * - * @param pid The pid we are searching package name for. - */ - @Nullable - public abstract String getPackageNameByPid(int pid); - /** * Sets if the given pid has an overlay UI or not. * diff --git a/core/java/android/os/logcat/ILogcatManagerService.aidl b/core/java/android/os/logcat/ILogcatManagerService.aidl index 02db2749bbe8a..68b5679919d69 100644 --- a/core/java/android/os/logcat/ILogcatManagerService.aidl +++ b/core/java/android/os/logcat/ILogcatManagerService.aidl @@ -22,7 +22,5 @@ package android.os.logcat; interface ILogcatManagerService { void startThread(in int uid, in int gid, in int pid, in int fd); void finishThread(in int uid, in int gid, in int pid, in int fd); - void approve(in int uid, in int gid, in int pid, in int fd); - void decline(in int uid, in int gid, in int pid, in int fd); } diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index 9db715bfc4e1d..85504ceae791a 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -6540,14 +6540,6 @@ android:exported="false"> - - - Harmful app detected - - System log access request - - Only this time - - Don\u2019t allow - - - %s requests system logs for functional debugging. - These logs might contain information that apps and services on your device have written. - - - Don\u2019t show again - %1$s wants to show %2$s slices diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index bf918c6d8756a..c232a8a474bda 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -3857,11 +3857,6 @@ - - - - - diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 3054f4a7d7638..2f87e4f846a15 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -16086,23 +16086,6 @@ public class ActivityManagerService extends IActivityManager.Stub return mSystemReady; } - /** - * Returns package name by pid. - */ - @Override - @Nullable - public String getPackageNameByPid(int pid) { - synchronized (mPidsSelfLocked) { - final ProcessRecord app = mPidsSelfLocked.get(pid); - - if (app != null && app.info != null) { - return app.info.packageName; - } - - return null; - } - } - /** * Sets if the given pid has an overlay UI or not. * diff --git a/services/core/java/com/android/server/logcat/LogAccessConfirmationActivity.java b/services/core/java/com/android/server/logcat/LogAccessConfirmationActivity.java deleted file mode 100644 index 6b442a6a395e9..0000000000000 --- a/services/core/java/com/android/server/logcat/LogAccessConfirmationActivity.java +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.server.logcat; - -import android.content.Context; -import android.content.DialogInterface; -import android.content.Intent; -import android.content.IntentSender; -import android.os.Bundle; -import android.os.ServiceManager; -import android.os.logcat.ILogcatManagerService; -import android.util.Slog; -import android.view.View; -import android.widget.TextView; - -import com.android.internal.R; -import com.android.internal.app.AlertActivity; -import com.android.internal.app.AlertController; - - -/** - * This dialog is shown to the user before an activity in a harmful app is launched. - * - * See {@code PackageManager.setLogcatAppInfo} for more info. - */ -public class LogAccessConfirmationActivity extends AlertActivity implements - DialogInterface.OnClickListener { - private static final String TAG = LogAccessConfirmationActivity.class.getSimpleName(); - - private String mPackageName; - private IntentSender mTarget; - private final ILogcatManagerService mLogcatManagerService = - ILogcatManagerService.Stub.asInterface(ServiceManager.getService("logcat")); - - private int mUid; - private int mGid; - private int mPid; - private int mFd; - - private static final String EXTRA_UID = "uid"; - private static final String EXTRA_GID = "gid"; - private static final String EXTRA_PID = "pid"; - private static final String EXTRA_FD = "fd"; - - @Override - protected void onCreate(Bundle savedInstanceState) { - super.onCreate(savedInstanceState); - - final Intent intent = getIntent(); - mPackageName = intent.getStringExtra(Intent.EXTRA_PACKAGE_NAME); - mUid = intent.getIntExtra("uid", 0); - mGid = intent.getIntExtra("gid", 0); - mPid = intent.getIntExtra("pid", 0); - mFd = intent.getIntExtra("fd", 0); - - final AlertController.AlertParams p = mAlertParams; - p.mTitle = getString(R.string.log_access_confirmation_title); - p.mView = createView(); - - p.mPositiveButtonText = getString(R.string.log_access_confirmation_allow); - p.mPositiveButtonListener = this; - p.mNegativeButtonText = getString(R.string.log_access_confirmation_deny); - p.mNegativeButtonListener = this; - - mAlert.installContent(mAlertParams); - } - - private View createView() { - final View view = getLayoutInflater().inflate(R.layout.harmful_app_warning_dialog, - null /*root*/); - ((TextView) view.findViewById(R.id.app_name_text)) - .setText(mPackageName); - ((TextView) view.findViewById(R.id.message)) - .setText(getIntent().getExtras().getString("body")); - return view; - } - - @Override - public void onClick(DialogInterface dialog, int which) { - switch (which) { - case DialogInterface.BUTTON_POSITIVE: - try { - mLogcatManagerService.approve(mUid, mGid, mPid, mFd); - } catch (Throwable t) { - Slog.e(TAG, "Could not start the LogcatManagerService.", t); - } - finish(); - break; - case DialogInterface.BUTTON_NEGATIVE: - try { - mLogcatManagerService.decline(mUid, mGid, mPid, mFd); - } catch (Throwable t) { - Slog.e(TAG, "Could not start the LogcatManagerService.", t); - } - finish(); - break; - } - } - - /** - * Create the Intent for a LogAccessConfirmationActivity. - */ - public static Intent createIntent(Context context, String targetPackageName, - IntentSender target, int uid, int gid, int pid, int fd) { - final Intent intent = new Intent(); - intent.setClass(context, LogAccessConfirmationActivity.class); - intent.putExtra(Intent.EXTRA_PACKAGE_NAME, targetPackageName); - intent.putExtra(EXTRA_UID, uid); - intent.putExtra(EXTRA_GID, gid); - intent.putExtra(EXTRA_PID, pid); - intent.putExtra(EXTRA_FD, fd); - - return intent; - } - -} diff --git a/services/core/java/com/android/server/logcat/LogcatManagerService.java b/services/core/java/com/android/server/logcat/LogcatManagerService.java index 140c6d48b57b7..ff6372aec3bd9 100644 --- a/services/core/java/com/android/server/logcat/LogcatManagerService.java +++ b/services/core/java/com/android/server/logcat/LogcatManagerService.java @@ -16,36 +16,20 @@ package com.android.server.logcat; -import android.annotation.NonNull; -import android.app.ActivityManager; -import android.app.ActivityManager.RunningAppProcessInfo; -import android.app.ActivityManagerInternal; -import android.app.Notification; -import android.app.NotificationManager; -import android.app.PendingIntent; import android.content.Context; -import android.content.Intent; -import android.content.pm.PackageManager; -import android.content.pm.PackageManager.NameNotFoundException; import android.os.ILogd; import android.os.RemoteException; import android.os.ServiceManager; -import android.os.UserHandle; import android.os.logcat.ILogcatManagerService; import android.util.Slog; -import com.android.internal.R; -import com.android.internal.notification.SystemNotificationChannels; -import com.android.internal.util.ArrayUtils; -import com.android.server.LocalServices; import com.android.server.SystemService; -import java.util.Arrays; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; /** - * Service responsible for managing the access to Logcat. + * Service responsible for manage the access to Logcat. */ public final class LogcatManagerService extends SystemService { @@ -54,43 +38,6 @@ public final class LogcatManagerService extends SystemService { private final BinderService mBinderService; private final ExecutorService mThreadExecutor; private ILogd mLogdService; - private NotificationManager mNotificationManager; - private @NonNull ActivityManager mActivityManager; - private ActivityManagerInternal mActivityManagerInternal; - private static final int MAX_UID_IMPORTANCE_COUNT_LISTENER = 2; - private static int sUidImportanceListenerCount = 0; - private static final int AID_SHELL_UID = 2000; - - // TODO This allowlist is just a temporary workaround for the tests: - // FrameworksServicesTests - // PlatformRuleTests - // After adapting the test suites, the allowlist will be removed in - // the upcoming bug fix patches. - private static final String[] ALLOWABLE_TESTING_PACKAGES = { - "android.platform.test.rule.tests", - "com.android.frameworks.servicestests" - }; - - // TODO Same as the above ALLOWABLE_TESTING_PACKAGES. - private boolean isAllowableTestingPackage(int uid) { - PackageManager pm = mContext.getPackageManager(); - - String[] packageNames = pm.getPackagesForUid(uid); - - if (ArrayUtils.isEmpty(packageNames)) { - return false; - } - - for (String name : packageNames) { - Slog.e(TAG, "isAllowableTestingPackage: " + name); - - if (Arrays.asList(ALLOWABLE_TESTING_PACKAGES).contains(name)) { - return true; - } - } - - return false; - }; private final class BinderService extends ILogcatManagerService.Stub { @Override @@ -104,197 +51,6 @@ public final class LogcatManagerService extends SystemService { // the logd data access is finished. mThreadExecutor.execute(new LogdMonitor(uid, gid, pid, fd, false)); } - - @Override - public void approve(int uid, int gid, int pid, int fd) { - try { - getLogdService().approve(uid, gid, pid, fd); - } catch (RemoteException e) { - e.printStackTrace(); - } - } - - @Override - public void decline(int uid, int gid, int pid, int fd) { - try { - getLogdService().decline(uid, gid, pid, fd); - } catch (RemoteException e) { - e.printStackTrace(); - } - } - } - - private ILogd getLogdService() { - synchronized (LogcatManagerService.this) { - if (mLogdService == null) { - LogcatManagerService.this.addLogdService(); - } - return mLogdService; - } - } - - private String getBodyString(Context context, String callingPackage, int uid) { - PackageManager pm = context.getPackageManager(); - try { - return context.getString( - com.android.internal.R.string.log_access_confirmation_body, - pm.getApplicationInfoAsUser(callingPackage, PackageManager.MATCH_DIRECT_BOOT_AUTO, - UserHandle.getUserId(uid)).loadLabel(pm)); - } catch (NameNotFoundException e) { - // App name is unknown. - return null; - } - } - - private void sendNotification(int notificationId, String clientInfo, int uid, int gid, int pid, - int fd) { - - final ActivityManagerInternal activityManagerInternal = - LocalServices.getService(ActivityManagerInternal.class); - - PackageManager pm = mContext.getPackageManager(); - String packageName = activityManagerInternal.getPackageNameByPid(pid); - if (packageName != null) { - String notificationBody = getBodyString(mContext, packageName, uid); - - final Intent mIntent = LogAccessConfirmationActivity.createIntent(mContext, - packageName, null, uid, gid, pid, fd); - - if (notificationBody == null) { - // Decline the logd access if the nofitication body is unknown - Slog.e(TAG, "Unknown notification body, declining the logd access"); - declineLogdAccess(uid, gid, pid, fd); - return; - } - - // TODO Next version will replace notification with dialogue - // per UX guidance. - generateNotificationWithBodyContent(notificationId, clientInfo, notificationBody, - mIntent); - return; - - } - - String[] packageNames = pm.getPackagesForUid(uid); - - if (ArrayUtils.isEmpty(packageNames)) { - // Decline the logd access if the app name is unknown - Slog.e(TAG, "Unknown calling package name, declining the logd access"); - declineLogdAccess(uid, gid, pid, fd); - return; - } - - String firstPackageName = packageNames[0]; - - if (firstPackageName == null || firstPackageName.length() == 0) { - // Decline the logd access if the package name from uid is unknown - Slog.e(TAG, "Unknown calling package name, declining the logd access"); - declineLogdAccess(uid, gid, pid, fd); - return; - } - - String notificationBody = getBodyString(mContext, firstPackageName, uid); - - final Intent mIntent = LogAccessConfirmationActivity.createIntent(mContext, - firstPackageName, null, uid, gid, pid, fd); - - if (notificationBody == null) { - Slog.e(TAG, "Unknown notification body, declining the logd access"); - declineLogdAccess(uid, gid, pid, fd); - return; - } - - // TODO Next version will replace notification with dialogue - // per UX guidance. - generateNotificationWithBodyContent(notificationId, clientInfo, - notificationBody, mIntent); - } - - private void declineLogdAccess(int uid, int gid, int pid, int fd) { - try { - getLogdService().decline(uid, gid, pid, fd); - } catch (RemoteException ex) { - Slog.e(TAG, "Fails to call remote functions ", ex); - } - } - - private void generateNotificationWithBodyContent(int notificationId, String clientInfo, - String notificationBody, Intent intent) { - final Notification.Builder notificationBuilder = new Notification.Builder( - mContext, - SystemNotificationChannels.ACCESSIBILITY_SECURITY_POLICY); - intent.setFlags( - Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TASK); - intent.setIdentifier(String.valueOf(notificationId) + clientInfo); - intent.putExtra("body", notificationBody); - - notificationBuilder - .setSmallIcon(R.drawable.ic_info) - .setContentTitle( - mContext.getString(R.string.log_access_confirmation_title)) - .setContentText(notificationBody) - .setContentIntent( - PendingIntent.getActivity(mContext, 0, intent, - PendingIntent.FLAG_IMMUTABLE)) - .setTicker(mContext.getString(R.string.log_access_confirmation_title)) - .setOnlyAlertOnce(true) - .setAutoCancel(true); - mNotificationManager.notify(notificationId, notificationBuilder.build()); - } - - /** - * A class which watches an uid for background access and notifies the logdMonitor when - * the package status becomes foreground (importance change) - */ - private class UidImportanceListener implements ActivityManager.OnUidImportanceListener { - private final int mExpectedUid; - private final int mExpectedGid; - private final int mExpectedPid; - private final int mExpectedFd; - private int mExpectedImportance; - private int mCurrentImportance = RunningAppProcessInfo.IMPORTANCE_GONE; - - UidImportanceListener(int uid, int gid, int pid, int fd, int importance) { - mExpectedUid = uid; - mExpectedGid = gid; - mExpectedPid = pid; - mExpectedFd = fd; - mExpectedImportance = importance; - } - - @Override - public void onUidImportance(int uid, int importance) { - if (uid == mExpectedUid) { - mCurrentImportance = importance; - - /** - * 1) If the process status changes to foreground, send a notification - * for user consent. - * 2) If the process status remains background, we decline logd access request. - **/ - if (importance <= RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE) { - String clientInfo = getClientInfo(uid, mExpectedGid, mExpectedPid, mExpectedFd); - sendNotification(0, clientInfo, uid, mExpectedGid, mExpectedPid, - mExpectedFd); - mActivityManager.removeOnUidImportanceListener(this); - - synchronized (LogcatManagerService.this) { - sUidImportanceListenerCount--; - } - } else { - try { - getLogdService().decline(uid, mExpectedGid, mExpectedPid, mExpectedFd); - } catch (RemoteException ex) { - Slog.e(TAG, "Fails to call remote functions ", ex); - } - } - } - } - } - - private static String getClientInfo(int uid, int gid, int pid, int fd) { - return "UID=" + Integer.toString(uid) + " GID=" + Integer.toString(gid) + " PID=" - + Integer.toString(pid) + " FD=" + Integer.toString(fd); } private class LogdMonitor implements Runnable { @@ -318,7 +74,9 @@ public final class LogcatManagerService extends SystemService { } /** - * LogdMonitor generates a prompt for users. + * The current version grant the permission by default. + * And track the logd access. + * The next version will generate a prompt for users. * The users decide whether the logd access is allowed. */ @Override @@ -328,61 +86,10 @@ public final class LogcatManagerService extends SystemService { } if (mStart) { - - // TODO See the comments of ALLOWABLE_TESTING_PACKAGES. - if (isAllowableTestingPackage(mUid)) { - try { - getLogdService().approve(mUid, mGid, mPid, mFd); - } catch (RemoteException e) { - e.printStackTrace(); - } - return; - } - - // If the access request is coming from adb shell, approve the logd access - if (mUid == AID_SHELL_UID) { - try { - getLogdService().approve(mUid, mGid, mPid, mFd); - } catch (RemoteException e) { - e.printStackTrace(); - } - return; - } - - final int procState = LocalServices.getService(ActivityManagerInternal.class) - .getUidProcessState(mUid); - // If the process is foreground, send a notification for user consent - if (procState <= ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE) { - String clientInfo = getClientInfo(mUid, mGid, mPid, mFd); - sendNotification(0, clientInfo, mUid, mGid, mPid, mFd); - } else { - /** - * If the process is background, add a background process change listener and - * monitor if the process status changes. - * To avoid clients registering multiple listeners, we limit the number of - * maximum listeners to MAX_UID_IMPORTANCE_COUNT_LISTENER. - **/ - if (mActivityManager == null) { - return; - } - - synchronized (LogcatManagerService.this) { - if (sUidImportanceListenerCount < MAX_UID_IMPORTANCE_COUNT_LISTENER) { - // Trigger addOnUidImportanceListener when there is an update from - // the importance of the process - mActivityManager.addOnUidImportanceListener(new UidImportanceListener( - mUid, mGid, mPid, mFd, - RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE), - RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE); - sUidImportanceListenerCount++; - } else { - try { - getLogdService().decline(mUid, mGid, mPid, mFd); - } catch (RemoteException e) { - e.printStackTrace(); - } - } - } + try { + mLogdService.approve(mUid, mGid, mPid, mFd); + } catch (RemoteException ex) { + Slog.e(TAG, "Fails to call remote functions ", ex); } } } @@ -393,8 +100,6 @@ public final class LogcatManagerService extends SystemService { mContext = context; mBinderService = new BinderService(); mThreadExecutor = Executors.newCachedThreadPool(); - mActivityManager = context.getSystemService(ActivityManager.class); - mNotificationManager = mContext.getSystemService(NotificationManager.class); } @Override @@ -409,4 +114,5 @@ public final class LogcatManagerService extends SystemService { private void addLogdService() { mLogdService = ILogd.Stub.asInterface(ServiceManager.getService("logd")); } + }