Resolve "cli: keys modify: ask salt and password on password prompt for security"

Closes #201 (closed)

lib/core/conf/src/keypairs/cli.rs

@@ -28,11 +28,36 @@
 )]
 
 use crate::*;
-use std::io;
+use std::io::{self, Write};
+use zeroize::Zeroize;
+
+#[cfg(test)]
+use mockall::*;
+
+#[derive(Zeroize)]
+#[zeroize(drop)]
+struct Secret {
+    secret: String,
+}
+
+#[cfg_attr(test, automock)]
+trait GetUserInput {
+    fn get_user_input(&self, prompt: &str) -> std::io::Result<Secret>;
+}
+
+impl GetUserInput for std::io::Stdin {
+    fn get_user_input(&self, prompt: &str) -> std::io::Result<Secret> {
+        print!("{}", prompt);
+        io::stdout().flush()?;
+        Ok(Secret {
+            secret: rpassword::read_password_with_reader(Some(self.lock()))?,
+        })
+    }
+}
 
 #[derive(Debug, Copy, Clone)]
-/// Errors encountered by the wizard
-pub enum WizardError {
+/// Errors encountered by the user interaction
+pub enum CliError {
     /// Canceled
     Canceled,
 
@@ -40,35 +65,38 @@ pub enum WizardError {
     BadInput,
 }
 
-impl From<std::io::Error> for WizardError {
+impl From<std::io::Error> for CliError {
     fn from(_e: std::io::Error) -> Self {
-        WizardError::BadInput
+        CliError::BadInput
     }
 }
 
 /// Modify network keys command
-pub fn modify_network_keys(
-    salt: String,
-    password: String,
+pub fn modify_network_keys(key_pairs: DuniterKeyPairs) -> Result<DuniterKeyPairs, CliError> {
+    inner_modify_network_keys(std::io::stdin(), key_pairs)
+}
+
+/// Private function to modify network keys
+fn inner_modify_network_keys<T: GetUserInput>(
+    stdin: T,
     mut key_pairs: DuniterKeyPairs,
-) -> DuniterKeyPairs {
-    let generator = ed25519::KeyPairFromSaltedPasswordGenerator::with_default_parameters();
-    key_pairs.network_keypair =
-        KeyPairEnum::Ed25519(generator.generate(ed25519::SaltedPassword::new(salt, password)));
-    key_pairs
+) -> Result<DuniterKeyPairs, CliError> {
+    key_pairs.network_keypair = salt_password_prompt(stdin)?;
+    Ok(key_pairs)
 }
 
 /// Modify member keys command
-pub fn modify_member_keys(
-    salt: String,
-    password: String,
+pub fn modify_member_keys(key_pairs: DuniterKeyPairs) -> Result<DuniterKeyPairs, CliError> {
+    inner_modify_member_keys(std::io::stdin(), key_pairs)
+}
+
+/// Private function to modify network keys
+fn inner_modify_member_keys<T: GetUserInput>(
+    stdin: T,
     mut key_pairs: DuniterKeyPairs,
-) -> DuniterKeyPairs {
-    let generator = ed25519::KeyPairFromSaltedPasswordGenerator::with_default_parameters();
-    key_pairs.member_keypair = Some(KeyPairEnum::Ed25519(
-        generator.generate(ed25519::SaltedPassword::new(salt, password)),
-    ));
-    key_pairs
+) -> Result<DuniterKeyPairs, CliError> {
+    key_pairs.member_keypair = Some(salt_password_prompt(stdin)?);
+    Ok(key_pairs)
 }
 
 /// Ask user for confirmation and Clear keys command
@@ -115,8 +143,18 @@ fn inner_clear_keys(
 
 /// Show keys command
 pub fn show_keys(key_pairs: DuniterKeyPairs) {
+    show_network_keys(&key_pairs);
+    show_member_keys(&key_pairs);
+}
+
+/// Show network keys
+pub fn show_network_keys(key_pairs: &DuniterKeyPairs) {
     println!("Network key: {}", key_pairs.network_keypair);
-    match key_pairs.member_keypair {
+}
+
+/// Show member keys
+pub fn show_member_keys(key_pairs: &DuniterKeyPairs) {
+    match &key_pairs.member_keypair {
         None => println!("No member key configured"),
         Some(key) => println!("Member key: {}", key),
     }
@@ -126,7 +164,7 @@ pub fn show_keys(key_pairs: DuniterKeyPairs) {
 pub fn save_keypairs(
     profile_path: PathBuf,
     keypairs_file_path: &Option<PathBuf>,
-    key_pairs: DuniterKeyPairs,
+    key_pairs: &DuniterKeyPairs,
 ) -> Result<(), std::io::Error> {
     let conf_keys_path: PathBuf = if let Some(keypairs_file_path) = keypairs_file_path {
         keypairs_file_path.to_path_buf()
@@ -139,7 +177,7 @@ pub fn save_keypairs(
     Ok(())
 }
 
-fn question_prompt<'a>(question: &str, answers: &[&'a str]) -> Result<&'a str, WizardError> {
+fn question_prompt<'a>(question: &str, answers: &[&'a str]) -> Result<&'a str, CliError> {
     let mut buf = String::new();
 
     println!("{} ({}):", question, answers.join("/"));
@@ -150,41 +188,42 @@ fn question_prompt<'a>(question: &str, answers: &[&'a str]) -> Result<&'a str, W
             let answer = answers.iter().find(|x| **x == buf.trim());
             match answer {
                 Some(&value) => Ok(value),
-                None => Err(WizardError::Canceled),
+                None => Err(CliError::Canceled),
             }
         }
-        Err(_) => Err(WizardError::Canceled),
+        Err(_) => Err(CliError::Canceled),
     }
 }
 
-fn salt_password_prompt() -> Result<KeyPairEnum, WizardError> {
-    let salt = rpassword::prompt_password_stdout("Salt: ")?;
-    if !salt.is_empty() {
-        let password = rpassword::prompt_password_stdout("Password: ")?;
-        if !password.is_empty() {
+fn salt_password_prompt<T: GetUserInput>(stdin: T) -> Result<KeyPairEnum, CliError> {
+    let salt = stdin.get_user_input("Salt: ")?;
+    if !salt.secret.is_empty() {
+        let password = stdin.get_user_input("Password: ")?;
+        if !password.secret.is_empty() {
             let generator = ed25519::KeyPairFromSaltedPasswordGenerator::with_default_parameters();
-            let key_pairs = KeyPairEnum::Ed25519(
-                generator.generate(ed25519::SaltedPassword::new(salt, password)),
-            );
+            let key_pairs = KeyPairEnum::Ed25519(generator.generate(ed25519::SaltedPassword::new(
+                salt.secret.clone(),
+                password.secret.clone(),
+            )));
             Ok(key_pairs)
         } else {
-            Err(WizardError::BadInput)
+            Err(CliError::BadInput)
         }
     } else {
-        Err(WizardError::BadInput)
+        Err(CliError::BadInput)
     }
 }
 
 /// The wizard key function
-pub fn key_wizard(mut key_pairs: DuniterKeyPairs) -> Result<DuniterKeyPairs, WizardError> {
+pub fn key_wizard(mut key_pairs: DuniterKeyPairs) -> Result<DuniterKeyPairs, CliError> {
     let mut answer = question_prompt("Modify your network keypair?", &["y", "n"])?;
     if answer == "y" {
-        key_pairs.network_keypair = salt_password_prompt()?;
+        key_pairs.network_keypair = salt_password_prompt(std::io::stdin())?;
     }
 
     answer = question_prompt("Modify your member keypair?", &["y", "n", "d"])?;
     if answer == "y" {
-        key_pairs.member_keypair = Some(salt_password_prompt()?);
+        key_pairs.member_keypair = Some(salt_password_prompt(std::io::stdin())?);
     } else if answer == "d" {
         println!("Deleting member keypair!");
         key_pairs.member_keypair = None;
@@ -220,8 +259,25 @@ mod tests {
             }),
             member_keypair: None,
         };
+        let mut stdin_mock = MockGetUserInput::new();
+        stdin_mock.expect_get_user_input().returning(|prompt| {
+            if prompt.starts_with("Salt:") {
+                Ok(Secret {
+                    secret: SALT_TEST.to_owned(),
+                })
+            } else if prompt.starts_with("Password:") {
+                Ok(Secret {
+                    secret: PASSWORD_TEST.to_owned(),
+                })
+            } else {
+                Err(std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    format!("should not be called with {}", prompt),
+                ))
+            }
+        });
         let result_key_pairs =
-            modify_member_keys(SALT_TEST.to_owned(), PASSWORD_TEST.to_owned(), key_pairs);
+            inner_modify_member_keys(stdin_mock, key_pairs).expect("Fail to read new member keys");
         // We expect network key not to change
         assert_eq!(
             result_key_pairs.network_keypair.public_key(),
@@ -269,8 +325,25 @@ mod tests {
             }),
             member_keypair: None,
         };
-        let result_key_pairs =
-            modify_network_keys(SALT_TEST.to_owned(), PASSWORD_TEST.to_owned(), key_pairs);
+        let mut stdin_mock = MockGetUserInput::new();
+        stdin_mock.expect_get_user_input().returning(|prompt| {
+            if prompt.starts_with("Salt:") {
+                Ok(Secret {
+                    secret: SALT_TEST.to_owned(),
+                })
+            } else if prompt.starts_with("Password:") {
+                Ok(Secret {
+                    secret: PASSWORD_TEST.to_owned(),
+                })
+            } else {
+                Err(std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    format!("should not be called with {}", prompt),
+                ))
+            }
+        });
+        let result_key_pairs = inner_modify_network_keys(stdin_mock, key_pairs)
+            .expect("Fail to read new network keys");
         // We expect network key to update
         assert_eq!(
             result_key_pairs.network_keypair.public_key(),