Skip to content

Commit d971072

Browse files
committed
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.
1 parent 9313e87 commit d971072

File tree

8 files changed

+252
-21
lines changed

8 files changed

+252
-21
lines changed

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public final class BinderChannelSmokeTest {
9797
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
9898
.build();
9999

100+
AndroidComponentAddress serverAddress;
100101
ManagedChannel channel;
101102
AtomicReference<Metadata> headersCapture = new AtomicReference<>();
102103
AtomicReference<PeerUid> clientUidCapture = new AtomicReference<>();
@@ -134,7 +135,7 @@ public void setUp() throws Exception {
134135
TestUtils.recordRequestHeadersInterceptor(headersCapture),
135136
PeerUids.newPeerIdentifyingServerInterceptor());
136137

137-
AndroidComponentAddress serverAddress = HostServices.allocateService(appContext);
138+
serverAddress = HostServices.allocateService(appContext);
138139
HostServices.configureService(
139140
serverAddress,
140141
HostServices.serviceParamsBuilder()
@@ -149,13 +150,15 @@ public void setUp() throws Exception {
149150
.build())
150151
.build());
151152

152-
channel =
153-
BinderChannelBuilder.forAddress(serverAddress, appContext)
153+
channel = newBinderChannelBuilder().build();
154+
}
155+
156+
BinderChannelBuilder newBinderChannelBuilder() {
157+
return BinderChannelBuilder.forAddress(serverAddress, appContext)
154158
.inboundParcelablePolicy(
155-
InboundParcelablePolicy.newBuilder()
156-
.setAcceptParcelableMetadataValues(true)
157-
.build())
158-
.build();
159+
InboundParcelablePolicy.newBuilder()
160+
.setAcceptParcelableMetadataValues(true)
161+
.build());
159162
}
160163

161164
@After
@@ -185,6 +188,18 @@ public void testBasicCall() throws Exception {
185188
assertThat(doCall("Hello").get()).isEqualTo("Hello");
186189
}
187190

