Skip to content
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
063f212
add `RemoteClusterTransportInterceptor` interface
slobodanadamovic Sep 5, 2025
6e6f79f
add RemoteClusterAuthenticationService interface
slobodanadamovic Sep 5, 2025
9d5d6b3
change SecurityNetty4Transport to depend on RemoteClusterAuthenticati…
slobodanadamovic Sep 5, 2025
d3f2df9
add empty CrossClusterAccessTransportInterceptor that implements Remo…
slobodanadamovic Sep 5, 2025
90a6c54
construct RemoteClusterTransportInterceptor in security interceptor
slobodanadamovic Sep 5, 2025
9b25155
move interceptForCrossClusterAccessRequests and RCS_INTERNAL_ACTIONS_…
slobodanadamovic Sep 5, 2025
5018173
move profile intialization logic into CrossClusterAccessTransportInte…
slobodanadamovic Sep 5, 2025
cbebc26
inline profile filters intitialization and remove initializeProfileFi…
slobodanadamovic Sep 5, 2025
1325d3b
change order of parameters in shouldRemoveParentAuthorizationFromThre…
slobodanadamovic Sep 5, 2025
f07cd23
replace Optional<String> with boolean
slobodanadamovic Sep 5, 2025
9a21578
implement isRemoteClusterConnection and invoke it instead resolving a…
slobodanadamovic Sep 5, 2025
201fcad
delegate last call to remoteClusterCredentialsResolver and remove it
slobodanadamovic Sep 5, 2025
179b339
remove other unused fields from SecurityServerTransportInterceptor
slobodanadamovic Sep 5, 2025
b493336
move RemoteClusterCredentials record
slobodanadamovic Sep 5, 2025
71e94a4
move remote access headers check into RemoteClusterTransportInterceptor
slobodanadamovic Sep 5, 2025
9b5aa27
add new SecurityServerTransportInterceptor constructor
slobodanadamovic Sep 5, 2025
4d54614
construct RemoteClusterTransportInterceptor in security plugin and ca…
slobodanadamovic Sep 5, 2025
99d7319
move to using new constructor part 1
slobodanadamovic Sep 5, 2025
4981394
move to using new constructor part 2
slobodanadamovic Sep 7, 2025
af00bcd
move to using new constructor part 3
slobodanadamovic Sep 7, 2025
35f2a44
spotless and remove repeat annotation
slobodanadamovic Sep 7, 2025
de29062
Merge branch 'main' of github.com:elastic/elasticsearch into rcs-inte…
slobodanadamovic Sep 7, 2025
b7b8aa7
introduce destructiveOperations field in test class
slobodanadamovic Sep 7, 2025
3dd666f
rename tryAuthenticate to authenticateHeaders
slobodanadamovic Sep 7, 2025
7450a83
renaming and javadoc
slobodanadamovic Sep 7, 2025
84ed0b5
inline `interceptForCrossClusterAccessRequests` method
slobodanadamovic Sep 7, 2025
75aa5ca
address review feedback
slobodanadamovic Sep 9, 2025
25d558c
Merge branch 'main' into rcs-interceptor-refactoring
slobodanadamovic Sep 9, 2025
2d96a32
Merge branch 'main' into rcs-interceptor-refactoring
slobodanadamovic Sep 9, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import org.elasticsearch.xpack.core.security.transport.SecurityTransportExceptionHandler;
import org.elasticsearch.xpack.core.ssl.SSLService;
import org.elasticsearch.xpack.core.ssl.SslProfile;
import org.elasticsearch.xpack.security.authc.CrossClusterAccessAuthenticationService;
import org.elasticsearch.xpack.security.authc.RemoteClusterAuthenticationService;

