From 14508dc0359bed47f2b77e8da160226e33dbf742 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Thu, 7 Aug 2025 11:27:00 -0700 Subject: [PATCH 1/5] Replace queryIntentServices() hack with SystemApis.createContextAsUser() Use a createContextAsUser() wrapper to make the call to PackageManager's resolveService() look the same in both the same-user and cross-user cases. This is how the Android team recommends accessing XXXAsUser() APIs in general (See b/72863821#4). Will convert bindServiceAsUser() too in a follow up PR. --- .../grpc/binder/internal/ServiceBinding.java | 67 +++++-------------- .../binder/internal/ServiceBindingTest.java | 9 +++ 2 files changed, 27 insertions(+), 49 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java index f0cbe9ec56b..115555e49fa 100644 --- a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java +++ b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java @@ -17,6 +17,7 @@ package io.grpc.binder.internal; import static com.google.common.base.Preconditions.checkState; +import static io.grpc.binder.internal.SystemApis.createContextAsUser; import android.app.admin.DevicePolicyManager; import android.content.ComponentName; @@ -32,13 +33,10 @@ import androidx.annotation.AnyThread; import androidx.annotation.MainThread; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.VerifyException; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.binder.BinderChannelCredentials; -import java.lang.reflect.Method; -import java.util.List; import java.util.concurrent.Executor; import java.util.logging.Level; import java.util.logging.Logger; @@ -93,8 +91,6 @@ public String methodName() { private final Observer observer; private final Executor mainThreadExecutor; - private static volatile Method queryIntentServicesAsUserMethod; - @GuardedBy("this") private State state; @@ -257,51 +253,10 @@ void unbindInternal(Status reason) { } } - // Sadly the PackageManager#resolveServiceAsUser() API we need isn't part of the SDK or even a - // @SystemApi as of this writing. Modern Android prevents even system apps from calling it, by any - // means (https://developer.android.com/guide/app-compatibility/restrictions-non-sdk-interfaces). - // So instead we call queryIntentServicesAsUser(), which does more than we need but *is* a - // @SystemApi in all the SDK versions where we support cross-user Channels. - @Nullable - private static ResolveInfo resolveServiceAsUser( - PackageManager packageManager, Intent intent, int flags, UserHandle targetUserHandle) { - List results = - queryIntentServicesAsUser(packageManager, intent, flags, targetUserHandle); - // The first query result is "what would be returned by resolveService", per the javadoc. - return (results != null && !results.isEmpty()) ? results.get(0) : null; - } - - // The cross-user Channel feature requires the client to be a system app so we assume @SystemApi - // queryIntentServicesAsUser() is visible to us at runtime. It would be visible at build time too, - // if our host system app were written to call it directly. We only have to use reflection here - // because grpc-java is a library built outside the Android source tree where the compiler can't - // see the "non-SDK" @SystemApis that we need. - @Nullable - @SuppressWarnings("unchecked") // Safe by PackageManager#queryIntentServicesAsUser spec in AOSP. - private static List queryIntentServicesAsUser( - PackageManager packageManager, Intent intent, int flags, UserHandle targetUserHandle) { - try { - if (queryIntentServicesAsUserMethod == null) { - synchronized (ServiceBinding.class) { - if (queryIntentServicesAsUserMethod == null) { - queryIntentServicesAsUserMethod = - PackageManager.class.getMethod( - "queryIntentServicesAsUser", Intent.class, int.class, UserHandle.class); - } - } - } - return (List) - queryIntentServicesAsUserMethod.invoke(packageManager, intent, flags, targetUserHandle); - } catch (ReflectiveOperationException e) { - throw new VerifyException(e); - } - } - @AnyThread @Override public ServiceInfo resolve() throws StatusException { checkState(sourceContext != null); - PackageManager packageManager = sourceContext.getPackageManager(); int flags = 0; if (Build.VERSION.SDK_INT >= 29) { // Filter out non-'directBootAware' s when 'targetUserHandle' is locked. Here's why: @@ -311,9 +266,9 @@ public ServiceInfo resolve() throws StatusException { flags |= PackageManager.MATCH_DIRECT_BOOT_AUTO; } ResolveInfo resolveInfo = - targetUserHandle != null - ? resolveServiceAsUser(packageManager, bindIntent, flags, targetUserHandle) - : packageManager.resolveService(bindIntent, flags); + getContextForTargetUser(sourceContext, targetUserHandle) + .getPackageManager() + .resolveService(bindIntent, flags); if (resolveInfo == null) { throw Status.UNIMPLEMENTED // Same status code as when bindService() returns false. .withDescription("resolveService(" + bindIntent + " / " + targetUserHandle + ") was null") @@ -322,6 +277,20 @@ public ServiceInfo resolve() throws StatusException { return resolveInfo.serviceInfo; } + private static Context getContextForTargetUser(Context context, UserHandle targetUser) + throws StatusException { + if (targetUser == null) { + return context; + } + try { + return createContextAsUser(context, targetUser, /* flags= */ 0); // @SystemApi since R. + } catch (ReflectiveOperationException e) { + throw Status.INTERNAL + .withDescription("Cross-user pre-auth requires SDK_INT >= R and @SystemApi visibility") + .asException(); + } + } + @MainThread private void clearReferences() { sourceContext = null; diff --git a/binder/src/test/java/io/grpc/binder/internal/ServiceBindingTest.java b/binder/src/test/java/io/grpc/binder/internal/ServiceBindingTest.java index bd51c522d15..223fa0a4f85 100644 --- a/binder/src/test/java/io/grpc/binder/internal/ServiceBindingTest.java +++ b/binder/src/test/java/io/grpc/binder/internal/ServiceBindingTest.java @@ -303,6 +303,15 @@ public void testResolveWithTargetUserHandle() throws Exception { assertThat(resolvedServiceInfo.processName).isEqualTo(serviceInfo.processName); } + @Test + @Config(sdk = 29) + public void testResolveWithUnsupportedTargetUserHandle() throws Exception { + binding = newBuilder().setTargetUserHandle(generateUserHandle(/* userId= */ 0)).build(); + StatusException statusException = assertThrows(StatusException.class, binding::resolve); + assertThat(statusException.getStatus().getCode()).isEqualTo(Code.INTERNAL); + assertThat(statusException.getStatus().getDescription()).contains("SDK_INT >= R"); + } + @Test public void testResolveNonExistentServiceThrows() throws Exception { ComponentName doesNotExistService = new ComponentName("does.not.exist", "NoService"); From 1f10fdcfb9240c4b3a5ab11557b77e35bd9cbbd2 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Fri, 15 Aug 2025 22:34:57 -0700 Subject: [PATCH 2/5] Add TODO --- .../src/main/java/io/grpc/binder/internal/ServiceBinding.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java index 45bf75e770a..eee11c0ea38 100644 --- a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java +++ b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java @@ -190,8 +190,10 @@ private static Status bindInternal( break; case BIND_SERVICE_AS_USER: if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { + // We don't need SystemApis because bindServiceAsUser() is simply public in R+. bindResult = context.bindServiceAsUser(bindIntent, conn, flags, targetUserHandle); } else { + // TODO(jdcormie): Use SystemApis to make this work pre-R. return Status.INTERNAL.withDescription("Cross user Channel requires Android R+"); } break; From 0ea470e2f566c299ff8079cf64982a0c3f952f84 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Fri, 15 Aug 2025 23:02:53 -0700 Subject: [PATCH 3/5] Refactor for ease of use later with bindServiceAsUser --- .../grpc/binder/internal/ServiceBinding.java | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java index eee11c0ea38..fc038c8a9d6 100644 --- a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java +++ b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java @@ -267,7 +267,6 @@ void unbindInternal(Status reason) { @AnyThread @Override public ServiceInfo resolve() throws StatusException { - checkState(sourceContext != null); int flags = 0; if (Build.VERSION.SDK_INT >= 29) { // Filter out non-'directBootAware' s when 'targetUserHandle' is locked. Here's why: @@ -276,10 +275,16 @@ public ServiceInfo resolve() throws StatusException { // 'directBootAware'-ness. This flag explicitly tells resolveService() to act the same way. flags |= PackageManager.MATCH_DIRECT_BOOT_AUTO; } + Context targetUserContext; + try { + targetUserContext = getContextForTargetUser(); + } catch (ReflectiveOperationException e) { + throw Status.INTERNAL + .withDescription("Cross-user pre-auth requires SDK_INT >= R and @SystemApi visibility") + .asException(); + } ResolveInfo resolveInfo = - getContextForTargetUser(sourceContext, targetUserHandle) - .getPackageManager() - .resolveService(bindIntent, flags); + targetUserContext.getPackageManager().resolveService(bindIntent, flags); if (resolveInfo == null) { throw Status.UNIMPLEMENTED // Same status code as when bindService() returns false. .withDescription("resolveService(" + bindIntent + " / " + targetUserHandle + ") was null") @@ -288,18 +293,12 @@ public ServiceInfo resolve() throws StatusException { return resolveInfo.serviceInfo; } - private static Context getContextForTargetUser(Context context, UserHandle targetUser) - throws StatusException { - if (targetUser == null) { - return context; - } - try { - return createContextAsUser(context, targetUser, /* flags= */ 0); // @SystemApi since R. - } catch (ReflectiveOperationException e) { - throw Status.INTERNAL - .withDescription("Cross-user pre-auth requires SDK_INT >= R and @SystemApi visibility") - .asException(); - } + private Context getContextForTargetUser() throws ReflectiveOperationException { + checkState(sourceContext != null); + return targetUserHandle == null + ? sourceContext + : createContextAsUser( + sourceContext, targetUserHandle, /* flags= */ 0); // @SystemApi since R. } @MainThread From 187e5fa13922182083f7dfe9c704094f3ca33e89 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Thu, 28 Aug 2025 07:09:38 +0100 Subject: [PATCH 4/5] update TODO --- .../src/main/java/io/grpc/binder/internal/ServiceBinding.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java index fc038c8a9d6..8720fa5a231 100644 --- a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java +++ b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java @@ -193,7 +193,7 @@ private static Status bindInternal( // We don't need SystemApis because bindServiceAsUser() is simply public in R+. bindResult = context.bindServiceAsUser(bindIntent, conn, flags, targetUserHandle); } else { - // TODO(jdcormie): Use SystemApis to make this work pre-R. + // TODO(#12279): Use SystemApis to make this work pre-R. return Status.INTERNAL.withDescription("Cross user Channel requires Android R+"); } break; @@ -294,7 +294,7 @@ public ServiceInfo resolve() throws StatusException { } private Context getContextForTargetUser() throws ReflectiveOperationException { - checkState(sourceContext != null); + checkState(sourceContext != null, "Already unbound!"); return targetUserHandle == null ? sourceContext : createContextAsUser( From 945164aa03a6b95a6d5cfdfa53934f4f1cf5f160 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Thu, 28 Aug 2025 09:01:33 +0100 Subject: [PATCH 5/5] move try/catch into getContextForTargetUser --- .../grpc/binder/internal/ServiceBinding.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java index 8720fa5a231..1d77ca8e1bc 100644 --- a/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java +++ b/binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java @@ -275,16 +275,10 @@ public ServiceInfo resolve() throws StatusException { // 'directBootAware'-ness. This flag explicitly tells resolveService() to act the same way. flags |= PackageManager.MATCH_DIRECT_BOOT_AUTO; } - Context targetUserContext; - try { - targetUserContext = getContextForTargetUser(); - } catch (ReflectiveOperationException e) { - throw Status.INTERNAL - .withDescription("Cross-user pre-auth requires SDK_INT >= R and @SystemApi visibility") - .asException(); - } ResolveInfo resolveInfo = - targetUserContext.getPackageManager().resolveService(bindIntent, flags); + getContextForTargetUser("Cross-user pre-auth") + .getPackageManager() + .resolveService(bindIntent, flags); if (resolveInfo == null) { throw Status.UNIMPLEMENTED // Same status code as when bindService() returns false. .withDescription("resolveService(" + bindIntent + " / " + targetUserHandle + ") was null") @@ -293,12 +287,17 @@ public ServiceInfo resolve() throws StatusException { return resolveInfo.serviceInfo; } - private Context getContextForTargetUser() throws ReflectiveOperationException { + private Context getContextForTargetUser(String purpose) throws StatusException { checkState(sourceContext != null, "Already unbound!"); - return targetUserHandle == null - ? sourceContext - : createContextAsUser( - sourceContext, targetUserHandle, /* flags= */ 0); // @SystemApi since R. + try { + return targetUserHandle == null + ? sourceContext + : createContextAsUser(sourceContext, targetUserHandle, /* flags= */ 0); + } catch (ReflectiveOperationException e) { + throw Status.INTERNAL + .withDescription(purpose + " requires SDK_INT >= R and @SystemApi visibility") + .asException(); + } } @MainThread