[Security] Apply validation when parsing certgen input (elastic/x-pack-elasticsearch#2711)

When certgen configuration was read from an input file (`-in` option) validation errors were collected but never reported. Depending on the type of error this may have caused the tool to exit with an internal error (e.g. NPE).

Validation is now applied after parsing the file and if errors are found the tool exits.

Original commit: elastic/x-pack-elasticsearch@b2262ed1d7

plugin/src/main/java/org/elasticsearch/xpack/ssl/CertUtils.java

@@ -38,6 +38,7 @@ import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
 
@@ -360,6 +361,9 @@ public class CertUtils {
      */
     static PKCS10CertificationRequest generateCSR(KeyPair keyPair, X500Principal principal, GeneralNames sanList)
             throws IOException, OperatorCreationException {
+        Objects.requireNonNull(keyPair, "Key-Pair must not be null");
+        Objects.requireNonNull(keyPair.getPublic(), "Public-Key must not be null");
+        Objects.requireNonNull(principal, "Principal must not be null");
         JcaPKCS10CertificationRequestBuilder builder = new JcaPKCS10CertificationRequestBuilder(principal, keyPair.getPublic());
         if (sanList != null) {
             ExtensionsGenerator extGen = new ExtensionsGenerator();

plugin/src/main/java/org/elasticsearch/xpack/ssl/CertificateTool.java

@@ -47,7 +47,10 @@ import org.bouncycastle.openssl.jcajce.JcePEMEncryptorBuilder;
 import org.bouncycastle.pkcs.PKCS10CertificationRequest;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.cli.EnvironmentAwareCommand;
+import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.cli.Terminal.Verbosity;
+import org.elasticsearch.cli.UserException;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
@@ -222,7 +225,7 @@ public class CertificateTool extends EnvironmentAwareCommand {
     static Collection<CertificateInformation> getCertificateInformationList(Terminal terminal, String inputFile)
             throws Exception {
         if (inputFile != null) {
-            return parseFile(resolvePath(inputFile).toAbsolutePath());
+            return parseAndValidateFile(terminal, resolvePath(inputFile).toAbsolutePath());
         }
         Map<String, CertificateInformation> map = new HashMap<>();
         boolean done = false;
@@ -267,6 +270,26 @@ public class CertificateTool extends EnvironmentAwareCommand {
         return map.values();
     }
 
+    static Collection<CertificateInformation> parseAndValidateFile(Terminal terminal, Path file) throws Exception {
+        final Collection<CertificateInformation> config = parseFile(file);
+        boolean hasError = false;
+        for (CertificateInformation certInfo : config) {
+            final List<String> errors = certInfo.validate();
+            if (errors.size() > 0) {
+                hasError = true;
+                terminal.println(Verbosity.SILENT, "Configuration for instance " + certInfo.name.originalName + " has invalid details");
+                for (String message : errors) {
+                    terminal.println(Verbosity.SILENT, " * " + message);
+                }
+                terminal.println("");
+            }
+        }
+        if (hasError) {
+            throw new UserException(ExitCodes.CONFIG, "File " + file + " contains invalid configuration details (see messages above)");
+        }
+        return config;
+    }
+
     /**
      * Parses the input file to retrieve the certificate information
      * @param file the file to parse

plugin/src/test/java/org/elasticsearch/xpack/ssl/CertificateToolTests.java

@@ -56,6 +56,8 @@ import org.bouncycastle.openssl.PEMEncryptedKeyPair;
 import org.bouncycastle.openssl.PEMParser;
 import org.bouncycastle.pkcs.PKCS10CertificationRequest;
 import org.elasticsearch.cli.MockTerminal;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.cli.UserException;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
@@ -211,6 +213,18 @@ public class CertificateToolTests extends ESTestCase {
         assertEquals("different file", certInfo.name.filename);
     }
 
+    public void testParsingFileWithInvalidDetails() throws Exception {
+        Path tempDir = initTempDir();
+        Path instanceFile = writeInvalidInstanceInformation(tempDir.resolve("instances-invalid.yml"));
+        final MockTerminal terminal = new MockTerminal();
+        final UserException exception = expectThrows(UserException.class,
+                () -> CertificateTool.parseAndValidateFile(terminal, instanceFile));
+        assertThat(exception.getMessage(), containsString("invalid configuration"));
+        assertThat(exception.getMessage(), containsString(instanceFile.toString()));
+        assertThat(terminal.getOutput(), containsString("THIS=not a,valid DN"));
+        assertThat(terminal.getOutput(), containsString("could not be converted to a valid DN"));
+    }
+
     public void testGeneratingCsr() throws Exception {
         Path tempDir = initTempDir();
         Path outputFile = tempDir.resolve("out.zip");
@@ -528,6 +542,17 @@ public class CertificateToolTests extends ESTestCase {
         return Files.write(path, instances, StandardCharsets.UTF_8);
     }
 
+    /**
+     * Writes the description of instances to a given {@link Path}
+     */
+    private Path writeInvalidInstanceInformation(Path path) throws IOException {
+        Iterable<String> instances = Arrays.asList(
+                "instances:",
+                "  - name: \"THIS=not a,valid DN\"",
+                "    ip: \"127.0.0.1\"");
+        return Files.write(path, instances, StandardCharsets.UTF_8);
+    }
+
     @SuppressForbidden(reason = "resolve paths against CWD for a CLI tool")
     private static Path resolvePath(String path) {
         return PathUtils.get(path).toAbsolutePath();