Skip to content

Commit 6658220

Browse files
jdcormiejdcormie
committed
Add pre-auth override EAG attr and tests
Parameterize RobolectricBinderSecurityTest
1 parent c2a7596 commit 6658220

8 files changed

Lines changed: 337 additions & 24 deletions

File tree

‎binder/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ dependencies {
7272
androidTestImplementation testFixtures(project(':grpc-core'))
7373

7474
testFixturesImplementation libraries.guava.testlib
75+
testFixturesImplementation testFixtures(project(':grpc-core'))
7576
}
7677

7778
import net.ltgt.gradle.errorprone.CheckSeverity

‎binder/src/main/java/io/grpc/binder/ApiConstants.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import android.content.Intent;
2020
import android.os.UserHandle;
21+
import io.grpc.Attributes;
22+
import io.grpc.EquivalentAddressGroup;
2123
import io.grpc.ExperimentalApi;
2224
import io.grpc.NameResolver;
2325

@@ -43,4 +45,15 @@ private ApiConstants() {}
4345
*/
4446
public static final NameResolver.Args.Key<UserHandle> TARGET_ANDROID_USER =
4547
NameResolver.Args.Key.create("target-android-user");
48+
49+
/**
50+
* Lets you override a Channel's pre-auth configuration (see {@link
51+
* BinderChannelBuilder#preAuthorizeServers(boolean)} for a given {@link EquivalentAddressGroup}.
52+
*
53+
* <p>A {@link NameResolver} that discovers servers from an untrusted source like PackageManager
54+
* can use this to force server pre-auth and prevent abuse.
55+
*/
56+
@EquivalentAddressGroup.Attr
57+
public static final Attributes.Key<Boolean> PRE_AUTH_SERVER_OVERRIDE =
58+
Attributes.Key.create("pre-auth-server-override");
4659
}

‎binder/src/main/java/io/grpc/binder/internal/BinderTransport.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.base.Preconditions.checkNotNull;
2020
import static com.google.common.base.Preconditions.checkState;
2121
import static com.google.common.util.concurrent.Futures.immediateFuture;
22+
import static io.grpc.binder.ApiConstants.PRE_AUTH_SERVER_OVERRIDE;
2223
import static java.util.concurrent.TimeUnit.MILLISECONDS;
2324

