github-mirrors/elasticsearch
1
0
Fork 0
mirror of https://github.com/elastic/elasticsearch.git synced 2025-04-25 07:37:19 -04:00
Code Issues Projects Releases Packages Wiki Activity

ESQL: LOOKUP JOIN security tests and privileges (#118447)

Browse source 
- Remove Enrich privilege for LOOKUP JOIN
- Add security tests to lookup, to ensure they work/fail depending on user roles and privileges
This commit is contained in:
Iván Cea Fontenla 2024-12-24 11:10:19 +01:00 • committed by GitHub
parent 43e6fad99c
commit 8dbb25df5a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 118 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

91
x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java
View file

@ -30,6 +30,7 @@ import org.junit.Before;
import org.junit.ClassRule;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@ -87,9 +88,11 @@ public class EsqlSecurityIT extends ESRestTestCase {
@Before
public void indexDocuments() throws IOException {
Settings lookupSettings = Settings.builder().put("index.mode", "lookup").build();
String mapping = """
"properties":{"value": {"type": "double"}, "org": {"type": "keyword"}}
""";
createIndex("index", Settings.EMPTY, mapping);
indexDocument("index", 1, 10.0, "sales");
indexDocument("index", 2, 20.0, "engineering");
@ -110,6 +113,16 @@ public class EsqlSecurityIT extends ESRestTestCase {
indexDocument("indexpartial", 2, 40.0, "sales");
refresh("indexpartial");
createIndex("lookup-user1", lookupSettings, mapping);
indexDocument("lookup-user1", 1, 12.0, "engineering");
indexDocument("lookup-user1", 2, 31.0, "sales");
refresh("lookup-user1");
createIndex("lookup-user2", lookupSettings, mapping);
indexDocument("lookup-user2", 1, 32.0, "marketing");
indexDocument("lookup-user2", 2, 40.0, "sales");
refresh("lookup-user2");
if (aliasExists("second-alias") == false) {
Request aliasRequest = new Request("POST", "_aliases");
aliasRequest.setJsonEntity("""
@ -126,6 +139,17 @@ public class EsqlSecurityIT extends ESRestTestCase {
}
}
},
{
"add": {
"alias": "lookup-first-alias",
"index": "lookup-user1",
"filter": {
"term": {
"org": "sales"
}
}
}
},
{
"add": {
"alias": "second-alias",
@ -229,22 +253,30 @@ public class EsqlSecurityIT extends ESRestTestCase {
public void testUnauthorizedIndices() throws IOException {
ResponseException error;
error = expectThrows(ResponseException.class, () -> runESQLCommand("user1", "from index-user2 | stats sum(value)"));
assertThat(error.getMessage(), containsString("Unknown index [index-user2]"));
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(400));
error = expectThrows(ResponseException.class, () -> runESQLCommand("user2", "from index-user1 | stats sum(value)"));
assertThat(error.getMessage(), containsString("Unknown index [index-user1]"));
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(400));
error = expectThrows(ResponseException.class, () -> runESQLCommand("alias_user2", "from index-user2 | stats sum(value)"));
assertThat(error.getMessage(), containsString("Unknown index [index-user2]"));
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(400));
error = expectThrows(ResponseException.class, () -> runESQLCommand("metadata1_read2", "from index-user1 | stats sum(value)"));
assertThat(error.getMessage(), containsString("Unknown index [index-user1]"));
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(400));
}
public void testInsufficientPrivilege() {
Exception error = expectThrows(Exception.class, () -> runESQLCommand("metadata1_read2", "FROM index-user1 | STATS sum=sum(value)"));
ResponseException error = expectThrows(
ResponseException.class,
() -> runESQLCommand("metadata1_read2", "FROM index-user1 | STATS sum=sum(value)")
);
logger.info("error", error);
assertThat(error.getMessage(), containsString("Unknown index [index-user1]"));
assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
}
public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Exception {
@ -511,6 +543,63 @@ public class EsqlSecurityIT extends ESRestTestCase {
}
}
public void testLookupJoinIndexAllowed() throws Exception {
Response resp = runESQLCommand(
"metadata1_read2",
"ROW x = 40.0 | EVAL value = x | LOOKUP JOIN `lookup-user2` ON value | KEEP x, org"
);
assertOK(resp);
Map<String, Object> respMap = entityAsMap(resp);
assertThat(
respMap.get("columns"),
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
);
assertThat(respMap.get("values"), equalTo(List.of(List.of(40.0, "sales"))));
// Alias, should find the index and the row
resp = runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN `lookup-first-alias` ON value | KEEP x, org");
assertOK(resp);
respMap = entityAsMap(resp);
assertThat(
respMap.get("columns"),
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
);
assertThat(respMap.get("values"), equalTo(List.of(List.of(31.0, "sales"))));
// Alias, for a row that's filtered out
resp = runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN `lookup-first-alias` ON value | KEEP x, org");
assertOK(resp);
respMap = entityAsMap(resp);
assertThat(
respMap.get("columns"),
equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword")))
);
assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(123.0, null))));
}
public void testLookupJoinIndexForbidden() {
var resp = expectThrows(
ResponseException.class,
() -> runESQLCommand("metadata1_read2", "FROM lookup-user2 | EVAL value = 10.0 | LOOKUP JOIN `lookup-user1` ON value | KEEP x")
);
assertThat(resp.getMessage(), containsString("Unknown index [lookup-user1]"));
assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
resp = expectThrows(
ResponseException.class,
() -> runESQLCommand("metadata1_read2", "ROW x = 10.0 | EVAL value = x | LOOKUP JOIN `lookup-user1` ON value | KEEP x")
);
assertThat(resp.getMessage(), containsString("Unknown index [lookup-user1]"));
assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
resp = expectThrows(
ResponseException.class,
() -> runESQLCommand("alias_user1", "ROW x = 10.0 | EVAL value = x | LOOKUP JOIN `lookup-user1` ON value | KEEP x")
);
assertThat(resp.getMessage(), containsString("Unknown index [lookup-user1]"));
assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
}
private void createEnrichPolicy() throws Exception {
createIndex("songs", Settings.EMPTY, """
"properties":{"song_id": {"type": "keyword"}, "title": {"type": "keyword"}, "artist": {"type": "keyword"} }

6
x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml
View file

@ -35,15 +35,15 @@ user2:
metadata1_read2:
cluster: []
indices:
- names: [ 'index-user1' ]
- names: [ 'index-user1', 'lookup-user1' ]
privileges: [ 'view_index_metadata' ]
- names: [ 'index-user2' ]
- names: [ 'index-user2', 'lookup-user2' ]
privileges: [ 'read' ]
alias_user1:
cluster: []
indices:
- names: [ 'first-alias' ]
- names: [ 'first-alias', 'lookup-first-alias' ]
privileges:
- read

17
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java
View file

@ -132,7 +132,6 @@ import java.util.stream.IntStream;
*/
abstract class AbstractLookupService<R extends AbstractLookupService.Request, T extends AbstractLookupService.TransportRequest> {
private final String actionName;
private final String privilegeName;
private final ClusterService clusterService;
private final SearchService searchService;
private final TransportService transportService;
@ -143,7 +142,6 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
AbstractLookupService(
String actionName,
String privilegeName,
ClusterService clusterService,
SearchService searchService,
TransportService transportService,
@ -152,7 +150,6 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
CheckedBiFunction<StreamInput, BlockFactory, T, IOException> readRequest
) {
this.actionName = actionName;
this.privilegeName = privilegeName;
this.clusterService = clusterService;
this.searchService = searchService;
this.transportService = transportService;
@ -237,9 +234,21 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
}));
}
/**
* Get the privilege required to perform the lookup.
* <p>
* If null is returned, no privilege check will be performed.
* </p>
*/
@Nullable
protected abstract String getRequiredPrivilege();
private void hasPrivilege(ActionListener<Void> outListener) {
final Settings settings = clusterService.getSettings();
if (settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false || XPackSettings.SECURITY_ENABLED.get(settings) == false) {
String privilegeName = getRequiredPrivilege();
if (privilegeName == null
|| settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false
|| XPackSettings.SECURITY_ENABLED.get(settings) == false) {
outListener.onResponse(null);
return;
}

16
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java
View file

@ -52,16 +52,7 @@ public class EnrichLookupService extends AbstractLookupService<EnrichLookupServi
BigArrays bigArrays,
BlockFactory blockFactory
) {
super(
LOOKUP_ACTION_NAME,
ClusterPrivilegeResolver.MONITOR_ENRICH.name(),
clusterService,
searchService,
transportService,
bigArrays,
blockFactory,
TransportRequest::readFrom
);
super(LOOKUP_ACTION_NAME, clusterService, searchService, transportService, bigArrays, blockFactory, TransportRequest::readFrom);
}
@Override
@ -90,6 +81,11 @@ public class EnrichLookupService extends AbstractLookupService<EnrichLookupServi
};
}
@Override
protected String getRequiredPrivilege() {
return ClusterPrivilegeResolver.MONITOR_ENRICH.name();
}
private static void validateTypes(DataType inputDataType, MappedFieldType fieldType) {
if (fieldType instanceof RangeFieldMapper.RangeFieldType rangeType) {
// For range policy types, the ENRICH index field type will be one of a list of supported range types,

17
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java
View file

@ -23,7 +23,6 @@ import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.search.SearchService;
import org.elasticsearch.tasks.TaskId;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
import org.elasticsearch.xpack.esql.action.EsqlQueryAction;
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
@ -50,16 +49,7 @@ public class LookupFromIndexService extends AbstractLookupService<LookupFromInde
BigArrays bigArrays,
BlockFactory blockFactory
) {
super(
LOOKUP_ACTION_NAME,
ClusterPrivilegeResolver.MONITOR_ENRICH.name(), // TODO some other privilege
clusterService,
searchService,
transportService,
bigArrays,
blockFactory,
TransportRequest::readFrom
);
super(LOOKUP_ACTION_NAME, clusterService, searchService, transportService, bigArrays, blockFactory, TransportRequest::readFrom);
}
@Override
@ -83,6 +73,11 @@ public class LookupFromIndexService extends AbstractLookupService<LookupFromInde
return termQueryList(fieldType, context, inputBlock, inputDataType);
}
@Override
protected String getRequiredPrivilege() {
return null;
}
private static void validateTypes(DataType inputDataType, MappedFieldType fieldType) {
// TODO: consider supporting implicit type conversion as done in ENRICH for some types
if (fieldType.typeName().equals(inputDataType.typeName()) == false) {