diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java index 58e7d7e2b31..a53b698b7ee 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java @@ -112,6 +112,7 @@ public BinderClientTransport( targetAddress, factory.inboundParcelablePolicy), factory.binderDecorator, + factory.inboundBinderDecorator, buildLogId(factory.sourceContext, targetAddress)); this.offloadExecutorPool = factory.offloadExecutorPool; this.securityPolicy = factory.securityPolicy; diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java index 459e064ad9b..9c9f8bd7723 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java @@ -51,6 +51,7 @@ public final class BinderClientTransportFactory implements ClientTransportFactor final BindServiceFlags bindServiceFlags; final InboundParcelablePolicy inboundParcelablePolicy; final OneWayBinderProxy.Decorator binderDecorator; + final LeakSafeOneWayBinder.Decorator inboundBinderDecorator; final long readyTimeoutMillis; final boolean preAuthorizeServers; // TODO(jdcormie): Default to true. final boolean useLegacyAuthStrategy; @@ -72,6 +73,7 @@ private BinderClientTransportFactory(Builder builder) { bindServiceFlags = checkNotNull(builder.bindServiceFlags); inboundParcelablePolicy = checkNotNull(builder.inboundParcelablePolicy); binderDecorator = checkNotNull(builder.binderDecorator); + inboundBinderDecorator = checkNotNull(builder.inboundBinderDecorator); readyTimeoutMillis = builder.readyTimeoutMillis; preAuthorizeServers = builder.preAuthorizeServers; useLegacyAuthStrategy = builder.useLegacyAuthStrategy; @@ -126,6 +128,7 @@ public static final class Builder implements ClientTransportFactoryBuilder { BindServiceFlags bindServiceFlags = BindServiceFlags.DEFAULTS; InboundParcelablePolicy inboundParcelablePolicy = InboundParcelablePolicy.DEFAULT; OneWayBinderProxy.Decorator binderDecorator = OneWayBinderProxy.IDENTITY_DECORATOR; + LeakSafeOneWayBinder.Decorator inboundBinderDecorator = LeakSafeOneWayBinder.IDENTITY_DECORATOR; long readyTimeoutMillis = 60_000; boolean preAuthorizeServers; boolean useLegacyAuthStrategy = true; // TODO(jdcormie): Default to false. @@ -191,6 +194,14 @@ public Builder setBinderDecorator(OneWayBinderProxy.Decorator binderDecorator) { return this; } + /** + * Decorates the incoming binder, for fault injection. + */ + public Builder setInboundBinderDecorator(LeakSafeOneWayBinder.Decorator inboundBinderDecorator) { + this.inboundBinderDecorator = checkNotNull(inboundBinderDecorator, "inboundBinderDecorator"); + return this; + } + /** * Limits how long it can take to for a new transport to become ready after being started. * diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java index f913775fcbe..6d1a4eff059 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java @@ -181,12 +181,13 @@ public synchronized boolean handleTransaction(int code, Parcel parcel) { checkNotNull(executor, "Not started?")); // Create a new transport and let our listener know about it. BinderServerTransport transport = - BinderServerTransport.create( - executorServicePool, - attrsBuilder.build(), - streamTracerFactories, - clientBinderDecorator, - callbackBinder); + new BinderServerTransport.Builder() + .setExecutorServicePool(executorServicePool) + .setAttributes(attrsBuilder.build()) + .setStreamTracerFactories(streamTracerFactories) + .setBinderDecorator(clientBinderDecorator) + .setCallbackBinder(callbackBinder) + .build(); transport.start(listener.transportCreated(transport)); return true; } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java index 784d833bdf5..90ba317b25f 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java @@ -15,7 +15,10 @@ */ package io.grpc.binder.internal; +import static com.google.common.base.Preconditions.checkNotNull; + import android.os.IBinder; +import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.grpc.Attributes; import io.grpc.Grpc; @@ -46,8 +49,14 @@ private BinderServerTransport( ObjectPool executorServicePool, Attributes attributes, List streamTracerFactories, - OneWayBinderProxy.Decorator binderDecorator) { - super(executorServicePool, attributes, binderDecorator, buildLogId(attributes)); + OneWayBinderProxy.Decorator binderDecorator, + LeakSafeOneWayBinder.Decorator inboundBinderDecorator) { + super( + executorServicePool, + attributes, + binderDecorator, + inboundBinderDecorator, + buildLogId(attributes)); this.streamTracerFactories = streamTracerFactories; } @@ -56,21 +65,22 @@ private BinderServerTransport( * * @param binderDecorator used to decorate 'callbackBinder', for fault injection. */ - public static BinderServerTransport create( - ObjectPool executorServicePool, - Attributes attributes, - List streamTracerFactories, - OneWayBinderProxy.Decorator binderDecorator, - IBinder callbackBinder) { + private static BinderServerTransport create(Builder builder) { BinderServerTransport transport = new BinderServerTransport( - executorServicePool, attributes, streamTracerFactories, binderDecorator); + checkNotNull(builder.executorServicePool, "executorServicePool"), + builder.attributes, + builder.streamTracerFactories, + builder.binderDecorator, + builder.inboundBinderDecorator); // TODO(jdcormie): Plumb in the Server's executor() and use it here instead. // No need to handle failure here because if 'callbackBinder' is already dead, we'll notice it // again in start() when we send the first transaction. synchronized (transport) { transport.setOutgoingBinder( - OneWayBinderProxy.wrap(callbackBinder, transport.getScheduledExecutorService())); + OneWayBinderProxy.wrap( + checkNotNull(builder.callbackBinder, "callbackBinder"), + transport.getScheduledExecutorService())); } return transport; } @@ -154,4 +164,50 @@ private static InternalLogId buildLogId(Attributes attributes) { return InternalLogId.allocate( BinderServerTransport.class, "from " + attributes.get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR)); } + + /** Builder for {@link BinderServerTransport} instances. */ + public static final class Builder { + private ObjectPool executorServicePool; + private Attributes attributes = Attributes.EMPTY; + private List streamTracerFactories = ImmutableList.of(); + private OneWayBinderProxy.Decorator binderDecorator = OneWayBinderProxy.IDENTITY_DECORATOR; + private LeakSafeOneWayBinder.Decorator inboundBinderDecorator = LeakSafeOneWayBinder.IDENTITY_DECORATOR; + private IBinder callbackBinder; + + public Builder() {} + + public Builder setExecutorServicePool(ObjectPool executorServicePool) { + this.executorServicePool = checkNotNull(executorServicePool, "executorServicePool"); + return this; + } + + public Builder setAttributes(Attributes attributes) { + this.attributes = checkNotNull(attributes, "attributes"); + return this; + } + + public Builder setStreamTracerFactories(List streamTracerFactories) { + this.streamTracerFactories = checkNotNull(streamTracerFactories, "streamTracerFactories"); + return this; + } + + public Builder setBinderDecorator(OneWayBinderProxy.Decorator binderDecorator) { + this.binderDecorator = checkNotNull(binderDecorator, "binderDecorator"); + return this; + } + + public Builder setInboundBinderDecorator(LeakSafeOneWayBinder.Decorator inboundBinderDecorator) { + this.inboundBinderDecorator = checkNotNull(inboundBinderDecorator, "inboundBinderDecorator"); + return this; + } + + public Builder setCallbackBinder(IBinder callbackBinder) { + this.callbackBinder = checkNotNull(callbackBinder, "callbackBinder"); + return this; + } + + public BinderServerTransport build() { + return BinderServerTransport.create(this); + } + } } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java index 2b7aa97bfd9..61494aadc7d 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java @@ -198,13 +198,15 @@ protected BinderTransport( ObjectPool executorServicePool, Attributes attributes, OneWayBinderProxy.Decorator binderDecorator, + LeakSafeOneWayBinder.Decorator inboundBinderDecorator, InternalLogId logId) { this.binderDecorator = binderDecorator; this.executorServicePool = executorServicePool; this.attributes = attributes; this.logId = logId; scheduledExecutorService = executorServicePool.getObject(); - incomingBinder = new LeakSafeOneWayBinder(this::handleTransaction); + incomingBinder = + new LeakSafeOneWayBinder(inboundBinderDecorator.decorate(this::handleTransaction)); ongoingCalls = new ConcurrentHashMap<>(); flowController = new FlowController(TRANSACTION_BYTES_WINDOW); } diff --git a/binder/src/main/java/io/grpc/binder/internal/LeakSafeOneWayBinder.java b/binder/src/main/java/io/grpc/binder/internal/LeakSafeOneWayBinder.java index c36bc7d5bd3..1a067fc59bc 100644 --- a/binder/src/main/java/io/grpc/binder/internal/LeakSafeOneWayBinder.java +++ b/binder/src/main/java/io/grpc/binder/internal/LeakSafeOneWayBinder.java @@ -63,6 +63,17 @@ public interface TransactionHandler { boolean handleTransaction(int code, Parcel data); } + /** + * Decorates a {@link TransactionHandler} to add behavior. + */ + @Internal + public interface Decorator { + TransactionHandler decorate(TransactionHandler input); + } + + /** A {@link Decorator} that does nothing. */ + public static final Decorator IDENTITY_DECORATOR = (x) -> x; + @Nullable private TransactionHandler handler; public LeakSafeOneWayBinder(TransactionHandler handler) { diff --git a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java b/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java index d261ce43c8c..681cfc548d2 100644 --- a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java +++ b/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java @@ -68,8 +68,8 @@ public void setUp() throws Exception { } // Provide defaults so that we can "include only relevant details in tests." - BinderServerTransportBuilder newBinderServerTransportBuilder() { - return new BinderServerTransportBuilder() + BinderServerTransport.Builder newBinderServerTransportBuilder() { + return new BinderServerTransport.Builder() .setExecutorServicePool(new FixedObjectPool<>(executorService)) .setAttributes(Attributes.EMPTY) .setStreamTracerFactories(ImmutableList.of()) @@ -126,44 +126,4 @@ public void testStartAfterShutdownNoIdle() throws Exception { assertThat(transportListener.isTerminated()).isTrue(); } - static class BinderServerTransportBuilder { - ObjectPool executorServicePool; - Attributes attributes; - List streamTracerFactories; - OneWayBinderProxy.Decorator binderDecorator; - IBinder callbackBinder; - - public BinderServerTransport build() { - return BinderServerTransport.create( - executorServicePool, attributes, streamTracerFactories, binderDecorator, callbackBinder); - } - - public BinderServerTransportBuilder setExecutorServicePool( - ObjectPool executorServicePool) { - this.executorServicePool = executorServicePool; - return this; - } - - public BinderServerTransportBuilder setAttributes(Attributes attributes) { - this.attributes = attributes; - return this; - } - - public BinderServerTransportBuilder setStreamTracerFactories( - List streamTracerFactories) { - this.streamTracerFactories = streamTracerFactories; - return this; - } - - public BinderServerTransportBuilder setBinderDecorator( - OneWayBinderProxy.Decorator binderDecorator) { - this.binderDecorator = binderDecorator; - return this; - } - - public BinderServerTransportBuilder setCallbackBinder(IBinder callbackBinder) { - this.callbackBinder = callbackBinder; - return this; - } - } } diff --git a/binder/src/test/java/io/grpc/binder/internal/RobolectricBinderTransportTest.java b/binder/src/test/java/io/grpc/binder/internal/RobolectricBinderTransportTest.java index 63c47bf4f19..c78c3fc76a5 100644 --- a/binder/src/test/java/io/grpc/binder/internal/RobolectricBinderTransportTest.java +++ b/binder/src/test/java/io/grpc/binder/internal/RobolectricBinderTransportTest.java @@ -252,16 +252,16 @@ protected String testAuthority(InternalServer server) { @Test public void clientAuthorizesServerUidsInOrder() throws Exception { - // TODO(jdcormie): In real Android, Binder#getCallingUid is thread-local but Robolectric only - // lets us fake value this *globally*. So the ShadowBinder#setCallingUid() here unrealistically - // affects the server's view of the client's uid too. For now this doesn't matter because this - // test never exercises server SecurityPolicy. - ShadowBinder.setCallingUid(EPHEMERAL_SERVER_UID); - serverPkgInfo.applicationInfo.uid = SERVER_APP_UID; shadowOf(application.getPackageManager()).installPackage(serverPkgInfo); shadowOf(application.getPackageManager()).addOrUpdateService(serviceInfo); - server = newServer(ImmutableList.of()); + server = + newServerBuilder() + .setClientBinderDecorator( + RobolectricUidPropagation.newUidPassingBinderDecorator(EPHEMERAL_SERVER_UID)) + .setStreamTracerFactories(ImmutableList.of()) + .build(); + registerServerWithRobolectric((BinderServer) server); server.start(serverListener); SettableAsyncSecurityPolicy securityPolicy = new SettableAsyncSecurityPolicy(); @@ -270,6 +270,7 @@ public void clientAuthorizesServerUidsInOrder() throws Exception { .setFactory( newClientTransportFactoryBuilder() .setSecurityPolicy(securityPolicy) + .setInboundBinderDecorator(RobolectricUidPropagation.newUidRestoringBinderDecorator()) .buildClientTransportFactory()) .build(); runIfNotNull(client.start(mockClientTransportListener)); diff --git a/binder/src/test/java/io/grpc/binder/internal/RobolectricUidPropagation.java b/binder/src/test/java/io/grpc/binder/internal/RobolectricUidPropagation.java new file mode 100644 index 00000000000..3ca9d715f6e --- /dev/null +++ b/binder/src/test/java/io/grpc/binder/internal/RobolectricUidPropagation.java @@ -0,0 +1,72 @@ +/* + * Copyright 2026 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.internal; + +import android.os.Binder; +import android.os.Parcel; +import android.os.RemoteException; +import org.robolectric.shadows.ShadowBinder; + +/** + * Helpers for Robolectric tests to propagate calling UID across in-process binder transactions. + * + *