import java.net.InetSocketAddress;
import java.net.SocketAddress;
Expand Down Expand Up @@ -79,7 +79,7 @@ public class SecurityNetty4Transport extends Netty4Transport {
private final boolean remoteClusterServerSslEnabled;
private final SslProfile remoteClusterClientSslProfile;
private final RemoteClusterClientBootstrapOptions remoteClusterClientBootstrapOptions;
private final CrossClusterAccessAuthenticationService crossClusterAccessAuthenticationService;
private final RemoteClusterAuthenticationService remoteClusterAuthenticationService;

public SecurityNetty4Transport(
final Settings settings,
Expand All @@ -91,7 +91,7 @@ public SecurityNetty4Transport(
final CircuitBreakerService circuitBreakerService,
final SSLService sslService,
final SharedGroupFactory sharedGroupFactory,
final CrossClusterAccessAuthenticationService crossClusterAccessAuthenticationService
final RemoteClusterAuthenticationService remoteClusterAuthenticationService
) {
super(
settings,
Expand All @@ -103,7 +103,7 @@ public SecurityNetty4Transport(
circuitBreakerService,
sharedGroupFactory
);
this.crossClusterAccessAuthenticationService = crossClusterAccessAuthenticationService;
this.remoteClusterAuthenticationService = remoteClusterAuthenticationService;
this.exceptionHandler = new SecurityTransportExceptionHandler(logger, lifecycle, (c, e) -> super.onException(c, e));
this.sslService = sslService;
this.transportSslEnabled = XPackSettings.TRANSPORT_SSL_ENABLED.get(settings);
Expand Down Expand Up @@ -180,7 +180,7 @@ protected void headerReceived(Header header) {
channel.config().setAutoRead(false);
// this prevents thread-context changes to propagate beyond the validation, as netty worker threads are reused
try (ThreadContext.StoredContext ignore = threadPool.getThreadContext().newStoredContext()) {
crossClusterAccessAuthenticationService.tryAuthenticate(
remoteClusterAuthenticationService.authenticateHeaders(
header.getRequestHeaders(),
ActionListener.runAfter(ActionListener.wrap(aVoid -> {
// authn is successful -> NOOP (the complete request will be subsequently authn & authz & audited)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,8 @@
import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent;
import org.elasticsearch.xpack.security.support.SecurityMigrations;
import org.elasticsearch.xpack.security.support.SecuritySystemIndices;
import org.elasticsearch.xpack.security.transport.CrossClusterAccessTransportInterceptor;
import org.elasticsearch.xpack.security.transport.RemoteClusterTransportInterceptor;
import org.elasticsearch.xpack.security.transport.SecurityHttpSettings;
import org.elasticsearch.xpack.security.transport.SecurityServerTransportInterceptor;
import org.elasticsearch.xpack.security.transport.filter.IPFilter;
Expand Down Expand Up @@ -1164,17 +1166,24 @@ Collection<Object> createComponents(
DestructiveOperations destructiveOperations = new DestructiveOperations(settings, clusterService.getClusterSettings());
crossClusterAccessAuthcService.set(new CrossClusterAccessAuthenticationService(clusterService, apiKeyService, authcService.get()));
components.add(crossClusterAccessAuthcService.get());

RemoteClusterTransportInterceptor remoteClusterTransportInterceptor = new CrossClusterAccessTransportInterceptor(
crossClusterAccessAuthcService.get(),
authcService.get(),
authzService,
getLicenseState(),
securityContext.get(),
threadPool,
settings
);
securityInterceptor.set(
new SecurityServerTransportInterceptor(
settings,
threadPool,
authcService.get(),
authzService,
getSslService(),
securityContext.get(),
destructiveOperations,
crossClusterAccessAuthcService.get(),
getLicenseState()
remoteClusterTransportInterceptor
)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import static org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo.CROSS_CLUSTER_ACCESS_SUBJECT_INFO_HEADER_KEY;
import static org.elasticsearch.xpack.security.authc.CrossClusterAccessHeaders.CROSS_CLUSTER_ACCESS_CREDENTIALS_HEADER_KEY;

public class CrossClusterAccessAuthenticationService {
public class CrossClusterAccessAuthenticationService implements RemoteClusterAuthenticationService {

private static final Logger logger = LogManager.getLogger(CrossClusterAccessAuthenticationService.class);

Expand All @@ -52,6 +52,7 @@ public CrossClusterAccessAuthenticationService(
this.authenticationService = authenticationService;
}

@Override
public void authenticate(final String action, final TransportRequest request, final ActionListener<Authentication> listener) {
final ThreadContext threadContext = clusterService.threadPool().getThreadContext();
final CrossClusterAccessHeaders crossClusterAccessHeaders;
Expand Down Expand Up @@ -117,7 +118,8 @@ public void authenticate(final String action, final TransportRequest request, fi
}
}

public void tryAuthenticate(Map<String, String> headers, ActionListener<Void> listener) {
@Override
public void authenticateHeaders(Map<String, String> headers, ActionListener<Void> listener) {
final ApiKeyService.ApiKeyCredentials credentials;
try {
credentials = extractApiKeyCredentialsFromHeaders(headers);
Expand All @@ -128,7 +130,8 @@ public void tryAuthenticate(Map<String, String> headers, ActionListener<Void> li
tryAuthenticate(credentials, listener);
}

public void tryAuthenticate(ApiKeyService.ApiKeyCredentials credentials, ActionListener<Void> listener) {
// package-private for testing
void tryAuthenticate(ApiKeyService.ApiKeyCredentials credentials, ActionListener<Void> listener) {
Objects.requireNonNull(credentials);
apiKeyService.tryAuthenticate(clusterService.threadPool().getThreadContext(), credentials, ActionListener.wrap(authResult -> {
if (authResult.isAuthenticated()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.authc;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;

import java.util.Map;

/**
* Service interface for authenticating remote cluster requests.
*
* <p>
* This service handles authentication for cross-cluster requests.
* It provides methods to authenticate both full transport requests
* and credential headers only.
*/
public interface RemoteClusterAuthenticationService {

/**
* Called to authenticates a remote cluster transport request.
*
* @param action the transport action being performed
* @param request the transport request containing authentication headers
* @param listener callback to receive the authenticated {@link Authentication}
* object on success, or an exception on failure
*/
void authenticate(String action, TransportRequest request, ActionListener<Authentication> listener);

/**
* This method is called to do a preliminary check if headers contain valid
* remote cluster credentials, without the overhead of full authentication
* processing.
*
* @param headers map of request headers containing authentication credentials
* @param listener callback to receive {@code null} on successful authentication,
* or an exception on authentication failure
*/
void authenticateHeaders(Map<String, String> headers, ActionListener<Void> listener);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little bit confused about the contract of this method.

  • The name of the method implies that is does authentication.
  • The javadoc implies that it doesn't actually do authentication (or isn't required to?).
  • The implementation in CrossClusterAccessAuthenticationService.tryAuthenticate appears to require and authenticate the API Key, but not set the Authentication.

I'm not quite sure what to make of that.

Perhaps the issue is what "if headers contain valid remote cluster credentials" actually means. I would read that and assume it was just checking whether the headers exist, but I think it might mean it does more than that. So, I guess my question is what doesn't it do? Is it just that it doesn't create an Authentication object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps the issue is what "if headers contain valid remote cluster credentials" actually means.

Yeah, this is confusing. It's vague because the "validation" may mean different for cross-project than for RCS 2.0.

The order of method calls is:

  1. read only headers on the remote cluster port and call authenticateHeaders
  2. consume the full request body and call authenticate

The authenticateHeaders method validates and verifies the cross cluster credentials based on the received headers. It's called early during request processing, and only for requests on the remote cluster port. We only read headers (body is not consumed yet) at the stage when this method is called. This means that required cross cluster headers must be present and credentials must be valid. Fro RCS 2.0, there is no authentication nor role building happening at this stage, hence the without the overhead of full authentication processing comment.

But thinking further, this may not hold true in all use cases. This method could chose to build authentication object and stash it in the thread context, but it's not required at this phase. It gets required when we call ServerTransportFilter to process inbound message.

Also, I leaned on the fact that all communication is going over RCS port and REMOTE_CLUSTER_PROFILE. This is explicit in SecurityNetty4Transport, but implicit when authenticateHeaders is called. I can try to make that contract clearer. And also make it explicit and change RemoteClusterTransportInterceptor#getProfileTransportFilters to provide a single filter for the REMOTE_CLUSTER_PROFILE only. Currently, it's pretty generic as I wanted to keep flexibility to inject different transport filters for other profiles. We may not need to be able to do it, but I couldn't foresee it at this stage. Let me know what you think.

Copy link
Contributor

@tvernum tvernum Sep 8, 2025

Choose a reason for hiding this comment

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

For RCS 2.0, there is no authentication

I think this is where the semantic mismatch occurs. For me, verifying the api key (which, unless I'm horribly confused, is done in authenticateHeaders) is authentication. That's the primary credential that determines whether this is an authenticated request.

We don't determine the identity of the end-user, their roles (intersection with the API key) or construct an Authentication object, but in my mental model, those are side-effects of the actual "authentication" which is checking the validity of the API key.

I don't want to bike shed this, or argue about precise terminology, but it seems we draw the box around "authentication" differently, and your mindset is "unless we've done all the things we typically do during authentication, then we haven't done authentication" and mine is "if we've verified credentials, that's authentication".

So calling it authenticateHeaders but saying it doesn't do "full authentication" is confusing to me, I don't know what that means.

Calling it authenticateConnection would be more meaningful to me, at least in the context of RCS2.0, but maybe that's not a terminology that fits with CPS (I'm not sure what you think we'll do in that method for CPS).

Copy link
Contributor Author

@slobodanadamovic slobodanadamovic Sep 8, 2025

Choose a reason for hiding this comment

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

I think semantics are important and this should have read as For RCS 2.0, there is no authentication object nor role building happening at this stage.

I tried to make distinction between authenticateHeaders and authenticate methods, but failed to name correctly the other typical things we do after we execute authentication. Which could be summed up as "building authentication informations and stashing them in the thread context".

Both methods are called for each request. I don't have full picture on why this was implemented the way it is, but I'm assuming it has to do with how thread context is handled at these different points in time.

We may need to do more refactoring here, because we execute authentication two times for the same RCS request. This may have not been a big of an issue for RCS 2.0, but I think it will be for CPS.

Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to do more refactoring here, because we execute authentication two times for the same RCS request. This may have not been a big of an issue for RCS 2.0, but I think it will be for CPS.

I think we can defer this conversation until we know what the CPS version looks like. That's going to require coming up with a clearer concept of why we have 2 methods and what we're supposed to do in each one, and trying to name the methods now is probably going to be a waste of effort.


}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import java.util.Arrays;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

public final class PreAuthorizationUtils {
Expand Down Expand Up @@ -118,17 +117,17 @@ private static boolean shouldPreAuthorizeChildActionOfParent(final String parent
}

public static boolean shouldRemoveParentAuthorizationFromThreadContext(
Optional<String> remoteClusterAlias,
String childAction,
SecurityContext securityContext
SecurityContext securityContext,
boolean isRemoteClusterRequest
) {
final ParentActionAuthorization parentAuthorization = securityContext.getParentAuthorization();
if (parentAuthorization == null) {
// Nothing to remove.
return false;
}

if (remoteClusterAlias.isPresent()) {
if (isRemoteClusterRequest) {
// We never want to send the parent authorization header to remote clusters.
return true;
}
Expand Down
Loading