binder: Let the server know when the client fails to authorize it. (grpc#12445)

Avoids waiting for the handshake timeout on the server side in this
case.

I also add test coverage for the `!setOutgoingBinder()` case to make
sure it works in the new location.

My ulterior motive for this change is simplifying the client handshake
code in preparation for grpc#12398 -- An (impossible) !isShutdown() clause
goes away for easy to understand reasons and I'll no longer have to pass
the server's binder as an arg from async function to function in two
separate handshake impls.

Fixes grpc#12438

Author: jdcormie

binder/src/androidTest/java/io/grpc/binder/internal/BinderClientTransportTest.java

@@ -39,6 +39,7 @@
 import io.grpc.Status.Code;
 import io.grpc.binder.AndroidComponentAddress;
 import io.grpc.binder.BinderServerBuilder;
+import io.grpc.binder.FakeDeadBinder;
 import io.grpc.binder.HostServices;
 import io.grpc.binder.SecurityPolicy;
 import io.grpc.binder.internal.OneWayBinderProxies.BlackHoleOneWayBinderProxy;
@@ -358,6 +359,20 @@ public void testTxnFailurePostSetup() throws Exception {
     assertThat(streamStatus.getCause()).isSameInstanceAs(doe);
   }
 
+  @Test
+  public void testServerBinderDeadOnArrival() throws Exception {
+    BlockingBinderDecorator<OneWayBinderProxy> decorator = new BlockingBinderDecorator<>();
+    transport = new BinderClientTransportBuilder().setBinderDecorator(decorator).build();
+    transport.start(transportListener).run();
+    decorator.putNextResult(decorator.takeNextRequest());  // Server's "Endpoint" Binder.
+    OneWayBinderProxy unusedServerBinder = decorator.takeNextRequest();
+    decorator.putNextResult(
+        OneWayBinderProxy.wrap(new FakeDeadBinder(), offloadServicePool.getObject()));
+    Status clientStatus = transportListener.awaitShutdown();
+    assertThat(clientStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(clientStatus.getDescription()).contains("Failed to observe outgoing binder");
+  }
+
   @Test
   public void testBlackHoleEndpointConnectTimeout() throws Exception {
     BlockingBinderDecorator<BlackHoleOneWayBinderProxy> decorator = new BlockingBinderDecorator<>();

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

@@ -87,7 +87,6 @@ public final class BinderClientTransport extends BinderTransport
   @GuardedBy("this")
   private ScheduledFuture<?> readyTimeoutFuture; // != null iff timeout scheduled.
 
-
   /**
    * Constructs a new transport instance.
    *
@@ -324,6 +323,9 @@ protected void handleSetupTransport(Parcel parcel) {
       } else if (binder == null) {
         shutdownInternal(
             Status.UNAVAILABLE.withDescription("Malformed SETUP_TRANSPORT data"), true);
+      } else if (!setOutgoingBinder(OneWayBinderProxy.wrap(binder, offloadExecutor))) {
+        shutdownInternal(
+            Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true);
       } else {
         restrictIncomingBinderToCallsFrom(remoteUid);
         attributes = setSecurityAttrs(attributes, remoteUid);
@@ -334,7 +336,7 @@ protected void handleSetupTransport(Parcel parcel) {
             new FutureCallback<Status>() {
               @Override
               public void onSuccess(Status result) {
-                handleAuthResult(binder, result);
+                handleAuthResult(result);
               }
 
               @Override
@@ -353,24 +355,17 @@ private ListenableFuture<Status> checkServerAuthorizationAsync(int remoteUid) {
         : Futures.submit(() -> securityPolicy.checkAuthorization(remoteUid), offloadExecutor);
   }
 
-  private synchronized void handleAuthResult(IBinder binder, Status authorization) {
+  private synchronized void handleAuthResult(Status authorization) {
     if (inState(TransportState.SETUP)) {
       if (!authorization.isOk()) {
         shutdownInternal(authorization, true);
-      } else if (!setOutgoingBinder(OneWayBinderProxy.wrap(binder, offloadExecutor))) {
-        shutdownInternal(
-            Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true);
       } else {
-        // Check state again, since a failure inside setOutgoingBinder (or a callback it
-        // triggers), could have shut us down.
-        if (!isShutdown()) {
-          setState(TransportState.READY);
-          attributes = clientTransportListener.filterTransport(attributes);
-          clientTransportListener.transportReady();
-          if (readyTimeoutFuture != null) {
-            readyTimeoutFuture.cancel(false);
-            readyTimeoutFuture = null;
-          }
+        setState(TransportState.READY);
+        attributes = clientTransportListener.filterTransport(attributes);
+        clientTransportListener.transportReady();
+        if (readyTimeoutFuture != null) {
+          readyTimeoutFuture.cancel(false);
+          readyTimeoutFuture = null;
         }
       }
     }
@@ -387,7 +382,6 @@ protected void handlePingResponse(Parcel parcel) {
     pingTracker.onPingResponse(parcel.readInt());
   }
 
-
   private static ClientStream newFailingClientStream(
       Status failure, Attributes attributes, Metadata headers, ClientStreamTracer[] tracers) {
     StatsTraceContext statsTraceContext =

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

@@ -43,6 +43,7 @@
 import androidx.test.core.content.pm.ApplicationInfoBuilder;
 import androidx.test.core.content.pm.PackageInfoBuilder;
 import com.google.common.collect.ImmutableList;
+import com.google.common.truth.TruthJUnit;
 import io.grpc.Attributes;
 import io.grpc.InternalChannelz.SocketStats;
 import io.grpc.ServerStreamTracer;
@@ -353,6 +354,29 @@ static void sendShutdownTransportTransactionAsUid(ClientTransport client, int se
     }
   }
 
+  @Test
+  public void clientReportsAuthzErrorToServer() throws Exception {
+    server.start(serverListener);
+    client =
+        newClientTransportBuilder()
+            .setFactory(
+                newClientTransportFactoryBuilder()
+                    .setSecurityPolicy(SecurityPolicies.permissionDenied("test"))
+                    .buildClientTransportFactory())
+            .build();
+    runIfNotNull(client.start(mockClientTransportListener));
+    verify(mockClientTransportListener, timeout(TIMEOUT_MS))
+        .transportShutdown(statusCaptor.capture());
+    assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.PERMISSION_DENIED);
+
+    TruthJUnit.assume().that(preAuthServersParam).isFalse();
+
+    MockServerTransportListener serverTransportListener =
+        serverListener.takeListenerOrFail(TIMEOUT_MS, MILLISECONDS);
+    serverTransportListener.waitForTermination(TIMEOUT_MS, MILLISECONDS);
+    assertThat(serverTransportListener.isTerminated()).isTrue();
+  }
+
   @Test
   @Override
   // We don't quite pass the official/abstract version of this test yet because

binder/src/testFixtures/java/io/grpc/binder/FakeDeadBinder.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright 2025 The gRPC Authors
+ *
+ * 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 io.grpc.binder;
+
+import android.os.DeadObjectException;
+import android.os.IBinder;
+import android.os.IInterface;
+import android.os.Parcel;
+import android.os.RemoteException;
+import java.io.FileDescriptor;
+
+/** An {@link IBinder} that behaves as if its hosting process has died, for testing. */
+public class FakeDeadBinder implements IBinder {
+  @Override
+  public boolean isBinderAlive() {
+    return false;
+  }
+
+  @Override
+  public IInterface queryLocalInterface(String descriptor) {
+    return null;
+  }
+
+  @Override
+  public String getInterfaceDescriptor() throws RemoteException {
+    throw new DeadObjectException();
+  }
+
+  @Override
+  public boolean pingBinder() {
+    return false;
+  }
+
+  @Override
+  public void dump(FileDescriptor fd, String[] args) throws RemoteException {
+    throw new DeadObjectException();
+  }
+
+  @Override
+  public void dumpAsync(FileDescriptor fd, String[] args) throws RemoteException {
+    throw new DeadObjectException();
+  }
+
+  @Override
+  public boolean transact(int code, Parcel data, Parcel reply, int flags) throws RemoteException {
+    throw new DeadObjectException();
+  }
+
+  @Override
+  public void linkToDeath(DeathRecipient r, int flags) throws RemoteException {
+    throw new DeadObjectException();
+  }
+
+  @Override
+  public boolean unlinkToDeath(DeathRecipient deathRecipient, int flags) {
+    // No need to check whether 'deathRecipient' was ever actually passed to linkToDeath(): Per our
+    // API contract, if "the IBinder has already died" we never throw and always return false.
+    return false;
+  }
+}