SQL: More tests and docs for permissions (elastic/x-pack-elasticsearch#3679)

Adds tests for what works and what doesn't when we're missing some of
SQL's required permissions.

Adds required permissions to the documentation of each SQL access
method.

relates elastic/x-pack-elasticsearch#3552

Original commit: elastic/x-pack-elasticsearch@971dabb3b4
This commit is contained in:
Nik Everett 2018-02-01 17:20:44 -05:00 committed by GitHub
parent 1b36133988
commit c53e1f4b1c
10 changed files with 255 additions and 58 deletions

View File

@ -37,3 +37,18 @@ James S.A. Corey |Leviathan Wakes |561 |1306972800000
--------------------------------------------------
// TODO it'd be lovely to be able to assert that this is correct but
// that is probably more work then it is worth right now.
[[sql-cli-permissions]]
[NOTE]
===============================
If you are using Security you need to add a few permissions to
users so they can run SQL. To run SQL using the CLI a user needs
`read`, `indices:admin/get`, and `"cluster:monitor/main"`. The
following example configures a role that can run SQL in the CLI
for the `test` and `bort` indices:
["source","yaml",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{sql-tests}/security/roles.yml[cli_jdbc]
--------------------------------------------------
===============================

View File

@ -50,3 +50,19 @@ connection. For example:
--------------------------------------------------
include-tagged::{jdbc-tests}/SimpleExampleTestCase.java[simple_example]
--------------------------------------------------
[[sql-jdbc-permissions]]
[NOTE]
===============================
If you are using Security you need to add a few permissions to
users so they can run SQL. To run SQL a user needs `read` and
`indices:admin/get`. Some parts of the API require
`"cluster:monitor/main"`. The following example configures a
role that can run SQL in JDBC querying the `test` and `bort`
indices:
["source","yaml",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{sql-tests}/security/roles.yml[cli_jdbc]
--------------------------------------------------
===============================

View File

@ -183,3 +183,17 @@ or fewer results though. `time_zone` is the time zone to use for date
functions and date parsing. `time_zone` defaults to `utc` and can take
any values documented
http://www.joda.org/joda-time/apidocs/org/joda/time/DateTimeZone.html[here].
[[sql-rest-permissions]]
[NOTE]
===============================
If you are using Security you need to add a few permissions to
users so they can run SQL. To run SQL a user needs `read` and
`indices:admin/get`. The following example configures a role
that can run SQL against the `test` and `bort` indices:
["source","yaml",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{sql-tests}/security/roles.yml[rest]
--------------------------------------------------
===============================

View File

@ -51,3 +51,18 @@ the normal <<search-request-body,search>> API.
The request body accepts all of the <<sql-rest-fields,fields>> that
the <<sql-rest,SQL REST API>> accepts except `cursor`.
[[sql-translate-permissions]]
[NOTE]
===============================
If you are using Security you need to add a few permissions to
users so they can run translate SQL. To translate SQL a user
needs `read` and `indices:admin/get`. The following example
configures a role that can run SQL against the `test` and
`bort` indices:
["source","yaml",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{sql-tests}/security/roles.yml[rest]
--------------------------------------------------
===============================

View File

@ -1,22 +1,32 @@
read_all:
cluster:
- "cluster:monitor/main" # Used by JDBC's MetaData
rest_minimal:
indices:
- names: test
privileges: [read, "indices:admin/get"]
- names: bort
privileges: [read, "indices:admin/get"]
# end::rest
# tag::cli_jdbc
cli_or_jdbc_minimal:
cluster:
- "cluster:monitor/main"
indices:
- names: test
privileges: [read, "indices:admin/get"]
- names: bort
privileges: [read, "indices:admin/get"]
# end::cli_jdbc
read_something_else:
cluster:
- "cluster:monitor/main" # Used by JDBC's MetaData
- "cluster:monitor/main"
indices:
- names: something_that_isnt_test
privileges: [read, "indices:admin/get"]
read_test_a:
cluster:
- "cluster:monitor/main" # Used by JDBC's MetaData
- "cluster:monitor/main"
indices:
- names: test
privileges: [read, "indices:admin/get"]
@ -25,7 +35,7 @@ read_test_a:
read_test_a_and_b:
cluster:
- "cluster:monitor/main" # Used by JDBC's MetaData
- "cluster:monitor/main"
indices:
- names: test
privileges: [read, "indices:admin/get"]
@ -35,7 +45,7 @@ read_test_a_and_b:
read_test_without_c_3:
cluster:
- "cluster:monitor/main" # Used by JDBC's MetaData
- "cluster:monitor/main"
indices:
- names: test
privileges: [read, "indices:admin/get"]
@ -54,7 +64,23 @@ read_test_without_c_3:
read_bort:
cluster:
- "cluster:monitor/main" # Used by JDBC's MetaData
- "cluster:monitor/main"
indices:
- names: bort
privileges: [read, "indices:admin/get"]
no_monitor_main:
indices:
- names: test
privileges: [read, "indices:admin/get"]
- names: bort
privileges: [read, "indices:admin/get"]
no_get_index:
cluster:
- "cluster:monitor/main"
indices:
- names: test
privileges: [read]
- names: bort
privileges: [read]

View File

@ -9,7 +9,7 @@ import org.elasticsearch.common.CheckedConsumer;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.xpack.qa.sql.cli.RemoteCli;
import org.elasticsearch.xpack.qa.sql.cli.RemoteCli.SecurityConfig;
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Path;
@ -50,6 +50,11 @@ public class CliSecurityIT extends SqlSecurityTestCase {
* Perform security test actions using the CLI.
*/
private static class CliActions implements Actions {
@Override
public String minimalPermissionsForAllActions() {
return "cli_or_jdbc_minimal";
}
private SecurityConfig userSecurity(String user) {
SecurityConfig admin = adminSecurityConfig();
if (user == null) {
@ -135,6 +140,11 @@ public class CliSecurityIT extends SqlSecurityTestCase {
assertEquals("---------------+---------------", cli.readLine());
for (String table : tables) {
String line = null;
/*
* Security automatically creates either a `.security` or a
* `.security6` index but it might not have created the index
* by the time the test runs.
*/
while (line == null || line.startsWith(".security")) {
line = cli.readLine();
}
@ -170,6 +180,18 @@ public class CliSecurityIT extends SqlSecurityTestCase {
assertThat(cli.readLine(), containsString("Unknown column [" + column + "][1;23;31m][0m"));
}
}
@Override
public void checkNoMonitorMain(String user) throws Exception {
@SuppressWarnings("resource") // forceClose will close it
RemoteCli cli = new RemoteCli(elasticsearchAddress(), true, userSecurity(user)) {
@Override
protected void assertConnectionTest() throws IOException {
assertThat(readLine(), containsString("action [cluster:monitor/main] is unauthorized for user [" + user + "]"));
}
};
cli.forceClose();
}
}
public CliSecurityIT() {

View File

@ -17,6 +17,7 @@ import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.SQLInvalidAuthorizationSpecException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.List;
@ -115,6 +116,11 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
}
private static class JdbcActions implements Actions {
@Override
public String minimalPermissionsForAllActions() {
return "cli_or_jdbc_minimal";
}
@Override
public void queryWorksAsAdmin() throws Exception {
try (Connection h2 = LocalH2.anonymousDb();
@ -182,9 +188,11 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
try (Connection es = es(userProperties(user))) {
ResultSet actual = es.createStatement().executeQuery("SHOW TABLES");
// depending on whether security is enabled and a test is run in isolation or suite
// .security or .security6 index can appear
// to filter these out, the result set is flatten to a list
/*
* Security automatically creates either a `.security` or a
* `.security6` index but it might not have created the index
* by the time the test runs.
*/
List<String> actualList = new ArrayList<>();
while (actual.next()) {
@ -215,6 +223,25 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
con -> con.createStatement().executeQuery(sql),
column);
}
@Override
public void checkNoMonitorMain(String user) throws Exception {
// Most SQL actually works fine without monitor/main
expectMatchesAdmin("SELECT * FROM test", user, "SELECT * FROM test");
expectMatchesAdmin("SHOW TABLES LIKE 'test'", user, "SHOW TABLES LIKE 'test'");
expectMatchesAdmin("DESCRIBE test", user, "DESCRIBE test");
// But there are a few things that don't work
try (Connection es = es(userProperties(user))) {
expectUnauthorized("cluster:monitor/main", user, () -> es.getMetaData().getDatabaseMajorVersion());
expectUnauthorized("cluster:monitor/main", user, () -> es.getMetaData().getDatabaseMinorVersion());
}
}
private void expectUnauthorized(String action, String user, ThrowingRunnable r) {
SQLInvalidAuthorizationSpecException e = expectThrows(SQLInvalidAuthorizationSpecException.class, r);
assertEquals("action [" + action + "] is unauthorized for user [" + user + "]", e.getMessage());
}
}
public JdbcSecurityIT() {
@ -223,7 +250,7 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
// Metadata methods only available to JDBC
public void testMetaDataGetTablesWithFullAccess() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", "cli_or_jdbc_minimal");
expectActionMatchesAdmin(
con -> con.getMetaData().getTables("%", "%", "%t", null),
@ -256,7 +283,7 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
}
public void testMetaDataGetColumnsWorksAsFullAccess() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", "cli_or_jdbc_minimal");
expectActionMatchesAdmin(
con -> con.getMetaData().getColumns("%", "%", "%t", "%"),
@ -314,4 +341,4 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
"no_3s",
con -> con.getMetaData().getColumns("%", "%", "test", "%"));
}
}
}

View File

@ -24,8 +24,10 @@ import java.sql.JDBCType;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.stream.Collectors;
@ -34,9 +36,15 @@ import static org.elasticsearch.xpack.qa.sql.rest.RestSqlTestCase.randomMode;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static java.util.Collections.emptyMap;
public class RestSqlSecurityIT extends SqlSecurityTestCase {
private static class RestActions implements Actions {
@Override
public String minimalPermissionsForAllActions() {
return "rest_minimal";
}
@Override
public void queryWorksAsAdmin() throws Exception {
String mode = randomMode();
@ -121,6 +129,12 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase {
expected.put("rows", rows);
Map<String, Object> actual = runSql(user, mode, "SHOW TABLES");
/*
* Security automatically creates either a `.security` or a
* `.security6` index but it might not have created the index
* by the time the test runs.
*/
@SuppressWarnings("unchecked")
List<List<String>> rowsNoSecurity = ((List<List<String>>) actual.get("rows"))
.stream()
.filter(ls -> ls.get(0).startsWith(".security") == false)
@ -149,6 +163,14 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase {
assertThat(e.getMessage(), containsString("Unknown column [" + column + "]"));
}
@Override
public void checkNoMonitorMain(String user) throws Exception {
// Without monitor/main everything should work just fine
expectMatchesAdmin("SELECT * FROM test", user, "SELECT * FROM test");
expectMatchesAdmin("SHOW TABLES LIKE 'test'", user, "SHOW TABLES LIKE 'test'");
expectMatchesAdmin("DESCRIBE test", user, "DESCRIBE test");
}
private static Map<String, Object> runSql(@Nullable String asUser, String mode, String sql) throws IOException {
return runSql(asUser, mode, new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON));
}
@ -190,7 +212,7 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase {
* paranoid we'd hack together something to test the others as well.
*/
public void testHijackScrollFails() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", "rest_minimal");
Map<String, Object> adminResponse = RestActions.runSql(null, randomMode(),
new StringEntity("{\"query\": \"SELECT * FROM test\", \"fetch_size\": 1}", ContentType.APPLICATION_JSON));

View File

@ -53,6 +53,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
* For methods that take {@code user} a {@code null} user means "use the admin".
*/
protected interface Actions {
String minimalPermissionsForAllActions();
void queryWorksAsAdmin() throws Exception;
/**
* Assert that running some sql as a user returns the same result as running it as
@ -69,6 +70,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
void expectForbidden(String user, String sql) throws Exception;
void expectUnknownIndex(String user, String sql) throws Exception;
void expectUnknownColumn(String user, String sql, String column) throws Exception;
void checkNoMonitorMain(String user) throws Exception;
}
protected static final String SQL_ACTION_NAME = "indices:data/read/sql";
@ -196,7 +198,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
}
public void testQueryWithFullAccess() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", actions.minimalPermissionsForAllActions());
actions.expectMatchesAdmin("SELECT * FROM test ORDER BY a", "full_access", "SELECT * FROM test ORDER BY a");
new AuditLogAsserter()
@ -206,7 +208,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
}
public void testScrollWithFullAccess() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", actions.minimalPermissionsForAllActions());
actions.expectScrollMatchesAdmin("SELECT * FROM test ORDER BY a", "full_access", "SELECT * FROM test ORDER BY a");
new AuditLogAsserter()
@ -342,7 +344,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
}
public void testShowTablesWorksAsFullAccess() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", actions.minimalPermissionsForAllActions());
actions.expectMatchesAdmin("SHOW TABLES LIKE '%t'", "full_access", "SHOW TABLES");
new AuditLogAsserter()
@ -394,7 +396,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
}
public void testDescribeWorksAsFullAccess() throws Exception {
createUser("full_access", "read_all");
createUser("full_access", actions.minimalPermissionsForAllActions());
actions.expectMatchesAdmin("DESCRIBE test", "full_access", "DESCRIBE test");
new AuditLogAsserter()
@ -455,6 +457,19 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
.assertLogs();
}
public void testNoMonitorMain() throws Exception {
createUser("no_monitor_main", "no_monitor_main");
actions.checkNoMonitorMain("no_monitor_main");
}
public void testNoGetIndex() throws Exception {
createUser("no_get_index", "no_get_index");
actions.expectForbidden("no_get_index", "SELECT * FROM test");
actions.expectForbidden("no_get_index", "SHOW TABLES LIKE 'test'");
actions.expectForbidden("no_get_index", "DESCRIBE test");
}
protected static void createUser(String name, String role) throws IOException {
XContentBuilder user = JsonXContent.contentBuilder().prettyPrint().startObject(); {
user.field("password", "testpass");

View File

@ -85,44 +85,61 @@ public class RemoteCli implements Closeable {
out = new PrintWriter(new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8), true);
in = new BufferedReader(new InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8));
// Start the CLI
String command;
if (security == null) {
command = elasticsearchAddress;
} else {
command = security.user + "@" + elasticsearchAddress;
if (security.https) {
command = "https://" + command;
} else if (randomBoolean()) {
command = "http://" + command;
try {
// Start the CLI
String command;
if (security == null) {
command = elasticsearchAddress;
} else {
command = security.user + "@" + elasticsearchAddress;
if (security.https) {
command = "https://" + command;
} else if (randomBoolean()) {
command = "http://" + command;
}
if (security.keystoreLocation != null) {
command = command + " -keystore_location " + security.keystoreLocation;
}
}
if (security.keystoreLocation != null) {
command = command + " -keystore_location " + security.keystoreLocation;
if (false == checkConnectionOnStartup) {
command += " -check false";
}
}
if (false == checkConnectionOnStartup) {
command += " -check false";
}
/* Don't use println because it emits \r\n on windows but we put the
* terminal in unix mode to make the tests consistent. */
out.print(command + "\n");
out.flush();
// Feed it passwords if needed
if (security != null && security.keystoreLocation != null) {
assertEquals("keystore password: ", readUntil(s -> s.endsWith(": ")));
out.print(security.keystorePassword + "\n");
/* Don't use println because it emits \r\n on windows but we put the
* terminal in unix mode to make the tests consistent. */
out.print(command + "\n");
out.flush();
}
if (security != null) {
assertEquals("password: ", readUntil(s -> s.endsWith(": ")));
out.print(security.password + "\n");
out.flush();
}
// Throw out the logo and warnings about making a dumb terminal
while (false == readLine().contains("SQL"));
// Throw out the empty line before all the good stuff
// Feed it passwords if needed
if (security != null && security.keystoreLocation != null) {
assertEquals("keystore password: ", readUntil(s -> s.endsWith(": ")));
out.print(security.keystorePassword + "\n");
out.flush();
}
if (security != null) {
assertEquals("password: ", readUntil(s -> s.endsWith(": ")));
out.print(security.password + "\n");
out.flush();
}
// Throw out the logo and warnings about making a dumb terminal
while (false == readLine().contains("SQL"));
assertConnectionTest();
} catch (AssertionError | Exception e) {
/* If there is an error during connection then try and
* force the socket shut. */
forceClose();
throw e;
}
}
/**
* Assert that result of the connection test. Default implementation
* asserts that the test passes but overridden to check places where
* we want to assert that it fails.
*/
protected void assertConnectionTest() throws IOException {
// After the connection test passess we emit an empty line and then the prompt
assertEquals("", readLine());
}
@ -146,13 +163,21 @@ public class RemoteCli implements Closeable {
}
assertThat("unconsumed lines", nonQuit, empty());
} finally {
out.close();
in.close();
// Most importantly, close the socket so the next test can use the fixture
socket.close();
forceClose();
}
}
/**
* Shutdown the connection to the remote CLI without attempting to shut
* the remote down in an orderly way.
*/
public void forceClose() throws IOException {
out.close();
in.close();
// Most importantly, close the socket so the next test can use the fixture
socket.close();
}
/**
* Send a command and assert the echo.
*/