Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: implement RBAC gRFC misc cases (1.41.x backport) #8540

Merged
merged 4 commits into from Sep 20, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions xds/src/main/java/io/grpc/xds/ClientXdsClient.java
Expand Up @@ -136,6 +136,10 @@ final class ClientXdsClient extends AbstractXdsClient {
static boolean enableRetry =
Strings.isNullOrEmpty(System.getenv("GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY"))
|| Boolean.parseBoolean(System.getenv("GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY"));
@VisibleForTesting
static boolean enableRbac =
Strings.isNullOrEmpty(System.getenv("GRPC_XDS_EXPERIMENTAL_RBAC"))
|| Boolean.parseBoolean(System.getenv("GRPC_XDS_EXPERIMENTAL_RBAC"));

private static final String TYPE_URL_HTTP_CONNECTION_MANAGER_V2 =
"type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2"
Expand Down Expand Up @@ -218,7 +222,7 @@ protected void handleLdsResponse(String versionInfo, List<Any> resources, String
listener, retainedRdsResources, enableFaultInjection && isResourceV3);
} else {
ldsUpdate = processServerSideListener(
listener, retainedRdsResources, enableFaultInjection && isResourceV3);
listener, retainedRdsResources, enableRbac);
}
} catch (ResourceInvalidException e) {
errors.add(
Expand Down Expand Up @@ -729,10 +733,14 @@ private static FilterChainMatch parseFilterChainMatch(
static io.grpc.xds.HttpConnectionManager parseHttpConnectionManager(
HttpConnectionManager proto, Set<String> rdsResources, FilterRegistry filterRegistry,
boolean parseHttpFilter, boolean isForClient) throws ResourceInvalidException {
if (proto.getXffNumTrustedHops() != 0) {
if (enableRbac && proto.getXffNumTrustedHops() != 0) {
throw new ResourceInvalidException(
"HttpConnectionManager with xff_num_trusted_hops unsupported");
}
if (enableRbac && !proto.getOriginalIpDetectionExtensionsList().isEmpty()) {
throw new ResourceInvalidException("HttpConnectionManager with "
+ "original_ip_detection_extensions unsupported");
}
// Obtain max_stream_duration from Http Protocol Options.
long maxStreamDuration = 0;
if (proto.hasCommonHttpProtocolOptions()) {
Expand Down
16 changes: 16 additions & 0 deletions xds/src/main/java/io/grpc/xds/RbacFilter.java
Expand Up @@ -28,6 +28,7 @@
import io.envoyproxy.envoy.config.rbac.v3.Principal;
import io.envoyproxy.envoy.extensions.filters.http.rbac.v3.RBAC;
import io.envoyproxy.envoy.extensions.filters.http.rbac.v3.RBACPerRoute;
import io.envoyproxy.envoy.type.v3.Int32Range;
import io.grpc.Metadata;
import io.grpc.ServerCall;
import io.grpc.ServerCallHandler;
Expand All @@ -45,6 +46,7 @@
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.AuthenticatedMatcher;
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.DestinationIpMatcher;
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.DestinationPortMatcher;
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.DestinationPortRangeMatcher;
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.InvertMatcher;
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.Matcher;
import io.grpc.xds.internal.rbac.engine.GrpcAuthorizationEngine.OrMatcher;
Expand Down Expand Up @@ -216,6 +218,8 @@ private static Matcher parsePermission(Permission permission) {
return createDestinationIpMatcher(permission.getDestinationIp());
case DESTINATION_PORT:
return createDestinationPortMatcher(permission.getDestinationPort());
case DESTINATION_PORT_RANGE:
return parseDestinationPortRangeMatcher(permission.getDestinationPortRange());
case NOT_RULE:
return new InvertMatcher(parsePermission(permission.getNotRule()));
case METADATA: // hard coded, never match.
Expand Down Expand Up @@ -291,6 +295,14 @@ private static RequestedServerNameMatcher parseRequestedServerNameMatcher(

private static AuthHeaderMatcher parseHeaderMatcher(
io.envoyproxy.envoy.config.route.v3.HeaderMatcher proto) {
if (proto.getName().startsWith("grpc-")) {
throw new IllegalArgumentException("Invalid header matcher config: [grpc-] prefixed "
+ "header name is not allowed.");
}
if (":scheme".equals(proto.getName())) {
throw new IllegalArgumentException("Invalid header matcher config: header name [:scheme] "
+ "is not allowed.");
}
return new AuthHeaderMatcher(MatcherParser.parseHeaderMatcher(proto));
}

Expand All @@ -304,6 +316,10 @@ private static DestinationPortMatcher createDestinationPortMatcher(int port) {
return new DestinationPortMatcher(port);
}

private static DestinationPortRangeMatcher parseDestinationPortRangeMatcher(Int32Range range) {
return new DestinationPortRangeMatcher(range.getStart(), range.getEnd());
}

private static DestinationIpMatcher createDestinationIpMatcher(CidrRange cidrRange) {
return new DestinationIpMatcher(Matchers.CidrMatcher.create(
resolve(cidrRange), cidrRange.getPrefixLen().getValue()));
Expand Down
15 changes: 11 additions & 4 deletions xds/src/main/java/io/grpc/xds/XdsServerWrapper.java
Expand Up @@ -639,6 +639,9 @@ public void run() {
if (!routeDiscoveryStates.containsKey(resourceName)) {
return;
}
if (savedVirtualHosts == null && !isPending) {
logger.log(Level.WARNING, "Received valid Rds {0} configuration.", resourceName);
}
savedVirtualHosts = ImmutableList.copyOf(update.virtualHosts);
updateRdsRoutingConfig();
maybeUpdateSelector();
Expand Down Expand Up @@ -746,8 +749,8 @@ public <ReqT, RespT> Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call,
virtualHosts, call.getAuthority());
if (virtualHost == null) {
call.close(
Status.UNAVAILABLE.withDescription("Could not find xDS virtual host matching RPC"),
new Metadata());
Status.UNAVAILABLE.withDescription("Could not find xDS virtual host matching RPC"),
new Metadata());
return new Listener<ReqT>() {};
}
Route selectedRoute = null;
Expand All @@ -760,11 +763,15 @@ public <ReqT, RespT> Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call,
}
}
if (selectedRoute == null) {
call.close(
Status.UNAVAILABLE.withDescription("Could not find xDS route matching RPC"),
call.close(Status.UNAVAILABLE.withDescription("Could not find xDS route matching RPC"),
new Metadata());
return new ServerCall.Listener<ReqT>() {};
}
if (selectedRoute.routeAction() != null) {
call.close(Status.UNAVAILABLE.withDescription("Invalid xDS route action for matching "
+ "route: only Route.non_forwarding_action should be allowed."), new Metadata());
return new ServerCall.Listener<ReqT>() {};
}
ServerInterceptor routeInterceptor = noopInterceptor;
Map<Route, ServerInterceptor> perRouteInterceptors = routingConfig.interceptors();
if (perRouteInterceptors != null && perRouteInterceptors.get(selectedRoute) != null) {
Expand Down
Expand Up @@ -20,6 +20,7 @@

import com.google.auto.value.AutoValue;
import com.google.common.base.Joiner;
import com.google.common.io.BaseEncoding;
import io.grpc.Grpc;
import io.grpc.Metadata;
import io.grpc.ServerCall;
Expand All @@ -35,6 +36,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -234,6 +236,23 @@ public boolean matches(EvaluateArgs args) {
}
}

public static final class DestinationPortRangeMatcher implements Matcher {
private final int start;
private final int end;

/** Start of the range is inclusive. End of the range is exclusive.*/
public DestinationPortRangeMatcher(int start, int end) {
this.start = start;
this.end = end;
}

@Override
public boolean matches(EvaluateArgs args) {
int port = args.getDestinationPort();
return port >= start && port < end;
}
}

public static final class RequestedServerNameMatcher implements Matcher {
private final Matchers.StringMatcher delegate;

Expand Down Expand Up @@ -316,9 +335,44 @@ private Collection<String> getPrincipalNames() {

@Nullable
private String getHeader(String headerName) {
if (headerName.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
headerName = headerName.toLowerCase(Locale.ROOT);
if ("te".equals(headerName)) {
return null;
}
if (":authority".equals(headerName)) {
headerName = "host";
}
if ("host".equals(headerName)) {
return serverCall.getAuthority();
}
if (":path".equals(headerName)) {
return getPath();
}
if (":method".equals(headerName)) {
return "POST";
}
return deserializeHeader(headerName);
}

@Nullable
private String deserializeHeader(String headerName) {
if (headerName.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
Metadata.Key<byte[]> key;
try {
key = Metadata.Key.of(headerName, Metadata.BINARY_BYTE_MARSHALLER);
} catch (IllegalArgumentException e) {
return null;
}
Iterable<byte[]> values = metadata.getAll(key);
if (values == null) {
return null;
}
List<String> encoded = new ArrayList<>();
for (byte[] v : values) {
encoded.add(BaseEncoding.base64().omitPadding().encode(v));
}
return Joiner.on(",").join(encoded);
}
Metadata.Key<String> key;
try {
key = Metadata.Key.of(headerName, Metadata.ASCII_STRING_MARSHALLER);
Expand Down
17 changes: 17 additions & 0 deletions xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java
Expand Up @@ -131,16 +131,20 @@ public class ClientXdsClientDataTest {
public final ExpectedException thrown = ExpectedException.none();
private final FilterRegistry filterRegistry = FilterRegistry.getDefaultRegistry();
private boolean originalEnableRetry;
private boolean originalEnableRbac;

@Before
public void setUp() {
originalEnableRetry = ClientXdsClient.enableRetry;
assertThat(originalEnableRetry).isTrue();
originalEnableRbac = ClientXdsClient.enableRbac;
assertThat(originalEnableRbac).isTrue();
}

@After
public void tearDown() {
ClientXdsClient.enableRetry = originalEnableRetry;
ClientXdsClient.enableRbac = originalEnableRbac;
}

@Test
Expand Down Expand Up @@ -1108,6 +1112,19 @@ public void parseHttpConnectionManager_xffNumTrustedHopsUnsupported()
hcm, new HashSet<String>(), filterRegistry, false /* does not matter */,
true /* does not matter */);
}

@Test
public void parseHttpConnectionManager_OriginalIpDetectionExtensionsMustEmpty()
throws ResourceInvalidException {
@SuppressWarnings("deprecation")
HttpConnectionManager hcm = HttpConnectionManager.newBuilder()
.addOriginalIpDetectionExtensions(TypedExtensionConfig.newBuilder().build())
.build();
thrown.expect(ResourceInvalidException.class);
thrown.expectMessage("HttpConnectionManager with original_ip_detection_extensions unsupported");
ClientXdsClient.parseHttpConnectionManager(
hcm, new HashSet<String>(), filterRegistry, false /* does not matter */, false);
}

@Test
public void parseHttpConnectionManager_missingRdsAndInlinedRouteConfiguration()
Expand Down
43 changes: 43 additions & 0 deletions xds/src/test/java/io/grpc/xds/RbacFilterTest.java
Expand Up @@ -41,6 +41,7 @@
import io.envoyproxy.envoy.type.matcher.v3.MetadataMatcher;
import io.envoyproxy.envoy.type.matcher.v3.PathMatcher;
import io.envoyproxy.envoy.type.matcher.v3.StringMatcher;
import io.envoyproxy.envoy.type.v3.Int32Range;
import io.grpc.Attributes;
import io.grpc.Grpc;
import io.grpc.Metadata;
Expand Down Expand Up @@ -109,6 +110,33 @@ public void ipPortParser() {
assertThat(decision.decision()).isEqualTo(GrpcAuthorizationEngine.Action.DENY);
}

@Test
@SuppressWarnings({"unchecked", "deprecation"})
public void portRangeParser() {
List<Permission> permissionList = Arrays.asList(
Permission.newBuilder().setDestinationPortRange(
Int32Range.newBuilder().setStart(1010).setEnd(65535).build()
).build());
List<Principal> principalList = Arrays.asList(
Principal.newBuilder().setRemoteIp(
CidrRange.newBuilder().setAddressPrefix("10.10.10.0")
.setPrefixLen(UInt32Value.of(24)).build()
).build());
ConfigOrError<?> result = parse(permissionList, principalList);
assertThat(result.errorDetail).isNull();
ServerCall<Void,Void> serverCall = mock(ServerCall.class);
Attributes attributes = Attributes.newBuilder()
.set(Grpc.TRANSPORT_ATTR_REMOTE_ADDR, new InetSocketAddress("10.10.10.0", 1))
.set(Grpc.TRANSPORT_ATTR_LOCAL_ADDR, new InetSocketAddress("10.10.10.0",9090))
.build();
when(serverCall.getAttributes()).thenReturn(attributes);
when(serverCall.getMethodDescriptor()).thenReturn(method().build());
GrpcAuthorizationEngine engine =
new GrpcAuthorizationEngine(((RbacConfig)result.config).authConfig());
AuthDecision decision = engine.evaluate(new Metadata(), serverCall);
assertThat(decision.decision()).isEqualTo(GrpcAuthorizationEngine.Action.DENY);
}

@Test
@SuppressWarnings("unchecked")
public void pathParser() {
Expand Down Expand Up @@ -172,6 +200,21 @@ public void headerParser() {
assertThat(decision.decision()).isEqualTo(GrpcAuthorizationEngine.Action.DENY);
}

@Test
@SuppressWarnings("deprecation")
public void headerParser_headerName() {
HeaderMatcher headerMatcher = HeaderMatcher.newBuilder()
.setName("grpc--feature").setExactMatch("win").build();
List<Permission> permissionList = Arrays.asList(
Permission.newBuilder().setHeader(headerMatcher).build());
HeaderMatcher headerMatcher2 = HeaderMatcher.newBuilder()
.setName(":scheme").setExactMatch("win").build();
List<Principal> principalList = Arrays.asList(
Principal.newBuilder().setHeader(headerMatcher2).build());
ConfigOrError<RbacConfig> result = parseOverride(permissionList, principalList);
assertThat(result.errorDetail).isNotNull();
}

@Test
@SuppressWarnings("unchecked")
public void compositeRules() {
Expand Down
63 changes: 56 additions & 7 deletions xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java
Expand Up @@ -865,6 +865,50 @@ public void run() {
assertThat(status.getDescription()).isEqualTo("Could not find xDS route matching RPC");
}

@Test
@SuppressWarnings("unchecked")
public void interceptor_invalidRouteAction() throws Exception {
ArgumentCaptor<ConfigApplyingInterceptor> interceptorCaptor =
ArgumentCaptor.forClass(ConfigApplyingInterceptor.class);
final SettableFuture<Server> start = SettableFuture.create();
Executors.newSingleThreadExecutor().execute(new Runnable() {
@Override
public void run() {
try {
start.set(xdsServerWrapper.start());
} catch (Exception ex) {
start.setException(ex);
}
}
});
xdsClient.ldsResource.get(5, TimeUnit.SECONDS);
verify(mockBuilder).intercept(interceptorCaptor.capture());
ConfigApplyingInterceptor interceptor = interceptorCaptor.getValue();
ServerRoutingConfig routingConfig = createRoutingConfig("/FooService/barMethod",
"foo.google.com", "filter-type-url", Route.RouteAction.forCluster(
"cluster", Collections.<Route.RouteAction.HashPolicy>emptyList(), null, null
));
ServerCall<Void, Void> serverCall = mock(ServerCall.class);
when(serverCall.getAttributes()).thenReturn(
Attributes.newBuilder()
.set(ATTR_SERVER_ROUTING_CONFIG, new AtomicReference<>(routingConfig)).build());
when(serverCall.getMethodDescriptor()).thenReturn(createMethod("FooService/barMethod"));
when(serverCall.getAuthority()).thenReturn("foo.google.com");

Filter filter = mock(Filter.class);
when(filter.typeUrls()).thenReturn(new String[]{"filter-type-url"});
filterRegistry.register(filter);
ServerCallHandler<Void, Void> next = mock(ServerCallHandler.class);
interceptor.interceptCall(serverCall, new Metadata(), next);
verify(next, never()).startCall(any(ServerCall.class), any(Metadata.class));
ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(Status.class);
verify(serverCall).close(statusCaptor.capture(), any(Metadata.class));
Status status = statusCaptor.getValue();
assertThat(status.getCode()).isEqualTo(Status.UNAVAILABLE.getCode());
assertThat(status.getDescription()).isEqualTo("Invalid xDS route action for matching "
+ "route: only Route.non_forwarding_action should be allowed.");
}

@Test
@SuppressWarnings("unchecked")
public void interceptor_failingRouterConfig() throws Exception {
Expand Down Expand Up @@ -1104,15 +1148,20 @@ private static EnvoyServerProtoData.FilterChainMatch createMatch() {

private static ServerRoutingConfig createRoutingConfig(String path, String domain,
String filterType) {
return createRoutingConfig(path, domain, filterType, null);
}

private static ServerRoutingConfig createRoutingConfig(String path, String domain,
String filterType, Route.RouteAction action) {
RouteMatch routeMatch =
RouteMatch.create(
PathMatcher.fromPath(path, true),
Collections.<HeaderMatcher>emptyList(), null);
RouteMatch.create(
PathMatcher.fromPath(path, true),
Collections.<HeaderMatcher>emptyList(), null);
VirtualHost virtualHost = VirtualHost.create(
"v1", Collections.singletonList(domain),
Arrays.asList(Route.forAction(routeMatch, null,
ImmutableMap.<String, FilterConfig>of())),
Collections.<String, FilterConfig>emptyMap());
"v1", Collections.singletonList(domain),
Arrays.asList(Route.forAction(routeMatch, action,
ImmutableMap.<String, FilterConfig>of())),
Collections.<String, FilterConfig>emptyMap());
FilterConfig f0 = mock(FilterConfig.class);
when(f0.typeUrl()).thenReturn(filterType);
return ServerRoutingConfig.create(ImmutableList.<VirtualHost>of(virtualHost),
Expand Down