Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public final class BinderClientTransport extends BinderTransport
@GuardedBy("this")
private ScheduledFuture<?> readyTimeoutFuture; // != null iff timeout scheduled.


/**
* Constructs a new transport instance.
*
Expand Down Expand Up @@ -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);
Expand All @@ -334,7 +336,7 @@ protected void handleSetupTransport(Parcel parcel) {
new FutureCallback<Status>() {
@Override
public void onSuccess(Status result) {
handleAuthResult(binder, result);
handleAuthResult(result);
}

@Override
Expand All @@ -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;
}
}
}
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions binder/src/testFixtures/java/io/grpc/binder/FakeDeadBinder.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading