feat: Warn if an unencrypted identity is used

CHANGELOG.md

@@ -20,6 +20,8 @@ Allows to overwrite `Content-Type`, `Content-Encoding`, and `Cache-Control` HTTP
 ```
 This change will trigger the update process for frontend canister (new module hash: `2ff0513123f11c57716d889ca487083fac7d94a4c9434d5879f8d0342ad9d759`). 
 
+### feat: warn if an unencrypted identity is used on mainnet
+
 ### fix: Save SNS canister IDs
 
 SNS canister IDs were not being parsed reliably.  Now the candid file is being specified explicitly, which resolves the issue in at least some cases.

e2e/assets/expect_scripts/get_ledger_balance.exp

@@ -0,0 +1,18 @@
+#!/usr/bin/expect -df
+
+# ASSUMPTION: init_alice_with_pw.exp run before this script
+
+match_max 100000
+set timeout 30
+
+spawn dfx ledger balance --network ic --identity alice
+expect "Please enter the passphrase for your identity: "
+send -- "testpassword\r"
+expect "Decryption complete.\r"
+expect {
+    "WARN" {
+        puts stderr "Warned incorrectly for an encrypted identity"
+        exit 1
+    }
+    eof {}
+}

e2e/tests-dfx/identity.bash

@@ -160,3 +160,13 @@ teardown() {
     assert_command dfx canister call --output idl e2e_project_frontend retrieve '("B")'
     assert_eq '(blob "hello")'
 }
+
+@test "using an unencrypted identity on mainnet provokes a warning" {
+    assert_command dfx ledger balance --network ic
+    assert_match "WARN: The default identity is not stored securely." "$stderr"
+    assert_command "${BATS_TEST_DIRNAME}/../assets/expect_scripts/init_alice_with_pw.exp"
+    assert_command "${BATS_TEST_DIRNAME}/../assets/expect_scripts/get_ledger_balance.exp"
+    dfx identity new bob --disable-encryption
+    assert_command dfx ledger balance --network ic --identity bob
+    assert_match "WARN: The bob identity is not stored securely." "$stderr"
+}

scripts/workflows/provision-linux.sh

@@ -39,7 +39,7 @@ if [ "$E2E_TEST" = "tests-dfx/certificate.bash" ]; then
     sudo tar --directory /usr/local/bin --extract --file mitmproxy.tar.gz
     echo "mitmproxy version: $(mitmproxy --version)"
 fi
-if [ "$E2E_TEST" = "tests-dfx/identity_encryption.bash" ]; then
+if [ "$E2E_TEST" = "tests-dfx/identity_encryption.bash" ] || [ "$E2E_TEST" = "tests-dfx/identity.bash" ]; then
     sudo apt-get install --yes expect
 fi
 

src/dfx/src/lib/environment.rs

@@ -11,7 +11,7 @@ use candid::Principal;
 use fn_error_context::context;
 use ic_agent::{Agent, Identity};
 use semver::Version;
-use slog::{Logger, Record};
+use slog::{warn, Logger, Record};
 use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::fs::create_dir_all;
@@ -234,6 +234,10 @@ impl<'a> AgentEnvironment<'a> {
         let logger = backend.get_logger().clone();
         let mut identity_manager = IdentityManager::new(backend)?;
         let identity = identity_manager.instantiate_selected_identity()?;
+        if network_descriptor.is_ic && identity.insecure {
+            warn!(logger, "The {} identity is not stored securely. Do not use it to control a lot of cycles/ICP. Create a new identity with `dfx identity create` \
+                and use it in mainnet-facing commands with the `--identity` flag", identity.name());
+        }
         let url = network_descriptor.first_provider()?;
 
         Ok(AgentEnvironment {

src/dfx/src/lib/identity/identity_manager.rs

@@ -244,7 +244,7 @@ impl IdentityManager {
 
         let config = self.get_identity_config_or_default(name)?;
         let pem_path = self.get_identity_pem_path(name, &config);
-        let pem = pem_encryption::load_pem_file(&pem_path, Some(&config))?;
+        let (pem, _) = pem_encryption::load_pem_file(&pem_path, Some(&config))?;
         validate_pem_file(&pem)?;
         String::from_utf8(pem).map_err(|e| anyhow!("Could not translate pem file to text: {}", e))
     }

src/dfx/src/lib/identity/mod.rs

@@ -62,6 +62,10 @@ pub struct Identity {
     /// The name of this Identity.
     name: String,
 
+    /// Whether this identity is stored in unencrypted form.
+    /// False for identities that are not stored at all.
+    pub insecure: bool,
+
     /// Inner implementation of this identity.
     inner: Box<dyn ic_agent::Identity + Sync + Send>,
 
@@ -147,7 +151,7 @@ impl Identity {
                 disable_encryption,
             } => {
                 identity_config.encryption = create_encryption_config(disable_encryption)?;
-                let src_pem_content = pem_encryption::load_pem_file(&src_pem_file, None)?;
+                let (src_pem_content, _) = pem_encryption::load_pem_file(&src_pem_file, None)?;
                 identity_utils::validate_pem_file(&src_pem_content)?;
                 let dst_pem_file =
                     manager.get_identity_pem_path(&temp_identity_name, &identity_config);
@@ -202,6 +206,7 @@ impl Identity {
         Self {
             name: ANONYMOUS_IDENTITY_NAME.to_string(),
             inner: Box::new(AnonymousIdentity {}),
+            insecure: false,
             dir: PathBuf::new(),
         }
     }
@@ -210,6 +215,7 @@ impl Identity {
         manager: &IdentityManager,
         name: &str,
         pem_content: &[u8],
+        was_encrypted: bool,
     ) -> DfxResult<Self> {
         let inner = Box::new(BasicIdentity::from_pem(pem_content).map_err(|e| {
             DfxError::new(IdentityError::CannotReadIdentityFile(
@@ -222,13 +228,15 @@ impl Identity {
             name: name.to_string(),
             inner,
             dir: manager.get_identity_dir_path(name),
+            insecure: !was_encrypted,
         })
     }
 
     fn load_secp256k1_identity(
         manager: &IdentityManager,
         name: &str,
         pem_content: &[u8],
+        was_encrypted: bool,
     ) -> DfxResult<Self> {
         let inner = Box::new(Secp256k1Identity::from_pem(pem_content).map_err(|e| {
             DfxError::new(IdentityError::CannotReadIdentityFile(
@@ -241,6 +249,7 @@ impl Identity {
             name: name.to_string(),
             inner,
             dir: manager.get_identity_dir_path(name),
+            insecure: !was_encrypted,
         })
     }
 
@@ -262,6 +271,7 @@ impl Identity {
             name: name.to_string(),
             inner,
             dir: manager.get_identity_dir_path(name),
+            insecure: false,
         })
     }
 
@@ -280,11 +290,15 @@ impl Identity {
             Identity::load_hardware_identity(manager, name, hsm)
         } else {
             let pem_path = manager.load_identity_pem_path(name)?;
-            let pem_content = pem_encryption::load_pem_file(&pem_path, Some(&config))?;
-
-            Identity::load_secp256k1_identity(manager, name, &pem_content).or_else(|e| {
-                Identity::load_basic_identity(manager, name, &pem_content).map_err(|_| e)
-            })
+            let (pem_content, was_encrypted) =
+                pem_encryption::load_pem_file(&pem_path, Some(&config))?;
+
+            Identity::load_secp256k1_identity(manager, name, &pem_content, was_encrypted).or_else(
+                |e| {
+                    Identity::load_basic_identity(manager, name, &pem_content, was_encrypted)
+                        .map_err(|_| e)
+                },
+            )
         }
     }
 

src/dfx/src/lib/identity/pem_encryption.rs

@@ -13,14 +13,18 @@ use argon2::{password_hash::PasswordHasher, Argon2};
 use fn_error_context::context;
 
 /// Transparently handles all complexities regarding pem file encryption, including prompting the user for the password.
+/// Returns the pem and whether the original was encrypted.
 ///
 /// Try to only load the pem file once, as the user may be prompted for the password every single time you call this function.
 #[context("Failed to load pem file {}.", path.to_string_lossy())]
-pub fn load_pem_file(path: &Path, config: Option<&IdentityConfiguration>) -> DfxResult<Vec<u8>> {
+pub fn load_pem_file(
+    path: &Path,
+    config: Option<&IdentityConfiguration>,
+) -> DfxResult<(Vec<u8>, bool)> {
     let content = std::fs::read(path)
         .with_context(|| format!("Failed to read {}.", path.to_string_lossy()))?;
-    let content = maybe_decrypt_pem(content.as_slice(), config)?;
-    Ok(content)
+    let (content, was_encrypted) = maybe_decrypt_pem(content.as_slice(), config)?;
+    Ok((content, was_encrypted))
 }
 
 /// Transparently handles all complexities regarding pem file encryption, including prompting the user for the password.
@@ -89,22 +93,22 @@ fn maybe_encrypt_pem(
 ///
 /// If the pem file should not be encrypted, then the content is returned as is.
 ///
+/// Additionally returns whether or not it was necessary to decrypt the file.
+///
 /// `maybe_encrypt_pem` does the opposite.
 #[context("Failed to decrypt pem file.")]
 fn maybe_decrypt_pem(
     pem_content: &[u8],
     config: Option<&IdentityConfiguration>,
-) -> DfxResult<Vec<u8>> {
+) -> DfxResult<(Vec<u8>, bool)> {
     if let Some(decryption_config) = config.and_then(|c| c.encryption.as_ref()) {
         let password = password_prompt(DecryptingToUse)?;
-        let result = decrypt(pem_content, decryption_config, &password);
-        if result.is_ok() {
-            // print to stderr so that output redirection works for the identity export command
-            eprintln!("Decryption complete.");
-        };
-        result
+        let pem = decrypt(pem_content, decryption_config, &password)?;
+        // print to stderr so that output redirection works for the identity export command
+        eprintln!("Decryption complete.");
+        Ok((pem, true))
     } else {
-        Ok(Vec::from(pem_content))
+        Ok((Vec::from(pem_content), false))
     }
 }