Tie zip output to command line options in certutil (elastic/x-pack-elasticsearch#4354)

Previously "certutil" would generate a zip file if there were multiple
certificates.
However, this means that if the user specified "-multiple" or "-in"
then the output format will vary based on whether they entered
multiple instance names (-multiple) or whether the input file
contained multiple instance records (-in).
It is better if the output format is predictable based on the command
line options, so this change forces zip output whenever any of the
following command line options are supplied:
    -pem
    -keep-ca-key
    -multiple
    -in

Original commit: elastic/x-pack-elasticsearch@344baa5f17

docs/en/commands/certutil.asciidoc

@@ -82,8 +82,9 @@ generating a CA, see the <<certutil-ca,CA mode of this command>>.
 By default, the `cert` mode produces a single PKCS#12 output file which holds
 the instance certificate, the instance private key, and the CA certificate. If
 you specify the `--pem` parameter, the command generates PEM formatted
-certificates and keys and packages them into a zip file. Likewise if you chose
+certificates and keys and packages them into a zip file.
-to generate output for multiple instances, the command produces a zip file.
+If you specify the `--keep-ca-key`, `--multiple` or `--in` parameters,
+the command produces a zip file containing the generated certificates and keys.
 
 [float]
 [[certutil-csr]]

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

@@ -183,7 +183,7 @@ public class CertificateTool extends LoggingAwareMultiCommand {
         OptionSpec<String> caDnSpec;
         OptionSpec<Void> keepCaKeySpec;
 
-        OptionSpec<Void> multipleNodes;
+        OptionSpec<Void> multipleNodesSpec;
         OptionSpec<String> nameSpec;
         OptionSpec<String> dnsNamesSpec;
         OptionSpec<String> ipAddressesSpec;
@@ -236,10 +236,10 @@ public class CertificateTool extends LoggingAwareMultiCommand {
         }
 
         final void acceptInstanceDetails() {
-            multipleNodes = parser.accepts("multiple", "generate files for multiple instances");
+            multipleNodesSpec = parser.accepts("multiple", "generate files for multiple instances");
-            nameSpec = parser.accepts("name", "name of the generated certificate").availableUnless(multipleNodes).withRequiredArg();
+            nameSpec = parser.accepts("name", "name of the generated certificate").availableUnless(multipleNodesSpec).withRequiredArg();
-            dnsNamesSpec = parser.accepts("dns", "comma separated DNS names").availableUnless(multipleNodes).withRequiredArg();
+            dnsNamesSpec = parser.accepts("dns", "comma separated DNS names").availableUnless(multipleNodesSpec).withRequiredArg();
-            ipAddressesSpec = parser.accepts("ip", "comma separated IP addresses").availableUnless(multipleNodes).withRequiredArg();
+            ipAddressesSpec = parser.accepts("ip", "comma separated IP addresses").availableUnless(multipleNodesSpec).withRequiredArg();
         }
 
         final void acceptInputFile() {
@@ -394,7 +394,7 @@ public class CertificateTool extends LoggingAwareMultiCommand {
             if (input != null) {
                 return parseAndValidateFile(terminal, input.toAbsolutePath());
             }
-            if (options.has(multipleNodes)) {
+            if (options.has(multipleNodesSpec)) {
                 return readMultipleCertificateInformation(terminal);
             } else {
                 final Function<String, Stream<? extends String>> splitByComma = v -> Arrays.stream(Strings.splitStringByCommaToArray(v));
@@ -669,18 +669,19 @@ public class CertificateTool extends LoggingAwareMultiCommand {
             terminal.println("    * The private key for the instance certificate");
             terminal.println("    * The CA certificate");
             terminal.println("");
-            terminal.println("If you elect to generate PEM format certificates (the -pem option), then the output will");
+            terminal.println("If you specify any of the following options:");
-            terminal.println("be a zip file containing individual files for the instance certificate, the key and the CA certificate");
+            terminal.println("    * -pem (PEM formatted output)");
-            terminal.println("");
+            terminal.println("    * -keep-ca-key (retain generated CA key)");
-            terminal.println("If you elect to generate multiple instances certificates, the output will be a zip file");
+            terminal.println("    * -multiple (generate multiple certificates)");
-            terminal.println("containing all the generated certificates");
+            terminal.println("    * -in (generate certificates from an input file)");
+            terminal.println("then the output will be be a zip file containing individual certificate/key files");
             terminal.println("");
 
             CAInfo caInfo = getCAInfo(terminal, options, env);
             Collection<CertificateInformation> certInfo = getCertificateInformationList(terminal, options);
             final boolean keepCaKey = keepCaKey(options);
             final boolean usePemFormat = usePemFormat(options);
-            final boolean writeZipFile = certInfo.size() > 1 || keepCaKey || usePemFormat;
+            final boolean writeZipFile = options.has(multipleNodesSpec) || options.has(inputFileSpec) || keepCaKey || usePemFormat;
 
             final String outputName;
             if (writeZipFile) {

qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/core/ssl/CertificateToolTests.java

@@ -9,7 +9,6 @@ import com.google.common.jimfs.Configuration;
 import com.google.common.jimfs.Jimfs;
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
-import org.elasticsearch.core.internal.io.IOUtils;
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
 import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.ASN1String;
@@ -34,6 +33,7 @@ import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.CollectionUtils;
+import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.test.ESTestCase;
@@ -53,7 +53,6 @@ import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509ExtendedKeyManager;
 import javax.net.ssl.X509ExtendedTrustManager;
 import javax.security.auth.x500.X500Principal;
-
 import java.io.IOException;
 import java.io.Reader;
 import java.net.InetAddress;
@@ -83,6 +82,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -725,6 +725,42 @@ public class CertificateToolTests extends ESTestCase {
         checkTrust(node2KeyStore, new char[0], node1TrustStore, true);
     }
 
+    public void testZipOutputFromCommandLineOptions() throws Exception {
+        final Path tempDir = initTempDir();
+
+        final MockTerminal terminal = new MockTerminal();
+        Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", tempDir).build());
+
+        final Path zip = tempDir.resolve("pem.zip");
+
+        final AtomicBoolean isZip = new AtomicBoolean(false);
+        final GenerateCertificateCommand genCommand = new PathAwareGenerateCertificateCommand(null, zip) {
+            @Override
+            void generateAndWriteSignedCertificates(Path output, boolean writeZipFile, OptionSet options,
+                                                    Collection<CertificateInformation> certs, CAInfo caInfo,
+                                                    Terminal terminal) throws Exception {
+                isZip.set(writeZipFile);
+                // do nothing, all we care about is the "zip" flag
+            }
+
+            @Override
+            Collection<CertificateInformation> getCertificateInformationList(Terminal terminal, OptionSet options) throws Exception {
+                // Regardless of the commandline options, just work with a single cert
+                return Collections.singleton(new CertificateInformation("node", "node",
+                        Collections.emptyList(), Collections.emptyList(), Collections.emptyList()));
+            }
+        };
+
+        final String optionThatTriggersZip = randomFrom("-pem", "-keep-ca-key", "-multiple", "-in=input.yml");
+        final OptionSet genOptions = genCommand.getParser().parse(
+                "-out", "<zip>",
+                optionThatTriggersZip
+        );
+        genCommand.execute(terminal, genOptions, env);
+
+        assertThat("For command line option " + optionThatTriggersZip, isZip.get(), equalTo(true));
+    }
+
     private int getKeySize(Key node1Key) {
         assertThat(node1Key, instanceOf(RSAKey.class));
         return ((RSAKey) node1Key).getModulus().bitLength();