From 477bc5f5931519d85fcb4d9face52651e7d71933 Mon Sep 17 00:00:00 2001 From: jeffgrunewald Date: Tue, 11 Apr 2023 16:04:31 -0400 Subject: [PATCH 1/3] fix double-encoding-to-vec on crypto signing responses --- iot_config/src/admin_service.rs | 8 ++++---- iot_config/src/gateway_service.rs | 2 +- iot_config/src/session_key_service.rs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/iot_config/src/admin_service.rs b/iot_config/src/admin_service.rs index 979c25e3d..1fb984a71 100644 --- a/iot_config/src/admin_service.rs +++ b/iot_config/src/admin_service.rs @@ -136,7 +136,7 @@ impl iot_config::Admin for AdminService { signer, signature: vec![], }; - resp.signature = self.sign_response(&resp.encode_to_vec())?; + resp.signature = self.sign_response(&resp)?; Ok(Response::new(resp)) } @@ -173,7 +173,7 @@ impl iot_config::Admin for AdminService { signer, signature: vec![], }; - resp.signature = self.sign_response(&resp.encode_to_vec())?; + resp.signature = self.sign_response(&resp)?; Ok(Response::new(resp)) } @@ -236,7 +236,7 @@ impl iot_config::Admin for AdminService { signer, signature: vec![], }; - resp.signature = self.sign_response(&resp.encode_to_vec())?; + resp.signature = self.sign_response(&resp)?; Ok(Response::new(resp)) } @@ -263,7 +263,7 @@ impl iot_config::Admin for AdminService { signature: vec![], timestamp, }; - resp.signature = self.sign_response(&resp.encode_to_vec())?; + resp.signature = self.sign_response(&resp)?; tracing::debug!(region = region.to_string(), "returning region params"); Ok(Response::new(resp)) } diff --git a/iot_config/src/gateway_service.rs b/iot_config/src/gateway_service.rs index fb5c40211..55726ad2a 100644 --- a/iot_config/src/gateway_service.rs +++ b/iot_config/src/gateway_service.rs @@ -212,7 +212,7 @@ impl iot_config::Gateway for GatewayService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp.encode_to_vec())?; + resp.signature = self.sign_response(&resp)?; Ok(Response::new(resp)) } diff --git a/iot_config/src/session_key_service.rs b/iot_config/src/session_key_service.rs index 41bdb587a..72ffe92dc 100644 --- a/iot_config/src/session_key_service.rs +++ b/iot_config/src/session_key_service.rs @@ -282,7 +282,7 @@ impl iot_config::SessionKeyFilter for SessionKeyFilterService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp.encode_to_vec())?; + resp.signature = self.sign_response(&resp)?; Ok(Response::new(resp)) } From 1964042aab82cf0a2550dbfcd1e7faabb307e0d1 Mon Sep 17 00:00:00 2001 From: jeffgrunewald Date: Tue, 11 Apr 2023 16:25:14 -0400 Subject: [PATCH 2/3] remove helium network key filter/setting --- iot_config/src/admin_service.rs | 16 +-------------- iot_config/src/org_service.rs | 33 ++++++++---------------------- iot_config/src/settings.rs | 3 --- mobile_config/src/admin_service.rs | 18 ++-------------- mobile_config/src/settings.rs | 3 --- 5 files changed, 11 insertions(+), 62 deletions(-) diff --git a/iot_config/src/admin_service.rs b/iot_config/src/admin_service.rs index 1fb984a71..c29d8144a 100644 --- a/iot_config/src/admin_service.rs +++ b/iot_config/src/admin_service.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Result}; use chrono::Utc; use file_store::traits::{MsgVerify, TimestampEncode}; use futures::future::TryFutureExt; -use helium_crypto::{Keypair, Network, PublicKey, PublicKeyBinary, Sign}; +use helium_crypto::{Keypair, PublicKey, PublicKeyBinary, Sign}; use helium_proto::{ services::iot_config::{ self, AdminAddKeyReqV1, AdminKeyResV1, AdminLoadRegionReqV1, AdminLoadRegionResV1, @@ -25,7 +25,6 @@ pub struct AdminService { pool: Pool, region_map: RegionMapReader, region_updater: watch::Sender, - required_network: Network, signing_key: Keypair, } @@ -44,7 +43,6 @@ impl AdminService { pool, region_map, region_updater, - required_network: settings.network, signing_key: settings.signing_keypair()?, }) } @@ -73,17 +71,6 @@ impl AdminService { Ok(()) } - fn verify_network(&self, public_key: PublicKey) -> Result { - if self.required_network == public_key.network { - Ok(public_key) - } else { - Err(Status::invalid_argument(format!( - "invalid network: {}", - public_key.network - ))) - } - } - fn sign_response(&self, response: &R) -> Result, Status> where R: Message, @@ -104,7 +91,6 @@ impl iot_config::Admin for AdminService { let key_type = request.key_type().into(); let pubkey = verify_public_key(request.pubkey.as_ref()) - .and_then(|pubkey| self.verify_network(pubkey)) .map_err(|_| Status::invalid_argument("invalid pubkey supplied"))?; admin::insert_key(request.pubkey.clone().into(), key_type, &self.pool) diff --git a/iot_config/src/org_service.rs b/iot_config/src/org_service.rs index bbc7e305a..7c3431f21 100644 --- a/iot_config/src/org_service.rs +++ b/iot_config/src/org_service.rs @@ -7,7 +7,7 @@ use crate::{ use anyhow::Result; use chrono::Utc; use file_store::traits::{MsgVerify, TimestampEncode}; -use helium_crypto::{Keypair, Network, PublicKey, Sign}; +use helium_crypto::{Keypair, PublicKey, Sign}; use helium_proto::{ services::iot_config::{ self, route_stream_res_v1, ActionV1, OrgCreateHeliumReqV1, OrgCreateRoamerReqV1, @@ -23,7 +23,6 @@ use tonic::{Request, Response, Status}; pub struct OrgService { auth_cache: AuthCache, pool: Pool, - required_network: Network, route_update_tx: Sender, signing_key: Keypair, } @@ -38,23 +37,11 @@ impl OrgService { Ok(Self { auth_cache, pool, - required_network: settings.network, route_update_tx, signing_key: settings.signing_keypair()?, }) } - fn verify_network(&self, public_key: PublicKey) -> Result { - if self.required_network == public_key.network { - Ok(public_key) - } else { - Err(Status::invalid_argument(format!( - "invalid network: {}", - public_key.network - ))) - } - } - fn verify_admin_request_signature( &self, signer: &PublicKey, @@ -164,12 +151,10 @@ impl iot_config::Org for OrgService { _ = verify_keys .iter() .map(|key| { - verify_public_key(key) - .and_then(|pub_key| self.verify_network(pub_key)) - .map_err(|err| { - tracing::error!("failed pubkey validation: {err}"); - Status::invalid_argument(format!("failed pubkey validation: {err}")) - }) + verify_public_key(key).map_err(|err| { + tracing::error!("failed pubkey validation: {err}"); + Status::invalid_argument(format!("failed pubkey validation: {err}")) + }) }) .collect::, Status>>()?; @@ -236,11 +221,9 @@ impl iot_config::Org for OrgService { _ = verify_keys .iter() .map(|key| { - verify_public_key(key) - .and_then(|pub_key| self.verify_network(pub_key)) - .map_err(|err| { - Status::invalid_argument(format!("failed pubkey validation: {err}")) - }) + verify_public_key(key).map_err(|err| { + Status::invalid_argument(format!("failed pubkey validation: {err}")) + }) }) .collect::, Status>>()?; diff --git a/iot_config/src/settings.rs b/iot_config/src/settings.rs index 8e4d825c1..76825e3ca 100644 --- a/iot_config/src/settings.rs +++ b/iot_config/src/settings.rs @@ -1,5 +1,4 @@ use config::{Config, Environment, File}; -use helium_crypto::Network; use serde::Deserialize; use std::{ net::{AddrParseError, SocketAddr}, @@ -20,8 +19,6 @@ pub struct Settings { pub keypair: String, /// B58 encoded public key of the admin keypair pub admin: String, - /// Network required in all public keys: mainnet | testnet - pub network: Network, pub database: db_store::Settings, /// Settings passed to the db_store crate for connecting to /// the database for Solana on-chain data diff --git a/mobile_config/src/admin_service.rs b/mobile_config/src/admin_service.rs index 1ebb32e61..f756eb546 100644 --- a/mobile_config/src/admin_service.rs +++ b/mobile_config/src/admin_service.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Result}; use chrono::Utc; use file_store::traits::{MsgVerify, TimestampEncode}; use futures::future::TryFutureExt; -use helium_crypto::{Keypair, Network, PublicKey, PublicKeyBinary, Sign}; +use helium_crypto::{Keypair, PublicKey, PublicKeyBinary, Sign}; use helium_proto::{ services::mobile_config::{self, AdminAddKeyReqV1, AdminKeyResV1, AdminRemoveKeyReqV1}, Message, @@ -20,7 +20,6 @@ pub struct AdminService { key_cache: KeyCache, key_cache_updater: watch::Sender, pool: Pool, - required_network: Network, signing_key: Keypair, } @@ -35,7 +34,6 @@ impl AdminService { key_cache, key_cache_updater, pool, - required_network: settings.network, signing_key: settings.signing_keypair()?, }) } @@ -54,17 +52,6 @@ impl AdminService { Ok(()) } - fn verify_network(&self, public_key: PublicKey) -> Result { - if self.required_network == public_key.network { - Ok(public_key) - } else { - Err(Status::invalid_argument(format!( - "invalid network: {}", - public_key.network - ))) - } - } - fn sign_response(&self, response: &R) -> Result, Status> where R: Message, @@ -84,8 +71,7 @@ impl mobile_config::Admin for AdminService { self.verify_admin_request_signature(&signer, &request)?; let key_type = request.key_type().into(); - let pubkey = verify_public_key(request.pubkey.as_ref()) - .and_then(|pubkey| self.verify_network(pubkey))?; + let pubkey = verify_public_key(request.pubkey.as_ref())?; key_cache::db::insert_key(request.pubkey.clone().into(), key_type, &self.pool) .and_then(|_| async move { diff --git a/mobile_config/src/settings.rs b/mobile_config/src/settings.rs index 763d10c83..a7d3f7a0b 100644 --- a/mobile_config/src/settings.rs +++ b/mobile_config/src/settings.rs @@ -1,5 +1,4 @@ use config::{Config, Environment, File}; -use helium_crypto::Network; use serde::Deserialize; use std::{ net::{AddrParseError, SocketAddr}, @@ -20,8 +19,6 @@ pub struct Settings { pub signing_keypair: String, /// B58 encoded public key of the default admin keypair pub admin_pubkey: String, - /// Network required in all public keys: mainnet | testnet - pub network: Network, /// Settings passed to the db_store crate for connecting to /// the config service's own persistence store pub database: db_store::Settings, From 968481db1c2e3f4e49a286e355bfe431a4e6667e Mon Sep 17 00:00:00 2001 From: jeffgrunewald Date: Wed, 12 Apr 2023 10:17:08 -0400 Subject: [PATCH 3/3] use type guard to prevent double-encoding again --- iot_config/src/admin_service.rs | 15 ++++++--------- iot_config/src/gateway_service.rs | 13 +++++-------- iot_config/src/org_service.rs | 23 ++++++++++------------- iot_config/src/route_service.rs | 21 +++++++++------------ iot_config/src/session_key_service.rs | 9 +++------ mobile_config/src/admin_service.rs | 11 ++++------- mobile_config/src/gateway_service.rs | 9 +++------ mobile_config/src/router_service.rs | 11 ++++------- 8 files changed, 44 insertions(+), 68 deletions(-) diff --git a/iot_config/src/admin_service.rs b/iot_config/src/admin_service.rs index c29d8144a..5c9231469 100644 --- a/iot_config/src/admin_service.rs +++ b/iot_config/src/admin_service.rs @@ -71,12 +71,9 @@ impl AdminService { Ok(()) } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } } @@ -122,7 +119,7 @@ impl iot_config::Admin for AdminService { signer, signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -159,7 +156,7 @@ impl iot_config::Admin for AdminService { signer, signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -222,7 +219,7 @@ impl iot_config::Admin for AdminService { signer, signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -249,7 +246,7 @@ impl iot_config::Admin for AdminService { signature: vec![], timestamp, }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; tracing::debug!(region = region.to_string(), "returning region params"); Ok(Response::new(resp)) } diff --git a/iot_config/src/gateway_service.rs b/iot_config/src/gateway_service.rs index 55726ad2a..56adacb4e 100644 --- a/iot_config/src/gateway_service.rs +++ b/iot_config/src/gateway_service.rs @@ -44,12 +44,9 @@ impl GatewayService { }) } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } @@ -105,7 +102,7 @@ impl iot_config::Gateway for GatewayService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -182,7 +179,7 @@ impl iot_config::Gateway for GatewayService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; tracing::debug!( pubkey = address.to_string(), region = region.to_string(), @@ -212,7 +209,7 @@ impl iot_config::Gateway for GatewayService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } diff --git a/iot_config/src/org_service.rs b/iot_config/src/org_service.rs index 7c3431f21..45cb59219 100644 --- a/iot_config/src/org_service.rs +++ b/iot_config/src/org_service.rs @@ -66,12 +66,9 @@ impl OrgService { Ok(()) } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } } @@ -92,7 +89,7 @@ impl iot_config::Org for OrgService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -130,7 +127,7 @@ impl iot_config::Org for OrgService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -200,7 +197,7 @@ impl iot_config::Org for OrgService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -265,7 +262,7 @@ impl iot_config::Org for OrgService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -312,7 +309,7 @@ impl iot_config::Org for OrgService { signer: signer.clone(), signature: vec![], }; - update.signature = self.sign_response(&update)?; + update.signature = self.sign_response(&update.encode_to_vec())?; if self.route_update_tx.send(update).is_err() { tracing::info!( route_id = route_id, @@ -330,7 +327,7 @@ impl iot_config::Org for OrgService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -377,7 +374,7 @@ impl iot_config::Org for OrgService { signer: signer.clone(), signature: vec![], }; - update.signature = self.sign_response(&update)?; + update.signature = self.sign_response(&update.encode_to_vec())?; if self.route_update_tx.send(update).is_err() { tracing::info!( route_id = route_id, @@ -395,7 +392,7 @@ impl iot_config::Org for OrgService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } diff --git a/iot_config/src/route_service.rs b/iot_config/src/route_service.rs index 9007007cd..19f6e4583 100644 --- a/iot_config/src/route_service.rs +++ b/iot_config/src/route_service.rs @@ -119,12 +119,9 @@ impl RouteService { } } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } @@ -163,7 +160,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -190,7 +187,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -238,7 +235,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -280,7 +277,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -316,7 +313,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -502,7 +499,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -646,7 +643,7 @@ impl iot_config::Route for RouteService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } diff --git a/iot_config/src/session_key_service.rs b/iot_config/src/session_key_service.rs index 72ffe92dc..fb5764179 100644 --- a/iot_config/src/session_key_service.rs +++ b/iot_config/src/session_key_service.rs @@ -108,12 +108,9 @@ impl SessionKeyFilterService { } } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } @@ -282,7 +279,7 @@ impl iot_config::SessionKeyFilter for SessionKeyFilterService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } diff --git a/mobile_config/src/admin_service.rs b/mobile_config/src/admin_service.rs index f756eb546..78c406df7 100644 --- a/mobile_config/src/admin_service.rs +++ b/mobile_config/src/admin_service.rs @@ -52,12 +52,9 @@ impl AdminService { Ok(()) } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } } @@ -100,7 +97,7 @@ impl mobile_config::Admin for AdminService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } @@ -134,7 +131,7 @@ impl mobile_config::Admin for AdminService { signer: self.signing_key.public_key().into(), signature: vec![], }; - resp.signature = self.sign_response(&resp)?; + resp.signature = self.sign_response(&resp.encode_to_vec())?; Ok(Response::new(resp)) } } diff --git a/mobile_config/src/gateway_service.rs b/mobile_config/src/gateway_service.rs index 1bc69a2b5..003506575 100644 --- a/mobile_config/src/gateway_service.rs +++ b/mobile_config/src/gateway_service.rs @@ -46,12 +46,9 @@ impl GatewayService { Err(Status::permission_denied("unauthorized request signature")) } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } } @@ -82,7 +79,7 @@ impl mobile_config::Gateway for GatewayService { signer: self.signing_key.public_key().into(), signature: vec![], }; - res.signature = self.sign_response(&res)?; + res.signature = self.sign_response(&res.encode_to_vec())?; Ok(Response::new(res)) }, ) diff --git a/mobile_config/src/router_service.rs b/mobile_config/src/router_service.rs index b577adfe9..c0c0bcc88 100644 --- a/mobile_config/src/router_service.rs +++ b/mobile_config/src/router_service.rs @@ -37,12 +37,9 @@ impl RouterService { Err(Status::permission_denied("unauthorized request signature")) } - fn sign_response(&self, response: &R) -> Result, Status> - where - R: Message, - { + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key - .sign(&response.encode_to_vec()) + .sign(response) .map_err(|_| Status::internal("response signing error")) } @@ -81,7 +78,7 @@ impl mobile_config::Router for RouterService { signer: self.signing_key.public_key().into(), signature: vec![], }; - response.signature = self.sign_response(&response)?; + response.signature = self.sign_response(&response.encode_to_vec())?; Ok(Response::new(response)) } @@ -106,7 +103,7 @@ impl mobile_config::Router for RouterService { signer: self.signing_key.public_key().into(), signature: vec![], }; - response.signature = self.sign_response(&response)?; + response.signature = self.sign_response(&response.encode_to_vec())?; Ok(Response::new(response)) } }