Skip to content

Commit

Permalink
Respect --pass option in certutil csr mode (elastic#106105) (elastic#…
Browse files Browse the repository at this point in the history 
…106120)

elasticsearch-certutil csr generates a private key and a certificate
signing request (CSR) file. It has always accepted the "--pass" command
line option, but ignore it and always generated an unencrypted private
key.

This commit fixes the utility so the --pass option is respected and the
private key is encrypted.
  • Loading branch information
@tvernum
tvernum authored Mar 8, 2024
1 parent 11bf603 commit 321c4e1
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 38 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/106105.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 106105
summary: Respect --pass option in certutil csr mode
area: TLS
type: bug
issues: []
71 changes: 49 additions & 22 deletions ...ugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,29 @@ static void verifyIssuer(Certificate certificate, CAInfo caInfo, Terminal termin
throw new UserException(ExitCodes.CONFIG, "Certificate verification failed");
}
}

protected void writePemPrivateKey(
Terminal terminal,
OptionSet options,
ZipOutputStream outputStream,
JcaPEMWriter pemWriter,
String keyFileName,
PrivateKey privateKey
) throws IOException {
final boolean usePassword = useOutputPassword(options);
final char[] outputPassword = getOutputPassword(options);
outputStream.putNextEntry(new ZipEntry(keyFileName));
if (usePassword) {
withPassword(keyFileName, outputPassword, terminal, true, password -> {
pemWriter.writeObject(privateKey, getEncrypter(password));
return null;
});
} else {
pemWriter.writeObject(privateKey);
}
pemWriter.flush();
outputStream.closeEntry();
}
}

