Filter out CA PrivateKeyEntry when creating a KeyManager (#73807)

In 8.0, with security on by default, we store the HTTP
layer CA PrivateKeyEntry in the http.ssl keystore (along
with the node certificate) so that it is available in our
Enrollment API transport actions.
When loading a keystore, the current behavior is that the
X509ExtendedKeyManager will iterate through the PrivateKeyEntry
objects and will return the first key/certificate that satisfies
the requirements of the client and the server configuration,
and lacks any additional logic/filters.
We need the KeyManager to deterministically pick the node
certificate/key in all cases as this is the intended entry to be
used for TLS on the HTTP layer.
This change introduces filtering when creating the in-memory
keystore the KeyManager is loaded with, so that it will not
include PrivateKeyEntry objects when:
- there are more than 1 PrivateKeyEntry objects in the keystore
- The leaf certificate associated with the PrivateKeyEntry is a
CA certificate
Related: #75097

Co-authored-by: Ioannis Kakavas <[email protected]>

Author: BigPandaToo

client/rest-high-level/build.gradle

@@ -14,6 +14,7 @@ apply plugin: 'elasticsearch.rest-test'
 apply plugin: 'elasticsearch.publish'
 apply plugin: 'com.github.johnrengelman.shadow'
 apply plugin: 'elasticsearch.rest-resources'
+apply plugin: 'elasticsearch.internal-test-artifact'
 
 group = 'org.elasticsearch.client'
 archivesBaseName = 'elasticsearch-rest-high-level-client'
@@ -67,8 +68,6 @@ tasks.named('forbiddenApisMain').configure {
 File nodeCert = file("./testnode.crt")
 File nodeTrustStore = file("./testnode.jks")
 File pkiTrustCert = file("./src/test/resources/org/elasticsearch/client/security/delegate_pki/testRootCA.crt")
-File httpCaKeystore = file("./httpCa.p12");
-File transportKeystore = file("./transport.p12");
 
 tasks.named("integTest").configure {
   systemProperty 'tests.rest.async', 'false'
@@ -118,8 +117,6 @@ testClusters.all {
   extraConfigFile nodeCert.name, nodeCert
   extraConfigFile nodeTrustStore.name, nodeTrustStore
   extraConfigFile pkiTrustCert.name, pkiTrustCert
-  extraConfigFile httpCaKeystore.name, httpCaKeystore
-  extraConfigFile transportKeystore.name, transportKeystore
 
   setting 'xpack.searchable.snapshot.shared_cache.size', '1mb'
   setting 'xpack.searchable.snapshot.shared_cache.region_size', '16kb'

client/rest-high-level/qa/ssl-enabled/build.gradle

@@ -0,0 +1,45 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import org.elasticsearch.gradle.internal.test.RestIntegTestTask
+import org.elasticsearch.gradle.internal.info.BuildParams
+
+apply plugin: 'elasticsearch.java-rest-test'
+dependencies {
+  javaRestTestImplementation(testArtifact(project(':client:rest-high-level')))
+}
+
+tasks.matching{ it.name == "javaRestTest" }.configureEach {
+  onlyIf { BuildParams.inFipsJvm == false}
+  systemProperty 'tests.rest.cluster.username', System.getProperty('tests.rest.cluster.username', 'test_user')
+  systemProperty 'tests.rest.cluster.password', System.getProperty('tests.rest.cluster.password', 'test-user-password')
+}
+
+testClusters.matching { it.name == 'javaRestTest' }.configureEach {
+  testDistribution = 'DEFAULT'
+  numberOfNodes = 2
+  setting 'xpack.license.self_generated.type', 'trial'
+  setting 'xpack.security.enabled', 'true'
+  setting 'xpack.security.authc.token.enabled', 'true'
+  setting 'xpack.security.authc.api_key.enabled', 'true'
+
+  extraConfigFile 'httpCa.p12', file('./src/javaRestTest/resources/httpCa.p12')
+  extraConfigFile 'transport.p12', file('./src/javaRestTest/resources/transport.p12')
+
+  // TBD: sync these settings (which options are set) with the ones we will be generating in #74868
+  setting 'xpack.security.http.ssl.enabled', 'true'
+  setting 'xpack.security.transport.ssl.enabled', 'true'
+  setting 'xpack.security.http.ssl.keystore.path', 'httpCa.p12'
+  setting 'xpack.security.transport.ssl.keystore.path', 'transport.p12'
+  setting 'xpack.security.transport.ssl.verification_mode', 'certificate'
+
+
+  keystore 'xpack.security.http.ssl.keystore.secure_password', 'password'
+  keystore 'xpack.security.transport.ssl.keystore.secure_password', 'password'
+  user username: 'admin_user', password: 'admin-password', role: 'superuser'
+}

client/rest-high-level/qa/ssl-enabled/src/javaRestTest/java/org/elasticsearch/client/EnrollmentIT.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.client;
+
+import org.elasticsearch.client.security.KibanaEnrollmentResponse;
+import org.elasticsearch.client.security.NodeEnrollmentResponse;
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.core.PathUtils;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+import java.io.FileNotFoundException;
+import java.net.URL;
+import java.nio.file.Path;
+import java.util.List;
+
+import static org.hamcrest.Matchers.endsWith;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.notNullValue;
+
+public class EnrollmentIT  extends ESRestHighLevelClientTestCase {
+    private static Path httpTrustStore;
+
+    @BeforeClass
+    public static void findTrustStore() throws Exception {
+        final URL resource = EnrollmentIT.class.getResource("/httpCa.p12");
+        if (resource == null) {
+            throw new FileNotFoundException("Cannot find classpath resource /httpCa.p12");
+        }
+        httpTrustStore = PathUtils.get(resource.toURI());
+    }
+
+    @AfterClass
+    public static void cleanupStatics() {
+        httpTrustStore = null;
+    }
+
+    @Override
+    protected String getProtocol() {
+        return "https";
+    }
+
+    @Override
+    protected Settings restClientSettings() {
+        String token = basicAuthHeaderValue("admin_user", new SecureString("admin-password".toCharArray()));
+        return Settings.builder()
+            .put(ThreadContext.PREFIX + ".Authorization", token)
+            .put(TRUSTSTORE_PATH, httpTrustStore)
+            .put(TRUSTSTORE_PASSWORD, "password")
+            .build();
+    }
+
+    public void testEnrollNode() throws Exception {
+        final NodeEnrollmentResponse nodeEnrollmentResponse =
+            execute(highLevelClient().security()::enrollNode, highLevelClient().security()::enrollNodeAsync, RequestOptions.DEFAULT);
+        assertThat(nodeEnrollmentResponse, notNullValue());
+        assertThat(nodeEnrollmentResponse.getHttpCaKey(), endsWith("K2S3vidA="));
+        assertThat(nodeEnrollmentResponse.getHttpCaCert(), endsWith("LfkRjirc="));
+        assertThat(nodeEnrollmentResponse.getTransportKey(), endsWith("1I-r8vOQ=="));
+        assertThat(nodeEnrollmentResponse.getTransportCert(), endsWith("OpTdtgJo="));
+        List<String> nodesAddresses = nodeEnrollmentResponse.getNodesAddresses();
+        assertThat(nodesAddresses.size(), equalTo(2));
+    }
+
+    public void testEnrollKibana() throws Exception {
+        KibanaEnrollmentResponse kibanaResponse =
+            execute(highLevelClient().security()::enrollKibana, highLevelClient().security()::enrollKibanaAsync, RequestOptions.DEFAULT);
+        assertThat(kibanaResponse, notNullValue());
+        assertThat(kibanaResponse.getHttpCa()
+            , endsWith("brcNC5xq6YE7C4_06nH7F6le4kE4Uo6c9fpkl4ehOxQxndNLn462tFF-8VBA8IftJ1PPWzqGxLsCTzM6p6w8sa-XhgNYglLfkRjirc="));
+        assertNotNull(kibanaResponse.getPassword());
+        assertThat(kibanaResponse.getPassword().toString().length(), equalTo(14));
+    }
+}

client/rest-high-level/httpCa.p12 renamed to client/rest-high-level/qa/ssl-enabled/src/javaRestTest/resources/httpCa.p12

@@ -21,7 +21,6 @@
 import org.elasticsearch.client.security.GetRolesResponse;
 import org.elasticsearch.client.security.GetUsersRequest;
 import org.elasticsearch.client.security.GetUsersResponse;
-import org.elasticsearch.client.security.NodeEnrollmentResponse;
 import org.elasticsearch.client.security.PutRoleRequest;
 import org.elasticsearch.client.security.PutRoleResponse;
 import org.elasticsearch.client.security.PutUserRequest;
@@ -34,7 +33,6 @@
 import org.elasticsearch.client.security.user.privileges.IndicesPrivileges;
 import org.elasticsearch.client.security.user.privileges.IndicesPrivilegesTests;
 import org.elasticsearch.client.security.user.privileges.Role;
-import org.elasticsearch.client.security.KibanaEnrollmentResponse;
 import org.elasticsearch.core.CharArrays;
 
 import java.io.IOException;
@@ -47,12 +45,10 @@
 import java.util.Map;
 
 import static org.hamcrest.Matchers.empty;
-import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
 public class SecurityIT extends ESRestHighLevelClientTestCase {
@@ -196,30 +192,6 @@ public void testPutRole() throws Exception {
         assertThat(deleteRoleResponse.isFound(), is(true));
     }
 
-    @AwaitsFix(bugUrl = "Determine behavior for keystore with multiple keys")
-    public void testEnrollNode() throws Exception {
-        final NodeEnrollmentResponse nodeEnrollmentResponse =
-            execute(highLevelClient().security()::enrollNode, highLevelClient().security()::enrollNodeAsync, RequestOptions.DEFAULT);
-        assertThat(nodeEnrollmentResponse, notNullValue());
-        assertThat(nodeEnrollmentResponse.getHttpCaKey(), endsWith("ECAwGGoA=="));
-        assertThat(nodeEnrollmentResponse.getHttpCaCert(), endsWith("ECAwGGoA=="));
-        assertThat(nodeEnrollmentResponse.getTransportKey(), endsWith("fSI09on8AgMBhqA="));
-        assertThat(nodeEnrollmentResponse.getTransportCert(), endsWith("fSI09on8AgMBhqA="));
-        List<String> nodesAddresses = nodeEnrollmentResponse.getNodesAddresses();
-        assertThat(nodesAddresses.size(), equalTo(1));
-    }
-
-    @AwaitsFix(bugUrl = "Determine behavior for keystores with multiple keys")
-    public void testEnrollKibana() throws Exception {
-        KibanaEnrollmentResponse kibanaResponse =
-            execute(highLevelClient().security()::enrollKibana, highLevelClient().security()::enrollKibanaAsync, RequestOptions.DEFAULT);
-        assertThat(kibanaResponse, notNullValue());
-        assertThat(kibanaResponse.getHttpCa()
-            , endsWith("OWFyeGNmcwovSDJReE1tSG1leXJRaWxYbXJPdk9PUDFTNGRrSTFXbFJLOFdaN3c9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"));
-        assertNotNull(kibanaResponse.getPassword());
-        assertThat(kibanaResponse.getPassword().toString().length(), equalTo(14));
-    }
-
     private void deleteUser(User user) throws IOException {
         final Request deleteUserRequest = new Request(HttpDelete.METHOD_NAME, "/_security/user/" + user.getUsername());
         highLevelClient().getLowLevelClient().performRequest(deleteUserRequest);

client/rest-high-level/transport.p12 renamed to client/rest-high-level/qa/ssl-enabled/src/javaRestTest/resources/transport.p12

@@ -13,6 +13,7 @@ List projects = [
   'docs',
   'client:rest',
   'client:rest-high-level',
+  'client:rest-high-level:qa:ssl-enabled',
   'client:sniffer',
   'client:transport',
   'client:test',

client/rest-high-level/src/test/java/org/elasticsearch/client/SecurityIT.java

@@ -231,6 +231,7 @@ public static List<Setting<?>> getAllSettings() {
         settings.add(API_KEY_SERVICE_ENABLED_SETTING);
         settings.add(USER_SETTING);
         settings.add(PASSWORD_HASHING_ALGORITHM);
+        settings.add(ENROLLMENT_ENABLED);
         return Collections.unmodifiableList(settings);
     }
 

settings.gradle

@@ -77,6 +77,23 @@ X509ExtendedKeyManager createKeyManager(@Nullable Environment environment) {
         try {
             KeyStore ks = getStore(ksPath, keyStoreType, keyStorePassword);
             checkKeyStore(ks);
+            // TBD: filter out only http.ssl.keystore
+            List<String> aliases = new ArrayList<>();
+            for (String s : Collections.list(ks.aliases())) {
+                if (ks.isKeyEntry(s)) {
+                    aliases.add(s);
+                }
+            }
+            if (aliases.size() > 1) {
+                for (String alias : aliases) {
+                    Certificate certificate = ks.getCertificate(alias);
+                    if (certificate instanceof X509Certificate) {
+                        if (((X509Certificate) certificate).getBasicConstraints() != -1) {
+                            ks.deleteEntry(alias);
+                        }
+                    }
+                }
+            }
             return CertParsingUtils.keyManager(ks, keyPassword.getChars(), keyStoreAlgorithm);
         } catch (FileNotFoundException | NoSuchFileException e) {
             throw missingKeyConfigFile(e, KEYSTORE_FILE, ksPath);

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java

@@ -16,7 +16,15 @@
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509ExtendedKeyManager;
 
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
 import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 
 import static org.elasticsearch.test.TestMatchers.throwableWithMessage;
 import static org.hamcrest.Matchers.containsString;
@@ -52,6 +60,52 @@ public void testKeyStorePathCanBeEmptyForPkcs11() throws Exception {
         assertThat(ee.getCause().getMessage(), containsString("PKCS11 not found"));
     }
 
+    public void testCreateKeyManagerFromPKCS12ContainingCA() throws Exception {
+        assumeFalse("Can't run in a FIPS JVM", inFipsJvm());
+        final Settings settings = Settings.builder().put("path.home", createTempDir()).build();
+        final Path path = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/httpCa.p12");
+        final SecureString keyStorePassword = new SecureString("password".toCharArray());
+        final StoreKeyConfig keyConfig = new StoreKeyConfig(path.toString(), "PKCS12", keyStorePassword, keyStorePassword,
+            KeyManagerFactory.getDefaultAlgorithm(), TrustManagerFactory.getDefaultAlgorithm());
+        KeyStore keyStore = KeyStore.getInstance("PKCS12");
+        try (InputStream in = Files.newInputStream(path)) {
+            keyStore.load(in, keyStorePassword.getChars());
+        }
+        List<String> aliases = new ArrayList<>();
+        for (String s : Collections.list(keyStore.aliases())) {
+            if (keyStore.isKeyEntry(s)) {
+                aliases.add(s);
+            }
+        }
+        assertThat(aliases.size(), equalTo(2));
+        final X509ExtendedKeyManager keyManager = keyConfig.createKeyManager(TestEnvironment.newEnvironment(settings));
+        for (String alias : aliases) {
+            PrivateKey key = keyManager.getPrivateKey(alias);
+            assertTrue(key == null || alias.equals("http"));
+        }
+        final String[] new_aliases = keyManager.getServerAliases("RSA", null);
+        final X509Certificate[] certificates = keyManager.getCertificateChain("http");
+        assertThat(new_aliases.length, equalTo(1));
+        assertThat(certificates.length, equalTo(2));
+    }
+
+    public void testCreateKeyManagerFromPKCS12ContainingCAOnly() throws Exception {
+        assumeFalse("Can't run in a FIPS JVM", inFipsJvm());
+        final Settings settings = Settings.builder().put("path.home", createTempDir()).build();
+        final String path = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/ca.p12").toString();
+        final SecureString keyStorePassword = new SecureString("password".toCharArray());
+        final StoreKeyConfig keyConfig = new StoreKeyConfig(path, "PKCS12", keyStorePassword, keyStorePassword,
+            KeyManagerFactory.getDefaultAlgorithm(), TrustManagerFactory.getDefaultAlgorithm());
+        final X509ExtendedKeyManager keyManager = keyConfig.createKeyManager(TestEnvironment.newEnvironment(settings));
+        final PrivateKey ca_key = keyManager.getPrivateKey("ca");
+        final String[] aliases = keyManager.getServerAliases("RSA", null);
+        final X509Certificate[] certificates = keyManager.getCertificateChain("ca");
+        assertThat(ca_key, notNullValue());
+        assertThat(aliases.length, equalTo(1));
+        assertThat(aliases[0], equalTo("ca"));
+        assertThat(certificates.length, equalTo(1));
+    }
+
     private void tryReadPrivateKeyFromKeyStore(String type, String extension) {
         final Settings settings = Settings.builder().put("path.home", createTempDir()).build();
         final String path = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode" + extension).toString();

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/StoreKeyConfig.java

@@ -101,7 +101,7 @@ public class TransportKibanaEnrollmentAction extends HandledTransportAction<Kiba
             final char[] password = generateKibanaSystemPassword();
             final ChangePasswordRequest changePasswordRequest =
                 new ChangePasswordRequestBuilder(client).username("kibana_system")
-                    .password(password, Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(environment.settings())))
+                    .password(password.clone(), Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(environment.settings())))
                     .request();
             client.execute(ChangePasswordAction.INSTANCE, changePasswordRequest, ActionListener.wrap(response -> {
                 logger.debug("Successfully set the password for user [kibana_system] during kibana enrollment");

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/StoreKeyConfigTests.java

@@ -101,20 +101,22 @@ protected void doExecute(Task task, NodeEnrollmentRequest request, ActionListene
                 for (NodeInfo nodeInfo : response.getNodes()) {
                     nodeList.add(nodeInfo.getInfo(TransportInfo.class).getAddress().publishAddress().toString());
                 }
+                try {
+                    final String httpCaKey = Base64.getUrlEncoder().encodeToString(httpCaKeysAndCertificates.get(0).v1().getEncoded());
+                    final String httpCaCert = Base64.getUrlEncoder().encodeToString(httpCaKeysAndCertificates.get(0).v2().getEncoded());
+                    final String transportKey =
+                        Base64.getUrlEncoder().encodeToString(transportKeysAndCertificates.get(0).v1().getEncoded());
+                    final String transportCert =
+                        Base64.getUrlEncoder().encodeToString(transportKeysAndCertificates.get(0).v2().getEncoded());
+                    listener.onResponse(new NodeEnrollmentResponse(httpCaKey,
+                        httpCaCert,
+                        transportKey,
+                        transportCert,
+                        nodeList));
+                } catch (CertificateEncodingException e) {
+                    listener.onFailure(new ElasticsearchException("Unable to enroll node", e));
+                }
             }, listener::onFailure
         ));
-        try {
-            final String httpCaKey = Base64.getUrlEncoder().encodeToString(httpCaKeysAndCertificates.get(0).v1().getEncoded());
-            final String httpCaCert = Base64.getUrlEncoder().encodeToString(httpCaKeysAndCertificates.get(0).v2().getEncoded());
-            final String transportKey = Base64.getUrlEncoder().encodeToString(transportKeysAndCertificates.get(0).v1().getEncoded());
-            final String transportCert = Base64.getUrlEncoder().encodeToString(transportKeysAndCertificates.get(0).v2().getEncoded());
-            listener.onResponse(new NodeEnrollmentResponse(httpCaKey,
-                httpCaCert,
-                transportKey,
-                transportCert,
-                nodeList));
-        } catch (CertificateEncodingException e) {
-            listener.onFailure(new ElasticsearchException("Unable to enroll node", e));
-        }
     }
 }