Fix use of password protected PKCS#8 keys for SSL (#55907)

PEMUtils would incorrectly fill the encryption password with zeros
(the '\0' character) after decrypting a PKCS#8 key.

Since PEMUtils did not take ownership of this password it should not
zero it out because it does not know whether the caller will use that
password array again. This is actually what PEMKeyConfig does - it
uses the key encryption password as the password for the ephemeral
keystore that it creates in order to build a KeyManager.

Backport of: #55457, #55567

Author: tvernum

libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java

@@ -351,7 +351,6 @@ private static PrivateKey parsePKCS8Encrypted(BufferedReader bReader, char[] key
         EncryptedPrivateKeyInfo encryptedPrivateKeyInfo = new EncryptedPrivateKeyInfo(keyBytes);
         SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(encryptedPrivateKeyInfo.getAlgName());
         SecretKey secretKey = secretKeyFactory.generateSecret(new PBEKeySpec(keyPassword));
-        Arrays.fill(keyPassword, '\u0000');
         Cipher cipher = Cipher.getInstance(encryptedPrivateKeyInfo.getAlgName());
         cipher.init(Cipher.DECRYPT_MODE, secretKey, encryptedPrivateKeyInfo.getAlgParameters());
         PKCS8EncodedKeySpec keySpec = encryptedPrivateKeyInfo.getKeySpec(cipher);
@@ -648,7 +647,7 @@ private static String getEcCurveNameFromOid(String oidString) throws GeneralSecu
             case "1.3.132.0.39":
                 return "sect571r1";
         }
-        throw new GeneralSecurityException("Error parsing EC named curve identifier. Named curve with OID: " + oidString 
+        throw new GeneralSecurityException("Error parsing EC named curve identifier. Named curve with OID: " + oidString
             + " is not supported");
     }
 

libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemKeyConfigTests.java