static class SigningRequestCommand extends CertificateCommand {
Expand Down Expand Up @@ -621,9 +644,7 @@ public void execute(Terminal terminal, OptionSet options, Environment env, Proce
terminal.println("");

final Path output = resolveOutputPath(terminal, options, DEFAULT_CSR_ZIP);
final int keySize = getKeySize(options);
Collection<CertificateInformation> certificateInformations = getCertificateInformationList(terminal, options);
generateAndWriteCsrs(output, keySize, certificateInformations);
generateAndWriteCsrs(terminal, options, output);

terminal.println("");
terminal.println("Certificate signing requests have been written to " + output);
Expand All @@ -639,12 +660,25 @@ public void execute(Terminal terminal, OptionSet options, Environment env, Proce
terminal.println("follow the SSL configuration instructions in the product guide.");
}

// For testing
void generateAndWriteCsrs(Terminal terminal, OptionSet options, Path output) throws Exception {
final int keySize = getKeySize(options);
Collection<CertificateInformation> certificateInformations = getCertificateInformationList(terminal, options);
generateAndWriteCsrs(terminal, options, output, keySize, certificateInformations);
}

/**
* Generates certificate signing requests and writes them out to the specified file in zip format
*
* @param certInfo the details to use in the certificate signing requests
*/
void generateAndWriteCsrs(Path output, int keySize, Collection<CertificateInformation> certInfo) throws Exception {
void generateAndWriteCsrs(
Terminal terminal,
OptionSet options,
Path output,
int keySize,
Collection<CertificateInformation> certInfo
) throws Exception {
fullyWriteZipFile(output, (outputStream, pemWriter) -> {
for (CertificateInformation certificateInformation : certInfo) {
KeyPair keyPair = CertGenUtils.generateKeyPair(keySize);
Expand All @@ -667,10 +701,14 @@ void generateAndWriteCsrs(Path output, int keySize, Collection<CertificateInform
outputStream.closeEntry();

// write private key
outputStream.putNextEntry(new ZipEntry(dirName + certificateInformation.name.filename + ".key"));
pemWriter.writeObject(keyPair.getPrivate());
pemWriter.flush();
outputStream.closeEntry();
super.writePemPrivateKey(
terminal,
options,
outputStream,
pemWriter,
dirName + certificateInformation.name.filename + ".key",
keyPair.getPrivate()
);
}
});
}
Expand Down Expand Up @@ -802,10 +840,8 @@ void generateAndWriteSignedCertificates(

final int keySize = getKeySize(options);
final int days = getDays(options);
final char[] outputPassword = super.getOutputPassword(options);
if (writeZipFile) {
final boolean usePem = usePemFormat(options);
final boolean usePassword = super.useOutputPassword(options);
fullyWriteZipFile(output, (outputStream, pemWriter) -> {
for (CertificateInformation certificateInformation : certs) {
CertificateAndKey pair = generateCertificateAndKey(certificateInformation, caInfo, keySize, days, terminal);
Expand All @@ -825,20 +861,10 @@ void generateAndWriteSignedCertificates(
outputStream.closeEntry();

// write private key
final String keyFileName = entryBase + ".key";
outputStream.putNextEntry(new ZipEntry(keyFileName));
if (usePassword) {
withPassword(keyFileName, outputPassword, terminal, true, password -> {
pemWriter.writeObject(pair.key, getEncrypter(password));
return null;
});
} else {
pemWriter.writeObject(pair.key);
}
pemWriter.flush();
outputStream.closeEntry();
writePemPrivateKey(terminal, options, outputStream, pemWriter, entryBase + ".key", pair.key);
} else {
final String fileName = entryBase + ".p12";
final char[] outputPassword = super.getOutputPassword(options);
outputStream.putNextEntry(new ZipEntry(fileName));
writePkcs12(
fileName,
Expand All @@ -855,6 +881,7 @@ void generateAndWriteSignedCertificates(
});
} else {
assert certs.size() == 1;
final char[] outputPassword = super.getOutputPassword(options);
CertificateInformation certificateInformation = certs.iterator().next();
CertificateAndKey pair = generateCertificateAndKey(certificateInformation, caInfo, keySize, days, terminal);
fullyWriteFile(
Expand Down
142 changes: 126 additions & 16 deletions ...security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
import org.bouncycastle.asn1.x509.GeneralName;
import org.bouncycastle.asn1.x509.GeneralNames;
import org.bouncycastle.cert.X509CertificateHolder;
import org.bouncycastle.openssl.PEMEncryptedKeyPair;
import org.bouncycastle.openssl.PEMKeyPair;
import org.bouncycastle.openssl.PEMParser;
import org.bouncycastle.openssl.bc.BcPEMDecryptorProvider;
import org.bouncycastle.pkcs.PKCS10CertificationRequest;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.MockTerminal;
Expand Down Expand Up @@ -77,6 +80,7 @@
import java.security.cert.X509Certificate;
import java.security.interfaces.RSAKey;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -88,6 +92,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.TrustManagerFactory;
Expand Down Expand Up @@ -266,17 +271,35 @@ public void testParsingFileWithInvalidDetails() throws Exception {
assertThat(terminal.getErrorOutput(), containsString("could not be converted to a valid DN"));
}

public void testGeneratingCsr() throws Exception {
public void testGeneratingCsrFromInstancesFile() throws Exception {
Path tempDir = initTempDir();
Path outputFile = tempDir.resolve("out.zip");
MockTerminal terminal = MockTerminal.create();
final List<String> args = new ArrayList<>();

Path instanceFile = writeInstancesTo(tempDir.resolve("instances.yml"));
Collection<CertificateInformation> certInfos = CertificateTool.parseFile(instanceFile);
assertEquals(4, certInfos.size());

assertFalse(Files.exists(outputFile));
int keySize = randomFrom(1024, 2048);

new CertificateTool.SigningRequestCommand().generateAndWriteCsrs(outputFile, keySize, certInfos);
final boolean encrypt = randomBoolean();
final String password = encrypt ? randomAlphaOfLengthBetween(8, 12) : null;
if (encrypt) {
args.add("--pass");
if (randomBoolean()) {
args.add(password);
} else {
for (var ignore : certInfos) {
terminal.addSecretInput(password);
}
}
}

final CertificateTool.SigningRequestCommand command = new CertificateTool.SigningRequestCommand();
final OptionSet options = command.getParser().parse(Strings.toStringArray(args));
command.generateAndWriteCsrs(terminal, options, outputFile, keySize, certInfos);
assertTrue(Files.exists(outputFile));

Set<PosixFilePermission> perms = Files.getPosixFilePermissions(outputFile);
Expand All @@ -292,7 +315,6 @@ public void testGeneratingCsr() throws Exception {
assertTrue(Files.exists(zipRoot.resolve(filename)));
final Path csr = zipRoot.resolve(filename + "/" + filename + ".csr");
assertTrue(Files.exists(csr));
assertTrue(Files.exists(zipRoot.resolve(filename + "/" + filename + ".key")));
PKCS10CertificationRequest request = readCertificateRequest(csr);
assertEquals(certInfo.name.x500Principal.getName(), request.getSubject().toString());
Attribute[] extensionsReq = request.getAttributes(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest);
Expand All @@ -304,9 +326,84 @@ public void testGeneratingCsr() throws Exception {
} else {
assertEquals(0, extensionsReq.length);
}

final Path keyPath = zipRoot.resolve(filename + "/" + filename + ".key");
assertTrue(Files.exists(keyPath));
PEMKeyPair key = readPrivateKey(keyPath, password);
assertNotNull(key);
}
}

public void testGeneratingCsrFromCommandLineParameters() throws Exception {
Path tempDir = initTempDir();
Path outputFile = tempDir.resolve("out.zip");
MockTerminal terminal = MockTerminal.create();
final List<String> args = new ArrayList<>();

final int keySize = randomFrom(1024, 2048);
args.add("--keysize");
args.add(String.valueOf(keySize));

final String name = randomAlphaOfLengthBetween(4, 16);
args.add("--name");
args.add(name);

final List<String> dns = randomList(0, 4, () -> randomAlphaOfLengthBetween(4, 8) + "." + randomAlphaOfLengthBetween(2, 5));
dns.stream().map(s -> "--dns=" + s).forEach(args::add);
final List<String> ip = randomList(
0,
2,
() -> Stream.generate(() -> randomIntBetween(10, 250)).limit(4).map(String::valueOf).collect(Collectors.joining("."))
);
ip.stream().map(s -> "--ip=" + s).forEach(args::add);

final boolean encrypt = randomBoolean();
final String password = encrypt ? randomAlphaOfLengthBetween(8, 12) : null;
if (encrypt) {
args.add("--pass");
if (randomBoolean()) {
args.add(password);
} else {
terminal.addSecretInput(password);
}
}

final CertificateTool.SigningRequestCommand command = new CertificateTool.SigningRequestCommand();
final OptionSet options = command.getParser().parse(Strings.toStringArray(args));
command.generateAndWriteCsrs(terminal, options, outputFile);
assertTrue(Files.exists(outputFile));

Set<PosixFilePermission> perms = Files.getPosixFilePermissions(outputFile);
assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_READ));
assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_WRITE));
assertEquals(perms.toString(), 2, perms.size());

final Path zipRoot = getRootPathOfZip(outputFile);

assertFalse(Files.exists(zipRoot.resolve("ca")));
assertTrue(Files.exists(zipRoot.resolve(name)));
final Path csr = zipRoot.resolve(name + "/" + name + ".csr");
assertTrue(Files.exists(csr));

PKCS10CertificationRequest request = readCertificateRequest(csr);
assertEquals("CN=" + name, request.getSubject().toString());

Attribute[] extensionsReq = request.getAttributes(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest);
if (dns.size() > 0 || ip.size() > 0) {
assertEquals(1, extensionsReq.length);
Extensions extensions = Extensions.getInstance(extensionsReq[0].getAttributeValues()[0]);
GeneralNames subjAltNames = GeneralNames.fromExtensions(extensions, Extension.subjectAlternativeName);
assertSubjAltNames(subjAltNames, ip, dns);
} else {
assertEquals(0, extensionsReq.length);
}

final Path keyPath = zipRoot.resolve(name + "/" + name + ".key");
assertTrue(Files.exists(keyPath));
PEMKeyPair key = readPrivateKey(keyPath, password);
assertNotNull(key);
}

public void testGeneratingSignedPemCertificates() throws Exception {
Path tempDir = initTempDir();
Path outputFile = tempDir.resolve("out.zip");
Expand Down Expand Up @@ -939,19 +1036,6 @@ private int getDurationInDays(X509Certificate cert) {
return (int) ChronoUnit.DAYS.between(cert.getNotBefore().toInstant(), cert.getNotAfter().toInstant());
}

private void assertSubjAltNames(Certificate certificate, String ip, String dns) throws Exception {
final X509CertificateHolder holder = new X509CertificateHolder(certificate.getEncoded());
final GeneralNames names = GeneralNames.fromExtensions(holder.getExtensions(), Extension.subjectAlternativeName);
final CertificateInformation certInfo = new CertificateInformation(
"n",
"n",
Collections.singletonList(ip),
Collections.singletonList(dns),
Collections.emptyList()
);
assertSubjAltNames(names, certInfo);
}

/**
* Checks whether there are keys in {@code keyStore} that are trusted by {@code trustStore}.
*/
Expand Down Expand Up @@ -981,13 +1065,39 @@ private PKCS10CertificationRequest readCertificateRequest(Path path) throws Exce
}
}

private PEMKeyPair readPrivateKey(Path path, String password) throws Exception {
try (Reader reader = Files.newBufferedReader(path); PEMParser pemParser = new PEMParser(reader)) {
Object object = pemParser.readObject();
if (password == null) {
assertThat(object, instanceOf(PEMKeyPair.class));
return (PEMKeyPair) object;
} else {
assertThat(object, instanceOf(PEMEncryptedKeyPair.class));
final PEMEncryptedKeyPair encryptedKeyPair = (PEMEncryptedKeyPair) object;
assertThat(encryptedKeyPair.getDekAlgName(), is("AES-128-CBC"));
return encryptedKeyPair.decryptKeyPair(new BcPEMDecryptorProvider(password.toCharArray()));
}
}
}

private X509Certificate readX509Certificate(InputStream input) throws Exception {
List<Certificate> list = CertParsingUtils.readCertificates(input);
assertEquals(1, list.size());
assertThat(list.get(0), instanceOf(X509Certificate.class));
return (X509Certificate) list.get(0);
}

private void assertSubjAltNames(Certificate certificate, String ip, String dns) throws Exception {
final X509CertificateHolder holder = new X509CertificateHolder(certificate.getEncoded());
final GeneralNames names = GeneralNames.fromExtensions(holder.getExtensions(), Extension.subjectAlternativeName);
assertSubjAltNames(names, Collections.singletonList(ip), Collections.singletonList(dns));
}

private void assertSubjAltNames(GeneralNames generalNames, List<String> ip, List<String> dns) throws Exception {
final CertificateInformation certInfo = new CertificateInformation("n", "n", ip, dns, Collections.emptyList());
assertSubjAltNames(generalNames, certInfo);
}

private void assertSubjAltNames(GeneralNames subjAltNames, CertificateInformation certInfo) throws Exception {
final int expectedCount = certInfo.ipAddresses.size() + certInfo.dnsNames.size() + certInfo.commonNames.size();
assertEquals(expectedCount, subjAltNames.getNames().length);
Expand Down

0 comments on commit 321c4e1

Please sign in to comment.