Because Robolectric tests run client and server in a single process, Android's automatic + * cross-process UID propagation does not occur. This class provides decorators to simulate this + * propagation by passing the fake calling UID inside the transaction {@link Parcel}. + * + *

The outgoing decorator prepends the UID to the parcel, and the incoming decorator extracts + * it and sets it as the calling UID for the duration of the transaction handling. + */ +public final class RobolectricUidPropagation { + private RobolectricUidPropagation() {} + + /** + * Returns a decorator that prepends the given UID to all outgoing transactions. + */ + public static OneWayBinderProxy.Decorator newUidPassingBinderDecorator(int uid) { + return input -> new OneWayBinderProxy(input.getDelegate()) { + @Override + public void transact(int code, ParcelHolder data) throws RemoteException { + try (ParcelHolder newData = ParcelHolder.obtain()) { + newData.get().writeInt(uid); + Parcel srcParcel = data.get(); + newData.get().appendFrom(srcParcel, 0, srcParcel.dataSize()); + data.close(); // We consume the original data. + input.transact(code, newData); + } + } + }; + } + + /** + * Returns a decorator that reads a prepended UID from incoming transactions + * and sets it as the calling UID using Robolectric's ShadowBinder. + */ + public static LeakSafeOneWayBinder.Decorator newUidRestoringBinderDecorator() { + return input -> (code, data) -> { + int uid = data.readInt(); + int originalUid = Binder.getCallingUid(); + try { + // TODO(jdcormie): Use the new thread-safe Robolectric API (https://github.com/robolectric/robolectric/commit/892a5f2a535606c96142010c295f5f6b0bdd6bfb) once it is released. + ShadowBinder.setCallingUid(uid); + return input.handleTransaction(code, data); + } finally { + ShadowBinder.setCallingUid(originalUid); + } + }; + } +}