Browse Source

Auto-generated TLS files under fixed config path (#81547)

We (mostly I) were initially advocating for the auto-generated files to
use unique names (the name containing a timestamp particle), in order to
avoid that subsequent invocations of the config step conflict with
itself. Moreover, I was wishing that these files will not have to be
handled directly by admins (that the enrollment process was to be used).
However, experience proved us otherwise, admins have to manipulate these
files, and unique configuration names are hard to deal with in scripts
and docs, so this PR is all about using a fixed name for all the
generated files. _Labeling as a bug fix because the feedback is that it
very negatively impacts usabilty._ Closes
https://github.com/elastic/elasticsearch/issues/81057
Albert Zaharovits 3 years ago
parent
commit
9e9a8cc7d0

+ 3 - 6
distribution/packages/src/common/scripts/postinst

@@ -62,12 +62,9 @@ if [ "x$IS_UPGRADE" != "xtrue" ]; then
     ES_ADDITIONAL_CLASSPATH_DIRECTORIES=lib/tools/security-cli \
     /usr/share/elasticsearch/bin/elasticsearch-cli <<< ""; then
         # Above command runs as root and TLS keystores are created group-owned by root. It's simple to correct the ownership here
-        for dir in "${ES_PATH_CONF}"/tls_auto_config_*
-        do
-            chown root:elasticsearch "${dir}"/http_keystore_local_node.p12
-            chown root:elasticsearch "${dir}"/http_ca.crt
-            chown root:elasticsearch "${dir}"/transport_keystore_all_nodes.p12
-        done
+        chown root:elasticsearch "${ES_PATH_CONF}"/certs/http.p12
+        chown root:elasticsearch "${ES_PATH_CONF}"/certs/http_ca.crt
+        chown root:elasticsearch "${ES_PATH_CONF}"/certs/transport.p12
         if INITIAL_PASSWORD=$(ES_MAIN_CLASS=org.elasticsearch.xpack.security.enrollment.tool.AutoConfigGenerateElasticPasswordHash \
         ES_ADDITIONAL_SOURCES="x-pack-env;x-pack-security-env" \
         ES_ADDITIONAL_CLASSPATH_DIRECTORIES=lib/tools/security-cli \

+ 3 - 6
distribution/packages/src/common/scripts/postrm

@@ -103,12 +103,9 @@ if [ "$REMOVE_DIRS" = "true" ]; then
 
     # delete the security auto config directory if we are purging
     if [ "$REMOVE_SECURITY_AUTO_CONFIG_DIRECTORY" = "true" ]; then
-      for dir in "${ES_PATH_CONF}"/tls_auto_config_*
-      do
-        echo -n "Deleting security auto-configuration directory..."
-        rm -rf "${dir}"
-        echo "OK"
-      done
+      echo -n "Deleting security auto-configuration directory..."
+      rm -rf "${ES_PATH_CONF}"/certs
+      echo "OK"
     fi
 
     # delete the elasticsearch keystore if we are purging

+ 1 - 2
docs/reference/setup/install/check-running.asciidoc

@@ -5,8 +5,7 @@ You can test that your {es} node is running by sending an HTTPS request to port
 
 ["source","sh",subs="attributes"]
 ----
-curl --cacert {os-dir}{slash}tls_auto_config_<timestamp>{slash}http_ca.crt \
--u elastic https://localhost:9200 <1>
+curl --cacert {os-dir}{slash}certs{slash}http_ca.crt -u elastic https://localhost:9200 <1>
 ----
 // NOTCONSOLE
 <1> Ensure that you use `https` in your call, or the request will fail.

+ 1 - 1
docs/reference/setup/install/deb.asciidoc

@@ -201,7 +201,7 @@ locations for a Debian-based system:
 
 | conf
 | Generated TLS keys and certificates for the transport and http layer.
-| /etc/elasticsearch/auto_config_tls_<timestamp>
+| /etc/elasticsearch/certs
 d|
 
 | data

+ 4 - 4
docs/reference/setup/install/docker.asciidoc

@@ -112,7 +112,7 @@ your local machine.
 +
 [source,sh]
 ----
-docker cp es01:/usr/share/elasticsearch/config/tls_auto_config_*/http_ca.crt .
+docker cp es01:/usr/share/elasticsearch/config/certs/http_ca.crt .
 ----
 
 . Open a new terminal and verify that you can connect to your {es} cluster by
@@ -185,7 +185,7 @@ serious development or go into production with {es}, review the
 
 When you start {es} for the first time, the following certificates and keys are
 generated in the
-`/usr/share/elasticsearch/config/tls_auto_config_initial_node_<timestamp>`
+`/usr/share/elasticsearch/config/certs`
 directory in the Docker container, and allow you to connect a {kib} instance
 to your secured {es} cluster and encrypt internode communication. The files are
 listed here for reference.
@@ -194,10 +194,10 @@ listed here for reference.
 The CA certificate that is used to sign the certificates for the HTTP layer of
 this {es} cluster.
 
-`http_keystore_local_node.p12`::
+`http.p12`::
 Keystore that contains the key and certificate for the HTTP layer for this node.
 
-`transport_keystore_all_nodes.p12`::
+`transport.p12`::
 Keystore that contains the key and certificate for the transport layer for all
 the nodes in your cluster.
 

+ 1 - 1
docs/reference/setup/install/rpm.asciidoc

@@ -194,7 +194,7 @@ locations for an RPM-based system:
 
 | conf
 | Generated TLS keys and certificates for the transport and http layer.
-| /etc/elasticsearch/auto_config_tls_<timestamp>
+| /etc/elasticsearch/certs
 d|
 
 | data

+ 2 - 2
docs/reference/setup/install/security-files-reference.asciidoc

@@ -10,9 +10,9 @@ The files are listed here for reference.
 The CA certificate that is used to sign the certificates for the HTTP layer of
 this {es} cluster.
 
-`http_keystore_local_node.p12`::
+`http.p12`::
 Keystore that contains the key and certificate for the HTTP layer for this node.
 
-`transport_keystore_all_nodes.p12`::
+`transport.p12`::
 Keystore that contains the key and certificate for the transport layer for all
 the nodes in your cluster.

+ 1 - 1
docs/reference/setup/install/targz.asciidoc

@@ -147,7 +147,7 @@ directory so that you do not delete important data later on.
 
 | conf
   | Generated TLS keys and certificates for the transport and HTTP layer.
-  | $ES_HOME/config/tls_auto_config_<timestamp>
+  | $ES_HOME/config/certs
  d|
 
 | data

+ 2 - 2
docs/reference/setup/install/zip-windows.asciidoc

@@ -123,7 +123,7 @@ C:\elasticsearch-{version}{backslash}bin>\bin\elasticsearch-reset-password -u el
 ----
 
 NOTE: While a JRE can be used for the {es} service, due to its use of a client
-VM (as opposed to a server JVM which offers better performance for long-running 
+VM (as opposed to a server JVM which offers better performance for long-running
 applications) its usage is discouraged and a warning will be issued.
 
 NOTE: The system environment variable `ES_JAVA_HOME` should be set to the path
@@ -289,7 +289,7 @@ directory so that you do not delete important data later on.
 
 | conf
   | Generated TLS keys and certificates for the transport and HTTP layer.
-  | %ES_HOME%\config\tls_auto_config_<timestamp>
+  | %ES_HOME%\config\certs
  d|
 
 | data

+ 2 - 2
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java

@@ -434,7 +434,7 @@ public class DockerTests extends PackagingTestCase {
         copyFromContainer(installation.config("elasticsearch.yml"), tempDir.resolve("elasticsearch.yml"));
         copyFromContainer(installation.config("elasticsearch.keystore"), tempDir.resolve("elasticsearch.keystore"));
         copyFromContainer(installation.config("log4j2.properties"), tempDir.resolve("log4j2.properties"));
-        final Path autoConfigurationDir = findInContainer(installation.config, "d", "\"tls_auto_config_*\"");
+        final Path autoConfigurationDir = findInContainer(installation.config, "d", "\"certs\"");
         final String autoConfigurationDirName = autoConfigurationDir.getFileName().toString();
         copyFromContainer(autoConfigurationDir, tempDir.resolve(autoConfigurationDirName));
 
@@ -530,7 +530,7 @@ public class DockerTests extends PackagingTestCase {
         copyFromContainer(installation.config("jvm.options"), tempEsConfigDir);
         copyFromContainer(installation.config("elasticsearch.keystore"), tempEsConfigDir);
         copyFromContainer(installation.config("log4j2.properties"), tempEsConfigDir);
-        final Path autoConfigurationDir = findInContainer(installation.config, "d", "\"tls_auto_config_*\"");
+        final Path autoConfigurationDir = findInContainer(installation.config, "d", "\"certs\"");
         assertThat(autoConfigurationDir, notNullValue());
         final String autoConfigurationDirName = autoConfigurationDir.getFileName().toString();
         copyFromContainer(autoConfigurationDir, tempEsConfigDir.resolve(autoConfigurationDirName));

+ 8 - 23
qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java

@@ -656,12 +656,12 @@ public abstract class PackagingTestCase extends Assert {
             // We chown the installation on Windows to Administrators so that we can auto-configure it.
             String owner = Platforms.WINDOWS ? "BUILTIN\\Administrators" : "elasticsearch";
             assertThat(es.config(autoConfigDirName.get()), FileMatcher.file(Directory, owner, owner, p750));
-            Stream.of("http_keystore_local_node.p12", "http_ca.crt", "transport_keystore_all_nodes.p12")
+            Stream.of("http.p12", "http_ca.crt", "transport.p12")
                 .forEach(file -> assertThat(es.config(autoConfigDirName.get()).resolve(file), FileMatcher.file(File, owner, owner, p660)));
             configLines = Files.readAllLines(es.config("elasticsearch.yml"));
         } else if (es.distribution.isDocker()) {
             assertThat(es.config(autoConfigDirName.get()), DockerFileMatcher.file(Directory, "elasticsearch", "root", p750));
-            Stream.of("http_keystore_local_node.p12", "http_ca.crt", "transport_keystore_all_nodes.p12")
+            Stream.of("http.p12", "http_ca.crt", "transport.p12")
                 .forEach(
                     file -> assertThat(
                         es.config(autoConfigDirName.get()).resolve(file),
@@ -676,7 +676,7 @@ public abstract class PackagingTestCase extends Assert {
         } else {
             assert es.distribution.isPackage();
             assertThat(es.config(autoConfigDirName.get()), FileMatcher.file(Directory, "root", "elasticsearch", p750));
-            Stream.of("http_keystore_local_node.p12", "http_ca.crt", "transport_keystore_all_nodes.p12")
+            Stream.of("http.p12", "http_ca.crt", "transport.p12")
                 .forEach(
                     file -> assertThat(
                         es.config(autoConfigDirName.get()).resolve(file),
@@ -692,24 +692,9 @@ public abstract class PackagingTestCase extends Assert {
 
         assertThat(configLines, hasItem("xpack.security.enrollment.enabled: true"));
         assertThat(configLines, hasItem("xpack.security.transport.ssl.verification_mode: certificate"));
-        assertThat(
-            configLines,
-            hasItem(
-                "xpack.security.transport.ssl.keystore.path: "
-                    + es.config(autoConfigDirName.get()).resolve("transport_keystore_all_nodes.p12")
-            )
-        );
-        assertThat(
-            configLines,
-            hasItem(
-                "xpack.security.transport.ssl.truststore.path: "
-                    + es.config(autoConfigDirName.get()).resolve("transport_keystore_all_nodes.p12")
-            )
-        );
-        assertThat(
-            configLines,
-            hasItem("xpack.security.http.ssl.keystore.path: " + es.config(autoConfigDirName.get()).resolve("http_keystore_local_node.p12"))
-        );
+        assertThat(configLines, hasItem("xpack.security.transport.ssl.keystore.path: certs/transport.p12"));
+        assertThat(configLines, hasItem("xpack.security.transport.ssl.truststore.path: certs/transport.p12"));
+        assertThat(configLines, hasItem("xpack.security.http.ssl.keystore.path: certs/http.p12"));
         if (es.distribution.isDocker() == false) {
             assertThat(configLines, hasItem("http.host: [_local_, _site_]"));
         }
@@ -734,7 +719,7 @@ public abstract class PackagingTestCase extends Assert {
         assertThat(configLines, not(contains(containsString("automatically generated in order to configure Security"))));
         Path caCert = ServerUtils.getCaCert(installation);
         if (caCert != null) {
-            assertThat(caCert.toString(), Matchers.not(Matchers.containsString("tls_auto_config")));
+            assertThat(caCert.toString(), Matchers.not(Matchers.containsString("certs")));
         }
     }
 
@@ -746,7 +731,7 @@ public abstract class PackagingTestCase extends Assert {
             lsResult = sh.run("find \"" + es.config + "\" -type d -maxdepth 1");
         }
         assertNotNull(lsResult.stdout);
-        return Arrays.stream(lsResult.stdout.split("\n")).filter(f -> f.contains("tls_auto_config_")).findFirst();
+        return Arrays.stream(lsResult.stdout.split("\n")).filter(f -> f.contains("certs")).findFirst();
     }
 
 }

+ 2 - 2
qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java

@@ -173,7 +173,7 @@ public class ServerUtils {
     public static Path getCaCert(Installation installation) throws IOException {
         if (installation.distribution.isDocker()) {
             final Path tempDir = PackagingTestCase.createTempDir("docker-ssl");
-            final Path autoConfigurationDir = findInContainer(installation.config, "d", "\"tls_auto_config_*\"");
+            final Path autoConfigurationDir = findInContainer(installation.config, "d", "\"certs\"");
             if (autoConfigurationDir != null) {
                 final Path hostHttpCaCert = tempDir.resolve("http_ca.crt");
                 copyFromContainer(autoConfigurationDir.resolve("http_ca.crt"), hostHttpCaCert);
@@ -199,7 +199,7 @@ public class ServerUtils {
         }
         if (enrollmentEnabled && httpSslEnabled) {
             assert Files.exists(caCert) == false;
-            List<Path> allAutoconfTLS = FileUtils.lsGlob(configPath, "tls_auto_config_*");
+            List<Path> allAutoconfTLS = FileUtils.lsGlob(configPath, "certs*");
             assertThat(allAutoconfTLS.size(), is(1));
             Path autoconfTLSDir = allAutoconfTLS.get(0);
             caCert = autoconfTLSDir.resolve("http_ca.crt");

+ 4 - 8
x-pack/docs/en/security/configuring-stack-security.asciidoc

@@ -60,13 +60,9 @@ user when prompted:
 +
 [source,shell]
 ----
-curl --cacert config/tls_auto_config_<timestamp>/http_ca.crt \
--u elastic https://localhost:9200
+curl --cacert config/certs/http_ca.crt -u elastic https://localhost:9200
 ----
 // NOTCONSOLE
-+
-`<timestamp>`:: The timestamp of when the auto-configuration process created
-the security files directory in your Docker container.
 
 . From the directory where you installed {kib}, start {kib}.
 +
@@ -108,7 +104,7 @@ can <<encrypt-kibana-browser,encrypt traffic between your browser and {kib}>>.
 === Security certificates and keys
 
 When you start {es} for the first time, the following certificates and keys are
-generated in the `config/tls_auto_config_<timestamp>` directory,
+generated in the `config/certs` directory,
 which are used to connect a {kib} instance to your secured {es} cluster and
 to encrypt internode communication. The files are listed here for reference.
 
@@ -116,10 +112,10 @@ to encrypt internode communication. The files are listed here for reference.
 The CA certificate that is used to sign the certificates for the HTTP layer of
 this {es} cluster.
 
-`http_keystore_local_node.p12`::
+`http.p12`::
 Keystore that contains the key and certificate for the HTTP layer for this node.
 
-`transport_keystore_all_nodes.p12`::
+`transport.p12`::
 Keystore that contains the key and certificate for the transport layer for all
 the nodes in your cluster.
 

+ 2 - 2
x-pack/docs/en/security/enroll-nodes.asciidoc

@@ -25,7 +25,7 @@ bin{slash}elasticsearch --enrollment-token <enrollment-token>
 +
 ["source","sh",subs="attributes"]
 ----
-config{slash}tls_auto_config_node_<timestamp>
+config{slash}certs
 ----
 
-. Repeat the previous step for any new nodes that you want to enroll.
+. Repeat the previous step for any new nodes that you want to enroll.

+ 184 - 90
x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigureNode.java

@@ -105,22 +105,22 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
     public static final String AUTO_CONFIG_ALT_DN = "CN=Elasticsearch security auto-configuration HTTP CA";
     // the transport keystore is also used as a truststore
     private static final String SIGNATURE_ALGORITHM = "SHA256withRSA";
-    private static final String TRANSPORT_AUTOGENERATED_KEYSTORE_NAME = "transport_keystore_all_nodes";
-    private static final String TRANSPORT_AUTOGENERATED_KEY_ALIAS = "transport_all_nodes_key";
-    private static final String TRANSPORT_AUTOGENERATED_CERT_ALIAS = "transport_all_nodes_cert";
-    private static final String HTTP_AUTOGENERATED_KEY_ALIAS = "http_local_node_key";
-    private static final String HTTP_CA_AUTOGENERATED_KEY_ALIAS = "http_ca_local_node_key";
+    private static final String TRANSPORT_AUTOGENERATED_KEYSTORE_NAME = "transport";
+    private static final String TRANSPORT_KEY_KEYSTORE_ENTRY = "transport";
+    private static final String TRANSPORT_CA_CERT_KEYSTORE_ENTRY = "transport_ca";
     private static final int TRANSPORT_CERTIFICATE_DAYS = 99 * 365;
     private static final int TRANSPORT_CA_CERTIFICATE_DAYS = 99 * 365;
     private static final int TRANSPORT_KEY_SIZE = 4096;
     private static final int TRANSPORT_CA_KEY_SIZE = 4096;
-    static final String HTTP_AUTOGENERATED_KEYSTORE_NAME = "http_keystore_local_node";
+    static final String HTTP_AUTOGENERATED_KEYSTORE_NAME = "http";
+    static final String HTTP_KEY_KEYSTORE_ENTRY = "http";
+    static final String HTTP_CA_KEY_KEYSTORE_ENTRY = "http_ca";
     static final String HTTP_AUTOGENERATED_CA_NAME = "http_ca";
     private static final int HTTP_CA_CERTIFICATE_DAYS = 3 * 365;
     private static final int HTTP_CA_KEY_SIZE = 4096;
     private static final int HTTP_CERTIFICATE_DAYS = 2 * 365;
     private static final int HTTP_KEY_SIZE = 4096;
-    static final String TLS_CONFIG_DIR_NAME_PREFIX = "tls_auto_config_";
+    static final String TLS_GENERATED_CERTS_DIR_NAME = "certs";
     static final String AUTO_CONFIGURATION_START_MARKER =
         "#----------------------- Security auto configuration start -----------------------#";
     static final String AUTO_CONFIGURATION_END_MARKER =
@@ -163,7 +163,6 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
 
         // pre-flight checks for the files that are going to be changed
         final Path ymlPath = env.configFile().resolve("elasticsearch.yml");
-        final Path keystorePath = KeyStoreWrapper.keystorePath(env.configFile());
         // it is odd for the `elasticsearch.yml` file to be missing or not be a regular (the node won't start)
         // but auto configuration should not be concerned with fixing it (by creating the file) and let the node startup fail
         if (false == Files.exists(ymlPath) || false == Files.isRegularFile(ymlPath, LinkOption.NOFOLLOW_LINKS)) {
@@ -184,6 +183,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
             );
             notifyOfFailure(inEnrollmentMode, terminal, Terminal.Verbosity.NORMAL, ExitCodes.NOOP, msg);
         }
+        final Path keystorePath = KeyStoreWrapper.keystorePath(env.configFile());
         // Inform that auto-configuration will not run if keystore cannot be read.
         if (Files.exists(keystorePath)
             && (false == Files.isRegularFile(keystorePath, LinkOption.NOFOLLOW_LINKS) || false == Files.isReadable(keystorePath))) {
@@ -197,7 +197,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
 
         if (options.has(reconfigure)) {
             if (false == inEnrollmentMode) {
-                throw new UserException(ExitCodes.USAGE, "enrollment-token is a mandatory parameter.");
+                throw new UserException(ExitCodes.USAGE, "enrollment-token is a mandatory parameter when reconfiguring the node");
             }
             env = possibleReconfigureNode(env, terminal);
         }
@@ -207,20 +207,20 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         checkExistingConfiguration(env.settings(), inEnrollmentMode, terminal);
 
         final ZonedDateTime autoConfigDate = ZonedDateTime.now(ZoneOffset.UTC);
-        final String instantAutoConfigName = TLS_CONFIG_DIR_NAME_PREFIX + autoConfigDate.toInstant().getEpochSecond();
-        final Path instantAutoConfigDir = env.configFile().resolve(instantAutoConfigName);
+        final Path tempGeneratedTlsCertsDir = env.configFile()
+            .resolve(String.format(Locale.ROOT, TLS_GENERATED_CERTS_DIR_NAME + ".%d.tmp", autoConfigDate.toInstant().getEpochSecond()));
         try {
             // it is useful to pre-create the sub-config dir in order to check that the config dir is writable and that file owners match
-            Files.createDirectory(instantAutoConfigDir);
+            Files.createDirectory(tempGeneratedTlsCertsDir);
             // set permissions to 750, don't rely on umask, we assume auto configuration preserves ownership so we don't have to
             // grant "group" or "other" permissions
-            PosixFileAttributeView view = Files.getFileAttributeView(instantAutoConfigDir, PosixFileAttributeView.class);
+            PosixFileAttributeView view = Files.getFileAttributeView(tempGeneratedTlsCertsDir, PosixFileAttributeView.class);
             if (view != null) {
                 view.setPermissions(PosixFilePermissions.fromString("rwxr-x---"));
             }
         } catch (Throwable t) {
             try {
-                deleteDirectory(instantAutoConfigDir);
+                deleteDirectory(tempGeneratedTlsCertsDir);
             } catch (Exception ex) {
                 t.addSuppressed(ex);
             }
@@ -235,17 +235,22 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         // This is a sort of sanity check.
         // If the node process works OK given the owner of the config dir, it should also tolerate the auto-created config dir,
         // provided that they both have the same owner and permissions.
-        final UserPrincipal newFileOwner = Files.getOwner(instantAutoConfigDir, LinkOption.NOFOLLOW_LINKS);
+        final UserPrincipal newFileOwner = Files.getOwner(tempGeneratedTlsCertsDir, LinkOption.NOFOLLOW_LINKS);
         if (false == newFileOwner.equals(Files.getOwner(env.configFile(), LinkOption.NOFOLLOW_LINKS))) {
-            deleteDirectory(instantAutoConfigDir);
             // the following is only printed once, if the node starts successfully
-            throw new UserException(
+            UserException userException = new UserException(
                 ExitCodes.CONFIG,
                 "Aborting auto configuration because of config dir ownership mismatch. Config dir is owned by "
                     + Files.getOwner(env.configFile(), LinkOption.NOFOLLOW_LINKS).getName()
                     + " but auto-configuration directory would be owned by "
                     + newFileOwner.getName()
             );
+            try {
+                deleteDirectory(tempGeneratedTlsCertsDir);
+            } catch (Exception ex) {
+                userException.addSuppressed(ex);
+            }
+            throw userException;
         }
         final X509Certificate transportCaCert;
         final PrivateKey transportKey;
@@ -266,7 +271,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 enrollmentToken = EnrollmentToken.decodeFromString(enrollmentTokenParam.value(options));
             } catch (Exception e) {
                 try {
-                    deleteDirectory(instantAutoConfigDir);
+                    deleteDirectory(tempGeneratedTlsCertsDir);
                 } catch (Exception ex) {
                     e.addSuppressed(ex);
                 }
@@ -306,22 +311,32 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 }
             }
             if (enrollResponse == null || enrollResponse.getHttpStatus() != 200) {
-                deleteDirectory(instantAutoConfigDir);
-                throw new UserException(
+                UserException userException = new UserException(
                     ExitCodes.UNAVAILABLE,
                     "Aborting enrolling to cluster. "
                         + "Could not communicate with the node on any of the addresses from the enrollment token. All of "
                         + enrollmentToken.getBoundAddress()
                         + " were attempted."
                 );
+                try {
+                    deleteDirectory(tempGeneratedTlsCertsDir);
+                } catch (Exception ex) {
+                    userException.addSuppressed(ex);
+                }
+                throw userException;
             }
             final Map<String, Object> responseMap = enrollResponse.getResponseBody();
             if (responseMap == null) {
-                deleteDirectory(instantAutoConfigDir);
-                throw new UserException(
+                UserException userException = new UserException(
                     ExitCodes.DATA_ERROR,
                     "Aborting enrolling to cluster. Empty response when calling the enroll node API (" + enrollNodeUrl + ")"
                 );
+                try {
+                    deleteDirectory(tempGeneratedTlsCertsDir);
+                } catch (Exception ex) {
+                    userException.addSuppressed(ex);
+                }
+                throw userException;
             }
             final List<String> missingFields = new ArrayList<>();
             final String httpCaKeyPem = (String) responseMap.get("http_ca_key");
@@ -349,8 +364,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 missingFields.add("nodes_addresses");
             }
             if (false == missingFields.isEmpty()) {
-                deleteDirectory(instantAutoConfigDir);
-                throw new UserException(
+                UserException userException = new UserException(
                     ExitCodes.DATA_ERROR,
                     "Aborting enrolling to cluster. Invalid response when calling the enroll node API ("
                         + enrollNodeUrl
@@ -358,6 +372,12 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                         + "The following fields were empty or missing : "
                         + missingFields
                 );
+                try {
+                    deleteDirectory(tempGeneratedTlsCertsDir);
+                } catch (Exception ex) {
+                    userException.addSuppressed(ex);
+                }
+                throw userException;
             }
             transportCaCert = parseCertificateFromPem(transportCaCertPem, terminal);
             httpCaKey = parseKeyFromPem(httpCaKeyPem, terminal);
@@ -409,7 +429,11 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                     SIGNATURE_ALGORITHM
                 );
             } catch (Throwable t) {
-                deleteDirectory(instantAutoConfigDir);
+                try {
+                    deleteDirectory(tempGeneratedTlsCertsDir);
+                } catch (Exception ex) {
+                    t.addSuppressed(ex);
+                }
                 // this is an error which mustn't be ignored during node startup
                 // the exit code for unhandled Exceptions is "1"
                 throw t;
@@ -432,7 +456,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
 
             // the HTTP CA PEM file is provided "just in case". The node doesn't use it, but clients (configured manually, outside of the
             // enrollment process) might indeed need it, and it is currently impossible to retrieve it
-            fullyWriteFile(instantAutoConfigDir, HTTP_AUTOGENERATED_CA_NAME + ".crt", false, stream -> {
+            fullyWriteFile(tempGeneratedTlsCertsDir, HTTP_AUTOGENERATED_CA_NAME + ".crt", false, stream -> {
                 try (
                     JcaPEMWriter pemWriter = new JcaPEMWriter(new BufferedWriter(new OutputStreamWriter(stream, StandardCharsets.UTF_8)))
                 ) {
@@ -440,21 +464,27 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 }
             });
         } catch (Throwable t) {
-            deleteDirectory(instantAutoConfigDir);
+            try {
+                deleteDirectory(tempGeneratedTlsCertsDir);
+            } catch (Exception ex) {
+                t.addSuppressed(ex);
+            }
             // this is an error which mustn't be ignored during node startup
             // the exit code for unhandled Exceptions is "1"
             throw t;
         }
 
-        // save original keystore before updating (replacing)
+        // save the existing keystore before replacing
         final Path keystoreBackupPath = env.configFile()
-            .resolve(KeyStoreWrapper.KEYSTORE_FILENAME + "." + autoConfigDate.toInstant().getEpochSecond() + ".orig");
+            .resolve(
+                String.format(Locale.ROOT, KeyStoreWrapper.KEYSTORE_FILENAME + ".%d.orig", autoConfigDate.toInstant().getEpochSecond())
+            );
         if (Files.exists(keystorePath)) {
             try {
                 Files.copy(keystorePath, keystoreBackupPath, StandardCopyOption.COPY_ATTRIBUTES);
             } catch (Throwable t) {
                 try {
-                    deleteDirectory(instantAutoConfigDir);
+                    deleteDirectory(tempGeneratedTlsCertsDir);
                 } catch (Exception ex) {
                     t.addSuppressed(ex);
                 }
@@ -468,7 +498,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
             return nodeKeystorePassword.get().clone();
         })) {
             // do not overwrite keystore entries
-            // instead expect the user to manually remove them herself
+            // instead expect the admin to manually remove them herself
             if (nodeKeystore.getSettingNames().contains("xpack.security.transport.ssl.keystore.secure_password")
                 || nodeKeystore.getSettingNames().contains("xpack.security.transport.ssl.truststore.secure_password")
                 || nodeKeystore.getSettingNames().contains("xpack.security.http.ssl.keystore.secure_password")) {
@@ -486,14 +516,14 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 transportKeystore.load(null);
                 // the PKCS12 keystore and the contained private key use the same password
                 transportKeystore.setKeyEntry(
-                    TRANSPORT_AUTOGENERATED_KEY_ALIAS,
+                    TRANSPORT_KEY_KEYSTORE_ENTRY,
                     transportKey,
                     transportKeystorePassword.getChars(),
                     new Certificate[] { transportCert }
                 );
-                transportKeystore.setCertificateEntry(TRANSPORT_AUTOGENERATED_CERT_ALIAS, transportCaCert);
+                transportKeystore.setCertificateEntry(TRANSPORT_CA_CERT_KEYSTORE_ENTRY, transportCaCert);
                 fullyWriteFile(
-                    instantAutoConfigDir,
+                    tempGeneratedTlsCertsDir,
                     TRANSPORT_AUTOGENERATED_KEYSTORE_NAME + ".p12",
                     false,
                     stream -> transportKeystore.store(stream, transportKeystorePassword.getChars())
@@ -508,19 +538,19 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 // the keystore contains both the node's and the CA's private keys
                 // both keys are encrypted using the same password as the PKCS12 keystore they're contained in
                 httpKeystore.setKeyEntry(
-                    HTTP_CA_AUTOGENERATED_KEY_ALIAS,
+                    HTTP_CA_KEY_KEYSTORE_ENTRY,
                     httpCaKey,
                     httpKeystorePassword.getChars(),
                     new Certificate[] { httpCaCert }
                 );
                 httpKeystore.setKeyEntry(
-                    HTTP_AUTOGENERATED_KEY_ALIAS,
+                    HTTP_KEY_KEYSTORE_ENTRY,
                     httpKey,
                     httpKeystorePassword.getChars(),
                     new Certificate[] { httpCert, httpCaCert }
                 );
                 fullyWriteFile(
-                    instantAutoConfigDir,
+                    tempGeneratedTlsCertsDir,
                     HTTP_AUTOGENERATED_KEYSTORE_NAME + ".p12",
                     false,
                     stream -> httpKeystore.store(stream, httpKeystorePassword.getChars())
@@ -533,7 +563,13 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
             // restore keystore to revert possible keystore bootstrap
             try {
                 if (Files.exists(keystoreBackupPath)) {
-                    Files.move(keystoreBackupPath, keystorePath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
+                    Files.move(
+                        keystoreBackupPath,
+                        keystorePath,
+                        StandardCopyOption.REPLACE_EXISTING,
+                        StandardCopyOption.ATOMIC_MOVE,
+                        StandardCopyOption.COPY_ATTRIBUTES
+                    );
                 } else {
                     Files.deleteIfExists(keystorePath);
                 }
@@ -541,7 +577,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                 t.addSuppressed(ex);
             }
             try {
-                deleteDirectory(instantAutoConfigDir);
+                deleteDirectory(tempGeneratedTlsCertsDir);
             } catch (Exception ex) {
                 t.addSuppressed(ex);
             }
@@ -552,6 +588,76 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
             }
         }
 
+        try {
+            // all certs and keys have been generated in the temp certs dir, therefore:
+            // 1. backup (move) any previously existing tls certs dir (this backup is NOT removed when auto-conf finishes)
+            if (Files.exists(env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME))) {
+                moveDirectory(
+                    env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME),
+                    env.configFile()
+                        .resolve(
+                            String.format(
+                                Locale.ROOT,
+                                TLS_GENERATED_CERTS_DIR_NAME + ".%d.orig",
+                                autoConfigDate.toInstant().getEpochSecond()
+                            )
+                        )
+                );
+            }
+            // 2. move the newly populated temp certs dir to its permanent static dir name
+            moveDirectory(tempGeneratedTlsCertsDir, env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME));
+        } catch (Throwable t) {
+            // restore keystore to revert possible keystore bootstrap
+            try {
+                if (Files.exists(keystoreBackupPath)) {
+                    Files.move(
+                        keystoreBackupPath,
+                        keystorePath,
+                        StandardCopyOption.REPLACE_EXISTING,
+                        StandardCopyOption.ATOMIC_MOVE,
+                        StandardCopyOption.COPY_ATTRIBUTES
+                    );
+                } else {
+                    Files.deleteIfExists(keystorePath);
+                }
+            } catch (Exception ex) {
+                t.addSuppressed(ex);
+            }
+            // revert any previously existing TLS certs
+            try {
+                if (Files.exists(
+                    env.configFile()
+                        .resolve(
+                            String.format(
+                                Locale.ROOT,
+                                TLS_GENERATED_CERTS_DIR_NAME + ".%d.orig",
+                                autoConfigDate.toInstant().getEpochSecond()
+                            )
+                        )
+                )) {
+                    moveDirectory(
+                        env.configFile()
+                            .resolve(
+                                String.format(
+                                    Locale.ROOT,
+                                    TLS_GENERATED_CERTS_DIR_NAME + ".%d.orig",
+                                    autoConfigDate.toInstant().getEpochSecond()
+                                )
+                            ),
+                        env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME)
+                    );
+                }
+            } catch (Exception ex) {
+                t.addSuppressed(ex);
+            }
+            try {
+                deleteDirectory(tempGeneratedTlsCertsDir);
+            } catch (Exception ex) {
+                t.addSuppressed(ex);
+            }
+            throw t;
+        }
+
         try {
             // final Environment to be used in the lambda below
             final Environment localFinalEnv = env;
@@ -603,7 +709,10 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                     bw.newLine();
                     bw.write(
                         "xpack.security.transport.ssl.keystore.path: "
-                            + instantAutoConfigDir.resolve(TRANSPORT_AUTOGENERATED_KEYSTORE_NAME + ".p12")
+                            + TLS_GENERATED_CERTS_DIR_NAME
+                            + "/"
+                            + TRANSPORT_AUTOGENERATED_KEYSTORE_NAME
+                            + ".p12"
                     );
                     bw.newLine();
                     // we use the keystore as a truststore in order to minimize the number of auto-generated resources,
@@ -611,7 +720,10 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                     // no one should only need the TLS cert without the associated key for the transport layer
                     bw.write(
                         "xpack.security.transport.ssl.truststore.path: "
-                            + instantAutoConfigDir.resolve(TRANSPORT_AUTOGENERATED_KEYSTORE_NAME + ".p12")
+                            + TLS_GENERATED_CERTS_DIR_NAME
+                            + "/"
+                            + TRANSPORT_AUTOGENERATED_KEYSTORE_NAME
+                            + ".p12"
                     );
                     bw.newLine();
 
@@ -619,7 +731,11 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                     bw.write("xpack.security.http.ssl.enabled: true");
                     bw.newLine();
                     bw.write(
-                        "xpack.security.http.ssl.keystore.path: " + instantAutoConfigDir.resolve(HTTP_AUTOGENERATED_KEYSTORE_NAME + ".p12")
+                        "xpack.security.http.ssl.keystore.path: "
+                            + TLS_GENERATED_CERTS_DIR_NAME
+                            + "/"
+                            + HTTP_AUTOGENERATED_KEYSTORE_NAME
+                            + ".p12"
                     );
                     bw.newLine();
                     if (inEnrollmentMode) {
@@ -679,28 +795,33 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         } catch (Throwable t) {
             try {
                 if (Files.exists(keystoreBackupPath)) {
-                    Files.move(
-                        keystoreBackupPath,
-                        keystorePath,
-                        StandardCopyOption.REPLACE_EXISTING,
-                        StandardCopyOption.ATOMIC_MOVE,
-                        StandardCopyOption.COPY_ATTRIBUTES
-                    );
+                    Files.move(keystoreBackupPath, keystorePath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
                 } else {
+                    // this removes a statically named file, so it is potentially dangerous
                     Files.deleteIfExists(keystorePath);
                 }
             } catch (Exception ex) {
                 t.addSuppressed(ex);
             }
             try {
-                deleteDirectory(instantAutoConfigDir);
+                // this removes a statically named directory, so it is potentially dangerous
+                deleteDirectory(env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME));
             } catch (Exception ex) {
                 t.addSuppressed(ex);
             }
+            Path backupCertsDir = env.configFile()
+                .resolve(
+                    String.format(Locale.ROOT, TLS_GENERATED_CERTS_DIR_NAME + ".%d.orig", autoConfigDate.toInstant().getEpochSecond())
+                );
+            if (Files.exists(backupCertsDir)) {
+                moveDirectory(backupCertsDir, env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME));
+            }
             throw t;
         }
-        // only delete the backed up file if all went well
-        Files.deleteIfExists(keystoreBackupPath);
+        // only delete the backed-up keystore file if all went well, because the new keystore contains its entries
+        if (Files.exists(keystoreBackupPath)) {
+            Files.delete(keystoreBackupPath);
+        }
     }
 
     private String initialMasterNodesSettingValue(Environment environment) {
@@ -722,7 +843,6 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         // We remove the existing auto-configuration stanza from elasticsearch.yml, the elastisearch.keystore and
         // the directory with the auto-configured TLS key material, and then proceed as if elasticsearch is started
         // with --enrolment-token token, in the first place.
-        final String autoConfigDirName = getAutoConfigDirName(env);
         final List<String> existingConfigLines;
         try {
             existingConfigLines = Files.readAllLines(env.configFile().resolve("elasticsearch.yml"), StandardCharsets.UTF_8);
@@ -731,12 +851,13 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
             throw new UserException(ExitCodes.IO_ERROR, "Aborting enrolling to cluster. Unable to read elasticsearch.yml.", e);
         }
         final List<String> existingConfigWithoutAutoconfiguration = removePreviousAutoconfiguration(existingConfigLines);
-        if (existingConfigLines.equals(existingConfigWithoutAutoconfiguration) == false) {
+        if (false == existingConfigLines.equals(existingConfigWithoutAutoconfiguration)
+            && Files.exists(env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME))) {
             terminal.println("");
             terminal.println("This node will be reconfigured to join an existing cluster, using the enrollment token that you provided.");
             terminal.println("This operation will overwrite the existing configuration. Specifically: ");
             terminal.println("  - Security auto configuration will be removed from elasticsearch.yml");
-            terminal.println("  - The " + autoConfigDirName + " directory will be removed");
+            terminal.println("  - The [" + TLS_GENERATED_CERTS_DIR_NAME + "] config directory will be removed");
             terminal.println("  - Security auto configuration related secure settings will be removed from the elasticsearch.keystore");
             final boolean shouldContinue = terminal.promptYesNo("Do you want to continue with the reconfiguration process", false);
             if (shouldContinue == false) {
@@ -752,7 +873,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                         }
                     }
                 });
-                deleteDirectory(env.configFile().resolve(autoConfigDirName));
+                deleteDirectory(env.configFile().resolve(TLS_GENERATED_CERTS_DIR_NAME));
             } catch (Throwable t) {
                 throw new UserException(
                     ExitCodes.IO_ERROR,
@@ -781,11 +902,16 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         }
     }
 
-    @SuppressForbidden(reason = "Uses File API because the commons io library does, which is useful for file manipulation")
+    @SuppressForbidden(reason = "Commons IO lib uses the File API")
     private void deleteDirectory(Path directory) throws IOException {
         FileUtils.deleteDirectory(directory.toFile());
     }
 
+    @SuppressForbidden(reason = "Commons IO lib uses the File API")
+    private void moveDirectory(Path srcDir, Path dstDir) throws IOException {
+        FileUtils.moveDirectory(srcDir.toFile(), dstDir.toFile());
+    }
+
     private GeneralNames getSubjectAltNames(Settings settings) throws IOException {
         Set<GeneralName> generalNameSet = new HashSet<>();
         for (InetAddress ip : NetworkUtils.getAllAddresses()) {
@@ -837,7 +963,7 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         }
         // Skip security auto configuration when Security is already configured.
         // Security is enabled implicitly, but if the admin chooses to enable it explicitly then
-        // skip the TLS auto-configuration, as this is a sign that the admin is opting for the default behavior
+        // skip the TLS auto-configuration, as this is a sign that the admin is opting for the default 7.x behavior
         if (XPackSettings.SECURITY_ENABLED.exists(settings)) {
             // do not try to validate, correct or fill in any incomplete security configuration,
             // instead rely on the regular node startup to do this validation
@@ -1015,38 +1141,6 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
         return (List<String>) responseMap.get("nodes_addresses");
     }
 
-    private String getAutoConfigDirName(Environment env) throws UserException {
-        final List<String> autoConfigDirNameList;
-        try {
-            autoConfigDirNameList = Files.list(env.configFile())
-                .map(Path::getFileName)
-                .map(Path::toString)
-                .filter(name -> name.startsWith(TLS_CONFIG_DIR_NAME_PREFIX))
-                .collect(Collectors.toList());
-        } catch (IOException e) {
-            throw new UserException(
-                ExitCodes.USAGE,
-                "Aborting enrolling to cluster. Error attempting to find the directory with generated keys and certificates for TLS",
-                e
-            );
-        }
-        if (autoConfigDirNameList.isEmpty()) {
-            throw new UserException(
-                ExitCodes.USAGE,
-                "Aborting enrolling to cluster. This node doesn't appear to be auto-configured for security. "
-                    + "The directory with generated keys and certificates for TLS is missing."
-            );
-        } else if (autoConfigDirNameList.size() > 1) {
-            throw new UserException(
-                ExitCodes.USAGE,
-                "Aborting enrolling to cluster. Multiple directories with generated keys and certificates for TLS found: "
-                    + autoConfigDirNameList
-            );
-        } else {
-            return autoConfigDirNameList.get(0);
-        }
-    }
-
     /**
      * Removes existing autoconfiguration from a configuration file, retaining configuration that is user added even within
      * the auto-configuration stanza. It only matches configuration setting keys for auto-configuration as the values depend

+ 6 - 3
x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/AutoConfigureNodeTests.java

@@ -17,7 +17,6 @@ import org.elasticsearch.common.settings.KeyStoreWrapper;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.ssl.KeyStoreUtil;
-import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
@@ -240,8 +239,12 @@ public class AutoConfigureNodeTests extends ESTestCase {
             }
         }
 
-        KeyStore httpKeystore = KeyStoreUtil.readKeyStore(PathUtils.get(httpKeystorePath), "PKCS12", httpKeystorePassword.getChars());
-        return (X509Certificate) httpKeystore.getCertificate("http_local_node_key");
+        KeyStore httpKeystore = KeyStoreUtil.readKeyStore(
+            configDir.resolve("config").resolve(httpKeystorePath),
+            "PKCS12",
+            httpKeystorePassword.getChars()
+        );
+        return (X509Certificate) httpKeystore.getCertificate("http");
     }
 
     @SuppressForbidden(reason = "Uses File API because the commons io library does, which is useful for file manipulation")