From ec8c5511afb4a1f6e1f1b300ef7ac5263a990cc1 Mon Sep 17 00:00:00 2001 From: Wenhao Wang Date: Wed, 2 Feb 2022 13:20:00 -0800 Subject: [PATCH] Revert "Revert "User consent for Logcat data access"" This reverts commit d7138b445c54248b50dd879d08798e71cc1e94b4. This patch fixes the breaking caused by commit b4f8ed9bb8a94575989d689d37be523ee99bc589. It's a workaround patch. The upcoming patch will have solid fix. Bug: 193708292 Bug: 217482811 Bug: 217495147 Test: atest PlatformRuleTests -- --abi x86_64 Test: atest FrameworksServicesTests -- --abi x86_64 Ignore-AOSP-First: pending fix for logcat privacy issue Change-Id: If18086130599f0ae027d16ccbb7b5b5cd73f2e72 --- .../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, 487 insertions(+), 9 deletions(-) create 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 cce7dd338b3dc..a58ceaa990226 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -214,6 +214,14 @@ 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 68b5679919d69..02db2749bbe8a 100644 --- a/core/java/android/os/logcat/ILogcatManagerService.aidl +++ b/core/java/android/os/logcat/ILogcatManagerService.aidl @@ -22,5 +22,7 @@ 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 85504ceae791a..9db715bfc4e1d 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -6540,6 +6540,14 @@ 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 c232a8a474bda..bf918c6d8756a 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -3857,6 +3857,11 @@ + + + + + diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 2f87e4f846a15..3054f4a7d7638 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -16086,6 +16086,23 @@ 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 new file mode 100644 index 0000000000000..6b442a6a395e9 --- /dev/null +++ b/services/core/java/com/android/server/logcat/LogAccessConfirmationActivity.java @@ -0,0 +1,130 @@ +/* + * 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 ff6372aec3bd9..140c6d48b57b7 100644 --- a/services/core/java/com/android/server/logcat/LogcatManagerService.java +++ b/services/core/java/com/android/server/logcat/LogcatManagerService.java @@ -16,20 +16,36 @@ 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 manage the access to Logcat. + * Service responsible for managing the access to Logcat. */ public final class LogcatManagerService extends SystemService { @@ -38,6 +54,43 @@ 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 @@ -51,6 +104,197 @@ 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 { @@ -74,9 +318,7 @@ public final class LogcatManagerService extends SystemService { } /** - * The current version grant the permission by default. - * And track the logd access. - * The next version will generate a prompt for users. + * LogdMonitor generates a prompt for users. * The users decide whether the logd access is allowed. */ @Override @@ -86,10 +328,61 @@ public final class LogcatManagerService extends SystemService { } if (mStart) { - try { - mLogdService.approve(mUid, mGid, mPid, mFd); - } catch (RemoteException ex) { - Slog.e(TAG, "Fails to call remote functions ", ex); + + // 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(); + } + } + } } } } @@ -100,6 +393,8 @@ 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 @@ -114,5 +409,4 @@ public final class LogcatManagerService extends SystemService { private void addLogdService() { mLogdService = ILogd.Stub.asInterface(ServiceManager.getService("logd")); } - }