From e2d078ef78d5ff1135479df727099f32fddff56f Mon Sep 17 00:00:00 2001 From: Robert Horvath Date: Thu, 2 Jun 2022 12:42:04 +0200 Subject: [PATCH] LogcatManagerService: Find packageName through parent PIDs Log access requests may come from forked processes, eg. by a logcat process started by an app. In such cases, `getPackageNameByPid` failed to return a package name, as the process (eg. logcat) does not have a package name associated with it. As a fallback we used the first package name returned by `PackageManager#getPackagesForUid`, but this method returns package names in an indeterministic order, so the same app doing multiple log accesses could trigger multiple log access dialogs if the order differed. To find a better package name match, this change walks up the process tree to find a package name that forked the requesting process and matches the requesting UID. As a fallback, the list returned by `PackageManager#getPackagesForUid` is sorted before picking the first element. Bug: 229279048 Test: Manual, sending feedback through assistant Test: atest FrameworksServicesTests:LogcatManagerServiceTest Change-Id: I7ac044ac012be8c3911f3c4ce35c06215d078fe6 --- .../server/logcat/LogcatManagerService.java | 27 ++++++++++++------ .../logcat/LogcatManagerServiceTest.java | 28 ++++++++++++++++++- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/logcat/LogcatManagerService.java b/services/core/java/com/android/server/logcat/LogcatManagerService.java index 1bcc21e66302b..d61d321ad0f76 100644 --- a/services/core/java/com/android/server/logcat/LogcatManagerService.java +++ b/services/core/java/com/android/server/logcat/LogcatManagerService.java @@ -16,6 +16,8 @@ package com.android.server.logcat; +import static android.os.Process.getParentPid; + import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -45,6 +47,7 @@ import com.android.server.SystemService; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Objects; @@ -340,13 +343,6 @@ public final class LogcatManagerService extends SystemService { * access */ private String getPackageName(LogAccessRequest request) { - if (mActivityManagerInternal != null) { - String packageName = mActivityManagerInternal.getPackageNameByPid(request.mPid); - if (packageName != null) { - return packageName; - } - } - PackageManager pm = mContext.getPackageManager(); if (pm == null) { // Decline the logd access if PackageManager is null @@ -355,15 +351,28 @@ public final class LogcatManagerService extends SystemService { } String[] packageNames = pm.getPackagesForUid(request.mUid); - 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"); return null; } - String firstPackageName = packageNames[0]; + if (mActivityManagerInternal != null) { + int pid = request.mPid; + String packageName = mActivityManagerInternal.getPackageNameByPid(pid); + while ((packageName == null || !ArrayUtils.contains(packageNames, packageName)) + && pid != -1) { + pid = getParentPid(pid); + packageName = mActivityManagerInternal.getPackageNameByPid(pid); + } + if (packageName != null && ArrayUtils.contains(packageNames, packageName)) { + return packageName; + } + } + + Arrays.sort(packageNames); + String firstPackageName = packageNames[0]; if (firstPackageName == null || firstPackageName.isEmpty()) { // Decline the logd access if the package name from uid is unknown Slog.e(TAG, "Unknown calling package name, declining the logd access"); diff --git a/services/tests/servicestests/src/com/android/server/logcat/LogcatManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/logcat/LogcatManagerServiceTest.java index f33001774263e..6a27f391ac5bc 100644 --- a/services/tests/servicestests/src/com/android/server/logcat/LogcatManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/logcat/LogcatManagerServiceTest.java @@ -16,6 +16,8 @@ package com.android.server.logcat; +import static android.os.Process.INVALID_UID; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; @@ -28,6 +30,7 @@ import static org.mockito.Mockito.when; import android.app.ActivityManager; import android.app.ActivityManagerInternal; import android.content.ContextWrapper; +import android.content.pm.PackageManager; import android.os.ILogd; import android.os.Looper; import android.os.UserHandle; @@ -69,6 +72,8 @@ public class LogcatManagerServiceTest { @Mock private ActivityManagerInternal mActivityManagerInternalMock; @Mock + private PackageManager mPackageManagerMock; + @Mock private ILogd mLogdMock; private LogcatManagerService mService; @@ -81,10 +86,17 @@ public class LogcatManagerServiceTest { public void setUp() throws Exception { MockitoAnnotations.initMocks(this); addLocalServiceMock(ActivityManagerInternal.class, mActivityManagerInternalMock); + when(mActivityManagerInternalMock.getInstrumentationSourceUid(anyInt())) + .thenReturn(INVALID_UID); + mContextSpy = spy(new ContextWrapper(ApplicationProvider.getApplicationContext())); mClock = new OffsettableClock.Stopped(); mTestLooper = new TestLooper(mClock::now); - + when(mContextSpy.getPackageManager()).thenReturn(mPackageManagerMock); + when(mPackageManagerMock.getPackagesForUid(APP1_UID)).thenReturn( + new String[]{APP1_PACKAGE_NAME}); + when(mPackageManagerMock.getPackagesForUid(APP2_UID)).thenReturn( + new String[]{APP2_PACKAGE_NAME}); when(mActivityManagerInternalMock.getPackageNameByPid(APP1_PID)).thenReturn( APP1_PACKAGE_NAME); when(mActivityManagerInternalMock.getPackageNameByPid(APP2_PID)).thenReturn( @@ -135,6 +147,20 @@ public class LogcatManagerServiceTest { verify(mContextSpy, never()).startActivityAsUser(any(), any()); } + @Test + public void test_RequestFromBackground_ApprovedIfInstrumented() throws Exception { + when(mActivityManagerInternalMock.getInstrumentationSourceUid(APP1_UID)) + .thenReturn(APP1_UID); + when(mActivityManagerInternalMock.getUidProcessState(APP1_UID)).thenReturn( + ActivityManager.PROCESS_STATE_RECEIVER); + mService.getBinderService().startThread(APP1_UID, APP1_GID, APP1_PID, FD1); + mTestLooper.dispatchAll(); + + verify(mLogdMock).approve(APP1_UID, APP1_GID, APP1_PID, FD1); + verify(mLogdMock, never()).decline(APP1_UID, APP1_GID, APP1_PID, FD1); + verify(mContextSpy, never()).startActivityAsUser(any(), any()); + } + @Test public void test_RequestFromForegroundService_DeclinedWithoutPrompt() throws Exception { when(mActivityManagerInternalMock.getUidProcessState(APP1_UID)).thenReturn(