Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix double-encoding-to-vec on crypto signing responses #438

Merged
merged 3 commits into from
Apr 12, 2023
Merged

fix double-encoding-to-vec on crypto signing responses #438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
477bc5f
fix double-encoding-to-vec on crypto signing responses
jeffgrunewald Apr 11, 2023
1964042
remove helium network key filter/setting
jeffgrunewald Apr 11, 2023
968481d
use type guard to prevent double-encoding again
jeffgrunewald Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 3 additions & 20 deletions iot_config/src/admin_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -25,7 +25,6 @@ pub struct AdminService {
pool: Pool<Postgres>,
region_map: RegionMapReader,
region_updater: watch::Sender<RegionMap>,
required_network: Network,
signing_key: Keypair,
}

Expand All @@ -44,7 +43,6 @@ impl AdminService {
pool,
region_map,
region_updater,
required_network: settings.network,
signing_key: settings.signing_keypair()?,
})
}
Expand Down Expand Up @@ -73,23 +71,9 @@ impl AdminService {
Ok(())
}

fn verify_network(&self, public_key: PublicKey) -> Result<PublicKey, Status> {
if self.required_network == public_key.network {
Ok(public_key)
} else {
Err(Status::invalid_argument(format!(
"invalid network: {}",
public_key.network
)))
}
}

fn sign_response<R>(&self, response: &R) -> Result<Vec<u8>, Status>
where
R: Message,
{
fn sign_response(&self, response: &[u8]) -> Result<Vec<u8>, Status> {
self.signing_key
.sign(&response.encode_to_vec())
.sign(response)
.map_err(|_| Status::internal("response signing error"))
}
}
Expand All @@ -104,7 +88,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)
Expand Down
11 changes: 4 additions & 7 deletions iot_config/src/gateway_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,9 @@ impl GatewayService {
})
}

fn sign_response<R>(&self, response: &R) -> Result<Vec<u8>, Status>
where
R: Message,
{
fn sign_response(&self, response: &[u8]) -> Result<Vec<u8>, Status> {
self.signing_key
.sign(&response.encode_to_vec())
.sign(response)
.map_err(|_| Status::internal("response signing error"))
}

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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(),
Expand Down
56 changes: 18 additions & 38 deletions iot_config/src/org_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,7 +23,6 @@ use tonic::{Request, Response, Status};
pub struct OrgService {
auth_cache: AuthCache,
pool: Pool<Postgres>,
required_network: Network,
route_update_tx: Sender<RouteStreamResV1>,
signing_key: Keypair,
}
Expand All @@ -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<PublicKey, Status> {
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<R>(
&self,
signer: &PublicKey,
Expand All @@ -79,12 +66,9 @@ impl OrgService {
Ok(())
}

fn sign_response<R>(&self, response: &R) -> Result<Vec<u8>, Status>
where
R: Message,
{
fn sign_response(&self, response: &[u8]) -> Result<Vec<u8>, Status> {
self.signing_key
.sign(&response.encode_to_vec())
.sign(response)
.map_err(|_| Status::internal("response signing error"))
}
}
Expand All @@ -105,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))
}
Expand Down Expand Up @@ -143,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))
}
Expand All @@ -164,12 +148,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::<Result<Vec<PublicKey>, Status>>()?;

Expand Down Expand Up @@ -215,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))
}
Expand All @@ -236,11 +218,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::<Result<Vec<PublicKey>, Status>>()?;

Expand Down Expand Up @@ -282,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))
}
Expand Down Expand Up @@ -329,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,
Expand All @@ -347,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))
}
Expand Down Expand Up @@ -394,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,
Expand All @@ -412,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))
}
Expand Down
21 changes: 9 additions & 12 deletions iot_config/src/route_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,9 @@ impl RouteService {
}
}

fn sign_response<R>(&self, response: &R) -> Result<Vec<u8>, Status>
where
R: Message,
{
fn sign_response(&self, response: &[u8]) -> Result<Vec<u8>, Status> {
self.signing_key
.sign(&response.encode_to_vec())
.sign(response)
.map_err(|_| Status::internal("response signing error"))
}

Expand Down Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down
7 changes: 2 additions & 5 deletions iot_config/src/session_key_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,9 @@ impl SessionKeyFilterService {
}
}

fn sign_response<R>(&self, response: &R) -> Result<Vec<u8>, Status>
where
R: Message,
{
fn sign_response(&self, response: &[u8]) -> Result<Vec<u8>, Status> {
self.signing_key
.sign(&response.encode_to_vec())
.sign(response)
.map_err(|_| Status::internal("response signing error"))
}

Expand Down
3 changes: 0 additions & 3 deletions iot_config/src/settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use config::{Config, Environment, File};
use helium_crypto::Network;
use serde::Deserialize;
use std::{
net::{AddrParseError, SocketAddr},
Expand All @@ -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
Expand Down
Loading