From 139073e8f7ad42d304787db3a7e26df96f93c927 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Thu, 1 Dec 2016 10:17:28 -0500 Subject: [PATCH] security: improve migrate tool output and remove trappy config option This commit improves the output of the migrate tool in cases when there are errors parsing entries in the roles or users files. This is done through the use of a logger that delegates its output to the terminal. Additionally, the `-c` option has been removed. This option was used to set the configuration directory but this should be handled one way only and that is through the use of the `-Epath.conf` setting. Closes elastic/elasticsearch#3757 Closes elastic/elasticsearch#3758 Original commit: elastic/x-pack-elasticsearch@811e36776659bab91ecfbae713a5f439352319af --- .../esnative/ESNativeRealmMigrateTool.java | 90 +++++++++++++++---- .../esnative/ESNativeMigrateToolTests.java | 31 ++++--- .../ESNativeRealmMigrateToolTests.java | 79 ++++++++++++++++ .../xpack/security/MigrateToolIT.java | 9 +- 4 files changed, 177 insertions(+), 32 deletions(-) diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java index df502a83d94..5cd54a39e40 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateTool.java @@ -10,12 +10,25 @@ import javax.net.ssl.HttpsURLConnection; import joptsimple.OptionParser; import joptsimple.OptionSet; import joptsimple.OptionSpec; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.LoggerConfig; +import org.apache.logging.log4j.core.layout.PatternLayout; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.cli.MultiCommand; import org.elasticsearch.cli.SettingCommand; import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.Terminal.Verbosity; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.ESLoggerFactory; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -31,12 +44,14 @@ import org.elasticsearch.xpack.security.authz.store.FileRolesStore; import org.elasticsearch.xpack.ssl.SSLService; import java.io.BufferedReader; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.URI; import java.net.URL; +import java.nio.file.Files; import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; @@ -72,7 +87,6 @@ public class ESNativeRealmMigrateTool extends MultiCommand { private final OptionSpec url; private final OptionSpec usersToMigrateCsv; private final OptionSpec rolesToMigrateCsv; - private final OptionSpec esConfigDir; public MigrateUserOrRoles() { super("Migrates users or roles from file to native realm"); @@ -91,9 +105,6 @@ public class ESNativeRealmMigrateTool extends MultiCommand { this.rolesToMigrateCsv = parser.acceptsAll(Arrays.asList("r", "roles"), "Roles to migrate from file to native realm") .withRequiredArg(); - this.esConfigDir = parser.acceptsAll(Arrays.asList("c", "config"), - "Configuration directory to use instead of default") - .withRequiredArg(); } // Visible for testing @@ -114,9 +125,6 @@ public class ESNativeRealmMigrateTool extends MultiCommand { terminal.println("starting migration of users and roles..."); Settings.Builder sb = Settings.builder(); sb.put(settings); - if (this.esConfigDir != null) { - sb.put("path.conf", this.esConfigDir.value(options)); - } Settings shieldSettings = sb.build(); Environment shieldEnv = new Environment(shieldSettings); importUsers(terminal, shieldSettings, shieldEnv, options); @@ -187,7 +195,7 @@ public class ESNativeRealmMigrateTool extends MultiCommand { } } - public Set getUsersThatExist(Terminal terminal, Settings settings, Environment env, OptionSet options) throws Exception { + Set getUsersThatExist(Terminal terminal, Settings settings, Environment env, OptionSet options) throws Exception { Set existingUsers = new HashSet<>(); String allUsersJson = postURL(settings, env, "GET", this.url.value(options) + "/_xpack/security/user/", options, null); try (XContentParser parser = JsonXContent.jsonXContent.createParser(allUsersJson)) { @@ -208,7 +216,7 @@ public class ESNativeRealmMigrateTool extends MultiCommand { return existingUsers; } - public static String createUserJson(String[] roles, char[] password) throws IOException { + static String createUserJson(String[] roles, char[] password) throws IOException { XContentBuilder builder = jsonBuilder(); builder.startObject(); { @@ -223,14 +231,21 @@ public class ESNativeRealmMigrateTool extends MultiCommand { return builder.string(); } - public void importUsers(Terminal terminal, Settings settings, Environment env, OptionSet options) { + void importUsers(Terminal terminal, Settings settings, Environment env, OptionSet options) throws FileNotFoundException { String usersCsv = usersToMigrateCsv.value(options); String[] usersToMigrate = (usersCsv != null) ? usersCsv.split(",") : Strings.EMPTY_ARRAY; Path usersFile = FileUserPasswdStore.resolveFile(env); Path usersRolesFile = FileUserRolesStore.resolveFile(env); + if (Files.exists(usersFile) == false) { + throw new FileNotFoundException("users file [" + usersFile + "] does not exist"); + } else if (Files.exists(usersRolesFile) == false) { + throw new FileNotFoundException("users_roles file [" + usersRolesFile + "] does not exist"); + } + terminal.println("importing users from [" + usersFile + "]..."); - Map userToHashedPW = FileUserPasswdStore.parseFile(usersFile, null, settings); - Map userToRoles = FileUserRolesStore.parseFile(usersRolesFile, null); + final Logger logger = getTerminalLogger(terminal); + Map userToHashedPW = FileUserPasswdStore.parseFile(usersFile, logger, settings); + Map userToRoles = FileUserRolesStore.parseFile(usersRolesFile, logger); Set existingUsers; try { existingUsers = getUsersThatExist(terminal, settings, env, options); @@ -242,7 +257,7 @@ public class ESNativeRealmMigrateTool extends MultiCommand { } for (String user : usersToMigrate) { if (userToHashedPW.containsKey(user) == false) { - terminal.println("no user [" + user + "] found, skipping"); + terminal.println("user [" + user + "] was not found in files, skipping"); continue; } else if (existingUsers.contains(user)) { terminal.println("user [" + user + "] already exists, skipping"); @@ -261,7 +276,7 @@ public class ESNativeRealmMigrateTool extends MultiCommand { } } - public Set getRolesThatExist(Terminal terminal, Settings settings, Environment env, OptionSet options) throws Exception { + Set getRolesThatExist(Terminal terminal, Settings settings, Environment env, OptionSet options) throws Exception { Set existingRoles = new HashSet<>(); String allRolesJson = postURL(settings, env, "GET", this.url.value(options) + "/_xpack/security/role/", options, null); try (XContentParser parser = JsonXContent.jsonXContent.createParser(allRolesJson)) { @@ -282,18 +297,22 @@ public class ESNativeRealmMigrateTool extends MultiCommand { return existingRoles; } - public static String createRoleJson(RoleDescriptor rd) throws IOException { + static String createRoleJson(RoleDescriptor rd) throws IOException { XContentBuilder builder = jsonBuilder(); rd.toXContent(builder, ToXContent.EMPTY_PARAMS); return builder.string(); } - public void importRoles(Terminal terminal, Settings settings, Environment env, OptionSet options) { + void importRoles(Terminal terminal, Settings settings, Environment env, OptionSet options) throws FileNotFoundException { String rolesCsv = rolesToMigrateCsv.value(options); String[] rolesToMigrate = (rolesCsv != null) ? rolesCsv.split(",") : Strings.EMPTY_ARRAY; Path rolesFile = FileRolesStore.resolveFile(env).toAbsolutePath(); + if (Files.exists(rolesFile) == false) { + throw new FileNotFoundException("roles.yml file [" + rolesFile + "] does not exist"); + } terminal.println("importing roles from [" + rolesFile + "]..."); - Map roles = FileRolesStore.parseRoleDescriptors(rolesFile, null, true, Settings.EMPTY); + Logger logger = getTerminalLogger(terminal); + Map roles = FileRolesStore.parseRoleDescriptors(rolesFile, logger, true, settings); Set existingRoles; try { existingRoles = getRolesThatExist(terminal, settings, env, options); @@ -324,4 +343,41 @@ public class ESNativeRealmMigrateTool extends MultiCommand { } } } + + /** + * Creates a new Logger that is detached from the ROOT logger and only has an appender that will output log messages to the terminal + */ + static Logger getTerminalLogger(final Terminal terminal) { + final Logger logger = ESLoggerFactory.getLogger(ESNativeRealmMigrateTool.class); + Loggers.setLevel(logger, Level.ALL); + + // create appender + final Appender appender = new AbstractAppender(ESNativeRealmMigrateTool.class.getName(), null, + PatternLayout.newBuilder().withPattern("%m").build()) { + @Override + public void append(LogEvent event) { + switch (event.getLevel().getStandardLevel()) { + case FATAL: + case ERROR: + terminal.println(Verbosity.NORMAL, event.getMessage().getFormattedMessage()); + break; + case OFF: + break; + default: + terminal.println(Verbosity.VERBOSE, event.getMessage().getFormattedMessage()); + break; + } + } + }; + appender.start(); + + // get the config, detach from parent, remove appenders, add custom appender + final LoggerContext ctx = (LoggerContext) LogManager.getContext(false); + final Configuration config = ctx.getConfiguration(); + final LoggerConfig loggerConfig = config.getLoggerConfig(ESNativeRealmMigrateTool.class.getName()); + loggerConfig.setParent(null); + loggerConfig.getAppenders().forEach((s, a) -> Loggers.removeAppender(logger, a)); + Loggers.addAppender(logger, appender); + return logger; + } } diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java index 6155afe89be..a06db65a590 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeMigrateToolTests.java @@ -34,7 +34,7 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { private static boolean useSSL; @BeforeClass - private static void setSSL() { + public static void setSSL() { useSSL = randomBoolean(); } @@ -54,13 +54,14 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { return useSSL; } - private String homePath() throws Exception { - Environment e = internalCluster().getInstances(Environment.class).iterator().next(); - return e.configFile().toAbsolutePath().toString(); + private Environment nodeEnvironment() throws Exception { + return internalCluster().getInstances(Environment.class).iterator().next(); } public void testRetrieveUsers() throws Exception { - String home = homePath(); + final Environment nodeEnvironment = nodeEnvironment(); + String home = Environment.PATH_HOME_SETTING.get(nodeEnvironment.settings()); + String conf = Environment.PATH_CONF_SETTING.get(nodeEnvironment.settings()); SecurityClient c = new SecurityClient(client()); logger.error("--> creating users"); int numToAdd = randomIntBetween(1,10); @@ -81,11 +82,14 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { Settings sslSettings = SecuritySettingsSource.getSSLSettingsForStore("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks", "testnode"); - Settings settings = Settings.builder().put(sslSettings).put("path.home", home).build(); + Settings settings = Settings.builder().put(sslSettings) + .put("path.home", home) + .put("path.conf", conf) + .build(); logger.error("--> retrieving users using URL: {}, home: {}", url, home); OptionParser parser = muor.getParser(); - OptionSet options = parser.parse("-u", username, "-p", password, "-U", url, "-c", home); + OptionSet options = parser.parse("-u", username, "-p", password, "-U", url, "-Epath.conf=" + conf); logger.info("--> options: {}", options.asMap()); Set users = muor.getUsersThatExist(t, settings, new Environment(settings), options); logger.info("--> output: \n{}", t.getOutput());; @@ -95,11 +99,13 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { } public void testRetrieveRoles() throws Exception { - String home = homePath(); + final Environment nodeEnvironment = nodeEnvironment(); + String home = Environment.PATH_HOME_SETTING.get(nodeEnvironment.settings()); + String conf = Environment.PATH_CONF_SETTING.get(nodeEnvironment.settings()); SecurityClient c = new SecurityClient(client()); logger.error("--> creating roles"); int numToAdd = randomIntBetween(1,10); - Set addedRoles = new HashSet(numToAdd); + Set addedRoles = new HashSet<>(numToAdd); for (int i = 0; i < numToAdd; i++) { String rname = randomAsciiOfLength(5); c.preparePutRole(rname) @@ -121,11 +127,14 @@ public class ESNativeMigrateToolTests extends NativeRealmIntegTestCase { Settings sslSettings = SecuritySettingsSource.getSSLSettingsForStore("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.jks", "testclient"); - Settings settings = Settings.builder().put(sslSettings).put("path.home", home).build(); + Settings settings = Settings.builder().put(sslSettings) + .put("path.home", home) + .put("path.conf", conf) + .build(); logger.error("--> retrieving roles using URL: {}, home: {}", url, home); OptionParser parser = muor.getParser(); - OptionSet options = parser.parse("-u", username, "-p", password, "-U", url, "-c", home); + OptionSet options = parser.parse("-u", username, "-p", password, "-U", url, "-Epath.conf", conf); Set roles = muor.getRolesThatExist(t, settings, new Environment(settings), options); logger.info("--> output: \n{}", t.getOutput());; for (String r : addedRoles) { diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateToolTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateToolTests.java index fa95a68d60f..91850990166 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateToolTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ESNativeRealmMigrateToolTests.java @@ -5,13 +5,29 @@ */ package org.elasticsearch.xpack.security.authc.esnative; +import joptsimple.OptionSet; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.Logger; import org.elasticsearch.cli.Command; import org.elasticsearch.cli.CommandTestCase; +import org.elasticsearch.cli.MockTerminal; +import org.elasticsearch.cli.Terminal.Verbosity; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; import org.elasticsearch.xpack.security.authz.RoleDescriptor; import org.elasticsearch.xpack.security.authz.permission.FieldPermissions; +import java.io.FileNotFoundException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.isEmptyString; /** * Unit tests for the {@code ESNativeRealmMigrateTool} @@ -46,4 +62,67 @@ public class ESNativeRealmMigrateToolTests extends CommandTestCase { "\"privileges\":[\"all\"],\"field_security\":{\"grant\":[\"body\"]}}],\"run_as\":[],\"metadata\":{}}")); } + public void testTerminalLogger() throws Exception { + Logger terminalLogger = ESNativeRealmMigrateTool.getTerminalLogger(terminal); + assertThat(terminal.getOutput(), isEmptyString()); + + // only error and fatal gets logged at normal verbosity + terminal.setVerbosity(Verbosity.NORMAL); + List nonLoggingLevels = new ArrayList<>(Arrays.asList(Level.values())); + nonLoggingLevels.removeAll(Arrays.asList(Level.ERROR, Level.FATAL)); + for (Level level : nonLoggingLevels) { + terminalLogger.log(level, "this level should not log " + level.name()); + assertThat(terminal.getOutput(), isEmptyString()); + } + + terminalLogger.log(Level.ERROR, "logging an error"); + assertEquals("logging an error" + System.lineSeparator(), terminal.getOutput()); + terminal.reset(); + assertThat(terminal.getOutput(), isEmptyString()); + + terminalLogger.log(Level.FATAL, "logging a fatal message"); + assertEquals("logging a fatal message" + System.lineSeparator(), terminal.getOutput()); + terminal.reset(); + assertThat(terminal.getOutput(), isEmptyString()); + + // everything will get logged at verbose! + terminal.setVerbosity(Verbosity.VERBOSE); + List loggingLevels = new ArrayList<>(Arrays.asList(Level.values())); + loggingLevels.remove(Level.OFF); + for (Level level : loggingLevels) { + terminalLogger.log(level, "this level should log " + level.name()); + assertEquals("this level should log " + level.name() + System.lineSeparator(), terminal.getOutput()); + terminal.reset(); + assertThat(terminal.getOutput(), isEmptyString()); + } + } + + public void testMissingFiles() throws Exception { + Path homeDir = createTempDir(); + Path confDir = homeDir.resolve("config"); + Path xpackConfDir = confDir.resolve("x-pack"); + Files.createDirectories(xpackConfDir); + + ESNativeRealmMigrateTool.MigrateUserOrRoles muor = new ESNativeRealmMigrateTool.MigrateUserOrRoles(); + OptionSet options = muor.getParser().parse("-u", "elastic", "-p", "changeme", "-U", "http://localhost:9200"); + Settings settings = Settings.builder() + .put("path.home", homeDir) + .put("path.conf", confDir) + .build(); + Environment environment = new Environment(settings); + MockTerminal mockTerminal = new MockTerminal(); + + FileNotFoundException fnfe = expectThrows(FileNotFoundException.class, + () -> muor.importUsers(mockTerminal, settings, environment, options)); + assertThat(fnfe.getMessage(), containsString("users file")); + + Files.createFile(xpackConfDir.resolve("users")); + fnfe = expectThrows(FileNotFoundException.class, + () -> muor.importUsers(mockTerminal, settings, environment, options)); + assertThat(fnfe.getMessage(), containsString("users_roles file")); + + fnfe = expectThrows(FileNotFoundException.class, + () -> muor.importRoles(mockTerminal, settings, environment, options)); + assertThat(fnfe.getMessage(), containsString("roles.yml file")); + } } diff --git a/qa/security-migrate-tests/src/test/java/org/elasticsearch/xpack/security/MigrateToolIT.java b/qa/security-migrate-tests/src/test/java/org/elasticsearch/xpack/security/MigrateToolIT.java index 8af7f56b5aa..6d4468d2c30 100644 --- a/qa/security-migrate-tests/src/test/java/org/elasticsearch/xpack/security/MigrateToolIT.java +++ b/qa/security-migrate-tests/src/test/java/org/elasticsearch/xpack/security/MigrateToolIT.java @@ -12,6 +12,7 @@ import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.client.Client; import org.elasticsearch.client.Requests; import org.elasticsearch.common.Priority; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.xpack.security.action.role.GetRolesResponse; @@ -46,18 +47,18 @@ public class MigrateToolIT extends MigrateToolTestCase { } public void testRunMigrateTool() throws Exception { + logger.info("--> CONF: {}", System.getProperty("tests.config.dir")); Settings settings = Settings.builder() - .put("path.home", createTempDir().toAbsolutePath().toString()) + .put("path.home", PathUtils.get(System.getProperty("tests.config.dir")).getParent()) + .put("path.conf", System.getProperty("tests.config.dir")) .build(); - String integHome = System.getProperty("tests.config.dir"); - logger.info("--> HOME: {}", integHome); // Cluster should already be up String url = "http://" + getHttpURL(); logger.info("--> using URL: {}", url); MockTerminal t = new MockTerminal(); ESNativeRealmMigrateTool.MigrateUserOrRoles muor = new ESNativeRealmMigrateTool.MigrateUserOrRoles(); OptionParser parser = muor.getParser(); - OptionSet options = parser.parse("-u", "test_admin", "-p", "changeme", "-U", url, "-c", integHome); + OptionSet options = parser.parse("-u", "test_admin", "-p", "changeme", "-U", url); muor.execute(t, options, settings.getAsMap()); logger.info("--> output:\n{}", t.getOutput());