Merge pull request #580 from QuorumEngineering/fix-keypair-validation…

…-errors Fix locked key missing password message
Consensys · Jan 2, 2019 · 9b9644d · 9b9644d
2 parents 0872f4f + d12b597
commit 9b9644d
Show file tree

Hide file tree

Showing 17 changed files with 105 additions and 428 deletions.
diff --git a/config/src/main/java/com/quorum/tessera/config/JaxbConfigFactory.java b/config/src/main/java/com/quorum/tessera/config/JaxbConfigFactory.java
@@ -42,7 +42,7 @@ public Config create(final InputStream configData, final List<ConfigKeyPair> new
                 } else if (config.getKeys().getPasswordFile() != null) {
                     this.createFile(config.getKeys().getPasswordFile());
                     Files.write(config.getKeys().getPasswordFile(), newPasswords, APPEND);
-                } else if (!newPasswords.stream().allMatch(""::equals)) {
+                } else if (!newPasswords.stream().allMatch(Objects::isNull)) {
                     final List<String> existingPasswords = config
                         .getKeys()
                         .getKeyData()

diff --git a/config/src/main/java/com/quorum/tessera/config/KeyData.java b/config/src/main/java/com/quorum/tessera/config/KeyData.java
@@ -1,8 +1,6 @@
 package com.quorum.tessera.config;
 
 import com.quorum.tessera.config.adapters.PathAdapter;
-import com.quorum.tessera.config.constraints.ValidBase64;
-import com.quorum.tessera.config.constraints.ValidKeyDataConfig;
 
 import javax.validation.constraints.Pattern;
 import javax.xml.bind.annotation.XmlAccessType;
@@ -14,13 +12,9 @@
 @XmlAccessorType(XmlAccessType.FIELD)
 public class KeyData extends ConfigItem {
 
-    @ValidKeyDataConfig
     @XmlElement
     private KeyDataConfig config;
 
-    @ValidBase64
-    @Pattern(regexp = "^((?!NACL_FAILURE).)*$",
-            message = "Could not decrypt the private key with the provided password, please double check the passwords provided")
     @XmlElement
     private String privateKey;
 

diff --git a/config/src/main/java/com/quorum/tessera/config/adapters/KeyDataAdapter.java b/config/src/main/java/com/quorum/tessera/config/adapters/KeyDataAdapter.java
@@ -9,7 +9,9 @@
 public class KeyDataAdapter extends XmlAdapter<KeyData, ConfigKeyPair> {
 
     public static final String NACL_FAILURE_TOKEN = "NACL_FAILURE";
-
+
+    public static final String MISSING_PASSWORD_TOKEN = "MISSING_PASSWORD";
+
     @Override
     public ConfigKeyPair unmarshal(final KeyData keyData) {
 

diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/Base64Validator.java b/config/src/main/java/com/quorum/tessera/config/constraints/Base64Validator.java
@@ -1,5 +1,6 @@
 package com.quorum.tessera.config.constraints;
 
+import static com.quorum.tessera.config.adapters.KeyDataAdapter.MISSING_PASSWORD_TOKEN;
 import static com.quorum.tessera.config.adapters.KeyDataAdapter.NACL_FAILURE_TOKEN;
 import java.util.Base64;
 import java.util.Objects;
@@ -19,7 +20,7 @@ public boolean isValid(String value, ConstraintValidatorContext cvc) {
             return true;
         }
 
-        if(value.startsWith(NACL_FAILURE_TOKEN)) {
+        if(value.startsWith(NACL_FAILURE_TOKEN) || value.startsWith(MISSING_PASSWORD_TOKEN)) {
             return true;
         }
 

diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/InlineKeypairValidator.java b/config/src/main/java/com/quorum/tessera/config/constraints/InlineKeypairValidator.java
diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/KeyDataConfigValidator.java b/config/src/main/java/com/quorum/tessera/config/constraints/KeyDataConfigValidator.java
diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/ValidInlineKeypair.java b/config/src/main/java/com/quorum/tessera/config/constraints/ValidInlineKeypair.java
diff --git a/config/src/main/java/com/quorum/tessera/config/constraints/ValidKeyDataConfig.java b/config/src/main/java/com/quorum/tessera/config/constraints/ValidKeyDataConfig.java
diff --git a/config/src/main/java/com/quorum/tessera/config/keypairs/FilesystemKeyPair.java b/config/src/main/java/com/quorum/tessera/config/keypairs/FilesystemKeyPair.java
@@ -2,12 +2,17 @@
 
 import com.quorum.tessera.config.KeyDataConfig;
 import com.quorum.tessera.config.adapters.PathAdapter;
+import com.quorum.tessera.config.constraints.ValidBase64;
 import com.quorum.tessera.config.constraints.ValidContent;
 import com.quorum.tessera.config.constraints.ValidPath;
 import com.quorum.tessera.config.util.JaxbUtil;
 import com.quorum.tessera.io.IOCallback;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Pattern;
+import javax.validation.constraints.Size;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 import java.nio.file.Files;
@@ -17,14 +22,16 @@
 
 public class FilesystemKeyPair implements ConfigKeyPair {
 
-    @ValidContent(minLines = 1,maxLines = 1,message = "file expected to contain a single non empty value")
+    private static final Logger LOGGER = LoggerFactory.getLogger(FilesystemKeyPair.class);
+
+    @ValidContent(minLines = 1, maxLines = 1, message = "file expected to contain a single non empty value")
     @NotNull
     @ValidPath(checkExists = true, message = "File does not exist")
     @XmlElement
     @XmlJavaTypeAdapter(PathAdapter.class)
     private final Path publicKeyPath;
 
-    @ValidContent(minLines = 1,message = "file expected to contain at least one line")
+    @ValidContent(minLines = 1, message = "file expected to contain at least one line")
     @NotNull
     @ValidPath(checkExists = true, message = "File does not exist")
     @XmlElement
@@ -33,28 +40,48 @@ public class FilesystemKeyPair implements ConfigKeyPair {
 
     private InlineKeypair inlineKeypair;
 
-    private String password = "";
+    private String password;
 
     public FilesystemKeyPair(final Path publicKeyPath, final Path privateKeyPath) {
         this.publicKeyPath = publicKeyPath;
         this.privateKeyPath = privateKeyPath;
+
+        try {
+            loadKeys();
+        } catch (final Exception ex) {
+            //silently discard errors as these get picked up by the validator
+            LOGGER.debug("Unable to read key files", ex);
+        }
     }
 
     @Override
+    @Size(min = 1)
+    @ValidBase64(message = "Invalid Base64 key provided")
     public String getPublicKey() {
-        loadKeys();
+        if (this.inlineKeypair == null) {
+            return null;
+        }
         return this.inlineKeypair.getPublicKey();
     }
 
     @Override
+    @Size(min = 1)
+    @ValidBase64(message = "Invalid Base64 key provided")
+    @Pattern(regexp = "^((?!NACL_FAILURE).)*$", message = "Could not decrypt the private key with the provided password, please double check the passwords provided")
+    @Pattern(regexp = "^((?!MISSING_PASSWORD).)*$", message = "{InlineKeyData.missingPassword.message}")
     public String getPrivateKey() {
-        loadKeys();
+        if (this.inlineKeypair == null) {
+            return null;
+        }
         return this.inlineKeypair.getPrivateKey();
     }
 
     @Override
     public void withPassword(final String password) {
         this.password = password;
+        if (this.inlineKeypair != null) {
+            this.inlineKeypair.withPassword(this.password);
+        }
     }
 
     @Override
@@ -70,24 +97,18 @@ public Path getPrivateKeyPath() {
         return privateKeyPath;
     }
 
-    private void loadKeys() {
-        if(inlineKeypair == null) {
-            this.inlineKeypair = new InlineKeypair(
-                IOCallback.execute(() -> new String(Files.readAllBytes(this.publicKeyPath), UTF_8)),
-                JaxbUtil.unmarshal(
-                    IOCallback.execute(() -> Files.newInputStream(privateKeyPath)),
-                    KeyDataConfig.class
-                )
-            );
-        }
-        this.inlineKeypair.withPassword(this.password);
-    }
-
-    //For testing only
-    //TODO: remove
     public InlineKeypair getInlineKeypair() {
-        loadKeys();
         return inlineKeypair;
     }
 
+    private void loadKeys() {
+        this.inlineKeypair = new InlineKeypair(
+            IOCallback.execute(() -> new String(Files.readAllBytes(this.publicKeyPath), UTF_8)),
+            JaxbUtil.unmarshal(
+                IOCallback.execute(() -> Files.newInputStream(privateKeyPath)),
+                KeyDataConfig.class
+            )
+        );
+    }
+
 }
diff --git a/config/src/main/java/com/quorum/tessera/config/keypairs/InlineKeypair.java b/config/src/main/java/com/quorum/tessera/config/keypairs/InlineKeypair.java
@@ -3,7 +3,6 @@
 import com.quorum.tessera.config.KeyDataConfig;
 import com.quorum.tessera.config.PrivateKeyData;
 import com.quorum.tessera.config.constraints.ValidBase64;
-import com.quorum.tessera.config.constraints.ValidInlineKeypair;
 import com.quorum.tessera.config.keys.KeyEncryptorFactory;
 import com.quorum.tessera.nacl.NaclException;
 
@@ -12,23 +11,20 @@
 import javax.xml.bind.annotation.XmlElement;
 
 import static com.quorum.tessera.config.PrivateKeyType.UNLOCKED;
-import com.quorum.tessera.encryption.PrivateKey;
 import javax.validation.constraints.Size;
 
-@ValidInlineKeypair
 public class InlineKeypair implements ConfigKeyPair {
 
-    @Size(min = 1)
-    @NotNull
-    @ValidBase64(message = "Invalid Base64 key provided")
     @XmlElement
     private final String publicKey;
 
     @NotNull
     @XmlElement(name = "config")
     private final KeyDataConfig privateKeyConfig;
 
-    private String password = "";
+    private String password;
+
+    private String cachedValue;
 
     public InlineKeypair(final String publicKey, final KeyDataConfig privateKeyConfig) {
         this.publicKey = publicKey;
@@ -40,25 +36,39 @@ public KeyDataConfig getPrivateKeyConfig() {
     }
 
     @Override
+    @Size(min = 1)
+    @NotNull
+    @ValidBase64(message = "Invalid Base64 key provided")
     public String getPublicKey() {
         return this.publicKey;
     }
 
     @Override
+    @NotNull
+    @Size(min = 1)
+    @ValidBase64(message = "Invalid Base64 key provided")
     @Pattern(regexp = "^((?!NACL_FAILURE).)*$", message = "Could not decrypt the private key with the provided password, please double check the passwords provided")
+    @Pattern(regexp = "^((?!MISSING_PASSWORD).)*$", message = "{InlineKeyData.missingPassword.message}")
     public String getPrivateKey() {
         final PrivateKeyData pkd = privateKeyConfig.getPrivateKeyData();
 
         if (privateKeyConfig.getType() == UNLOCKED) {
             return privateKeyConfig.getValue();
-        } else {
-            try {
-                PrivateKey privateKey = KeyEncryptorFactory.create().decryptPrivateKey(pkd, password);
-                return privateKey.encodeToBase64();
-            } catch (final NaclException ex) {
-                return "NACL_FAILURE";
+        }
+
+        if (this.cachedValue == null) {
+            if (password == null) {
+                this.cachedValue = "MISSING_PASSWORD";
+            } else {
+                try {
+                    this.cachedValue = KeyEncryptorFactory.create().decryptPrivateKey(pkd, password).encodeToBase64();
+                } catch (final NaclException ex) {
+                    this.cachedValue = "NACL_FAILURE";
+                }
             }
         }
+
+        return this.cachedValue;
     }
 
     @Override