Skip to content

Commit 3f216d1

Browse files
ywangdywangd
authored
Remove action allowlist on the server side (#96936)
With the new cross-cluster API keys (#95714), RCS 2.0 is more tightly controlled. This removes the need for the action allowlist on the server side because cross-cluster API keys (1) require manage_security to create and (2) are specifized to give out only cross-cluster operation related privileges. Hence this PR removes the allowlist.
1 parent d6ff43b commit 3f216d1

File tree

2 files changed

+27
-155
lines changed

2 files changed

+27
-155
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/CrossClusterAccessServerTransportFilter.java‎

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -10,41 +10,23 @@
1010
import org.apache.logging.log4j.LogManager;
1111
import org.apache.logging.log4j.Logger;
1212
import org.elasticsearch.action.ActionListener;
13-
import org.elasticsearch.action.admin.cluster.remote.RemoteClusterNodesAction;
14-
import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsAction;
15-
import org.elasticsearch.action.admin.cluster.state.ClusterStateAction;
16-
import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction;
17-
import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction;
18-
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesAction;
19-
import org.elasticsearch.action.search.SearchAction;
20-
import org.elasticsearch.action.search.SearchShardsAction;
21-
import org.elasticsearch.action.search.SearchTransportService;
22-
import org.elasticsearch.action.search.TransportOpenPointInTimeAction;
2313
import org.elasticsearch.action.support.DestructiveOperations;
2414
import org.elasticsearch.common.util.concurrent.ThreadContext;
25-
import org.elasticsearch.index.seqno.RetentionLeaseActions;
2615
import org.elasticsearch.license.LicenseUtils;
2716
import org.elasticsearch.license.XPackLicenseState;
2817
import org.elasticsearch.tasks.Task;
29-
import org.elasticsearch.transport.TransportActionProxy;
3018
import org.elasticsearch.transport.TransportRequest;
31-
import org.elasticsearch.xpack.core.action.XPackInfoAction;
3219
import org.elasticsearch.xpack.core.security.SecurityContext;
33-
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction;
3420
import org.elasticsearch.xpack.core.security.authc.Authentication;
35-
import org.elasticsearch.xpack.core.transform.action.GetCheckpointAction;
3621
import org.elasticsearch.xpack.security.Security;
3722
import org.elasticsearch.xpack.security.audit.AuditUtil;
3823
import org.elasticsearch.xpack.security.authc.CrossClusterAccessAuthenticationService;
3924
import org.elasticsearch.xpack.security.authz.AuthorizationService;
4025

4126
import java.util.HashSet;
4227
import java.util.Set;
43-
import java.util.stream.Collectors;
44-
import java.util.stream.Stream;
4528

4629
import static org.elasticsearch.core.Strings.format;
47-
import static org.elasticsearch.transport.RemoteClusterService.REMOTE_CLUSTER_HANDSHAKE_ACTION_NAME;
4830
import static org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo.CROSS_CLUSTER_ACCESS_SUBJECT_INFO_HEADER_KEY;
4931
import static org.elasticsearch.xpack.security.authc.CrossClusterAccessHeaders.CROSS_CLUSTER_ACCESS_CREDENTIALS_HEADER_KEY;
5032

@@ -63,56 +45,6 @@ final class CrossClusterAccessServerTransportFilter extends ServerTransportFilte
6345
ALLOWED_TRANSPORT_HEADERS = Set.copyOf(allowedHeaders);
6446
}
6547

66-
// package private for testing
67-
static final Set<String> CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST;
68-
static {
69-
CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST = Stream.concat(
70-
// These actions have proxy equivalents, so we need to allow-list the action name and the action name with the proxy action
71-
// prefix
72-
Stream.of(
73-
SearchTransportService.FREE_CONTEXT_SCROLL_ACTION_NAME,
74-
SearchTransportService.FREE_CONTEXT_ACTION_NAME,
75-
SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME,
76-
SearchTransportService.DFS_ACTION_NAME,
77-
SearchTransportService.QUERY_ACTION_NAME,
78-
SearchTransportService.QUERY_ID_ACTION_NAME,
79-
SearchTransportService.QUERY_SCROLL_ACTION_NAME,
80-
SearchTransportService.QUERY_FETCH_SCROLL_ACTION_NAME,
81-
SearchTransportService.FETCH_ID_SCROLL_ACTION_NAME,
82-
SearchTransportService.FETCH_ID_ACTION_NAME,
83-
SearchTransportService.QUERY_CAN_MATCH_NAME,
84-
SearchTransportService.QUERY_CAN_MATCH_NODE_NAME,
85-
TransportOpenPointInTimeAction.OPEN_SHARD_READER_CONTEXT_NAME,
86-
// CCR actions
87-
"indices:data/read/xpack/ccr/shard_changes",
88-
"indices:internal/admin/ccr/restore/session/clear",
89-
"indices:internal/admin/ccr/restore/file_chunk/get"
90-
).flatMap(name -> Stream.of(name, TransportActionProxy.getProxyAction(name))),
91-
// These actions don't have proxy equivalents
92-
Stream.of(
93-
REMOTE_CLUSTER_HANDSHAKE_ACTION_NAME,
94-
RemoteClusterNodesAction.NAME,
95-
SearchAction.NAME,
96-
ClusterSearchShardsAction.NAME,
97-
SearchShardsAction.NAME,
98-
ResolveIndexAction.NAME,
99-
FieldCapabilitiesAction.NAME,
100-
FieldCapabilitiesAction.NAME + "[n]",
101-
"indices:data/read/eql",
102-
XPackInfoAction.NAME,
103-
GetCheckpointAction.NAME,
104-
// CCR actions
105-
ClusterStateAction.NAME,
106-
HasPrivilegesAction.NAME,
107-
IndicesStatsAction.NAME,
108-
RetentionLeaseActions.Add.ACTION_NAME,
109-
RetentionLeaseActions.Remove.ACTION_NAME,
110-
RetentionLeaseActions.Renew.ACTION_NAME,
111-
"indices:internal/admin/ccr/restore/session/put"
112-
)
113-
).collect(Collectors.toUnmodifiableSet());
114-
}
115-
11648
private final CrossClusterAccessAuthenticationService crossClusterAccessAuthcService;
11749
private final XPackLicenseState licenseState;
11850

@@ -150,17 +82,6 @@ protected void authenticate(
15082
authenticationListener,
15183
LicenseUtils.newComplianceException(Security.ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.getName())
15284
);
153-
} else if (false == CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST.contains(securityAction)) {
154-
onFailureWithDebugLog(
155-
securityAction,
156-
request,
157-
authenticationListener,
158-
new IllegalArgumentException(
159-
"action ["
160-
+ securityAction
161-
+ "] is not allowed as a cross cluster operation on the dedicated remote cluster server port"
162-
)
163-
);
16485
} else {
16586
try {
16687
validateHeaders();

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterTests.java‎

Lines changed: 27 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
4949
import static org.elasticsearch.xpack.core.security.support.Exceptions.authorizationError;
5050
import static org.elasticsearch.xpack.security.authc.CrossClusterAccessHeaders.CROSS_CLUSTER_ACCESS_CREDENTIALS_HEADER_KEY;
51-
import static org.elasticsearch.xpack.security.transport.CrossClusterAccessServerTransportFilter.CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST;
5251
import static org.hamcrest.Matchers.arrayWithSize;
5352
import static org.hamcrest.Matchers.containsString;
5453
import static org.hamcrest.Matchers.equalTo;
@@ -109,66 +108,42 @@ public void testInbound() {
109108
public void testCrossClusterAccessInbound() {
110109
TransportRequest request = mock(TransportRequest.class);
111110
Authentication authentication = AuthenticationTestHelper.builder().build();
112-
boolean allowlisted = randomBoolean();
113-
String action = allowlisted ? randomFrom(CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST) : "_action";
111+
String action = randomAlphaOfLengthBetween(10, 20);
114112
doAnswer(getAnswer(authentication)).when(authcService).authenticate(eq(action), eq(request), eq(true), anyActionListener());
115113
doAnswer(getAnswer(authentication, true)).when(crossClusterAccessAuthcService)
116114
.authenticate(eq(action), eq(request), anyActionListener());
117115
ServerTransportFilter filter = getNodeCrossClusterAccessFilter();
118116
PlainActionFuture<Void> listener = spy(new PlainActionFuture<>());
119117
filter.inbound(action, request, channel, listener);
120-
if (allowlisted) {
121-
verify(authzService).authorize(eq(authentication), eq(action), eq(request), anyActionListener());
122-
verify(crossClusterAccessAuthcService).authenticate(anyString(), any(), anyActionListener());
123-
verify(authcService, never()).authenticate(anyString(), any(), anyBoolean(), anyActionListener());
124-
} else {
125-
var actual = expectThrows(IllegalArgumentException.class, listener::actionGet);
126-
assertThat(
127-
actual.getMessage(),
128-
equalTo("action [" + action + "] is not allowed as a cross cluster operation on the dedicated remote cluster server port")
129-
);
130-
verify(authcService, never()).authenticate(anyString(), any(), anyBoolean(), anyActionListener());
131-
verify(crossClusterAccessAuthcService, never()).authenticate(anyString(), any(), anyActionListener());
132-
verifyNoMoreInteractions(authzService);
133-
}
118+
verify(authzService).authorize(eq(authentication), eq(action), eq(request), anyActionListener());
119+
verify(crossClusterAccessAuthcService).authenticate(anyString(), any(), anyActionListener());
120+
verify(authcService, never()).authenticate(anyString(), any(), anyBoolean(), anyActionListener());
134121
}
135122

136123
public void testCrossClusterAccessInboundInvalidHeadersFail() {
137124
TransportRequest request = mock(TransportRequest.class);
138125
Authentication authentication = AuthenticationTestHelper.builder().build();
139-
boolean allowlisted = randomBoolean();
140-
String action = allowlisted ? randomFrom(CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST) : "_action";
126+
String action = randomAlphaOfLengthBetween(10, 20);
141127
doAnswer(getAnswer(authentication)).when(authcService).authenticate(eq(action), eq(request), eq(true), anyActionListener());
142128
doAnswer(getAnswer(authentication, true)).when(crossClusterAccessAuthcService)
143129
.authenticate(eq(action), eq(request), anyActionListener());
144130
ServerTransportFilter filter = getNodeCrossClusterAccessFilter(Set.copyOf(randomNonEmptySubsetOf(SECURITY_HEADER_FILTERS)));
145131
PlainActionFuture<Void> listener = new PlainActionFuture<>();
146132
filter.inbound(action, request, channel, listener);
147133
var actual = expectThrows(IllegalArgumentException.class, listener::actionGet);
148-
if (allowlisted) {
149-
verifyNoMoreInteractions(authcService);
150-
verifyNoMoreInteractions(authzService);
151-
assertThat(
152-
actual.getMessage(),
153-
containsString("is not allowed for cross cluster requests through the dedicated remote cluster server port")
154-
);
155-
} else {
156-
verify(authcService, never()).authenticate(anyString(), any(), anyBoolean(), anyActionListener());
157-
verify(crossClusterAccessAuthcService, never()).authenticate(anyString(), any(), anyActionListener());
158-
verifyNoMoreInteractions(authzService);
159-
assertThat(
160-
actual.getMessage(),
161-
equalTo("action [" + action + "] is not allowed as a cross cluster operation on the dedicated remote cluster server port")
162-
);
163-
}
134+
verifyNoMoreInteractions(authcService);
135+
verifyNoMoreInteractions(authzService);
136+
assertThat(
137+
actual.getMessage(),
138+
containsString("is not allowed for cross cluster requests through the dedicated remote cluster server port")
139+
);
164140
verify(crossClusterAccessAuthcService, never()).authenticate(anyString(), any(), anyActionListener());
165141
}
166142

167143
public void testCrossClusterAccessInboundMissingHeadersFail() {
168144
TransportRequest request = mock(TransportRequest.class);
169145
Authentication authentication = AuthenticationTestHelper.builder().build();
170-
boolean allowlisted = randomBoolean();
171-
String action = allowlisted ? randomFrom(CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST) : "_action";
146+
String action = randomAlphaOfLengthBetween(10, 20);
172147
doAnswer(getAnswer(authentication)).when(authcService).authenticate(eq(action), eq(request), eq(true), anyActionListener());
173148
doAnswer(getAnswer(authentication, true)).when(crossClusterAccessAuthcService)
174149
.authenticate(eq(action), eq(request), anyActionListener());
@@ -198,27 +173,17 @@ public void testCrossClusterAccessInboundMissingHeadersFail() {
198173
filter.inbound(action, request, channel, listener);
199174
var actual = expectThrows(IllegalArgumentException.class, listener::actionGet);
200175

201-
if (allowlisted) {
202-
verifyNoMoreInteractions(authcService);
203-
verifyNoMoreInteractions(authzService);
204-
assertThat(
205-
actual.getMessage(),
206-
equalTo(
207-
"Cross cluster requests through the dedicated remote cluster server port require transport header ["
208-
+ firstMissingHeader
209-
+ "] but none found. "
210-
+ "Please ensure you have configured remote cluster credentials on the cluster originating the request."
211-
)
212-
);
213-
} else {
214-
verify(authcService, never()).authenticate(anyString(), any(), anyBoolean(), anyActionListener());
215-
verify(crossClusterAccessAuthcService, never()).authenticate(anyString(), any(), anyActionListener());
216-
verifyNoMoreInteractions(authzService);
217-
assertThat(
218-
actual.getMessage(),
219-
equalTo("action [" + action + "] is not allowed as a cross cluster operation on the dedicated remote cluster server port")
220-
);
221-
}
176+
verifyNoMoreInteractions(authcService);
177+
verifyNoMoreInteractions(authzService);
178+
assertThat(
179+
actual.getMessage(),
180+
equalTo(
181+
"Cross cluster requests through the dedicated remote cluster server port require transport header ["
182+
+ firstMissingHeader
183+
+ "] but none found. "
184+
+ "Please ensure you have configured remote cluster credentials on the cluster originating the request."
185+
)
186+
);
222187
verify(crossClusterAccessAuthcService, never()).authenticate(anyString(), any(), anyActionListener());
223188
}
224189

@@ -230,26 +195,14 @@ public void testInboundDestructiveOperations() {
230195
);
231196
Authentication authentication = AuthenticationTestHelper.builder().build();
232197
doAnswer(getAnswer(authentication)).when(authcService).authenticate(eq(action), eq(request), eq(true), anyActionListener());
233-
boolean crossClusterAccess = randomBoolean();
234-
ServerTransportFilter filter = crossClusterAccess ? getNodeCrossClusterAccessFilter() : getNodeFilter();
198+
ServerTransportFilter filter = getNodeFilter();
235199
PlainActionFuture<Void> listener = spy(new PlainActionFuture<>());
236200
filter.inbound(action, request, channel, listener);
237201
if (failDestructiveOperations) {
238202
expectThrows(IllegalArgumentException.class, listener::actionGet);
239203
verifyNoMoreInteractions(authzService);
240204
} else {
241-
if (crossClusterAccess) {
242-
var actual = expectThrows(IllegalArgumentException.class, listener::actionGet);
243-
assertThat(
244-
actual.getMessage(),
245-
equalTo(
246-
"action [" + action + "] is not allowed as a cross cluster operation on the dedicated remote cluster server port"
247-
)
248-
);
249-
verifyNoMoreInteractions(authzService);
250-
} else {
251-
verify(authzService).authorize(eq(authentication), eq(action), eq(request), anyActionListener());
252-
}
205+
verify(authzService).authorize(eq(authentication), eq(action), eq(request), anyActionListener());
253206
}
254207
}
255208

@@ -279,9 +232,7 @@ public void testInboundAuthenticationException() {
279232
public void testCrossClusterAccessInboundAuthenticationException() {
280233
TransportRequest request = mock(TransportRequest.class);
281234
Exception authE = authenticationError("authc failed");
282-
// Only pick allowlisted action -- it does not make sense to pick one that isn't because we will never get to authenticate in that
283-
// case
284-
String action = randomFrom(CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST);
235+
String action = randomAlphaOfLengthBetween(10, 20);
285236
doAnswer(i -> {
286237
final Object[] args = i.getArguments();
287238
assertThat(args, arrayWithSize(3));
@@ -344,7 +295,7 @@ public void testCrossClusterAccessInboundFailsWithUnsupportedLicense() {
344295

345296
ServerTransportFilter crossClusterAccessFilter = getNodeCrossClusterAccessFilter(unsupportedLicenseState);
346297
PlainActionFuture<Void> listener = new PlainActionFuture<>();
347-
String action = randomBoolean() ? randomFrom(CROSS_CLUSTER_ACCESS_ACTION_ALLOWLIST) : "_action";
298+
String action = randomAlphaOfLengthBetween(10, 20);
348299
crossClusterAccessFilter.inbound(action, mock(TransportRequest.class), channel, listener);
349300

350301
ElasticsearchSecurityException actualException = expectThrows(ElasticsearchSecurityException.class, listener::actionGet);

0 commit comments

Comments
 (0)