binder: Ensure the security interceptor is always closest to the actual transport. (#9716)

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

@@ -29,9 +29,12 @@ import com.google.common.base.Function;
 import com.google.protobuf.Empty;
 import io.grpc.CallOptions;
 import io.grpc.ManagedChannel;
+import io.grpc.Metadata;
 import io.grpc.MethodDescriptor;
 import io.grpc.Server;
+import io.grpc.ServerCall;
 import io.grpc.ServerCallHandler;
+import io.grpc.ServerInterceptor;
 import io.grpc.ServerServiceDefinition;
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
@@ -59,6 +62,7 @@ public final class BinderSecurityTest {
   @Nullable ManagedChannel channel;
   Map<String, MethodDescriptor<Empty, Empty>> methods = new HashMap<>();
   List<MethodDescriptor<Empty, Empty>> calls = new ArrayList<>();
+  CountingServerInterceptor countingServerInterceptor;
 
   @Before
   public void setupServiceDefinitionsAndMethods() {
@@ -86,6 +90,7 @@ public final class BinderSecurityTest {
       }
       serviceDefinitions.add(builder.build());
     }
+    countingServerInterceptor = new CountingServerInterceptor();
   }
 
   @After
@@ -120,6 +125,7 @@ public final class BinderSecurityTest {
       ServerSecurityPolicy serverPolicy) {
     BinderServerBuilder serverBuilder = BinderServerBuilder.forAddress(listenAddr, receiver);
     serverBuilder.securityPolicy(serverPolicy);
+    serverBuilder.intercept(countingServerInterceptor);
 
     for (ServerServiceDefinition serviceDefinition : serviceDefinitions) {
       serverBuilder.addService(serviceDefinition);
@@ -195,6 +201,27 @@ public final class BinderSecurityTest {
     }
   }
 
+  @Test
+  public void testSecurityInterceptorIsClosestToTransport() throws Exception {
+    createChannel(
+        ServerSecurityPolicy.newBuilder()
+            .servicePolicy("foo", policy((uid) -> true))
+            .servicePolicy("bar", policy((uid) -> false))
+            .servicePolicy("baz", policy((uid) -> false))
+            .build(),
+        SecurityPolicies.internalOnly());
+    assertThat(countingServerInterceptor.numInterceptedCalls).isEqualTo(0);
+    for (MethodDescriptor<Empty, Empty> method : methods.values()) {
+      try {
+        ClientCalls.blockingUnaryCall(channel, method, CallOptions.DEFAULT, null);
+      } catch (StatusRuntimeException sre) {
+        // Ignore.
+      }
+    }
+    // Only the foo calls should have made it to the user interceptor.
+    assertThat(countingServerInterceptor.numInterceptedCalls).isEqualTo(2);
+  }
+
   private static SecurityPolicy policy(Function<Integer, Boolean> func) {
     return new SecurityPolicy() {
       @Override
@@ -203,4 +230,17 @@ public final class BinderSecurityTest {
       }
     };
   }
+
+  private final class CountingServerInterceptor implements ServerInterceptor {
+    int numInterceptedCalls;
+
+    @Override
+    public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
+        ServerCall<ReqT, RespT> call,
+        Metadata headers,
+        ServerCallHandler<ReqT, RespT> next) {
+      numInterceptedCalls += 1;
+      return next.startCall(call, headers);
+    }
+  }
 }

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

@@ -81,6 +81,7 @@ public final class BinderServerBuilder
       SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE);
   private ServerSecurityPolicy securityPolicy;
   private InboundParcelablePolicy inboundParcelablePolicy;
+  private boolean isBuilt;
 
   private BinderServerBuilder(
       AndroidComponentAddress listenAddress,
@@ -107,8 +108,6 @@ public final class BinderServerBuilder
     // Disable stats and tracing by default.
     serverImplBuilder.setStatsEnabled(false);
     serverImplBuilder.setTracingEnabled(false);
-
-    BinderTransportSecurity.installAuthInterceptor(this);
   }
 
   @Override
@@ -177,6 +176,11 @@ public final class BinderServerBuilder
    */
   @Override // For javadoc refinement only.
   public Server build() {
+    // Since we install a final interceptor here, we need to ensure we're only built once.
+    checkState(!isBuilt, "BinderServerBuilder can only be used to build one server instance.");
+    isBuilt = true;
+    // We install the security interceptor last, so it's closest to the transport.
+    BinderTransportSecurity.installAuthInterceptor(this);
     return super.build();
   }
 }