Add error message if using password list in config (#941)

* Remove support for providing plaintext passwords in config/tessera CLI Existing configs containing an empty password list will still be allowed. * Only validate key passwords once the rest of config has been validated This is to prevent keys being decrypted before the config validation flags the use of the "passwords" field as invalid. * Remove support for passwords field in config when running w/ keygen * Account for keys configuration containing only the passwordFile field * Unit tests for deprecated passwords config field
Consensys · Dec 6, 2019 · 8fdd19e · 8fdd19e
1 parent bb4bafa
commit 8fdd19e
Show file tree

Hide file tree

Showing 10 changed files with 511 additions and 68 deletions.
diff --git a/cli/cli-api/src/main/java/com/quorum/tessera/cli/parsers/ConfigurationParser.java b/cli/cli-api/src/main/java/com/quorum/tessera/cli/parsers/ConfigurationParser.java
@@ -1,6 +1,7 @@
 package com.quorum.tessera.cli.parsers;
 
 import com.quorum.tessera.config.Config;
+import com.quorum.tessera.config.ConfigException;
 import com.quorum.tessera.config.KeyConfiguration;
 import com.quorum.tessera.config.keypairs.ConfigKeyPair;
 import com.quorum.tessera.config.util.ConfigFileStore;
@@ -13,25 +14,26 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import static java.nio.file.StandardOpenOption.APPEND;
-import java.util.List;
-import java.util.Objects;
-
-import static java.nio.file.StandardOpenOption.CREATE_NEW;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.nio.file.StandardOpenOption.APPEND;
+import static java.nio.file.StandardOpenOption.CREATE_NEW;
+
 public class ConfigurationParser implements Parser<Config> {
 
     protected static final Set<PosixFilePermission> NEW_PASSWORD_FILE_PERMS =
             Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE).collect(Collectors.toSet());
 
+    protected static final String passwordsMessage = "Configfile must contain \"passwordFile\" field. The \"passwords\" field is no longer supported.";
+
     private final List<ConfigKeyPair> newlyGeneratedKeys;
 
     private final FilesDelegate filesDelegate;
@@ -56,7 +58,7 @@ public Config parse(final CommandLine commandLine) throws IOException {
         if (commandLine.hasOption("configfile") && !isGeneratingWithKeyVault) {
             final Path path = Paths.get(commandLine.getOptionValue("configfile"));
 
-            if (!Files.exists(path)) {
+            if (!filesDelegate.exists(path)) {
                 throw new FileNotFoundException(String.format("%s not found.", path));
             }
 
@@ -68,6 +70,9 @@ public Config parse(final CommandLine commandLine) throws IOException {
                         config.setKeys(new KeyConfiguration());
                         config.getKeys().setKeyData(new ArrayList<>());
                     }
+                    if (config.getKeys().getKeyData() == null) {
+                        config.getKeys().setKeyData(new ArrayList<>());
+                    }
                     doPasswordStuff(config);
                     config.getKeys().getKeyData().addAll(newlyGeneratedKeys);
                 }
@@ -84,12 +89,12 @@ public Config parse(final CommandLine commandLine) throws IOException {
         return config;
     }
 
-    private static void output(CommandLine commandLine, Config config) throws IOException {
+    private void output(CommandLine commandLine, Config config) throws IOException {
 
         if (commandLine.hasOption("output")) {
             final Path outputConfigFile = Paths.get(commandLine.getOptionValue("output"));
 
-            try (OutputStream out = Files.newOutputStream(outputConfigFile, CREATE_NEW)) {
+            try (OutputStream out = filesDelegate.newOutputStream(outputConfigFile, CREATE_NEW)) {
                 JaxbUtil.marshal(config, out);
             }
         } else {
@@ -107,35 +112,21 @@ private void createFile(Path fileToMake) {
         }
     }
 
-    public Config doPasswordStuff(Config config) {
-
+    public Config doPasswordStuff(Config config) throws ConfigException {
         final List<String> newPasswords =
-                newlyGeneratedKeys.stream().map(ConfigKeyPair::getPassword).collect(Collectors.toList());
+            newlyGeneratedKeys.stream().map(ConfigKeyPair::getPassword).collect(Collectors.toList());
 
-        if (config.getKeys().getPasswords() != null) {
-            config.getKeys().getPasswords().addAll(newPasswords);
-            return config;
-        }
+        boolean hasNewPasswords = newPasswords.stream().anyMatch(p -> Objects.nonNull(p) && !p.isEmpty());
+        boolean isUsingPasswordFile = Objects.nonNull(config.getKeys().getPasswordFile());
+
+        if (hasNewPasswords) {
+            if (!isUsingPasswordFile) {
+                throw new ConfigException(new RuntimeException(passwordsMessage));
+            }
 
-        if (config.getKeys().getPasswordFile() != null) {
             Path passwordFile = config.getKeys().getPasswordFile();
             createFile(passwordFile);
             filesDelegate.write(passwordFile, newPasswords, APPEND);
-            return config;
-        }
-
-        /*
-         * Populate transient list of passwords that is consumed by KeyPasswordResolver
-         */
-        if (newPasswords.stream().anyMatch(Objects::nonNull) && config.getKeys().getKeyData() != null) {
-
-            final List<String> existingPasswords =
-                    config.getKeys().getKeyData().stream().map(k -> "").collect(Collectors.toList());
-
-            existingPasswords.addAll(newPasswords);
-            config.getKeys().setPasswords(existingPasswords);
-
-            return config;
         }
 
         return config;