Sql security take 1 (elastic/x-pack-elasticsearch#2106)
Add some basic security testing/integration. The good news: 1. Basic security now works. Users without access to an index can't run sql queries against it. Without this change they could. 2. Document level security works! At least so far as I can tell. The work left to do: 1. Field level security doesn't work properly. I mean, it kind of works in that the field's values don't leak but it just looks like they all have null values. 2. We will need to test scrolling. 3. I've only added tests for the rest sql action. I'll need to add tests for jdbc and the CLI as well. 4. I've only added tests for `SELECT` and have ignored stuff like `DESCRIBE` and `SHOW TABLES`. Original commit: elastic/x-pack-elasticsearch@b9909bbda0
This commit is contained in:
Nik Everett
GitHub
parent
2ac45f18fe
commit
ec36875872
|
|
@ -53,7 +53,10 @@ case $key in
|
|||
"-psql"
|
||||
"check"
|
||||
":x-pack-elasticsearch:plugin:precommit"
|
||||
":x-pack-elasticsearch:qa:sql-security:check"
|
||||
"-xforbiddenPatterns"
|
||||
"-x:x-pack-elasticsearch:plugin:forbiddenPatterns"
|
||||
"-x:x-pack-elasticsearch:qa:sql-security:forbiddenPatterns"
|
||||
)
|
||||
;;
|
||||
jdk9)
|
||||
|
|
|
|||
|
|
@ -315,8 +315,11 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I
|
|||
components.addAll(upgrade.createComponents(internalClient, clusterService, threadPool, resourceWatcherService,
|
||||
scriptService, xContentRegistry));
|
||||
|
||||
/* Note that we need *client*, not *internalClient* because client preserves the
|
||||
* authenticated user while internalClient throws that user away and acts as the
|
||||
* x-pack user. */
|
||||
components.addAll(
|
||||
sql.createComponents(internalClient, clusterService, threadPool, resourceWatcherService, scriptService, xContentRegistry));
|
||||
sql.createComponents(client, clusterService, threadPool, resourceWatcherService, scriptService, xContentRegistry));
|
||||
|
||||
// just create the reloader as it will pull all of the loaded ssl configurations and start watching them
|
||||
new SSLConfigurationReloader(settings, env, sslService, resourceWatcherService);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,51 @@
|
|||
import org.elasticsearch.gradle.test.RunTask
|
||||
|
||||
apply plugin: 'elasticsearch.standalone-rest-test'
|
||||
apply plugin: 'elasticsearch.rest-test'
|
||||
|
||||
dependencies {
|
||||
testCompile project(path: ':x-pack-elasticsearch:plugin', configuration: 'runtime')
|
||||
testCompile project(path: ':x-pack-elasticsearch:plugin', configuration: 'testArtifacts')
|
||||
testCompile project(path: ':modules:reindex')
|
||||
}
|
||||
|
||||
// NOCOMMIT we should try this on multiple nodes
|
||||
|
||||
integTestCluster {
|
||||
plugin ':x-pack-elasticsearch:plugin'
|
||||
setting 'xpack.ml.enabled', 'false'
|
||||
setting 'xpack.monitoring.enabled', 'false'
|
||||
extraConfigFile 'x-pack/roles.yml', 'roles.yml'
|
||||
setupCommand 'setupUser#test_admin',
|
||||
'bin/x-pack/users', 'useradd', 'test_admin', '-p', 'x-pack-test-password', '-r', 'superuser'
|
||||
waitCondition = { node, ant ->
|
||||
File tmpFile = new File(node.cwd, 'wait.success')
|
||||
ant.get(src: "http://${node.httpUri()}/_cluster/health?wait_for_nodes=>=${numNodes}&wait_for_status=yellow",
|
||||
dest: tmpFile.toString(),
|
||||
username: 'test_admin',
|
||||
password: 'x-pack-test-password',
|
||||
ignoreerrors: true,
|
||||
retries: 10)
|
||||
return tmpFile.exists()
|
||||
}
|
||||
}
|
||||
|
||||
task run(type: RunTask) {
|
||||
distribution = 'zip' // NOCOMMIT make double sure we want all the modules
|
||||
plugin ':x-pack-elasticsearch:plugin'
|
||||
setting 'xpack.ml.enabled', 'false'
|
||||
setting 'xpack.monitoring.enabled', 'false'
|
||||
extraConfigFile 'x-pack/roles.yml', 'roles.yml'
|
||||
setupCommand 'setupUser#test_admin',
|
||||
'bin/x-pack/users', 'useradd', 'test_admin', '-p', 'x-pack-test-password', '-r', 'superuser'
|
||||
waitCondition = { node, ant ->
|
||||
File tmpFile = new File(node.cwd, 'wait.success')
|
||||
ant.get(src: "http://${node.httpUri()}/_cluster/health?wait_for_nodes=>=${numNodes}&wait_for_status=yellow",
|
||||
dest: tmpFile.toString(),
|
||||
username: 'test_admin',
|
||||
password: 'x-pack-test-password',
|
||||
ignoreerrors: true,
|
||||
retries: 10)
|
||||
return tmpFile.exists()
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
read_test:
|
||||
indices:
|
||||
- names: test
|
||||
privileges: [read]
|
||||
|
||||
read_nothing:
|
||||
|
||||
read_something_else:
|
||||
indices:
|
||||
- names: something_that_isnt_test
|
||||
privileges: [read]
|
||||
|
||||
read_test_a:
|
||||
indices:
|
||||
- names: test
|
||||
privileges: [read]
|
||||
field_security:
|
||||
grant: [a]
|
||||
|
||||
read_test_a_and_b:
|
||||
indices:
|
||||
- names: test
|
||||
privileges: [read]
|
||||
field_security:
|
||||
grant: ["*"]
|
||||
except: [c]
|
||||
|
||||
read_test_without_c_3:
|
||||
indices:
|
||||
- names: test
|
||||
privileges: [read]
|
||||
query: |
|
||||
{
|
||||
"bool": {
|
||||
"must_not": [
|
||||
{
|
||||
"match": {
|
||||
"c": 3
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,161 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License;
|
||||
* you may not use this file except in compliance with the Elastic License.
|
||||
*/
|
||||
package org.elasticsearch.xpack.sql;
|
||||
|
||||
import org.elasticsearch.client.Response;
|
||||
import org.elasticsearch.client.ResponseException;
|
||||
import org.elasticsearch.client.http.Header;
|
||||
import org.elasticsearch.client.http.entity.ContentType;
|
||||
import org.elasticsearch.client.http.entity.StringEntity;
|
||||
import org.elasticsearch.client.http.message.BasicHeader;
|
||||
import org.elasticsearch.common.Nullable;
|
||||
import org.elasticsearch.common.settings.SecureString;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.util.concurrent.ThreadContext;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.common.xcontent.XContentHelper;
|
||||
import org.elasticsearch.common.xcontent.json.JsonXContent;
|
||||
import org.elasticsearch.test.NotEqualMessageBuilder;
|
||||
import org.elasticsearch.test.rest.ESRestTestCase;
|
||||
import org.junit.Before;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.util.Arrays;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static java.util.Collections.emptyMap;
|
||||
import static java.util.Collections.singletonMap;
|
||||
import static org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
|
||||
public class SqlSecurityIT extends ESRestTestCase {
|
||||
/**
|
||||
* All tests run as a an administrative user but use
|
||||
* <code>es-security-runas-user</code> to become a less privileged user when needed.
|
||||
*/
|
||||
@Override
|
||||
protected Settings restClientSettings() {
|
||||
String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray()));
|
||||
return Settings.builder()
|
||||
.put(ThreadContext.PREFIX + ".Authorization", token)
|
||||
.build();
|
||||
}
|
||||
|
||||
@Before
|
||||
public void createTestData() throws IOException {
|
||||
StringBuilder bulk = new StringBuilder();
|
||||
bulk.append("{\"index\":{\"_id\":\"1\"}\n");
|
||||
bulk.append("{\"a\": 1, \"b\": 2, \"c\": 3}\n");
|
||||
bulk.append("{\"index\":{\"_id\":\"2\"}\n");
|
||||
bulk.append("{\"a\": 4, \"b\": 5, \"c\": 6}\n");
|
||||
client().performRequest("PUT", "/test/test/_bulk", singletonMap("refresh", "true"),
|
||||
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
|
||||
}
|
||||
|
||||
// NOCOMMIT we're going to need to test jdbc and cli with these too!
|
||||
// NOCOMMIT we'll have to test scrolling as well
|
||||
// NOCOMMIT tests for describing a table and showing tables
|
||||
|
||||
public void testSqlWorksAsAdmin() throws IOException {
|
||||
Map<String, Object> expected = new HashMap<>();
|
||||
Map<String, Object> columns = new HashMap<>();
|
||||
columns.put("a", singletonMap("type", "long"));
|
||||
columns.put("b", singletonMap("type", "long"));
|
||||
columns.put("c", singletonMap("type", "long"));
|
||||
expected.put("columns", columns);
|
||||
Map<String, Object> row1 = new HashMap<>();
|
||||
row1.put("a", 1);
|
||||
row1.put("b", 2);
|
||||
row1.put("c", 3);
|
||||
Map<String, Object> row2 = new HashMap<>();
|
||||
row2.put("a", 4);
|
||||
row2.put("b", 5);
|
||||
row2.put("c", 6);
|
||||
expected.put("rows", Arrays.asList(row1, row2));
|
||||
expected.put("size", 2);
|
||||
assertResponse(expected, runSql("SELECT * FROM test.test ORDER BY a", null));
|
||||
}
|
||||
|
||||
public void testSqlWithFullAccess() throws IOException {
|
||||
createUser("full_access", "read_test");
|
||||
|
||||
assertResponse(runSql("SELECT * FROM test.test ORDER BY a", null), runSql("SELECT * FROM test.test ORDER BY a", "full_access"));
|
||||
}
|
||||
|
||||
public void testSqlNoAccess() throws IOException {
|
||||
createUser("no_access", "read_nothing");
|
||||
|
||||
ResponseException e = expectThrows(ResponseException.class, () -> runSql("SELECT * FROM test.test", "no_access"));
|
||||
assertThat(e.getMessage(), containsString("403 Forbidden"));
|
||||
}
|
||||
|
||||
public void testSqlWrongAccess() throws IOException {
|
||||
createUser("wrong_access", "read_something_else");
|
||||
|
||||
ResponseException e = expectThrows(ResponseException.class, () -> runSql("SELECT * FROM test.test", "no_access"));
|
||||
assertThat(e.getMessage(), containsString("403 Forbidden"));
|
||||
}
|
||||
|
||||
@AwaitsFix(bugUrl="https://github.com/elastic/x-pack-elasticsearch/issues/2074")
|
||||
public void testSqlSingleFieldGranted() throws IOException {
|
||||
createUser("only_a", "read_test_a");
|
||||
|
||||
/* This doesn't work because sql doesn't see the field level security
|
||||
* and still adds the metadata even though the field "doesn't exist" */
|
||||
assertResponse(runSql("SELECT a FROM test.test", null), runSql("SELECT * FROM test.test", "only_a"));
|
||||
/* This should probably be a 400 level error complaining about field
|
||||
* that do not exist because that is what makes sense in SQL. */
|
||||
assertResponse(emptyMap(), runSql("SELECT * FROM test.test WHERE c = 3", "only_a"));
|
||||
}
|
||||
|
||||
@AwaitsFix(bugUrl="https://github.com/elastic/x-pack-elasticsearch/issues/2074")
|
||||
public void testSqlSingleFieldExcepted() throws IOException {
|
||||
createUser("not_c", "read_test_a_and_b");
|
||||
|
||||
/* This doesn't work because sql doesn't see the field level security
|
||||
* and still adds the metadata even though the field "doesn't exist" */
|
||||
assertResponse(runSql("SELECT a, b FROM test.test", null), runSql("SELECT * FROM test.test", "not_c"));
|
||||
/* This should probably be a 400 level error complaining about field
|
||||
* that do not exist because that is what makes sense in SQL. */
|
||||
assertResponse(emptyMap(), runSql("SELECT * FROM test.test WHERE c = 3", "not_c"));
|
||||
}
|
||||
|
||||
public void testSqlDocumentExclued() throws IOException {
|
||||
createUser("no_3s", "read_test_without_c_3");
|
||||
|
||||
assertResponse(runSql("SELECT * FROM test.test WHERE c != 3", null), runSql("SELECT * FROM test.test", "no_3s"));
|
||||
}
|
||||
|
||||
private void assertResponse(Map<String, Object> expected, Map<String, Object> actual) {
|
||||
if (false == expected.equals(actual)) {
|
||||
NotEqualMessageBuilder message = new NotEqualMessageBuilder();
|
||||
message.compareMaps(actual, expected);
|
||||
fail("Response does not match:\n" + message.toString());
|
||||
}
|
||||
}
|
||||
|
||||
private Map<String, Object> runSql(String sql, @Nullable String asUser) throws IOException {
|
||||
Header[] headers = asUser == null ? new Header[0] : new Header[] {new BasicHeader("es-security-runas-user", asUser)};
|
||||
Response response = client().performRequest("POST", "/_sql", emptyMap(),
|
||||
new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON),
|
||||
headers);
|
||||
try (InputStream content = response.getEntity().getContent()) {
|
||||
return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
|
||||
}
|
||||
}
|
||||
|
||||
private void createUser(String name, String role) throws IOException {
|
||||
XContentBuilder user = JsonXContent.contentBuilder().prettyPrint().startObject(); {
|
||||
user.field("password", "not_used");
|
||||
user.field("roles", role);
|
||||
}
|
||||
user.endObject();
|
||||
client().performRequest("PUT", "/_xpack/security/user/" + name, emptyMap(),
|
||||
new StringEntity(user.string(), ContentType.APPLICATION_JSON));
|
||||
}
|
||||
}
|
||||
|
|
@ -24,7 +24,6 @@ import org.elasticsearch.xpack.sql.analysis.catalog.EsCatalog;
|
|||
import org.elasticsearch.xpack.sql.execution.PlanExecutor;
|
||||
import org.elasticsearch.xpack.sql.session.RowSetCursor;
|
||||
import org.elasticsearch.xpack.sql.session.SqlSettings;
|
||||
import org.joda.time.DateTimeZone;
|
||||
|
||||
import java.util.function.Supplier;
|
||||
|
||||
|
|
@ -61,14 +60,11 @@ public class TransportSqlAction extends HandledTransportAction<SqlRequest, SqlRe
|
|||
protected void doExecute(SqlRequest request, ActionListener<SqlResponse> listener) {
|
||||
String sessionId = request.sessionId();
|
||||
String query = request.query();
|
||||
DateTimeZone timeZone = request.timeZone();
|
||||
|
||||
SqlSettings sqlCfg = new SqlSettings(
|
||||
Settings.builder()
|
||||
// .put(SqlSettings.PAGE_SIZE, req.fetchSize)
|
||||
.put(SqlSettings.TIMEZONE_ID, request.timeZone().getID())
|
||||
.build());
|
||||
|
||||
// NOCOMMIT this should be linked up
|
||||
// SqlSettings sqlCfg = new SqlSettings(Settings.builder()
|
||||
// // .put(SqlSettings.PAGE_SIZE, req.fetchSize)
|
||||
// .put(SqlSettings.TIMEZONE_ID, request.timeZone().getID()).build());
|
||||
try {
|
||||
if (sessionId == null) {
|
||||
if (!Strings.hasText(query)) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue