Skip to content

Commit

Permalink
Fix use of password protected PKCS#8 keys for SSL (elastic#55567)
Browse files Browse the repository at this point in the history
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: elastic#55457
  • Loading branch information
tvernum authored Apr 22, 2020
1 parent 32e46bf commit 8b566ae
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,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);
Expand Down Expand Up @@ -639,7 +638,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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 4 additions & 0 deletions libs/ssl-config/src/test/resources/certs/README.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,7 @@ elasticsearch-certutil ca --pem --out ${PWD}/ca1-b.zip --days 9999 --ca-dn "CN=T
unzip ca1-b.zip
mv ca ca1-b

# 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"
28 changes: 28 additions & 0 deletions libs/ssl-config/src/test/resources/certs/cert1/cert1-pkcs8.key
Original file line number Diff line number Diff line change
@@ -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-----
29 changes: 29 additions & 0 deletions libs/ssl-config/src/test/resources/certs/cert2/cert2-pkcs8.key
Original file line number Diff line number Diff line change
@@ -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-----
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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 {
final Environment env = TestEnvironment.newEnvironment(buildEnvSettings(Settings.EMPTY));
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();
}

}

0 comments on commit 8b566ae

Please sign in to comment.