From 06afa6f6da401412c03dc2fe29d2f19d76760e9f Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Wed, 20 Nov 2024 10:55:53 -0500 Subject: [PATCH] feat: error when using insecure identity on mainnet (#4005) * error when using insecure identity on mainnet no warning on non-mainnet (playground, local replica) * changelog * surpress the error in irrelevant tests * fix typo --- CHANGELOG.md | 8 ++++++++ e2e/tests-dfx/canister_url.bash | 6 ++++-- e2e/tests-dfx/fabricate_cycles.bash | 2 ++ e2e/tests-dfx/identity.bash | 12 ++++++------ e2e/tests-dfx/network.bash | 8 ++++---- e2e/tests-dfx/sign_send.bash | 3 ++- src/dfx/src/lib/environment.rs | 22 +++++++++++++++++----- 7 files changed, 43 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8740bce112..a3ed50fef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ # UNRELEASED +### feat: error when using insecure identity on mainnet + +This used to be a warning. A hard error can abort the command so that no insecure state will be on the mainnet. + +Users can surpress this error by setting `export DFX_WARNING=-mainnet_plaintext_identity`. + +The warning won't display when executing commands like `dfx deploy --playground`. + # 0.24.3 ### feat: Bitcoin support in PocketIC diff --git a/e2e/tests-dfx/canister_url.bash b/e2e/tests-dfx/canister_url.bash index 974c64627e..cd5f6f03d3 100644 --- a/e2e/tests-dfx/canister_url.bash +++ b/e2e/tests-dfx/canister_url.bash @@ -4,7 +4,9 @@ load ../utils/_ setup() { standard_setup - + # some of the tests run on mainnet with default plaintext identity + # so we need to set this to avoid the error + export DFX_WARNING=-mainnet_plaintext_identity dfx_new_assets hello } @@ -58,7 +60,7 @@ teardown() { echo "{}" > canister_ids.json jq '.hello_frontend.ic = "qsgof-4qaaa-aaaan-qekqq-cai"' canister_ids.json | sponge canister_ids.json frontend_id=$(dfx canister id hello_frontend --ic) - + assert_command dfx canister url hello_frontend --ic assert_match "https://${frontend_id}.icp0.io" diff --git a/e2e/tests-dfx/fabricate_cycles.bash b/e2e/tests-dfx/fabricate_cycles.bash index 82f5c61da5..6d811adb03 100644 --- a/e2e/tests-dfx/fabricate_cycles.bash +++ b/e2e/tests-dfx/fabricate_cycles.bash @@ -39,6 +39,8 @@ teardown() { @test "ledger fabricate-cycles fails on real IC" { install_asset greet + # without DFX_WARNING, the command would fail with different error (Failed to create AgentEnvironment...) + export DFX_WARNING=-mainnet_plaintext_identity assert_command_fail dfx ledger fabricate-cycles --all --network ic assert_match "Cannot run this on the real IC." assert_command_fail dfx ledger fabricate-cycles --all --ic diff --git a/e2e/tests-dfx/identity.bash b/e2e/tests-dfx/identity.bash index 291b720906..a6f8568e3a 100644 --- a/e2e/tests-dfx/identity.bash +++ b/e2e/tests-dfx/identity.bash @@ -186,15 +186,15 @@ teardown() { assert_eq '(blob "hello")' "$stdout" } -@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" +@test "using an unencrypted identity on mainnet provokes a hard error which can be suppressed" { + assert_command_fail dfx ledger balance --network ic + assert_match "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 --storage-mode plaintext - assert_command dfx ledger balance --network ic --identity bob - assert_match "WARN: The bob identity is not stored securely." "$stderr" - + assert_command_fail dfx ledger balance --network ic --identity bob + assert_match "The bob identity is not stored securely." "$stderr" + # can suppress the error export DFX_WARNING=-mainnet_plaintext_identity assert_command dfx ledger balance --network ic --identity bob assert_not_contains "not stored securely" "$stderr" diff --git a/e2e/tests-dfx/network.bash b/e2e/tests-dfx/network.bash index dcb12ac8da..857c3dfedc 100644 --- a/e2e/tests-dfx/network.bash +++ b/e2e/tests-dfx/network.bash @@ -99,13 +99,13 @@ teardown() { assert_command_fail dfx diagnose --network ic assert_contains "The test_id identity is not stored securely." - assert_contains "use it in mainnet-facing commands" - assert_contains "No wallet found; nothing to do" + assert_contains "in mainnet-facing commands" + assert_contains "you can suppress this warning" assert_command_fail dfx diagnose --ic assert_contains "The test_id identity is not stored securely." - assert_contains "use it in mainnet-facing commands" - assert_contains "No wallet found; nothing to do" + assert_contains "in mainnet-facing commands" + assert_contains "you can suppress this warning" assert_command dfx diagnose assert_not_contains "identity is not stored securely" diff --git a/e2e/tests-dfx/sign_send.bash b/e2e/tests-dfx/sign_send.bash index b266510a1e..5a828eb052 100644 --- a/e2e/tests-dfx/sign_send.bash +++ b/e2e/tests-dfx/sign_send.bash @@ -44,7 +44,8 @@ teardown() { cd "$E2E_TEMP_DIR" mkdir not-a-project-dir cd not-a-project-dir - + # suppress the error + export DFX_WARNING=-mainnet_plaintext_identity assert_command dfx canister sign --query rwlgt-iiaaa-aaaaa-aaaaa-cai read --network ic assert_match "Query message generated at \[message.json\]" } diff --git a/src/dfx/src/lib/environment.rs b/src/dfx/src/lib/environment.rs index 5b9ecc4881..6962f5d975 100644 --- a/src/dfx/src/lib/environment.rs +++ b/src/dfx/src/lib/environment.rs @@ -3,12 +3,12 @@ use crate::config::dfx_version; use crate::lib::error::DfxResult; use crate::lib::progress_bar::ProgressBar; use crate::lib::warning::{is_warning_disabled, DfxWarning::MainnetPlainTextIdentity}; -use anyhow::anyhow; +use anyhow::{anyhow, bail}; use candid::Principal; use dfx_core::config::cache::Cache; use dfx_core::config::model::canister_id_store::CanisterIdStore; use dfx_core::config::model::dfinity::{Config, NetworksConfig}; -use dfx_core::config::model::network_descriptor::NetworkDescriptor; +use dfx_core::config::model::network_descriptor::{NetworkDescriptor, NetworkTypeDescriptor}; use dfx_core::error::canister_id_store::CanisterIdStoreError; use dfx_core::error::identity::NewIdentityManagerError; use dfx_core::error::load_dfx_config::LoadDfxConfigError; @@ -17,7 +17,7 @@ use dfx_core::identity::identity_manager::{IdentityManager, InitializeIdentity}; use fn_error_context::context; use ic_agent::{Agent, Identity}; use semver::Version; -use slog::{warn, Logger, Record}; +use slog::{Logger, Record}; use std::borrow::Cow; use std::cell::RefCell; use std::path::PathBuf; @@ -288,11 +288,23 @@ impl<'a> AgentEnvironment<'a> { identity_manager.instantiate_selected_identity(&logger)? }; if network_descriptor.is_ic + && !matches!( + network_descriptor.r#type, + NetworkTypeDescriptor::Playground { .. } + ) && identity.insecure && !is_warning_disabled(MainnetPlainTextIdentity) { - 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 new` \ - and use it in mainnet-facing commands with the `--identity` flag", identity.name()); + bail!( + "The {} identity is not stored securely. Do not use it to control a lot of cycles/ICP. +- For enhanced security, create a new identity using the command: + dfx identity new + Then, specify the new identity in mainnet-facing commands with the `--identity` flag. +- If you understand the risks and still wish to use the insecure plaintext identity, you can suppress this warning by running: + export DFX_WARNING=-mainnet_plaintext_identity + After setting this environment variable, re-run the command.", + identity.name() + ); } let url = network_descriptor.first_provider()?; let effective_canister_id = if let Some(d) = &network_descriptor.local_server_descriptor {