@@ -45,22 +45,38 @@ public class PemKeyConfigTests extends ESTestCase {
     private static final int IP_NAME = 7;
     private static final int DNS_NAME = 2;
 
-    public void testBuildKeyConfigFromPemFilesWithoutPassword() throws Exception {
+    public void testBuildKeyConfigFromPkcs1PemFilesWithoutPassword() throws Exception {
         final Path cert = getDataPath("/certs/cert1/cert1.crt");
         final Path key = getDataPath("/certs/cert1/cert1.key");
         final PemKeyConfig keyConfig = new PemKeyConfig(cert, key, new char[0]);
         assertThat(keyConfig.getDependentFiles(), Matchers.containsInAnyOrder(cert, key));
         assertCertificateAndKey(keyConfig, "CN=cert1");
     }
 
-    public void testBuildKeyConfigFromPemFilesWithPassword() throws Exception {
+    public void testBuildKeyConfigFromPkcs1PemFilesWithPassword() throws Exception {
         final Path cert = getDataPath("/certs/cert2/cert2.crt");
         final Path key = getDataPath("/certs/cert2/cert2.key");
         final PemKeyConfig keyConfig = new PemKeyConfig(cert, key, "c2-pass".toCharArray());
         assertThat(keyConfig.getDependentFiles(), Matchers.containsInAnyOrder(cert, key));
         assertCertificateAndKey(keyConfig, "CN=cert2");
     }
 
+    public void testBuildKeyConfigFromPkcs8PemFilesWithoutPassword() throws Exception {
+        final Path cert = getDataPath("/certs/cert1/cert1.crt");
+        final Path key = getDataPath("/certs/cert1/cert1-pkcs8.key");
+        final PemKeyConfig keyConfig = new PemKeyConfig(cert, key, new char[0]);
+        assertThat(keyConfig.getDependentFiles(), Matchers.containsInAnyOrder(cert, key));
+        assertCertificateAndKey(keyConfig, "CN=cert1");
+    }
+
+    public void testBuildKeyConfigFromPkcs8PemFilesWithPassword() throws Exception {
+        final Path cert = getDataPath("/certs/cert2/cert2.crt");
+        final Path key = getDataPath("/certs/cert2/cert2-pkcs8.key");
+        final PemKeyConfig keyConfig = new PemKeyConfig(cert, key, "c2-pass".toCharArray());
+        assertThat(keyConfig.getDependentFiles(), Matchers.containsInAnyOrder(cert, key));
+        assertCertificateAndKey(keyConfig, "CN=cert2");
+    }
+
     public void testKeyManagerFailsWithIncorrectPassword() throws Exception {
         final Path cert = getDataPath("/certs/cert2/cert2.crt");
         final Path key = getDataPath("/certs/cert2/cert2.key");

libs/ssl-config/src/test/resources/certs/README.txt

@@ -73,3 +73,9 @@ do
     keytool -keypasswd -keystore cert-all/certs.jks -alias $Cert -keypass p12-pass -new key-pass -storepass jks-pass
 done
 
+# 11. (Not relevant on 6.8 branch)
+
+# 12. Convert certifcate keys to pkcs8
+
+openssl pkcs8 -topk8 -inform PEM -in cert1/cert1.key -outform PEM -out cert1/cert1-pkcs8.key -nocrypt
+openssl pkcs8 -topk8 -inform PEM -in cert2/cert2.key -outform PEM -out cert2/cert2-pkcs8.key -passin pass:"c2-pass" -passout pass:"c2-pass"

libs/ssl-config/src/test/resources/certs/cert1/cert1-pkcs8.key

@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCWz6ITDTlkTLue
+B30Jx0+7sWHdlM5ObZjWhMQ1eyJD0gYU/gkH2C88IN/PtSv04tzFS6PA4KPDLIya
+AhczPlGElSansiui//CpieCI4tt5c2BgVo3XdJaylYoW3CRILUrlSBOMUmJCQEok
+verxMrz8DeppNxRfj99pQkoxUkmFMZj/C7XNVYrTttdF1li5FUtWJxw234OUfum3
+PQIzz6YTmoPtLrJ2fB8I4CH8R5hwGcryhBSAqq8pgy61aTPCgEBZ1c4Dvl65X8dG
+2QEVPjwMZnnbGjvlZefOgkmAWJ1VjihA3GVgO2mx4tB4D2x5K/OAxh2foZkDVhqJ
+fBkOblLnAgMBAAECggEAEKKUkR9rTjn8lADldPesPtrhHaz1WMdUDY2ViwSrEeoP
+y6791gStqSdDKMkmMRv5GDYwuOzOg4/dbnt+jaN5IHPHUMYhdBhhNoJD5zWG2g20
++stxV+u/V7GRCtZ7lg6Q7VuW9Gp99irbQtREHxjmqbLrQXHW6HeZQCYUwv39qBhU
+mjjILGc0OsyD1SMXhuf/f245/oLuYRUlFOXsRPnBxt2XXQQ5ZbabrSEk5AtI5j03
+V5p8UQ75XZdvzQ8La3cyq3n5PufQIoLH+n3gXtADP6Gx+SxjehNRRfAvQqglMt0m
+uMKgbiZYn9F7eoQomqYx9PhekkqsXsd+BwpELIfTqQKBgQD/lNVArgbUNzgudIKY
+Jv4QJC+YZEG0roCWHdPFvgjAYcMYx8Lxbg6C3TcDxkT3amMhMd3X5wvKQoGyZpWY
+S2LIPtgvkbea6ZHVh0wUQFJl3E5N4P3n5otjFOh8+wMhQg9IV5xFbCuLtPJhy9Qb
+INyEvS8Nzoan3n9kUWC3bq7ECwKBgQCXDt6DYxwfCz1xIB+i5Qtspt/qNfPLbW3o
+J6okYgP+SN1TCzremtxC+YW0Udau/eso7T2GBvcp3eBTPy320APcXvXxtJnDMsBL
+A7m6M8dlBVFOi262AIZ4o+BerPJVZ6jjmOLgN9CXneMbRoDUPOhlIDf+jBy5foJU
+Y8vNCeF6FQKBgAd71S66qcqG/2ck1DoeUiwo0xf0P5RJ08wRfYT5xonTkwHjv4qQ
+PW6JibXblWNlQxfSvPs4cbjvb5rItDKsam0QogXqj2TC2BlXh9vD8mW3KLfREb47
+mvNAxnn6Y6ISrB3jKtlBjJjfqIVCkahlsu9UFs+hr4G02ygV1e4pGIb3AoGAGMmF
+1cVzndx4TpHY3x/6ie+wGnyT7rOcL1Yi4yl6QkWum6viExkSP6M2P2qWccyUw/h5
++f42nJYd80sQvclQeN7UOL9L4+32A9kuptFMTNVcjCjxF8hqSG2Lqb1zXnROEFrM
+D8LY5agw1g7xoOIFuGJbDdfr9rw9op9ll9WhPCkCgYBIJPHnm5OGX2s35cMoVc3n
+TQriv5D04E4SR8O4NQRGkGiFR+jLx07l6JOYaKrHk/SGjEQLvS+fDl8nPhmlInst
+ykJ+rZN7OSck/cxkrBN0Ez+k4NI8xTlgJbhP2hwRC6Lk8o5MuVWjVAqsbzDp/9gx
+9m4Zr3M/8uZMtkRp2j2e6Q==
+-----END PRIVATE KEY-----

libs/ssl-config/src/test/resources/certs/cert2/cert2-pkcs8.key

@@ -0,0 +1,29 @@
+-----BEGIN ENCRYPTED PRIVATE KEY-----
+MIIE6TAbBgkqhkiG9w0BBQMwDgQI3LMAczRe1pICAggABIIEyIfF+k/Z7mEPN+gu
+rrV4slxmFjnIzyjoOsWbFBN9j5W8kZGewzWvjnYAzVMVeMUXXW27EMOC818aF8Ry
+20BUVkmGNAUFVEslayeF0/S2sYR9xBPqYGLF0DI2VDTBIn8zrTdvQ0DYuuLf4g3W
+nY69paCGSVjP24xrnEck+X6PjwIQ89MsJlVLn2chPEetvgu2jB+OdRneSA+lqBSW
+2sDuKeXAmYdm3nCVz9do6T22OC9PUkzICGUvMyhYO4CD9dI3FFHa4ncKsr4+htfp
+p3eppDczSneDKTPmGJg41UrEVdfENvqYLbYrX2ZZRnWUeuLsGbZUsWyfMT5TT2yz
+sZXSS9+WRF8IDDhzKjsoSpsuNwMj8KMmc4oIrBNPysI+Bv0sQzPBlMJkHJ+Swb+W
+I2wWC7NS7cwGFEEzwXKYAYI/34e/fAa+oOHd/aNdpGDhhnRyG+Eut3GbojlODiXN
+ntLhDIZ+lQclJEkmxP2QfhIvjqkNBkPv+8Xt2Ami3ueF2iNj9PxQHUdx3aht1VMg
+uVNLc0qLl98fDYx7+U2q5Y4L1mViZrCwGW+lcaY6a8hscPuasqW9aolFTE95YLVU
+yFeUOZlUh1g5FYepISESR5jm5k9wf/WV2cp/KyAzCftdx2iRQtgvyQtSITFtthDa
+kR7jI/T/HE2aPqiEcd0Sx+66aSH2JspseJ4VsLGkpg2lU0FPtPaAsl8lIhlfdoDA
+43kOPKe5q2YT4QaUNZJROAuyJAlRbE8+LNlfAlZCm0UTQhgiXPzkEuVQFM4DNplq
+FkBZnW4R/hD2rP81oy/SISxIANTyAB0FfCRxrvSRP6xe0XMYTIqsUVt9gpszI6AY
+B+KTKlJR68cI8Gs1Jq6otn1ExAlCX45mwfaFO6Be89U0YNCZXdJ0X66SFNPmsMrg
+eNbRLGziUhyoVUuPEhLXrxtuaGUO0LdHDSCQQGVNKCwNKbcWOE1clYWcJ1Lj0a7T
+BTb4y1Nqt7LOHm5En5RtSaourgEKvi6PS2EcemBSUG4xSSP1pEoV/nQnvl2PFsG7
+182571HDc7FRZplzE4fhRKetS5WUuKax2FIqljQXS0CJZwV65boJwfJ2FosxdHj4
+eZK/wYHxi8R8LQB478bLqp3xZt28/EhsMjtLy/nO9SZfSy7Ajgq2mk2w4HZke23K
+qv6JY8/ofu5nBp0QSJIPQj35arkxXnEYjzgqJD1IIDNckt3jRSpeX3OMfmVWDXus
+a/wEwNaBACo5lgUYAwzXiPFyLEa6RkbD04z55GqsIjwZhIKrC1glyb0gmtxyY/N8
+QfPaSc4aXgUQK2mrgsR22eKf735MhM0pHOykErJzrIV92lRfSuQvfIoqTeFV0fWY
+Yxns7HEIBln7/9udgZpf+EWnzM3kpPjRX1iYNtAsMAKEMrK/lnt3jZTkKY2Drem5
+NGjtEWNmzKOLH/S0TLcpHldoDJGdno7CytNGasrkFTaDLEhF68QI85zbMmyotKiZ
+FKooG78oyxWCYqqPJ9vVBGwsMNfDMJP56T9UNhL72FwNqzSCaEcAUlDm2zCiQWKd
+NpOfYzRx8uLp7UyzXbl959GjHWfTA74Yidug13eSHRgKwLXLuro05awG8yNXeDeL
+wDswgDyKmlV7JDJjQw==
+-----END ENCRYPTED PRIVATE KEY-----

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

@@ -328,7 +328,6 @@ private static PrivateKey parsePKCS8Encrypted(BufferedReader bReader, char[] key
         EncryptedPrivateKeyInfo encryptedPrivateKeyInfo = new EncryptedPrivateKeyInfo(keyBytes);
         SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(encryptedPrivateKeyInfo.getAlgName());
         SecretKey secretKey = secretKeyFactory.generateSecret(new PBEKeySpec(keyPassword));
-        Arrays.fill(keyPassword, '\u0000');
         Cipher cipher = Cipher.getInstance(encryptedPrivateKeyInfo.getAlgName());
         cipher.init(Cipher.DECRYPT_MODE, secretKey, encryptedPrivateKeyInfo.getAlgParameters());
         PKCS8EncodedKeySpec keySpec = encryptedPrivateKeyInfo.getKeySpec(cipher);

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

@@ -0,0 +1,64 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+package org.elasticsearch.xpack.core.ssl;
+
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.TestEnvironment;
+import org.elasticsearch.test.ESTestCase;
+import org.hamcrest.Matchers;
+
+import javax.net.ssl.X509ExtendedKeyManager;
+
+import static org.hamcrest.Matchers.notNullValue;
+
+public class PEMKeyConfigTests extends ESTestCase {
+
+    public static final SecureString NO_PASSWORD = new SecureString("".toCharArray());
+    public static final SecureString TESTNODE_PASSWORD = new SecureString("testnode".toCharArray());
+
+    public void testEncryptedPkcs8RsaKey() throws Exception {
+        verifyKeyConfig("testnode.crt", "key_pkcs8_encrypted.pem", TESTNODE_PASSWORD);
+    }
+
+    public void testUnencryptedPkcs8RsaKey() throws Exception {
+        verifyKeyConfig("testnode.crt", "rsa_key_pkcs8_plain.pem", NO_PASSWORD);
+    }
+
+    public void testUnencryptedPkcs8DsaKey() throws Exception {
+        verifyKeyConfig("testnode.crt", "dsa_key_pkcs8_plain.pem", NO_PASSWORD);
+    }
+
+    public void testUnencryptedPkcs8EcKey() throws Exception {
+        verifyKeyConfig("testnode.crt", "ec_key_pkcs8_plain.pem", NO_PASSWORD);
+    }
+
+    public void testEncryptedPkcs1RsaKey() throws Exception {
+        verifyKeyConfig("testnode.crt", "testnode-aes" + randomFrom(128, 192, 256) + ".pem", TESTNODE_PASSWORD);
+    }
+
+    public void testUnencryptedPkcs1RsaKey() throws Exception {
+        verifyKeyConfig("testnode.crt", "testnode-unprotected.pem", NO_PASSWORD);
+    }
+
+    private void verifyKeyConfig(String certName, String keyName, SecureString keyPassword) throws Exception {
+        Settings settings = Settings.builder().put("path.home", createTempDir()).build();
+        final Environment env = TestEnvironment.newEnvironment(settings);
+        PEMKeyConfig config = new PEMKeyConfig(getPath(keyName), keyPassword, getPath(certName));
+        assertThat(config.certificates(env), Matchers.iterableWithSize(1));
+        X509ExtendedKeyManager keyManager = config.createKeyManager(env);
+        assertThat(keyManager, notNullValue());
+        assertThat(keyManager.getPrivateKey("key"), notNullValue());
+    }
+
+    private String getPath(String fileName) {
+        return getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/" + fileName)
+            .toAbsolutePath().toString();
+    }
+
+}