Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.17] Respect --pass option in certutil csr mode #109834

Merged
merged 1 commit into from
Jun 18, 2024
Merged

[7.17] Respect --pass option in certutil csr mode #109834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/106105.yaml
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,29 @@ static void writePkcs12(
}
});
}

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 @@ -620,9 +643,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
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 @@ -638,12 +659,25 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
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 @@ -666,10 +700,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 @@ -805,10 +843,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) -> {
// write out the CA info first if it was generated
if (caInfo != null && caInfo.generated) {
Expand Down Expand Up @@ -838,20 +874,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 @@ -868,6 +894,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);
fullyWriteFile(
Expand Down
147 changes: 131 additions & 16 deletions ...security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import org.bouncycastle.cert.X509CertificateHolder;
import org.bouncycastle.openssl.PEMDecryptorProvider;
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.MockTerminal;
import org.elasticsearch.cli.Terminal;
Expand Down Expand Up @@ -59,6 +61,7 @@
import java.io.Reader;
import java.net.InetAddress;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
Expand All @@ -74,6 +77,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 @@ -85,6 +89,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 @@ -257,17 +262,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 = new MockTerminal();
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 (Object 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 @@ -284,7 +307,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 @@ -296,7 +318,82 @@ 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 = new MockTerminal();
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 {
Expand Down Expand Up @@ -968,19 +1065,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 @@ -1014,13 +1098,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 Expand Up @@ -1102,6 +1212,11 @@ private static Path resolvePath(String path) {
return PathUtils.get(path).toAbsolutePath();
}

private static Path getRootPathOfZip(Path pemZip) throws IOException, URISyntaxException {
FileSystem zipFS = FileSystems.newFileSystem(new URI("jar:" + pemZip.toUri()), Collections.emptyMap());
return zipFS.getPath("/");
}

private String generateCA(Path caFile, MockTerminal terminal, Environment env) throws Exception {
final int caKeySize = randomIntBetween(4, 8) * 512;
final int days = randomIntBetween(7, 1500);
Expand Down