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

Servlerless API protection with annotations #93607

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/93607.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 93607
summary: Servlerless API protection with annotations
area: Indices APIs
type: enhancement
issues: []
43 changes: 39 additions & 4 deletions server/src/main/java/org/elasticsearch/action/ActionModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@
import org.elasticsearch.action.update.UpdateAction;
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.NamedRegistry;
Expand Down Expand Up @@ -448,6 +449,12 @@
public class ActionModule extends AbstractModule {

private static final Logger logger = LogManager.getLogger(ActionModule.class);
/**
* This RestHandler is used as a placeholder for any routes that are unreachable (i.e. have no ServerlessScope annotation) when
* running in serverless mode. It does nothing, and its handleRequest method is never called. It just provides a way to register the
* routes so that we know they do exist.
*/
private static final RestHandler placeholderRestHandler = (request, channel, client) -> {};

private final Settings settings;
private final IndexNameExpressionResolver indexNameExpressionResolver;
Expand All @@ -464,6 +471,7 @@ public class ActionModule extends AbstractModule {
private final RequestValidators<IndicesAliasesRequest> indicesAliasesRequestRequestValidators;
private final ThreadPool threadPool;
private final ReservedClusterStateService reservedClusterStateService;
private final boolean serverlessEnabled;

public ActionModule(
Settings settings,
Expand All @@ -488,6 +496,7 @@ public ActionModule(
this.settingsFilter = settingsFilter;
this.actionPlugins = actionPlugins;
this.threadPool = threadPool;
this.serverlessEnabled = DiscoveryNode.isServerless();
actions = setupActions(actionPlugins);
actionFilters = setupActionFilters(actionPlugins);
autoCreateIndex = new AutoCreateIndex(settings, clusterSettings, indexNameExpressionResolver, systemIndices);
Expand Down Expand Up @@ -530,7 +539,15 @@ public ActionModule(
actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).toList()
);

restController = new RestController(headers, restInterceptor, nodeClient, circuitBreakerService, usageService, tracer);
restController = new RestController(
headers,
restInterceptor,
nodeClient,
circuitBreakerService,
usageService,
tracer,
serverlessEnabled
);
reservedClusterStateService = new ReservedClusterStateService(clusterService, reservedStateHandlers);
}

Expand Down Expand Up @@ -730,10 +747,18 @@ private static ActionFilters setupActionFilters(List<ActionPlugin> actionPlugins
public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
List<AbstractCatAction> catActions = new ArrayList<>();
Consumer<RestHandler> registerHandler = handler -> {
if (handler instanceof AbstractCatAction) {
catActions.add((AbstractCatAction) handler);
if (shouldKeepRestHandler(handler)) {
if (handler instanceof AbstractCatAction) {
catActions.add((AbstractCatAction) handler);
}
restController.registerHandler(handler);
} else {
/*
* There's no way this handler can be reached, so we just register a placeholder so that requests for it are routed to
* RestController for proper error messages.
*/
handler.routes().forEach(route -> restController.registerHandler(route, placeholderRestHandler));
}
restController.registerHandler(handler);
};
registerHandler.accept(new RestAddVotingConfigExclusionAction());
registerHandler.accept(new RestClearVotingConfigExclusionsAction());
Expand Down Expand Up @@ -918,6 +943,16 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
registerHandler.accept(new RestCatAction(catActions));
}

/**
* This method is used to determine whether a RestHandler ought to be kept in memory or not. Returns true if serverless mode is
* disabled, or if there is any ServlerlessScope annotation on the RestHandler.
* @param handler
* @return
*/
private boolean shouldKeepRestHandler(final RestHandler handler) {
return serverlessEnabled == false || handler.getServerlessScope() != null;
}

@Override
protected void configure() {
bind(ActionFilters.class).toInstance(actionFilters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public static boolean isStateless(final Settings settings) {
}
}

/**
* Check if the serverless feature flag is present and set to {@code true}, indicating that the node is
* part of a serverless deployment.
*
* @return true if the serverless feature flag is present and set
*/
public static boolean isServerless() {
return DiscoveryNodeRole.hasServerlessFeatureFlag();
}

static final String COORDINATING_ONLY = "coordinating_only";
public static final TransportVersion EXTERNAL_ID_VERSION = TransportVersion.V_8_3_0;
public static final Comparator<DiscoveryNode> DISCOVERY_NODE_COMPARATOR = Comparator.comparing(DiscoveryNode::getName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ public class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole> {
USE_STATELESS_FEATURE_FLAG = useStateless;
}

/**
* A feature flag to indicate if serverless is available or not. Defaults to false.
*/
private static final String USE_SERVERLESS_SYSTEM_PROPERTY = "es.serverless";
private static final Boolean USE_SERVERLESS_FEATURE_FLAG;
static {
final Boolean useStateless = Booleans.parseBoolean(System.getProperty(USE_SERVERLESS_SYSTEM_PROPERTY), false);
if (useStateless && Build.CURRENT.isSnapshot() == false) {
throw new IllegalArgumentException("Enabling serverless usage is only supported in snapshot builds");
}
USE_SERVERLESS_FEATURE_FLAG = useStateless;
}

private final String roleName;

/**
Expand Down Expand Up @@ -407,6 +420,10 @@ public static boolean hasStatelessFeatureFlag() {
return USE_STATELESS_FEATURE_FLAG;
}

public static boolean hasServerlessFeatureFlag() {
return USE_SERVERLESS_FEATURE_FLAG;
}

private static void ensureNoStatelessFeatureFlag(DiscoveryNodeRole role) {
if (hasStatelessFeatureFlag()) {
throw new IllegalArgumentException("Role [" + role.roleName() + "] is only supported on non-stateless deployments");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
handler.handleRequest(request, channel, client);
}

@Override
public RestHandler getConcreteRestHandler() {
return handler.getConcreteRestHandler();
}

@Override
public boolean supportsContentStream() {
return handler.supportsContentStream();
Expand Down
52 changes: 44 additions & 8 deletions server/src/main/java/org/elasticsearch/rest/RestController.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ public class RestController implements HttpServerTransport.Dispatcher {
static final Set<String> SAFELISTED_MEDIA_TYPES = Set.of("application/x-www-form-urlencoded", "multipart/form-data", "text/plain");

static final String ELASTIC_PRODUCT_HTTP_HEADER = "X-elastic-product";
static final String ELASTIC_INTERNAL_ORIGIN_HTTP_HEADER = "X-elastic-internal-origin";
static final String ELASTIC_PRODUCT_HTTP_HEADER_VALUE = "Elasticsearch";
static final Set<String> RESERVED_PATHS = Set.of("/__elb_health__", "/__elb_health__/zk", "/_health", "/_health/zk");

private static final BytesReference FAVICON_RESPONSE;

static {
Expand All @@ -97,14 +97,17 @@ public class RestController implements HttpServerTransport.Dispatcher {
private final Set<RestHeaderDefinition> headersToCopy;
private final UsageService usageService;
private final Tracer tracer;
// If true, the ServerlessScope annotations will be enforced
private final boolean serverlessEnabled;

public RestController(
Set<RestHeaderDefinition> headersToCopy,
UnaryOperator<RestHandler> handlerWrapper,
NodeClient client,
CircuitBreakerService circuitBreakerService,
UsageService usageService,
Tracer tracer
Tracer tracer,
boolean serverlessEnabled
) {
this.headersToCopy = headersToCopy;
this.usageService = usageService;
Expand All @@ -115,12 +118,8 @@ public RestController(
this.handlerWrapper = handlerWrapper;
this.client = client;
this.circuitBreakerService = circuitBreakerService;
registerHandlerNoWrap(
RestRequest.Method.GET,
"/favicon.ico",
RestApiVersion.current(),
(request, channel, clnt) -> channel.sendResponse(new RestResponse(RestStatus.OK, "image/x-icon", FAVICON_RESPONSE))
);
registerHandlerNoWrap(RestRequest.Method.GET, "/favicon.ico", RestApiVersion.current(), new RestFavIconHandler());
this.serverlessEnabled = serverlessEnabled;
}

/**
Expand Down Expand Up @@ -371,6 +370,20 @@ private void dispatchRequest(RestRequest request, RestChannel channel, RestHandl
}
}
RestChannel responseChannel = channel;
if (serverlessEnabled) {
Scope scope = handler.getServerlessScope();
if (Scope.INTERNAL.equals(scope)) {
final String internalOrigin = request.header(ELASTIC_INTERNAL_ORIGIN_HTTP_HEADER);
boolean internalRequest = internalOrigin != null;
if (internalRequest == false) {
handleServerlessRequestToProtectedResource(request.uri(), request.method(), responseChannel);
return;
}
} else if (Scope.PUBLIC.equals(scope) == false) {
handleServerlessRequestToProtectedResource(request.uri(), request.method(), responseChannel);
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps my suggestion about a better error message could be handled with a static dummy handler (that has no scope), and then an else case here which returns the nicer error response?

}
try {
if (handler.canTripCircuitBreaker()) {
inFlightRequestsBreaker(circuitBreakerService).addEstimateBytesAndMaybeBreak(contentLength, "<http_request>");
Expand Down Expand Up @@ -674,6 +687,21 @@ public static void handleBadRequest(String uri, RestRequest.Method method, RestC
}
}

public static void handleServerlessRequestToProtectedResource(String uri, RestRequest.Method method, RestChannel channel)
throws IOException {
try (XContentBuilder builder = channel.newErrorBuilder()) {
builder.startObject();
{
builder.field(
"error",
"uri [" + uri + "] with method [" + method + "] exists but is not available when running in " + "serverless mode"
);
}
builder.endObject();
channel.sendResponse(new RestResponse(BAD_REQUEST, builder));
}
}

/**
* Get the valid set of HTTP methods for a REST request.
*/
Expand Down Expand Up @@ -779,4 +807,12 @@ private static CircuitBreaker inFlightRequestsBreaker(CircuitBreakerService circ
// We always obtain a fresh breaker to reflect changes to the breaker configuration.
return circuitBreakerService.getBreaker(CircuitBreaker.IN_FLIGHT_REQUESTS);
}

@ServerlessScope(Scope.PUBLIC)
private static final class RestFavIconHandler implements RestHandler {
@Override
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
channel.sendResponse(new RestResponse(RestStatus.OK, "image/x-icon", FAVICON_RESPONSE));
}
}
}
20 changes: 20 additions & 0 deletions server/src/main/java/org/elasticsearch/rest/RestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,26 @@ default boolean supportsContentStream() {
return false;
}

/**
* Returns the concrete RestHandler for this RestHandler. That is, if this is a delegating RestHandler it returns the delegate.
* Otherwise it returns itself.
* @return The underlying RestHandler
*/
default RestHandler getConcreteRestHandler() {
return this;
}

/**
* Returns the serverless Scope of this RestHandler. This is only meaningful when running in a servlerless environment. If a
* RestHandler has no ServerlessScope annotation, then this method returns null, meaning that this RestHandler is not visible at all in
* Serverless mode.
* @return The Scope for this handler, or null if there is no ServerlessScope annotation
*/
default Scope getServerlessScope() {
ServerlessScope serverlessScope = getConcreteRestHandler().getClass().getAnnotation(ServerlessScope.class);
return serverlessScope == null ? null : serverlessScope.value();
}

/**
* Indicates if the RestHandler supports working with pooled buffers. If the request handler will not escape the return
* {@link RestRequest#content()} or any buffers extracted from it then there is no need to make a copies of any pooled buffers in the
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/rest/Scope.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.rest;

public enum Scope {
PUBLIC, // available to all requests
INTERNAL // available only to requests with a X-elastic-internal-origin header
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest API spec encodes Visibility which is eerily similar to Scope

Should we look into merging the two concepts?

See: #56104

cc @sethmlarson

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is slightly different, in that Scope here is specifically about serverless, while Visibility applies to self managed ES. The name "scope" should probably be more specific to make that clear.

25 changes: 25 additions & 0 deletions server/src/main/java/org/elasticsearch/rest/ServerlessScope.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.rest;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* This annotation is meant to be applied to RestHandler classes, and is used to determine which RestHandlers are available to requests
* at runtime in Serverless mode. This annotation is unused when not running in serverless mode. If this annotation is not present in a
* RestHandler, then that RestHandler is not available at all in Serverless mode.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface ServerlessScope {
Scope value();
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.rest.Scope;
import org.elasticsearch.rest.ServerlessScope;
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
Expand All @@ -24,6 +26,7 @@
import static org.elasticsearch.rest.RestRequest.Method.GET;
import static org.elasticsearch.rest.RestRequest.Method.HEAD;

@ServerlessScope(Scope.INTERNAL)
public class RestMainAction extends BaseRestHandler {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure the main action is in scope for serverless? In its current form it exposes a bunch of information that will not make sense in serverless: cluster name, cluster uuid, build information (which includes the self managed version number). Maybe this should be internal to start?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh hmm good point. I'll change it to internal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


@Override
Expand Down
Loading