[Certutil] Verify certificates after generation (#103948)

This commit modifies the "elasticsearch-certutil cert" command to add a
verify step after the generation of the new certificate.

The primary purpose of this verify is to ensure that the key used to
sign the certificate matches the issuing certificate that was provided.
That is, if the CA is provided using PEM files, that the `--ca-cert` and
`--ca-key` are a matching pair.

There are more efficient ways to tell is a certificate and key are a
matching pair if you know what type of key is being used (RSA, EC, etc)
but post-signing verification is the most straight-forward way to
perform a key-type agnostic check, and is a useful general purpose check
to include in certificate generation.

Closes: #98207

docs/changelog/103948.yaml

@@ -0,0 +1,6 @@
+pr: 103948
+summary: '''elasticsearch-certutil cert'' now verifies the issuing chain of the generated
+  certificate'
+area: TLS
+type: enhancement
+issues: []

x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java

@@ -57,6 +57,7 @@ import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 import java.nio.file.attribute.PosixFileAttributeView;
 import java.nio.file.attribute.PosixFilePermission;
+import java.security.GeneralSecurityException;
 import java.security.Key;
 import java.security.KeyPair;
 import java.security.KeyStore;
@@ -570,6 +571,25 @@ class CertificateTool extends MultiCommand {
                 }
             });
         }
+
+        /**
+         * Verify that the provided certificate is validly signed by the provided CA
+         */
+        static void verifyIssuer(Certificate certificate, CAInfo caInfo, Terminal terminal) throws UserException {
+            try {
+                certificate.verify(caInfo.certAndKey.cert.getPublicKey());
+            } catch (GeneralSecurityException e) {
+                terminal.errorPrintln("");
+                terminal.errorPrintln("* ERROR *");
+                terminal.errorPrintln("Verification of generated certificate failed.");
+                terminal.errorPrintln("This usually occurs if the provided CA certificate does not match with the CA key.");
+                terminal.errorPrintln("Cause: " + e);
+                for (var c = e.getCause(); c != null; c = c.getCause()) {
+                    terminal.errorPrintln("     - " + c);
+                }
+                throw new UserException(ExitCodes.CONFIG, "Certificate verification failed");
+            }
+        }
     }
 
     static class SigningRequestCommand extends CertificateCommand {
@@ -788,7 +808,7 @@ class CertificateTool extends MultiCommand {
                 final boolean usePassword = super.useOutputPassword(options);
                 fullyWriteZipFile(output, (outputStream, pemWriter) -> {
                     for (CertificateInformation certificateInformation : certs) {
-                        CertificateAndKey pair = generateCertificateAndKey(certificateInformation, caInfo, keySize, days);
+                        CertificateAndKey pair = generateCertificateAndKey(certificateInformation, caInfo, keySize, days, terminal);
 
                         final String dirName = certificateInformation.name.filename + "/";
                         ZipEntry zipEntry = new ZipEntry(dirName);
@@ -836,7 +856,7 @@ class CertificateTool extends MultiCommand {
             } else {
                 assert certs.size() == 1;
                 CertificateInformation certificateInformation = certs.iterator().next();
-                CertificateAndKey pair = generateCertificateAndKey(certificateInformation, caInfo, keySize, days);
+                CertificateAndKey pair = generateCertificateAndKey(certificateInformation, caInfo, keySize, days, terminal);
                 fullyWriteFile(
                     output,
                     stream -> writePkcs12(
@@ -856,7 +876,8 @@ class CertificateTool extends MultiCommand {
             CertificateInformation certificateInformation,
             CAInfo caInfo,
             int keySize,
-            int days
+            int days,
+            Terminal terminal
         ) throws Exception {
             KeyPair keyPair = CertGenUtils.generateKeyPair(keySize);
             Certificate certificate;
@@ -873,6 +894,7 @@ class CertificateTool extends MultiCommand {
                     caInfo.certAndKey.key,
                     days
                 );
+                verifyIssuer(certificate, caInfo, terminal);
             } else {
                 certificate = CertGenUtils.generateSignedCertificate(
                     certificateInformation.name.x500Principal,
@@ -940,6 +962,7 @@ class CertificateTool extends MultiCommand {
                 );
             }
         }
+
     }
 
     @SuppressForbidden(reason = "resolve paths against CWD for a CLI tool")

x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java

@@ -27,6 +27,7 @@ import org.bouncycastle.asn1.x509.GeneralNames;
 import org.bouncycastle.cert.X509CertificateHolder;
 import org.bouncycastle.openssl.PEMParser;
 import org.bouncycastle.pkcs.PKCS10CertificationRequest;
+import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.cli.ProcessInfo;
 import org.elasticsearch.cli.Terminal;
@@ -36,6 +37,7 @@ import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.ssl.KeyStoreUtil;
 import org.elasticsearch.common.ssl.PemUtils;
+import org.elasticsearch.common.util.ArrayUtils;
 import org.elasticsearch.common.util.CollectionUtils;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.core.IOUtils;
@@ -60,6 +62,7 @@ import java.io.InputStream;
 import java.io.Reader;
 import java.net.InetAddress;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
@@ -281,8 +284,7 @@ public class CertificateToolTests extends ESTestCase {
         assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_WRITE));
         assertEquals(perms.toString(), 2, perms.size());
 
-        FileSystem fileSystem = FileSystems.newFileSystem(new URI("jar:" + outputFile.toUri()), Collections.emptyMap());
-        Path zipRoot = fileSystem.getPath("/");
+        final Path zipRoot = getRootPathOfZip(outputFile);
 
         assertFalse(Files.exists(zipRoot.resolve("ca")));
         for (CertificateInformation certInfo : certInfos) {
@@ -341,8 +343,7 @@ public class CertificateToolTests extends ESTestCase {
         assertTrue(perms.toString(), perms.contains(PosixFilePermission.OWNER_WRITE));
         assertEquals(perms.toString(), 2, perms.size());
 
-        FileSystem fileSystem = FileSystems.newFileSystem(new URI("jar:" + outputFile.toUri()), Collections.emptyMap());
-        Path zipRoot = fileSystem.getPath("/");
+        final Path zipRoot = getRootPathOfZip(outputFile);
 
         assertFalse(Files.exists(zipRoot.resolve("ca")));
 
@@ -460,8 +461,7 @@ public class CertificateToolTests extends ESTestCase {
         Certificate caCert = caKeyStore.getCertificate("ca");
         assertThat(caCert, notNullValue());
 
-        FileSystem zip = FileSystems.newFileSystem(new URI("jar:" + pemZipFile.toUri()), Collections.emptyMap());
-        Path zipRoot = zip.getPath("/");
+        final Path zipRoot = getRootPathOfZip(pemZipFile);
 
         final Path keyPath = zipRoot.resolve("cert/cert.key");
         final PrivateKey key = PemUtils.readPrivateKey(keyPath, () -> longPassword.toCharArray());
@@ -645,7 +645,7 @@ public class CertificateToolTests extends ESTestCase {
         final String node2Ip = "200.182." + randomIntBetween(1, 250) + "." + randomIntBetween(1, 250);
         final String node3Ip = "200.183." + randomIntBetween(1, 250) + "." + randomIntBetween(1, 250);
 
-        final String caPassword = generateCA(caFile, terminal, env);
+        final String caPassword = generateCA(caFile, terminal, env, false);
 
         final GenerateCertificateCommand gen1Command = new PathAwareGenerateCertificateCommand(caFile, node1File);
         final OptionSet gen1Options = gen1Command.getParser()
@@ -716,7 +716,7 @@ public class CertificateToolTests extends ESTestCase {
             node3Ip
         );
         gen3Args.add("-self-signed");
-        final GenerateCertificateCommand gen3Command = new PathAwareGenerateCertificateCommand(null, node3File);
+        final GenerateCertificateCommand gen3Command = new PathAwareGenerateCertificateCommand(Map.of(), node3File);
         final OptionSet gen3Options = gen3Command.getParser().parse(Strings.toStringArray(gen3Args));
         gen3Command.execute(terminal, gen3Options, env, processInfo);
 
@@ -773,7 +773,7 @@ public class CertificateToolTests extends ESTestCase {
         Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", tempDir).build());
 
         final Path caFile = tempDir.resolve("ca.p12");
-        final String caPassword = generateCA(caFile, terminal, env);
+        final String caPassword = generateCA(caFile, terminal, env, false);
 
         final Path node1Pkcs12 = tempDir.resolve("node1.p12");
         final Path pemZip = tempDir.resolve("pem.zip");
@@ -831,8 +831,7 @@ public class CertificateToolTests extends ESTestCase {
 
         assertThat(pemZip, pathExists());
 
-        FileSystem zip2FS = FileSystems.newFileSystem(new URI("jar:" + pemZip.toUri()), Collections.emptyMap());
-        Path zip2Root = zip2FS.getPath("/");
+        final Path zip2Root = getRootPathOfZip(pemZip);
 
         final Path ca2 = zip2Root.resolve("ca/ca.p12");
         assertThat(ca2, not(pathExists()));
@@ -861,7 +860,7 @@ public class CertificateToolTests extends ESTestCase {
         final Path zip = tempDir.resolve("pem.zip");
 
         final AtomicBoolean isZip = new AtomicBoolean(false);
-        final GenerateCertificateCommand genCommand = new PathAwareGenerateCertificateCommand(null, zip) {
+        final GenerateCertificateCommand genCommand = new PathAwareGenerateCertificateCommand(Map.of(), zip) {
             @Override
             void generateAndWriteSignedCertificates(
                 Path output,
@@ -892,6 +891,45 @@ public class CertificateToolTests extends ESTestCase {
         assertThat("For command line option " + optionThatTriggersZip, isZip.get(), equalTo(true));
     }
 
+    public void testErrorIfSigningCertificateAndKeyDontMatch() throws Exception {
+        final Path tempDir = initTempDir();
+
+        final var terminal = MockTerminal.create();
+        final var env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", tempDir).build());
+        final var processInfo = new ProcessInfo(Map.of(), Map.of(), createTempDir());
+
+        final Path ca1zip = tempDir.resolve("ca1.zip");
+        final String ca1Password = generateCA(ca1zip, terminal, env, true);
+        terminal.reset();
+        final Path ca2zip = tempDir.resolve("ca2.zip");
+        final String ca2Password = generateCA(ca2zip, terminal, env, true);
+
+        var ca1Root = getRootPathOfZip(ca1zip);
+        var ca1Cert = ca1Root.resolve("ca/ca.crt");
+        var ca1Key = ca1Root.resolve("ca/ca.key");
+
+        var ca2Root = getRootPathOfZip(ca2zip);
+        var ca2Key = ca2Root.resolve("ca/ca.key");
+
+        var p12Out = tempDir.resolve("certs.p12");
+        var p12Password = randomAlphaOfLength(8);
+
+        final var gen1Command = new PathAwareGenerateCertificateCommand(Map.of("ca-cert", ca1Cert, "ca-key", ca2Key), p12Out);
+        final var gen1Options = gen1Command.getParser()
+            .parse("--ca-cert", "<ca_crt>", "--ca-key", "<ca_key>", "--ca-pass", ca2Password, "--out", "<p12>", "--pass", p12Password);
+
+        final UserException e = expectThrows(UserException.class, () -> gen1Command.execute(terminal, gen1Options, env, processInfo));
+        assertThat(e.exitCode, is(ExitCodes.CONFIG));
+        assertThat(e.getMessage(), containsString("Certificate verification failed"));
+        assertThat(p12Out, not(pathExists()));
+
+        final var gen2Command = new PathAwareGenerateCertificateCommand(Map.of("ca-cert", ca1Cert, "ca-key", ca1Key), p12Out);
+        final var gen2Options = gen2Command.getParser()
+            .parse("--ca-cert", "<ca_crt>", "--ca-key", "<ca_key>", "--ca-pass", ca1Password, "--out", "<p12>", "--pass", p12Password);
+        gen2Command.execute(terminal, gen2Options, env, processInfo);
+        assertThat(p12Out, pathExists());
+    }
+
     private int getKeySize(Key node1Key) {
         assertThat(node1Key, instanceOf(RSAKey.class));
         return ((RSAKey) node1Key).getModulus().bitLength();
@@ -1034,25 +1072,32 @@ public class CertificateToolTests extends ESTestCase {
         return PathUtils.get(path).toAbsolutePath();
     }
 
-    private String generateCA(Path caFile, MockTerminal terminal, Environment env) throws Exception {
+    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, boolean pem) throws Exception {
         final int caKeySize = randomIntBetween(4, 8) * 512;
         final int days = randomIntBetween(7, 1500);
         final String caPassword = randomFrom("", randomAlphaOfLengthBetween(4, 80));
 
         final CertificateAuthorityCommand caCommand = new PathAwareCertificateAuthorityCommand(caFile);
-        final OptionSet caOptions = caCommand.getParser()
-            .parse(
-                "-ca-dn",
-                "CN=My ElasticSearch Cluster",
-                "-pass",
-                caPassword,
-                "-out",
-                caFile.toString(),
-                "-keysize",
-                String.valueOf(caKeySize),
-                "-days",
-                String.valueOf(days)
-            );
+        String[] args = {
+            "-ca-dn",
+            "CN=My ElasticSearch Cluster",
+            "-pass",
+            caPassword,
+            "-out",
+            caFile.toString(),
+            "-keysize",
+            String.valueOf(caKeySize),
+            "-days",
+            String.valueOf(days) };
+        if (pem) {
+            args = ArrayUtils.append(args, "--pem");
+        }
+        final OptionSet caOptions = caCommand.getParser().parse(args);
         final ProcessInfo processInfo = new ProcessInfo(Map.of(), Map.of(), createTempDir());
         caCommand.execute(terminal, caOptions, env, processInfo);
 
@@ -1091,20 +1136,26 @@ public class CertificateToolTests extends ESTestCase {
      * This class works around that by sticking with the original path objects
      */
     private static class PathAwareGenerateCertificateCommand extends GenerateCertificateCommand {
-        private final Path caFile;
+        private final Map<String, Path> inputPaths;
         private final Path outFile;
 
         PathAwareGenerateCertificateCommand(Path caFile, Path outFile) {
-            this.caFile = caFile;
+            this(Map.of("ca", caFile), outFile);
+        }
+
+        PathAwareGenerateCertificateCommand(Map<String, Path> inputPaths, Path outFile) {
+            this.inputPaths = Map.copyOf(inputPaths);
             this.outFile = outFile;
         }
 
         @Override
         protected Path resolvePath(OptionSet options, OptionSpec<String> spec) {
-            if (spec.options().contains("ca")) {
-                return caFile;
-            }
-            return super.resolvePath(options, spec);
+            return this.inputPaths.entrySet()
+                .stream()
+                .filter(entry -> spec.options().contains(entry.getKey()))
+                .findFirst()
+                .map(Entry::getValue)
+                .orElseGet(() -> super.resolvePath(options, spec));
         }
 
         @Override