diff --git a/dev-tools/ci b/dev-tools/ci index c663447bc78..8513791c874 100755 --- a/dev-tools/ci +++ b/dev-tools/ci @@ -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) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java b/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java index b01dbf3a78e..8079e8f8981 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java @@ -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); diff --git a/qa/sql-security/build.gradle b/qa/sql-security/build.gradle new file mode 100644 index 00000000000..b864c0bca20 --- /dev/null +++ b/qa/sql-security/build.gradle @@ -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() + } +} diff --git a/qa/sql-security/roles.yml b/qa/sql-security/roles.yml new file mode 100644 index 00000000000..7c6aa681306 --- /dev/null +++ b/qa/sql-security/roles.yml @@ -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 + } + } + ] + } + } diff --git a/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java b/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java new file mode 100644 index 00000000000..7d16b8415ad --- /dev/null +++ b/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java @@ -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 + * es-security-runas-user 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 expected = new HashMap<>(); + Map 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 row1 = new HashMap<>(); + row1.put("a", 1); + row1.put("b", 2); + row1.put("c", 3); + Map 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 expected, Map actual) { + if (false == expected.equals(actual)) { + NotEqualMessageBuilder message = new NotEqualMessageBuilder(); + message.compareMaps(actual, expected); + fail("Response does not match:\n" + message.toString()); + } + } + + private Map 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)); + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java index 75369bc9320..8e16dcace54 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java @@ -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 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)) {