From 1b5c297dedade49bae7192b395d0c062772b4aca Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 18 Dec 2023 11:02:47 -0700 Subject: [PATCH 1/3] formatting --- src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index aa7da61..b2630ec 100644 --- a/src/client.rs +++ b/src/client.rs @@ -650,7 +650,7 @@ impl SkfClient { devaddr: skf.devaddr.into(), session_key: skf.session_key.to_owned(), action: ActionV1::Remove.into(), - max_copies: 0 + max_copies: 0, }) .collect(), timestamp: current_timestamp()?, From d3a942b91807c09c70a18f7437bdd1f3a18a0bab Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 18 Dec 2023 11:02:59 -0700 Subject: [PATCH 2/3] remove defaults for creating routes Default NetID has led to quite a bit of confusion. Default multi-buy could lead to same confusion if someone doesn't know where the value is coming from. Better to be explicit. --- src/cmds/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmds/mod.rs b/src/cmds/mod.rs index ec7d910..252ebc2 100644 --- a/src/cmds/mod.rs +++ b/src/cmds/mod.rs @@ -191,11 +191,11 @@ pub struct GetRoute { #[derive(Debug, Args)] pub struct NewRoute { - #[arg(long, env = ENV_NET_ID, default_value = "000024")] + #[arg(long, env = ENV_NET_ID)] pub net_id: HexNetID, #[arg(long, env = ENV_OUI)] pub oui: Oui, - #[arg(long, env = ENV_MAX_COPIES, default_value = "5")] + #[arg(long, env = ENV_MAX_COPIES)] pub max_copies: u32, #[arg(from_global)] From 99ca268b97975902f8006c4dace21315876627d7 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 18 Dec 2023 11:04:09 -0700 Subject: [PATCH 3/3] add section to `env info` Make a fake request, sign it, then try and verify the request. If you open a key in some editors, there's a chance the key becomes corrupted. The public key can still be read out many times, but it cannot sign a valid message. --- src/client.rs | 19 +++++++++++++++++ src/cmds/env.rs | 55 ++++++++++++++++++++++++++++++++++--------------- src/cmds/mod.rs | 2 +- tests/common.rs | 2 +- 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/client.rs b/src/client.rs index b2630ec..a996e7a 100644 --- a/src/client.rs +++ b/src/client.rs @@ -757,6 +757,24 @@ fn current_timestamp() -> Result { Ok(SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() as u64) } +pub fn verify_keypair(keypair: &Keypair) -> Result { + let mut req = RouteListReqV1 { + oui: 0, + timestamp: current_timestamp()?, + signer: keypair.public_key().to_vec(), + signature: vec![], + }; + req.signature = req + .sign(keypair) + .map_err(|e| anyhow!("failed to sign: {e:?}"))?; + + let _verified = req + .verify(&keypair.public_key()) + .map_err(|e| anyhow!("keypair corrupted: {e:?}"))?; + + Ok(true) +} + pub trait MsgSign: Message + std::clone::Clone { fn sign(&self, keypair: &Keypair) -> Result> where @@ -819,6 +837,7 @@ macro_rules! impl_verify { }; } +impl_verify!(RouteListReqV1, signature); impl_verify!(OrgListResV1, signature); impl_verify!(OrgResV1, signature); impl_verify!(OrgEnableResV1, signature); diff --git a/src/cmds/env.rs b/src/cmds/env.rs index 1e90152..f0884db 100644 --- a/src/cmds/env.rs +++ b/src/cmds/env.rs @@ -4,7 +4,7 @@ use std::{env, fs, path::PathBuf}; use super::{ EnvInfo, GenerateKeypair, ENV_CONFIG_HOST, ENV_KEYPAIR_BIN, ENV_MAX_COPIES, ENV_NET_ID, ENV_OUI, }; -use crate::{hex_field, Msg, Oui, PrettyJson, Result}; +use crate::{client::verify_keypair, hex_field, Msg, Oui, PrettyJson, Result}; use anyhow::Context; use dialoguer::Input; use helium_crypto::Keypair; @@ -65,10 +65,10 @@ pub async fn env_init() -> Result { pub fn env_info(args: EnvInfo) -> Result { let env_keypair = env::var(ENV_KEYPAIR_BIN).ok().map(|i| i.into()); - let (env_keypair_location, env_public_key, env_key_type) = - get_public_key_from_path(env_keypair); - let (arg_keypair_location, arg_public_key, arg_key_type) = - get_public_key_from_path(args.keypair); + let (env_keypair_location, env_public_key, env_key_type, env_verifies) = + get_public_key_from_path(&env_keypair); + let (arg_keypair_location, arg_public_key, arg_key_type, arg_verifies) = + get_public_key_from_path(&args.keypair); let output = json!({ "environment": { @@ -79,6 +79,7 @@ pub fn env_info(args: EnvInfo) -> Result { ENV_KEYPAIR_BIN: env_keypair_location, "public_key_from_keypair": env_public_key, "key_type_from_keypair": env_key_type, + "keypair_verifies_own_sig": env_verifies, }, "arguments": { "config_host": args.config_host, @@ -87,7 +88,8 @@ pub fn env_info(args: EnvInfo) -> Result { "max_copies": args.max_copies, "keypair": arg_keypair_location, "public_key_from_keypair": arg_public_key, - "key_type_from_keypair": arg_key_type + "key_type_from_keypair": arg_key_type, + "keypair_verifies_own_sig": arg_verifies, } }); Msg::ok(output.pretty_json()?) @@ -123,24 +125,38 @@ pub fn generate_keypair(args: GenerateKeypair) -> Result { )) } -pub fn get_public_key_from_path(path: Option) -> (String, String, String) { +pub fn get_public_key_from_path(path: &Option) -> (String, String, String, String) { match path { None => ( "unset".to_string(), "unset".to_string(), "unset".to_string(), + "unset".to_string(), ), Some(path) => { let display_path = path.as_path().display().to_string(); match fs::read(path).with_context(|| format!("path does not exist: {display_path}")) { - Err(e) => (e.to_string(), "".to_string(), "".to_string()), + Err(e) => ( + e.to_string(), + "".to_string(), + "".to_string(), + "".to_string(), + ), Ok(data) => match Keypair::try_from(&data[..]) { - Err(e) => (display_path, e.to_string(), "".to_string()), - Ok(keypair) => ( - display_path, - keypair.public_key().to_string(), - keypair.key_tag().key_type.to_string(), - ), + Err(e) => (display_path, e.to_string(), "".to_string(), "".to_string()), + Ok(keypair) => { + let verified = match verify_keypair(&keypair) { + Err(e) => format!("failed to verify: {e:?}"), + Ok(_) => "verified".to_string(), + }; + + ( + display_path, + keypair.public_key().to_string(), + keypair.key_tag().key_type.to_string(), + verified, + ) + } }, } } @@ -226,10 +242,12 @@ mod tests { #[test] fn get_keypair_does_not_exist() { - let (location, pubkey, key_type) = get_public_key_from_path(Some("./nowhere.bin".into())); + let (location, pubkey, key_type, verified) = + get_public_key_from_path(&Some("./nowhere.bin".into())); assert_eq!(location, "path does not exist: ./nowhere.bin"); assert!(pubkey.is_empty()); assert!(key_type.is_empty()); + assert!(verified.is_empty()); } #[test] @@ -240,17 +258,20 @@ mod tests { fs::write(arg_keypair.clone(), "invalid key").unwrap(); // ======= - let (location, pubkey, key_type) = get_public_key_from_path(Some(arg_keypair.clone())); + let (location, pubkey, key_type, verified) = + get_public_key_from_path(&Some(arg_keypair.clone())); assert_eq!(location, arg_keypair.display().to_string()); assert_eq!(pubkey, "decode error"); assert_eq!(key_type, ""); + assert_eq!(verified, ""); } #[test] fn get_keypair_not_provided() { - let (location, pubkey, key_type) = get_public_key_from_path(None); + let (location, pubkey, key_type, verified) = get_public_key_from_path(&None); assert_eq!(location, "unset"); assert_eq!(pubkey, "unset"); assert_eq!(key_type, "unset"); + assert_eq!(verified, "unset"); } } diff --git a/src/cmds/mod.rs b/src/cmds/mod.rs index 252ebc2..2c11302 100644 --- a/src/cmds/mod.rs +++ b/src/cmds/mod.rs @@ -952,7 +952,7 @@ pub trait PathBufKeypair { impl PathBufKeypair for PathBuf { fn to_keypair(&self) -> Result { - let data = std::fs::read(self).context("reading keypair file")?; + let data = std::fs::read(&self).context("reading keypair file")?; Ok(helium_crypto::Keypair::try_from(&data[..])?) } } diff --git a/tests/common.rs b/tests/common.rs index aa01b3a..3dcc301 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -33,7 +33,7 @@ pub fn generate_keypair(path: PathBuf) -> Result { commit: true, })?; info!("generate_keypair: {out}"); - let (_, public_key, _) = cmds::env::get_public_key_from_path(Some(path)); + let (_, public_key, _, _) = cmds::env::get_public_key_from_path(&Some(path)); let public_key = PublicKey::from_str(&public_key)?; Ok(public_key) }