2425
import android.content.Context;
@@ -607,7 +608,9 @@ public BinderClientTransport(
607608
this.securityPolicy = factory.securityPolicy;
608609
this.offloadExecutor = offloadExecutorPool.getObject();
609610
this.readyTimeoutMillis = factory.readyTimeoutMillis;
610-
this.preAuthorizeServer = factory.preAuthorizeServers;
611+
Boolean preAuthServerOverride = options.getEagAttributes().get(PRE_AUTH_SERVER_OVERRIDE);
612+
this.preAuthorizeServer =
613+
preAuthServerOverride != null ? preAuthServerOverride : factory.preAuthorizeServers;
611614
numInUseStreams = new AtomicInteger();
612615
pingTracker = new PingTracker(Ticker.systemTicker(), (id) -> sendPing(id));
613616

‎binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@
2222
import static org.robolectric.Shadows.shadowOf;
2323

2424
import android.app.Application;
25+
import android.content.pm.ApplicationInfo;
26+
import android.content.pm.PackageInfo;
27+
import android.content.pm.ServiceInfo;
2528
import androidx.test.core.app.ApplicationProvider;
29+
import androidx.test.core.content.pm.ApplicationInfoBuilder;
30+
import androidx.test.core.content.pm.PackageInfoBuilder;
31+
import com.google.common.collect.ImmutableList;
2632
import com.google.common.util.concurrent.Futures;
2733
import com.google.common.util.concurrent.ListenableFuture;
2834
import com.google.common.util.concurrent.SettableFuture;
@@ -46,11 +52,13 @@
4652
import org.junit.Before;
4753
import org.junit.Test;
4854
import org.junit.runner.RunWith;
49-
import org.robolectric.RobolectricTestRunner;
55+
import org.robolectric.ParameterizedRobolectricTestRunner;
56+
import org.robolectric.ParameterizedRobolectricTestRunner.Parameter;
57+
import org.robolectric.ParameterizedRobolectricTestRunner.Parameters;
5058
import org.robolectric.annotation.LooperMode;
5159
import org.robolectric.annotation.LooperMode.Mode;
5260

53-
@RunWith(RobolectricTestRunner.class)
61+
@RunWith(ParameterizedRobolectricTestRunner.class)
5462
@LooperMode(Mode.INSTRUMENTATION_TEST)
5563
public final class RobolectricBinderSecurityTest {
5664

@@ -62,10 +70,33 @@ public final class RobolectricBinderSecurityTest {
6270
private ManagedChannel channel;
6371
private Server server;
6472

73+
@Parameter public boolean preAuthServersParam;
74+
75+
@Parameters(name = "preAuthServersParam={0}")
76+
public static ImmutableList<Boolean> data() {
77+
return ImmutableList.of(true, false);
78+
}
79+
6580
@Before
6681
public void setUp() {
67-
AndroidComponentAddress listenAddress =
68-
AndroidComponentAddress.forRemoteComponent(context.getPackageName(), "HostService");
82+
ApplicationInfo serverAppInfo = ApplicationInfoBuilder.newBuilder()
83+
.setPackageName(context.getPackageName())
84+
.build();
85+
serverAppInfo.uid = android.os.Process.myUid();
86+
PackageInfo serverPkgInfo = PackageInfoBuilder.newBuilder()
87+
.setPackageName(serverAppInfo.packageName)
88+
.setApplicationInfo(serverAppInfo)
89+
.build();
90+
shadowOf(context.getPackageManager()).installPackage(serverPkgInfo);
91+
92+
ServiceInfo serviceInfo = new ServiceInfo();
93+
serviceInfo.name = "SomeService";
94+
serviceInfo.packageName = serverAppInfo.packageName;
95+
serviceInfo.applicationInfo = serverAppInfo;
96+
shadowOf(context.getPackageManager()).addOrUpdateService(serviceInfo);
97+
98+
AndroidComponentAddress listenAddress = AndroidComponentAddress.forRemoteComponent(
99+
serviceInfo.packageName, serviceInfo.name);
69100

70101
MethodDescriptor<Empty, Empty> methodDesc = getMethodDescriptor();
71102
ServerCallHandler<Empty, Empty> callHandler =
@@ -110,6 +141,7 @@ public ListenableFuture<Status> checkAuthorizationAsync(int uid) {
110141
checkNotNull(binderReceiver.get()));
111142
channel =
112143
BinderChannelBuilder.forAddress(listenAddress, context)
144+
.preAuthorizeServers(preAuthServersParam)
113145
.build();
114146
}
115147

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

Lines changed: 139 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,54 @@
1616

1717
package io.grpc.binder.internal;
1818

19+
import static com.google.common.truth.Truth.assertThat;
20+
import static java.util.concurrent.TimeUnit.MILLISECONDS;
21+
import static org.mockito.Mockito.never;
22+
import static org.mockito.Mockito.timeout;
23+
import static org.mockito.Mockito.verify;
1924
import static org.robolectric.Shadows.shadowOf;
2025

2126
import android.app.Application;
2227
import android.content.Intent;
28+
import android.content.pm.ApplicationInfo;
29+
import android.content.pm.PackageInfo;
30+
import android.content.pm.ServiceInfo;
2331
import androidx.test.core.app.ApplicationProvider;
32+
import androidx.test.core.content.pm.ApplicationInfoBuilder;
33+
import androidx.test.core.content.pm.PackageInfoBuilder;
2434
import com.google.common.collect.ImmutableList;
35+
import io.grpc.Attributes;
2536
import io.grpc.ServerStreamTracer;
37+
import io.grpc.Status;
2638
import io.grpc.binder.AndroidComponentAddress;
39+
import io.grpc.binder.ApiConstants;
40+
import io.grpc.binder.AsyncSecurityPolicy;
41+
import io.grpc.binder.internal.SettableAsyncSecurityPolicy.AuthRequest;
2742
import io.grpc.internal.AbstractTransportTest;
2843
import io.grpc.internal.ClientTransportFactory.ClientTransportOptions;
2944
import io.grpc.internal.GrpcUtil;
3045
import io.grpc.internal.InternalServer;
3146
import io.grpc.internal.ManagedClientTransport;
3247
import io.grpc.internal.ObjectPool;
3348
import io.grpc.internal.SharedResourcePool;
49+
import java.util.Collections;
3450
import java.util.List;
3551
import java.util.concurrent.Executor;
3652
import java.util.concurrent.ScheduledExecutorService;
3753
import org.junit.Before;
3854
import org.junit.Ignore;
55+
import org.junit.Rule;
3956
import org.junit.Test;
4057
import org.junit.runner.RunWith;
58+
import org.mockito.Mock;
59+
import org.mockito.junit.MockitoJUnit;
60+
import org.mockito.junit.MockitoRule;
4161
import org.robolectric.ParameterizedRobolectricTestRunner;
4262
import org.robolectric.ParameterizedRobolectricTestRunner.Parameter;
4363
import org.robolectric.ParameterizedRobolectricTestRunner.Parameters;
4464
import org.robolectric.annotation.LooperMode;
4565
import org.robolectric.annotation.LooperMode.Mode;
66+
import org.robolectric.shadows.ShadowBinder;
4667

4768
/**
4869
* All of the AbstractTransportTest cases applied to {@link BinderTransport} running in a
@@ -68,15 +89,44 @@ public final class RobolectricBinderTransportTest extends AbstractTransportTest
6889
private final ObjectPool<Executor> serverExecutorPool =
6990
SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR);
7091

92+
@Rule public MockitoRule mocks = MockitoJUnit.rule();
93+
94+
@Mock AsyncSecurityPolicy mockClientSecurityPolicy;
95+
96+
ApplicationInfo serverAppInfo;
97+
PackageInfo serverPkgInfo;
98+
ServiceInfo serviceInfo;
99+
71100
private int nextServerAddress;
72101

73-
@Parameter public boolean preAuthorizeServers;
102+
@Parameter public boolean preAuthServersParam;
74103

75-
@Parameters(name = "preAuthorizeServers={0}")
104+
@Parameters(name = "preAuthServersParam={0}")
76105
public static ImmutableList<Boolean> data() {
77106
return ImmutableList.of(true, false);
78107
}
79108

109+
@Override
110+
public void setUp() {
111+
serverAppInfo =
112+
ApplicationInfoBuilder.newBuilder().setPackageName("the.server.package").build();
113+
serverAppInfo.uid = android.os.Process.myUid();
114+
serverPkgInfo =
115+
PackageInfoBuilder.newBuilder()
116+
.setPackageName(serverAppInfo.packageName)
117+
.setApplicationInfo(serverAppInfo)
118+
.build();
119+
shadowOf(application.getPackageManager()).installPackage(serverPkgInfo);
120+
121+
serviceInfo = new ServiceInfo();
122+
serviceInfo.name = "SomeService";
123+
serviceInfo.packageName = serverAppInfo.packageName;
124+
serviceInfo.applicationInfo = serverAppInfo;
125+
shadowOf(application.getPackageManager()).addOrUpdateService(serviceInfo);
126+
127+
super.setUp();
128+
}
129+
80130
@Before
81131
public void requestRealisticBindServiceBehavior() {
82132
shadowOf(application).setBindServiceCallsOnServiceConnectedDirectly(false);
@@ -85,10 +135,11 @@ public void requestRealisticBindServiceBehavior() {
85135

86136
@Override
87137
protected InternalServer newServer(List<ServerStreamTracer.Factory> streamTracerFactories) {
88-
AndroidComponentAddress listenAddr = AndroidComponentAddress.forBindIntent(
89-
new Intent()
90-
.setClassName(application.getPackageName(), "HostService")
91-
.setAction("io.grpc.action.BIND." + nextServerAddress++));
138+
AndroidComponentAddress listenAddr =
139+
AndroidComponentAddress.forBindIntent(
140+
new Intent()
141+
.setClassName(serviceInfo.packageName, serviceInfo.name)
142+
.setAction("io.grpc.action.BIND." + nextServerAddress++));
92143

93144
BinderServer binderServer =
94145
new BinderServer.Builder()
@@ -115,30 +166,101 @@ protected InternalServer newServer(
115166
return newServer(streamTracerFactories);
116167
}
117168

169+
BinderClientTransportFactory.Builder newClientTransportFactoryBuilder() {
170+
return new BinderClientTransportFactory.Builder()
171+
.setPreAuthorizeServers(preAuthServersParam)
172+
.setSourceContext(application)
173+
.setScheduledExecutorPool(executorServicePool)
174+
.setOffloadExecutorPool(offloadExecutorPool);
175+
}
176+
177+
BinderClientTransportBuilder newClientTransportBuilder() {
178+
return new BinderClientTransportBuilder()
179+
.setClientTransportFactory(newClientTransportFactoryBuilder().buildClientTransportFactory())
180+
.setServerAddress(server.getListenSocketAddress());
181+
}
182+
118183
@Override
119184
protected ManagedClientTransport newClientTransport(InternalServer server) {
120-
BinderClientTransportFactory.Builder builder =
121-
new BinderClientTransportFactory.Builder()
122-
.setPreAuthorizeServers(preAuthorizeServers)
123-
.setSourceContext(application)
124-
.setScheduledExecutorPool(executorServicePool)
125-
.setOffloadExecutorPool(offloadExecutorPool);
126-
127185
ClientTransportOptions options = new ClientTransportOptions();
128186
options.setEagAttributes(eagAttrs());
129187
options.setChannelLogger(transportLogger());
130188

131-
return new BinderTransport.BinderClientTransport(
132-
builder.buildClientTransportFactory(),
133-
(AndroidComponentAddress) server.getListenSocketAddress(),
134-
options);
189+
return newClientTransportBuilder()
190+
.setServerAddress(server.getListenSocketAddress())
191+
.setClientTransportOptions(options)
192+
.build();
135193
}
136194

137195
@Override
138196
protected String testAuthority(InternalServer server) {
139197
return ((AndroidComponentAddress) server.getListenSocketAddress()).getAuthority();
140198
}
141199

200+
@Test
201+
public void clientAuthorizesServerUidsInOrder() throws Exception {
202+
serverAppInfo.uid = 11111;
203+
shadowOf(application.getPackageManager()).installPackage(serverPkgInfo);
204+
shadowOf(application.getPackageManager()).addOrUpdateService(serviceInfo);
205+
server = newServer(Collections.emptyList());
206+
server.start(serverListener);
207+
208+
SettableAsyncSecurityPolicy securityPolicy = new SettableAsyncSecurityPolicy();
209+
client =
210+
newClientTransportBuilder()
211+
.setClientTransportFactory(
212+
newClientTransportFactoryBuilder()
213+
.setSecurityPolicy(securityPolicy)
214+
.buildClientTransportFactory())
215+
.build();
216+
217+
// TODO(jdcormie): In real Android, Binder#getCallingUid is thread-local but Robolectric only
218+
// lets us fake value this *globally*. So the ShadowBinder#setCallingUid() here unrealistically
219+
// affects the server's view of the client's uid too. For now this doesn't matter because this
220+
// test never exercises server SecurityPolicy.
221+
ShadowBinder.setCallingUid(22222); // UID of the server *process* which != serverAppInfo.uid.
222+
runIfNotNull(client.start(mockClientTransportListener));
223+
224+
if (preAuthServersParam) {
225+
AuthRequest preAuthRequest = securityPolicy.takeNextAuthRequest(TIMEOUT_MS, MILLISECONDS);
226+
assertThat(preAuthRequest.uid).isEqualTo(serverAppInfo.uid);
227+
verify(mockClientTransportListener, never()).transportReady();
228+
preAuthRequest.setResult(Status.OK);
229+
}
230+
231+
AuthRequest authRequest = securityPolicy.takeNextAuthRequest(TIMEOUT_MS, MILLISECONDS);
232+
assertThat(authRequest.uid).isEqualTo(22222);
233+
verify(mockClientTransportListener, never()).transportReady();
234+
authRequest.setResult(Status.OK);
235+
236+
verify(mockClientTransportListener, timeout(TIMEOUT_MS)).transportReady();
237+
}
238+
239+
@Test
240+
public void eagAttributeCanOverrideChannelPreAuthServerSetting() throws Exception {
241+
server.start(serverListener);
242+
SettableAsyncSecurityPolicy securityPolicy = new SettableAsyncSecurityPolicy();
243+
ClientTransportOptions options = new ClientTransportOptions();
244+
options.setEagAttributes(
245+
Attributes.newBuilder().set(ApiConstants.PRE_AUTH_SERVER_OVERRIDE, true).build());
246+
client =
247+
newClientTransportFactoryBuilder()
248+
.setSecurityPolicy(securityPolicy)
249+
.buildClientTransportFactory()
250+
.newClientTransport(server.getListenSocketAddress(), options, null);
251+
runIfNotNull(client.start(mockClientTransportListener));
252+
253+
AuthRequest preAuthRequest = securityPolicy.takeNextAuthRequest(TIMEOUT_MS, MILLISECONDS);
254+
verify(mockClientTransportListener, never()).transportReady();
255+
preAuthRequest.setResult(Status.OK);
256+
257+
AuthRequest authRequest = securityPolicy.takeNextAuthRequest(TIMEOUT_MS, MILLISECONDS);
258+
verify(mockClientTransportListener, never()).transportReady();
259+
authRequest.setResult(Status.OK);
260+
261+
verify(mockClientTransportListener, timeout(TIMEOUT_MS)).transportReady();
262+
}
263+
142264
@Test
143265
@Ignore("See BinderTransportTest#socketStats.")
144266
@Override

0 commit comments

Comments
 (0)