191+
@Test
192+
public void testBasicCallWithLegacyAuthStrategy() throws Exception {
193+
channel = newBinderChannelBuilder().useLegacyAuthStrategy().build();
194+
assertThat(doCall("Hello").get()).isEqualTo("Hello");
195+
}
196+
197+
@Test
198+
public void testBasicCallWithV2AuthStrategy() throws Exception {
199+
channel = newBinderChannelBuilder().useV2AuthStrategy().build();
200+
assertThat(doCall("Hello").get()).isEqualTo("Hello");
201+
}
202+
188203
@Test
189204
public void testPeerUidIsRecorded() throws Exception {
190205
assertThat(doCall("Hello").get()).isEqualTo("Hello");

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,67 @@ public BinderChannelBuilder preAuthorizeServers(boolean preAuthorize) {
280280
return this;
281281
}
282282

283+
/**
284+
* Specifies how and when to authorize a server against this Channel's {@link SecurityPolicy}.
285+
*
286+
* <p>This method selects the original "legacy" authorization strategy, which is no longer
287+
* preferred for two reasons: First, the legacy strategy considers the UID of the server *process*
288+
* we connect to. This is problematic for services using the `android:isolatedProcess` attribute,
289+
* which runs them under a different "ephemeral" UID. This UID lacks all the privileges of the
290+
* hosting app -- any non-trivial SecurityPolicy would fail to authorize it. Second, the legacy
291+
* authorization strategy performs SecurityPolicy checks later in the connection handshake, which
292+
* means the calling UID must be rechecked on every subsequent RPC. For these reasons, prefer
293+
* {@link #useV2AuthStrategy} instead.
294+
*
295+
* <p>The server does not know which authorization strategy a client is using. Both strategies
296+
* work with all versions of the grpc-binder server.
297+
*
298+
* <p>Callers need not specify an authorization strategy, but the default is unspecified and will
299+
* eventually become {@link #useV2AuthStrategy()}. Clients that require the legacy strategy should
300+
* configure it explicitly using this method. Eventually, however, legacy support will be
301+
* deprecated and removed.
302+
*
303+
* @return this
304+
*/
305+
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12397")
306+
public BinderChannelBuilder useLegacyAuthStrategy() {
307+
transportFactoryBuilder.setUseLegacyAuthStrategy(true);
308+
return this;
309+
}
310+
311+
/**
312+
* Specifies how and when to authorize a server against this Channel's {@link SecurityPolicy}.
313+
*
314+
* <p>This method selects the v2 authorization strategy. It improves on the original strategy
315+
* ({@link #useLegacyAuthStrategy}), by considering the UID of the server *app* we connect to,
316+
* rather than the server *process*. This allows clients to connect to services configured with
317+
* the `android:isolatedProcess` attribute, which run with the same authority as the hosting app,
318+
* but under a different "ephemeral" UID that any non-trivial SecurityPolicy would fail to
319+
* authorize.
320+
*
321+
* <p>Furthermore, the v2 authorization strategy performs SecurityPolicy checks earlier in the
322+
* connection handshake, which allows subsequent RPCs over that connection to proceed securely
323+
* without further UID checks. For these reasons, clients should prefer the v2 strategy.
324+
*
325+
* <p>The server does not know which authorization strategy a client is using. Both strategies
326+
* work with all versions of the grpc-binder server.
327+
*
328+
* <p>Callers need not specify an authorization strategy, but the default is unspecified and can
329+
* change over time. Clients that require the v2 strategy should configure it explicitly using
330+
* this method. Eventually, this strategy will become the default and legacy support will be
331+
* removed.
332+
*
333+
* <p>If moving to the new authorization strategy causes a robolectric test to fail, ensure your
334+
* fake Service component is registered with `ShadowPackageManager` using `addOrUpdateService()`.
335+
*
336+
* @return this
337+
*/
338+
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12397")
339+
public BinderChannelBuilder useV2AuthStrategy() {
340+
transportFactoryBuilder.setUseLegacyAuthStrategy(false);
341+
return this;
342+
}
343+
283344
@Override
284345
public BinderChannelBuilder idleTimeout(long value, TimeUnit unit) {
285346
checkState(

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@ interface Observer {
5454
* before giving them a chance to run. However, note that the identity/existence of the resolved
5555
* Service can change between the time this method returns and the time you actually bind/connect
5656
* to it. For example, suppose the target package gets uninstalled or upgraded right after this
57-
* method returns. In {@link Observer#onBound}, you should verify that the server you resolved is
58-
* the same one you connected to.
57+
* method returns.
58+
*
59+
* <p>Compare with {@link #getConnectedServiceInfo()}, which can only be called after {@link
60+
* Observer#onBound(IBinder)} but can be used to learn about the service you actually connected
61+
* to.
5962
*/
6063
@AnyThread
6164
ServiceInfo resolve() throws StatusException;
@@ -68,6 +71,21 @@ interface Observer {
6871
@AnyThread
6972
void bind();
7073

74+
/**
75+
* Asks PackageManager for details about the remote Service we *actually* connected to.
76+
*
77+
* <p>Can only be called after {@link Observer#onBound}.
78+
*
79+
* <p>Compare with {@link #resolve()}, which reports which service would be selected as of now but
80+
* *without* connecting.
81+
*
82+
* @throws StatusException UNIMPLEMENTED if the connected service isn't found (an {@link
83+
* Observer#onUnbound} callback has likely already happened or is on its way!)
84+
* @throws IllegalStateException if {@link Observer#onBound} has not "happened-before" this call
85+
*/
86+
@AnyThread
87+
ServiceInfo getConnectedServiceInfo() throws StatusException;
88+
7189
/**
7290
* Unbind from the remote service if connected.
7391
*

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

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,10 @@ public BinderClientTransport(
119119
Boolean preAuthServerOverride = options.getEagAttributes().get(PRE_AUTH_SERVER_OVERRIDE);
120120
this.preAuthorizeServer =
121121
preAuthServerOverride != null ? preAuthServerOverride : factory.preAuthorizeServers;
122-
this.handshake = new LegacyClientHandshake();
122+
this.handshake =
123+
factory.useLegacyAuthStrategy ? new LegacyClientHandshake() : new V2ClientHandshake();
123124
numInUseStreams = new AtomicInteger();
124125
pingTracker = new PingTracker(Ticker.systemTicker(), (id) -> sendPing(id));
125-
126126
serviceBinding =
127127
new ServiceBinding(
128128
factory.mainThreadExecutor,
@@ -386,6 +386,56 @@ private synchronized void handleAuthResult(Status authorization) {
386386
handshake.onServerAuthorizationOk();
387387
}
388388

389+
private final class V2ClientHandshake implements ClientHandshake {
390+
391+
private OneWayBinderProxy endpointBinder;
392+
393+
@Override
394+
@GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
395+
public void onBound(OneWayBinderProxy endpointBinder) {
396+
this.endpointBinder = endpointBinder;
397+
Futures.addCallback(
398+
Futures.submit(serviceBinding::getConnectedServiceInfo, offloadExecutor),
399+
new FutureCallback<ServiceInfo>() {
400+
@Override
401+
public void onSuccess(ServiceInfo result) {
402+
synchronized (BinderClientTransport.this) {
403+
onConnectedServiceInfo(result);
404+
}
405+
}
406+
407+
@Override
408+
public void onFailure(Throwable t) {
409+
synchronized (BinderClientTransport.this) {
410+
shutdownInternal(Status.fromThrowable(t), true);
411+
}
412+
}
413+
},
414+
offloadExecutor);
415+
}
416+
417+
@GuardedBy("BinderClientTransport.this")
418+
private void onConnectedServiceInfo(ServiceInfo serviceInfo) {
419+
if (!inState(TransportState.SETUP)) {
420+
return;
421+
}
422+
attributes = setSecurityAttrs(attributes, serviceInfo.applicationInfo.uid);
423+
checkServerAuthorization(serviceInfo.applicationInfo.uid);
424+
}
425+
426+
@Override
427+
@GuardedBy("BinderClientTransport.this")
428+
public void onServerAuthorizationOk() {
429+
sendSetupTransaction(endpointBinder);
430+
}
431+
432+
@Override
433+
@GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
434+
public void handleSetupTransport() {
435+
onHandshakeComplete();
436+
}
437+
}
438+
389439
@GuardedBy("this")
390440
private void onHandshakeComplete() {
391441
setState(TransportState.READY);

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public final class BinderClientTransportFactory implements ClientTransportFactor
5353
final OneWayBinderProxy.Decorator binderDecorator;
5454
final long readyTimeoutMillis;
5555
final boolean preAuthorizeServers; // TODO(jdcormie): Default to true.
56+
final boolean useLegacyAuthStrategy;
5657

5758
ScheduledExecutorService executorService;
5859
Executor offloadExecutor;
@@ -73,6 +74,7 @@ private BinderClientTransportFactory(Builder builder) {
7374
binderDecorator = checkNotNull(builder.binderDecorator);
7475
readyTimeoutMillis = builder.readyTimeoutMillis;
7576
preAuthorizeServers = builder.preAuthorizeServers;
77+
useLegacyAuthStrategy = builder.useLegacyAuthStrategy;
7678

7779
executorService = scheduledExecutorPool.getObject();
7880
offloadExecutor = offloadExecutorPool.getObject();
@@ -126,6 +128,7 @@ public static final class Builder implements ClientTransportFactoryBuilder {
126128
OneWayBinderProxy.Decorator binderDecorator = OneWayBinderProxy.IDENTITY_DECORATOR;
127129
long readyTimeoutMillis = 60_000;
128130
boolean preAuthorizeServers;
131+
boolean useLegacyAuthStrategy = true; // TODO(jdcormie): Default to false.
129132

130133
@Override
131134
public BinderClientTransportFactory buildClientTransportFactory() {
@@ -219,5 +222,11 @@ public Builder setPreAuthorizeServers(boolean preAuthorizeServers) {
219222
this.preAuthorizeServers = preAuthorizeServers;
220223
return this;
221224
}
225+
226+
/** Specifies which version of the client handshake to use. */
227+
public Builder setUseLegacyAuthStrategy(boolean useLegacyAuthStrategy) {
228+
this.useLegacyAuthStrategy = useLegacyAuthStrategy;
229+
return this;
230+
}
222231
}
223232
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ public String methodName() {
102102

103103
private State reportedState; // Only used on the main thread.
104104

105+
@GuardedBy("this")
106+
private ComponentName connectedServiceName;
107+
105108
@AnyThread
106109
ServiceBinding(
107110
Executor mainThreadExecutor,
@@ -305,13 +308,34 @@ private void clearReferences() {
305308
sourceContext = null;
306309
}
307310

311+
@AnyThread
312+
@Override
313+
public ServiceInfo getConnectedServiceInfo() throws StatusException {
314+
try {
315+
return getContextForTargetUser("cross-user v2 handshake")
316+
.getPackageManager()
317+
.getServiceInfo(getConnectedServiceName(), /* flags= */ 0);
318+
} catch (PackageManager.NameNotFoundException e) {
319+
throw Status.UNIMPLEMENTED
320+
.withCause(e)
321+
.withDescription("connected remote service was uninstalled/disabled during handshake")
322+
.asException();
323+
}
324+
}
325+
326+
private synchronized ComponentName getConnectedServiceName() {
327+
checkState(connectedServiceName != null, "onBound() not yet called!");
328+
return connectedServiceName;
329+
}
330+
308331
@Override
309332
@MainThread
310333
public void onServiceConnected(ComponentName className, IBinder binder) {
311334
boolean bound = false;
312335
synchronized (this) {
313336
if (state == State.BINDING) {
314337
state = State.BOUND;
338+
connectedServiceName = className;
315339
bound = true;
316340
}
317341
}

0 commit comments

Comments
 (0)