binder: Introduce server authorization strategy v2

Adds support for android:isolatedProcess Services (fixing #12293) and moves all peer authorization checks to the handshake, allowing subsequent transactions to go unchecked.

Author: jdcormie

binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java

@@ -97,6 +97,7 @@ public final class BinderChannelSmokeTest {
  .setType(MethodDescriptor.MethodType.BIDI_STREAMING)
  .build();
 
+ AndroidComponentAddress serverAddress;
  ManagedChannel channel;
  AtomicReference<Metadata> headersCapture = new AtomicReference<>();
  AtomicReference<PeerUid> clientUidCapture = new AtomicReference<>();
@@ -134,7 +135,7 @@ public void setUp() throws Exception {
  TestUtils.recordRequestHeadersInterceptor(headersCapture),
  PeerUids.newPeerIdentifyingServerInterceptor());
 
- AndroidComponentAddress serverAddress = HostServices.allocateService(appContext);
+ serverAddress = HostServices.allocateService(appContext);
  HostServices.configureService(
  serverAddress,
  HostServices.serviceParamsBuilder()
@@ -149,13 +150,15 @@ public void setUp() throws Exception {
  .build())
  .build());
 
- channel =
- BinderChannelBuilder.forAddress(serverAddress, appContext)
+ channel = newBinderChannelBuilder().build();
+ }
+
+ BinderChannelBuilder newBinderChannelBuilder() {
+ return BinderChannelBuilder.forAddress(serverAddress, appContext)
  .inboundParcelablePolicy(
- InboundParcelablePolicy.newBuilder()
- .setAcceptParcelableMetadataValues(true)
- .build())
- .build();
+ InboundParcelablePolicy.newBuilder()
+ .setAcceptParcelableMetadataValues(true)
+ .build());
  }
 
  @After
@@ -185,6 +188,18 @@ public void testBasicCall() throws Exception {
  assertThat(doCall("Hello").get()).isEqualTo("Hello");
  }
 
