Security: use x-pack config files when present (#33688)

In versions prior to 6.3, x-pack configuration files were kept within a
x-pack subdirectory. In 6.3 these files were moved to the config
directory with a backwards compatibilty layer to look for the files in
the old location if file does not exist in the new location. This works
for files that are not pre-created for the user, but in the case of
security there are several files created by default. This change
updates the security code to check if the file in the new location is
a default file that has not been modified. If it is a default file, the
code will also see if a file exists in the pre 6.3 location and use it
instead.

Closes #33464

Author: jaymode

x-pack/plugin/security/build.gradle

@@ -137,6 +137,13 @@ artifacts {
     archives jar
     testArtifacts testJar
 }
+
+processResources {
+    from('src/main/config') {
+        into 'config'
+    }
+}
+
 sourceSets.test.resources {
     srcDir '../core/src/test/resources'
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.security;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.Version;
@@ -27,8 +28,8 @@
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.util.Providers;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.logging.LoggerMessageFormat;
-import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.regex.Regex;
@@ -221,7 +222,10 @@
 import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Clock;
 import java.util.ArrayList;
@@ -253,7 +257,8 @@
 public class Security extends Plugin implements ActionPlugin, IngestPlugin, NetworkPlugin, ClusterPlugin, DiscoveryPlugin, MapperPlugin,
         ExtensiblePlugin {
 
-    private static final Logger logger = Loggers.getLogger(Security.class);
+    private static final Logger LOGGER = LogManager.getLogger(Security.class);
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LOGGER);
 
     public static final String NAME4 = XPackField.SECURITY + "4";
     public static final Setting<Optional<String>> USER_SETTING =
@@ -535,7 +540,7 @@ private AuthenticationFailureHandler createAuthenticationFailureHandler(final Re
             extensionName = extension.toString();
         }
         if (failureHandler == null) {
-            logger.debug("Using default authentication failure handler");
+            LOGGER.debug("Using default authentication failure handler");
             final Map<String, List<String>> defaultFailureResponseHeaders = new HashMap<>();
             realms.asList().stream().forEach((realm) -> {
                 Map<String, List<String>> realmFailureHeaders = realm.getAuthenticationFailureHeaders();
@@ -556,7 +561,7 @@ private AuthenticationFailureHandler createAuthenticationFailureHandler(final Re
             }
             failureHandler = new DefaultAuthenticationFailureHandler(defaultFailureResponseHeaders);
         } else {
-            logger.debug("Using authentication failure handler from extension [" + extensionName + "]");
+            LOGGER.debug("Using authentication failure handler from extension [" + extensionName + "]");
         }
         return failureHandler;
     }
@@ -949,7 +954,7 @@ static void validateAutoCreateIndex(Settings settings) {
                 }
             }
 
-            logger.warn("the [action.auto_create_index] setting is configured to be restrictive [{}]. " +
+            LOGGER.warn("the [action.auto_create_index] setting is configured to be restrictive [{}]. " +
                     " for the next 6 months audit indices are allowed to be created, but please make sure" +
                     " that any future history indices after 6 months with the pattern " +
                     "[.security_audit_log*] are allowed to be created", value);
@@ -1039,7 +1044,7 @@ public UnaryOperator<Map<String, IndexTemplateMetaData>> getIndexTemplateMetaDat
                 templates.put(SECURITY_TEMPLATE_NAME, IndexTemplateMetaData.Builder.fromXContent(parser, SECURITY_TEMPLATE_NAME));
             } catch (IOException e) {
                 // TODO: should we handle this with a thrown exception?
-                logger.error("Error loading template [{}] as part of metadata upgrading", SECURITY_TEMPLATE_NAME);
+                LOGGER.error("Error loading template [{}] as part of metadata upgrading", SECURITY_TEMPLATE_NAME);
             }
 
             final byte[] auditTemplate = TemplateUtils.loadTemplate("/" + IndexAuditTrail.INDEX_TEMPLATE_NAME + ".json",
@@ -1049,12 +1054,12 @@ public UnaryOperator<Map<String, IndexTemplateMetaData>> getIndexTemplateMetaDat
                     .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, auditTemplate)) {
                 IndexTemplateMetaData auditMetadata = new IndexTemplateMetaData.Builder(
                         IndexTemplateMetaData.Builder.fromXContent(parser, IndexAuditTrail.INDEX_TEMPLATE_NAME))
-                        .settings(IndexAuditTrail.customAuditIndexSettings(settings, logger))
+                        .settings(IndexAuditTrail.customAuditIndexSettings(settings, LOGGER))
                         .build();
                 templates.put(IndexAuditTrail.INDEX_TEMPLATE_NAME, auditMetadata);
             } catch (IOException e) {
                 // TODO: should we handle this with a thrown exception?
-                logger.error("Error loading template [{}] as part of metadata upgrading", IndexAuditTrail.INDEX_TEMPLATE_NAME);
+                LOGGER.error("Error loading template [{}] as part of metadata upgrading", IndexAuditTrail.INDEX_TEMPLATE_NAME);
             }
 
             return templates;
@@ -1167,4 +1172,55 @@ public void accept(DiscoveryNode node, ClusterState state) {
     public void reloadSPI(ClassLoader loader) {
         securityExtensions.addAll(SecurityExtension.loadExtensions(loader));
     }
+
+    public static Path resolveConfigFile(Environment env, String name) {
+        final Path config = env.configFile().resolve(name);
+        final Path legacyConfig = env.configFile().resolve("x-pack").resolve(name);
+        // config and legacy config can be the same path if name is an absolute path
+        if (config.equals(legacyConfig) == false) {
+            final boolean configFileExists = Files.exists(config);
+            final boolean legacyConfigExists = Files.exists(legacyConfig);
+            if (configFileExists == false) {
+                if (legacyConfigExists) {
+                    DEPRECATION_LOGGER.deprecated("Config file [" + name + "] is in a deprecated location. Move from " +
+                        legacyConfig.toString() + " to " + config.toString());
+                    return legacyConfig;
+                }
+            } else if (legacyConfigExists) {
+                // there is a file in both locations
+                if (isDefaultFile(name, config)) {
+                    // use the legacy file as the new file is the default but warn user
+                    DEPRECATION_LOGGER.deprecated("Config file [" + name + "] exists in a deprecated location and non-deprecated " +
+                        "location. The file in the non-deprecated location is the default file. Using file found in the deprecated " +
+                        "location. Move " + legacyConfig.toString() + " to " + config.toString());
+                    return legacyConfig;
+                } else {
+                    // the regular file has been modified, but the old still exists, warn the user
+                    DEPRECATION_LOGGER.deprecated("Config file [" + name + "] exists in a deprecated location and non-deprecated " +
+                        "location. Using file found in the non-deprecated location [" + config.toString() + "]. Determine which file " +
+                        "should be kept and move it to " + config.toString() + ", then remove " + legacyConfig.toString());
+                }
+            }
+        }
+        return config;
+    }
+
+    static boolean isDefaultFile(String name, Path file) {
+        try (InputStream in = XPackPlugin.class.getResourceAsStream("/config/" + name)) {
+            if (in != null) {
+                try (InputStream fin = Files.newInputStream(file)) {
+                    int inValue = in.read();
+                    int finValue = fin.read();
+                    while (inValue != -1 && finValue != -1 && inValue == finValue) {
+                        inValue = in.read();
+                        finValue = fin.read();
+                    }
+                    return inValue == finValue;
+                }
+            }
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+        return false;
+    }
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStore.java

@@ -17,7 +17,6 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
@@ -26,6 +25,7 @@
 import org.elasticsearch.xpack.core.security.support.Validation;
 import org.elasticsearch.xpack.core.security.support.Validation.Users;
 import org.elasticsearch.xpack.core.security.user.User;
+import org.elasticsearch.xpack.security.Security;
 import org.elasticsearch.xpack.security.support.SecurityFiles;
 
 import java.io.IOException;
@@ -92,7 +92,7 @@ public boolean userExists(String username) {
     }
 
     public static Path resolveFile(Environment env) {
-        return XPackPlugin.resolveConfigFile(env, "users");
+        return Security.resolveConfigFile(env, "users");
     }
 
     /**

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java

@@ -16,10 +16,10 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.support.NoOpLogger;
 import org.elasticsearch.xpack.core.security.support.Validation;
+import org.elasticsearch.xpack.security.Security;
 import org.elasticsearch.xpack.security.support.SecurityFiles;
 
 import java.io.IOException;
@@ -79,7 +79,7 @@ public String[] roles(String username) {
     }
 
     public static Path resolveFile(Environment env) {
-        return XPackPlugin.resolveConfigFile(env, "users_roles");
+        return Security.resolveConfigFile(env, "users_roles");
     }
 
     /**

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java

@@ -31,10 +31,10 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.support.CachingRealm;
 import org.elasticsearch.xpack.core.security.authc.support.DnRoleMapperSettings;
+import org.elasticsearch.xpack.security.Security;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableMap;
@@ -80,7 +80,7 @@ synchronized void addListener(Runnable listener) {
 
     public static Path resolveFile(Settings settings, Environment env) {
         String location = DnRoleMapperSettings.ROLE_MAPPING_FILE_SETTING.get(settings);
-        return XPackPlugin.resolveConfigFile(env, location);
+        return Security.resolveConfigFile(env, location);
     }
 
     /**

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java

@@ -22,13 +22,13 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
 import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
 import org.elasticsearch.xpack.core.security.support.NoOpLogger;
 import org.elasticsearch.xpack.core.security.support.Validation;
+import org.elasticsearch.xpack.security.Security;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -130,7 +130,7 @@ Set<String> getAllRoleNames() {
     }
 
     public static Path resolveFile(Environment env) {
-        return XPackPlugin.resolveConfigFile(env, "roles.yml");
+        return Security.resolveConfigFile(env, "roles.yml");
     }
 
     public static Set<String> parseFileForRoleNames(Path path, Logger logger) {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java

@@ -49,6 +49,9 @@
 import org.junit.Before;
 
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -456,4 +459,63 @@ public void testGetFieldFilterSecurityEnabledDeprecatedSetting() throws Exceptio
                     "future release! See the breaking changes documentation for the next major version.");
         }
     }
+
+    /**
+     * Tests the default file comparison check. Note: this will fail when run in an IDE as the
+     * processing of resources isn't handled properly.
+     *
+     * TODO: can we add an assume for whether the test should run if it is an IDE context?
+     */
+    public void testIsDefaultFileCheck() throws Exception {
+        Path homeDir = createTempDir();
+        Path configDir = homeDir.resolve("config");
+        Path xPackConfigDir = configDir.resolve("x-pack");
+        Environment environment = new Environment(Settings.builder().put("path.home", homeDir).build(), configDir);
+
+        List<String> defaultFiles = Arrays.asList("roles.yml", "role_mapping.yml", "users", "users_roles");
+        Files.createDirectories(xPackConfigDir);
+
+        for (String defaultFileName : defaultFiles) {
+            logger.info("testing default file: {}", defaultFileName);
+            Path defaultFile = getDataPath("/config/" + defaultFileName);
+            final byte[] defaultBytes = Files.readAllBytes(defaultFile);
+            final Path defaultFileConfigPath = configDir.resolve(defaultFileName);
+            Path resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(defaultFileConfigPath, resolvedPath);
+
+            Files.write(defaultFileConfigPath, defaultBytes);
+            assertTrue(Security.isDefaultFile(defaultFileName, defaultFileConfigPath));
+
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(defaultFileConfigPath, resolvedPath);
+
+            // put a file in x-pack dir
+            final Path xPackFilePath = xPackConfigDir.resolve(defaultFileName);
+            Files.write(xPackFilePath, Collections.singletonList(randomAlphaOfLength(8)));
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(xPackFilePath, resolvedPath);
+            assertWarnings("Config file [" + defaultFileName + "] exists in a deprecated location and non-deprecated location. The" +
+                " file in the non-deprecated location is the default file. Using file found in the deprecated location. Move " +
+                xPackFilePath + " to " + defaultFileConfigPath);
+
+            // modify file in new location
+            Files.write(defaultFileConfigPath, Collections.singletonList(randomAlphaOfLength(8)),
+                randomBoolean() ? StandardOpenOption.TRUNCATE_EXISTING : StandardOpenOption.APPEND);
+
+            assertFalse(Security.isDefaultFile(defaultFileName, defaultFileConfigPath));
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(defaultFileConfigPath, resolvedPath);
+            assertWarnings("Config file [" + defaultFileName + "] exists in a deprecated location and non-deprecated location. " +
+                "Using file found in the non-deprecated location [" + defaultFileConfigPath + "]. Determine which file should be kept and" +
+                    " move it to " + defaultFileConfigPath + ", then remove " + xPackFilePath);
+
+            // remove default file
+            Files.delete(defaultFileConfigPath);
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(xPackFilePath, resolvedPath);
+            assertWarnings("Config file [" + defaultFileName + "] is in a deprecated location. Move from " +
+                xPackFilePath + " to " + defaultFileConfigPath);
+        }
+
+    }
 }