Skip to content

Commit

Permalink
Encrypt generated key with AES (#51019) (#51076)
Browse files Browse the repository at this point in the history
Replace DES with AES to align with modern encryption standards
Backport also fixs Files.readString API that is not available in Java 8

Resolves: #50843
  • Loading branch information
ywangd committed Jan 16, 2020
1 parent dd419f0 commit c1a6d5d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ static Collection<CertificateInformation> parseFile(Path file) throws Exception
}

static PEMEncryptor getEncrypter(char[] password) {
return new JcePEMEncryptorBuilder("DES-EDE3-CBC").setProvider(BC_PROV).build(password);
return new JcePEMEncryptorBuilder("AES-128-CBC").setProvider(BC_PROV).build(password);
}

private static <T, E extends Exception> T withPassword(String description, char[] password, Terminal terminal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.bouncycastle.asn1.x509.GeneralName;
import org.bouncycastle.asn1.x509.GeneralNames;
import org.bouncycastle.cert.X509CertificateHolder;
import org.bouncycastle.openssl.PEMDecryptorProvider;
import org.bouncycastle.openssl.PEMEncryptedKeyPair;
import org.bouncycastle.openssl.PEMParser;
import org.bouncycastle.pkcs.PKCS10CertificationRequest;
Expand Down Expand Up @@ -50,6 +51,7 @@
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.BeforeClass;
import org.mockito.Mockito;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.TrustManagerFactory;
Expand Down Expand Up @@ -349,6 +351,16 @@ public void testGeneratingSignedPemCertificates() throws Exception {
PEMParser pemParser = new PEMParser(reader);
Object parsed = pemParser.readObject();
assertThat(parsed, instanceOf(PEMEncryptedKeyPair.class));
// Verify we are using AES encryption
final PEMDecryptorProvider pemDecryptorProvider = Mockito.mock(PEMDecryptorProvider.class);
try {
((PEMEncryptedKeyPair) parsed).decryptKeyPair(pemDecryptorProvider);
} catch (Exception e) {
// Catch error thrown by the empty mock, we are only interested in the argument passed in
}
finally {
Mockito.verify(pemDecryptorProvider).get("AES-128-CBC");
}
char[] zeroChars = new char[caInfo.password.length];
Arrays.fill(zeroChars, (char) 0);
assertArrayEquals(zeroChars, caInfo.password);
Expand All @@ -368,7 +380,13 @@ public void testGeneratingSignedPemCertificates() throws Exception {
assertTrue(Files.exists(zipRoot.resolve(filename)));
final Path cert = zipRoot.resolve(filename + "/" + filename + ".crt");
assertTrue(Files.exists(cert));
assertTrue(Files.exists(zipRoot.resolve(filename + "/" + filename + ".key")));
Path keyFile = zipRoot.resolve(filename + "/" + filename + ".key");
assertTrue(Files.exists(keyFile));
if (keyPassword != null) {
assertTrue(new String(Files.readAllBytes(keyFile), StandardCharsets.US_ASCII).contains("DEK-Info: AES-128-CBC"));
} else {
assertFalse(new String(Files.readAllBytes(keyFile), StandardCharsets.US_ASCII).contains("DEK-Info:"));
}
final Path p12 = zipRoot.resolve(filename + "/" + filename + ".p12");
try (InputStream input = Files.newInputStream(cert)) {
X509Certificate certificate = readX509Certificate(input);
Expand Down

0 comments on commit c1a6d5d

Please sign in to comment.