+ @Test
+ public void testBasicCallWithLegacyAuthStrategy() throws Exception {
+ channel = newBinderChannelBuilder().useLegacyAuthStrategy().build();
+ assertThat(doCall("Hello").get()).isEqualTo("Hello");
+ }
+
+ @Test
+ public void testBasicCallWithV2AuthStrategy() throws Exception {
+ channel = newBinderChannelBuilder().useV2AuthStrategy().build();
+ assertThat(doCall("Hello").get()).isEqualTo("Hello");
+ }
+
  @Test
  public void testPeerUidIsRecorded() throws Exception {
  assertThat(doCall("Hello").get()).isEqualTo("Hello");

binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java

@@ -280,6 +280,62 @@ public BinderChannelBuilder preAuthorizeServers(boolean preAuthorize) {
  return this;
  }
 
+ /**
+ * Specifies how and when to authorize a server against this Channel's {@link SecurityPolicy}.
+ *
+ * <p>This method selects the original "legacy" authorization strategy, which is no longer
+ * preferred for two reasons. First, the legacy strategy considers the UID of the server *process*
+ * we connect to. This is problematic for services using the `android:isolatedProcess` attribute,
+ * which runs them under a different UID and without any of the privileges of the hosting app.
+ * Second, the legacy authorization strategy performs SecurityPolicy checks later in the
+ * handshake, which means the calling UID must be rechecked on every subsequent transaction. For
+ * these reasons, prefer {@link #useV2AuthStrategy()} instead.
+ *
+ * <p>The server does not know which authorization strategy a client is using. Both strategies
+ * work with all versions of the grpc-binder server.
+ *
+ * <p>The default authorization strategy is unspecified. Clients that require the legacy strategy
+ * should configure it explicitly using this method. Eventually support for the legacy strategy
+ * will be removed.
+ *
+ * @return this
+ */
+ public BinderChannelBuilder useLegacyAuthStrategy() {
+ transportFactoryBuilder.setUseLegacyAuthStrategy(true);
+ return this;
+ }
+
+ /**
+ * Specifies how and when to authorize a server against this Channel's {@link SecurityPolicy}.
+ *
+ * <p>This method selects the v2 authorization strategy. It improves on {@link
+ * #useLegacyAuthStrategy()}, by considering the UID of the server *app* we connect to, rather
+ * than the server *process*. This allows clients to connect to services using the
+ * `android:isolatedProcess` attribute, which runs them under a different ephemeral UID and
+ * without any of the privileges of the hosting app.
+ *
+ * <p>Furthermore, the v2 authorization strategy performs SecurityPolicy checks earlier the
+ * handshake, which allows subsequent transactions over the connection to proceed securely without
+ * further UID checks. For these reasons, clients should prefer the v2 strategy.
+ *
+ * <p>The server does not know which authorization strategy a client is using. Both strategies
+ * work with all versions of the grpc-binder server.
+ *
+ * <p>The default authorization strategy is unspecified. Clients that require the v2 strategy
+ * should configure it explicitly using this method. Eventually support for the legacy strategy
+ * will be removed.
+ *
+ * <p>If moving to the new authorization strategy causes a robolectric test to fail, ensure your
+ * fake Service component is registered with `ShadowPackageManager` using `addOrUpdateService()`.
+ *
+ * @return this
+ */
+ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12397")
+ public BinderChannelBuilder useV2AuthStrategy() {
+ transportFactoryBuilder.setUseLegacyAuthStrategy(false);
+ return this;
+ }
+
  @Override
  public BinderChannelBuilder idleTimeout(long value, TimeUnit unit) {
  checkState(

binder/src/main/java/io/grpc/binder/internal/Bindable.java

@@ -54,8 +54,11 @@ interface Observer {
  * before giving them a chance to run. However, note that the identity/existence of the resolved
  * Service can change between the time this method returns and the time you actually bind/connect
  * to it. For example, suppose the target package gets uninstalled or upgraded right after this
- * method returns. In {@link Observer#onBound}, you should verify that the server you resolved is
- * the same one you connected to.
+ * method returns.
+ *
+ * <p>Compare with {@link #getConnectedServiceInfo()}, which can only be called after {@link
+ * Observer#onBound(IBinder)} but can be used to learn about the service you actually connected
+ * to.
  */
  @AnyThread
  ServiceInfo resolve() throws StatusException;
@@ -68,6 +71,21 @@ interface Observer {
  @AnyThread
  void bind();
 
+ /**
+ * Asks PackageManager for details about the remote Service we *actually* connected to.
+ *
+ * <p>Can only be called after {@link Observer#onBound}.
+ *
+ * <p>Compare with {@link #resolve()}, which reports which service would be selected as of now but
+ * *without* connecting.
+ *
+ * @throws StatusException UNIMPLEMENTED if the connected service isn't found (an {@link
+ * Observer#onUnbound} callback has likely already happened or is on its way!)
+ * @throws IllegalStateException if {@link Observer#onBound} has not "happened-before" this call
+ */
+ @AnyThread
+ ServiceInfo getConnectedServiceInfo() throws StatusException;
+
  /**
  * Unbind from the remote service if connected.
  *

binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java

@@ -120,10 +120,10 @@ public BinderClientTransport(
  Boolean preAuthServerOverride = options.getEagAttributes().get(PRE_AUTH_SERVER_OVERRIDE);
  this.preAuthorizeServer =
  preAuthServerOverride != null ? preAuthServerOverride : factory.preAuthorizeServers;
- this.handshake = new LegacyClientHandshake();
+ this.handshake =
+ factory.useLegacyAuthStrategy ? new LegacyClientHandshake() : new NewClientHandshake();
  numInUseStreams = new AtomicInteger();
  pingTracker = new PingTracker(Ticker.systemTicker(), (id) -> sendPing(id));
-
  serviceBinding =
  new ServiceBinding(
  factory.mainThreadExecutor,
@@ -398,6 +398,78 @@ private void handleAuthResult(OneWayBinderProxy binder, Status authorization) {
  }
  }
 
+ private class NewClientHandshake implements ClientHandshake {
+ @Override
+ @GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
+ public void onBound(OneWayBinderProxy endpointBinder) {
+ Futures.addCallback(
+ Futures.submit(serviceBinding::getConnectedServiceInfo, offloadExecutor),
+ new FutureCallback<ServiceInfo>() {
+ @Override
+ public void onSuccess(ServiceInfo result) {
+ synchronized (BinderClientTransport.this) {
+ onConnectedServiceInfo(endpointBinder, result);
+ }
+ }
+
+ @Override
+ public void onFailure(Throwable t) {
+ synchronized (BinderClientTransport.this) {
+ shutdownInternal(Status.fromThrowable(t), true);
+ }
+ }
+ },
+ offloadExecutor);
+ }
+
+ @GuardedBy("BinderClientTransport.this")
+ private void onConnectedServiceInfo(OneWayBinderProxy endpointBinder, ServiceInfo serviceInfo) {
+ if (inState(TransportState.SETUP)) {
+ attributes = setSecurityAttrs(attributes, serviceInfo.applicationInfo.uid);
+ ListenableFuture<Status> authResultFuture =
+ register(checkServerAuthorizationAsync(serviceInfo.applicationInfo.uid));
+ Futures.addCallback(
+ authResultFuture,
+ new FutureCallback<Status>() {
+ @Override
+ public void onSuccess(Status result) {
+ synchronized (BinderClientTransport.this) {
+ handleAuthResult(endpointBinder, result);
+ }
+ }
+
+ @Override
+ public void onFailure(Throwable t) {
+ BinderClientTransport.this.handleAuthResult(t);
+ }
+ },
+ offloadExecutor);
+ }
+ }
+
+ @GuardedBy("BinderClientTransport.this")
+ private void handleAuthResult(OneWayBinderProxy endpointBinder, Status authResult) {
+ if (inState(TransportState.SETUP)) {
+ if (!authResult.isOk()) {
+ shutdownInternal(authResult, true);
+ } else {
+ sendSetupTransaction(endpointBinder);
+ }
+ }
+ }
+
+ @Override
+ @GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
+ public void handleSetupTransport(OneWayBinderProxy serverBinder) {
+ if (!setOutgoingBinder(serverBinder)) {
+ shutdownInternal(
+ Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true);
+ } else {
+ onHandshakeComplete();
+ }
+ }
+ }
+
  @GuardedBy("this")
  private void onHandshakeComplete() {
  setState(TransportState.READY);

binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java

@@ -53,6 +53,7 @@ public final class BinderClientTransportFactory implements ClientTransportFactor
  final OneWayBinderProxy.Decorator binderDecorator;
  final long readyTimeoutMillis;
  final boolean preAuthorizeServers; // TODO(jdcormie): Default to true.
+ final boolean useLegacyAuthStrategy;
 
  ScheduledExecutorService executorService;
  Executor offloadExecutor;
@@ -73,6 +74,7 @@ private BinderClientTransportFactory(Builder builder) {
  binderDecorator = checkNotNull(builder.binderDecorator);
  readyTimeoutMillis = builder.readyTimeoutMillis;
  preAuthorizeServers = builder.preAuthorizeServers;
+ useLegacyAuthStrategy = builder.useLegacyAuthStrategy;
 
  executorService = scheduledExecutorPool.getObject();
  offloadExecutor = offloadExecutorPool.getObject();
@@ -126,6 +128,7 @@ public static final class Builder implements ClientTransportFactoryBuilder {
  OneWayBinderProxy.Decorator binderDecorator = OneWayBinderProxy.IDENTITY_DECORATOR;
  long readyTimeoutMillis = 60_000;
  boolean preAuthorizeServers;
+ boolean useLegacyAuthStrategy = true; // TODO(jdcormie): Default to false.
 
  @Override
  public BinderClientTransportFactory buildClientTransportFactory() {
@@ -219,5 +222,11 @@ public Builder setPreAuthorizeServers(boolean preAuthorizeServers) {
  this.preAuthorizeServers = preAuthorizeServers;
  return this;
  }
+
+ /** Specifies which version of the client handshake to use. */
+ public Builder setUseLegacyAuthStrategy(boolean useLegacyAuthStrategy) {
+ this.useLegacyAuthStrategy = useLegacyAuthStrategy;
+ return this;
+ }
  }
 }

binder/src/main/java/io/grpc/binder/internal/ServiceBinding.java

@@ -102,6 +102,9 @@ public String methodName() {
 
  private State reportedState; // Only used on the main thread.
 
+ @GuardedBy("this")
+ private ComponentName connectedServiceName;
+
  @AnyThread
  ServiceBinding(
  Executor mainThreadExecutor,
@@ -305,13 +308,34 @@ private void clearReferences() {
  sourceContext = null;
  }
 
+ @AnyThread
+ @Override
+ public ServiceInfo getConnectedServiceInfo() throws StatusException {
+ try {
+ return getContextForTargetUser("cross-user v2 handshake")
+ .getPackageManager()
+ .getServiceInfo(getConnectedServiceName(), /* flags= */ 0);
+ } catch (PackageManager.NameNotFoundException e) {
+ throw Status.UNIMPLEMENTED
+ .withCause(e)
+ .withDescription("connected remote service was uninstalled/disabled during handshake")
+ .asException();
+ }
+ }
+
+ private synchronized ComponentName getConnectedServiceName() {
+ checkState(connectedServiceName != null, "onBound() not yet called!");
+ return connectedServiceName;
+ }
+
  @Override
  @MainThread
  public void onServiceConnected(ComponentName className, IBinder binder) {
  boolean bound = false;
  synchronized (this) {
  if (state == State.BINDING) {
  state = State.BOUND;
+ connectedServiceName = className;
  bound = true;
  }
  }

binder/src/test/java/io/grpc/binder/internal/RobolectricBinderTransportTest.java

@@ -25,6 +25,7 @@
 import static io.grpc.binder.internal.BinderTransport.SHUTDOWN_TRANSPORT;
 import static io.grpc.binder.internal.BinderTransport.WIRE_FORMAT_VERSION;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.junit.Assume.assumeTrue;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -119,11 +120,19 @@ public final class RobolectricBinderTransportTest extends AbstractTransportTest
 
  private int nextServerAddress;
 
- @Parameter public boolean preAuthServersParam;
+ @Parameter(value = 0)
+ public boolean preAuthServersParam;
 
- @Parameters(name = "preAuthServersParam={0}")
- public static ImmutableList<Boolean> data() {
- return ImmutableList.of(true, false);
+ @Parameter(value = 1)
+ public boolean useLegacyAuthStrategy;
+
+ @Parameters(name = "preAuthServersParam={0};useLegacyAuthStrategy={1}")
+ public static ImmutableList<Object[]> data() {
+ return ImmutableList.of(
+ new Object[] {false, false},
+ new Object[] {false, true},
+ new Object[] {true, false},
+ new Object[] {true, true});
  }
 
  @Override
@@ -189,6 +198,7 @@ protected InternalServer newServer(
  BinderClientTransportFactory.Builder newClientTransportFactoryBuilder() {
  return new BinderClientTransportFactory.Builder()
  .setPreAuthorizeServers(preAuthServersParam)
+ .setUseLegacyAuthStrategy(useLegacyAuthStrategy)
  .setSourceContext(application)
  .setScheduledExecutorPool(executorServicePool)
  .setOffloadExecutorPool(offloadExecutorPool);
@@ -249,7 +259,11 @@ public void clientAuthorizesServerUidsInOrder() throws Exception {
  }
 
  AuthRequest authRequest = securityPolicy.takeNextAuthRequest(TIMEOUT_MS, MILLISECONDS);
- assertThat(authRequest.uid).isEqualTo(11111);
+ if (useLegacyAuthStrategy) {
+ assertThat(authRequest.uid).isEqualTo(11111);
+ } else {
+ assertThat(authRequest.uid).isEqualTo(22222);
+ }
  verify(mockClientTransportListener, never()).transportReady();
  authRequest.setResult(Status.OK);
 
@@ -320,6 +334,10 @@ public void clientIgnoresDuplicateSetupTransaction() throws Exception {
  @Test
  public void clientIgnoresTransactionFromNonServerUids() throws Exception {
  server.start(serverListener);
+
+ // This test is not applicable to the new auth strategy which keeps the client Binder a secret.
+ assumeTrue(useLegacyAuthStrategy);
+
  client = newClientTransport(server);
  startTransport(client, mockClientTransportListener);