From fe059f9d435ada8ca027b7ff527af52de6c0715a Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 22 Apr 2024 11:52:05 -0700 Subject: [PATCH] Gateway service info perms (#792) * add constructor to make KeyCache without settings file * Add test for who is authorized to request gateway info I'm not sure where the mobile_hotspot_infos table exists, but I think this test does well enough keying off expected grpc error statuses. * Add verify method that allows gateway to request info about self * use KeyTag::default in KeyPair generation for tests * tighten gw info perm check to require proto request The method before was named specifically for the info request, but the function signature was much looser than the name implied. Passing the request reduces the chance of messing up argument order, or misunderstanding the arguments. If at some point more methods require the ability for self authorization, hopefully a better abstraction will reveal itself. --- Cargo.lock | 1 + ingest/tests/iot_ingest.rs | 10 +-- iot_config/tests/route_service.rs | 10 +-- mobile_config/Cargo.toml | 4 ++ mobile_config/src/gateway_service.rs | 15 ++++- mobile_config/src/key_cache.rs | 12 ++-- mobile_config/src/main.rs | 2 +- mobile_config/tests/gateway_service.rs | 93 ++++++++++++++++++++++++++ 8 files changed, 124 insertions(+), 23 deletions(-) create mode 100644 mobile_config/tests/gateway_service.rs diff --git a/Cargo.lock b/Cargo.lock index b8444348a..ca88fa849 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4492,6 +4492,7 @@ dependencies = [ "metrics-exporter-prometheus", "poc-metrics", "prost", + "rand 0.8.5", "retainer", "serde", "serde_json", diff --git a/ingest/tests/iot_ingest.rs b/ingest/tests/iot_ingest.rs index 21282d16f..ab39472c6 100644 --- a/ingest/tests/iot_ingest.rs +++ b/ingest/tests/iot_ingest.rs @@ -2,7 +2,7 @@ use std::{net::SocketAddr, str::FromStr}; use backon::{ExponentialBuilder, Retryable}; use file_store::file_sink::{FileSinkClient, Message as SinkMessage}; -use helium_crypto::{KeyTag, KeyType, Keypair, Network, PublicKey, Sign}; +use helium_crypto::{KeyTag, Keypair, Network, PublicKey, Sign}; use helium_proto::services::poc_lora::{ lora_stream_request_v1::Request as StreamRequest, lora_stream_response_v1::Response as StreamResponse, poc_lora_client::PocLoraClient, @@ -555,13 +555,7 @@ fn create_test_server( } fn generate_keypair() -> Keypair { - Keypair::generate( - KeyTag { - network: Network::MainNet, - key_type: KeyType::Ed25519, - }, - &mut OsRng, - ) + Keypair::generate(KeyTag::default(), &mut OsRng) } fn seconds(s: u64) -> std::time::Duration { diff --git a/iot_config/tests/route_service.rs b/iot_config/tests/route_service.rs index f54d3a14f..314e06292 100644 --- a/iot_config/tests/route_service.rs +++ b/iot_config/tests/route_service.rs @@ -3,7 +3,7 @@ use std::{net::SocketAddr, str::FromStr, sync::Arc}; use backon::{ExponentialBuilder, Retryable}; use chrono::Utc; use futures::{Future, StreamExt, TryFutureExt}; -use helium_crypto::{KeyTag, KeyType as CryptoKeyType, Keypair, Network, PublicKey, Sign}; +use helium_crypto::{KeyTag, Keypair, PublicKey, Sign}; use helium_proto::services::iot_config::{ self as proto, config_org_client::OrgClient, config_route_client::RouteClient, RouteStreamReqV1, }; @@ -487,13 +487,7 @@ fn socket_addr(port: u64) -> anyhow::Result { } fn generate_keypair() -> Keypair { - Keypair::generate( - KeyTag { - network: Network::MainNet, - key_type: CryptoKeyType::Ed25519, - }, - &mut OsRng, - ) + Keypair::generate(KeyTag::default(), &mut OsRng) } fn get_port() -> u64 { diff --git a/mobile_config/Cargo.toml b/mobile_config/Cargo.toml index ccfb2e047..63cd3f5a6 100644 --- a/mobile_config/Cargo.toml +++ b/mobile_config/Cargo.toml @@ -43,3 +43,7 @@ tracing-subscriber = {workspace = true} triggered = {workspace = true} task-manager = { path = "../task_manager" } solana-sdk = {workspace = true} + +[dev-dependencies] +rand = { workspace = true } +tokio-stream = { workspace = true, features = ["net"] } \ No newline at end of file diff --git a/mobile_config/src/gateway_service.rs b/mobile_config/src/gateway_service.rs index 7e3a2ca1a..b4304bbf4 100644 --- a/mobile_config/src/gateway_service.rs +++ b/mobile_config/src/gateway_service.rs @@ -47,6 +47,18 @@ impl GatewayService { Err(Status::permission_denied("unauthorized request signature")) } + fn verify_request_signature_for_info(&self, request: &GatewayInfoReqV1) -> Result<(), Status> { + let signer = verify_public_key(&request.signer)?; + let address = verify_public_key(&request.address)?; + + if address == signer && request.verify(&signer).is_ok() { + tracing::debug!(%signer, "self authorized"); + return Ok(()); + } + + self.verify_request_signature(&signer, request) + } + fn sign_response(&self, response: &[u8]) -> Result, Status> { self.signing_key .sign(response) @@ -60,8 +72,7 @@ impl mobile_config::Gateway for GatewayService { let request = request.into_inner(); telemetry::count_request("gateway", "info"); - let signer = verify_public_key(&request.signer)?; - self.verify_request_signature(&signer, &request)?; + self.verify_request_signature_for_info(&request)?; let pubkey: PublicKeyBinary = request.address.into(); tracing::debug!(pubkey = pubkey.to_string(), "fetching gateway info"); diff --git a/mobile_config/src/key_cache.rs b/mobile_config/src/key_cache.rs index 8a46ca7b0..6056ebc25 100644 --- a/mobile_config/src/key_cache.rs +++ b/mobile_config/src/key_cache.rs @@ -13,7 +13,13 @@ pub struct KeyCache { } impl KeyCache { - pub async fn new( + pub fn new(stored_keys: CacheKeys) -> (watch::Sender, Self) { + let (cache_sender, cache_receiver) = watch::channel(stored_keys); + + (cache_sender, Self { cache_receiver }) + } + + pub async fn from_settings( settings: &Settings, db: impl sqlx::PgExecutor<'_> + Copy, ) -> anyhow::Result<(watch::Sender, Self)> { @@ -22,9 +28,7 @@ impl KeyCache { let mut stored_keys = db::fetch_stored_keys(db).await?; stored_keys.insert((config_admin, KeyRole::Administrator)); - let (cache_sender, cache_receiver) = watch::channel(stored_keys); - - Ok((cache_sender, Self { cache_receiver })) + Ok(Self::new(stored_keys)) } pub fn verify_signature(&self, signer: &PublicKey, request: &R) -> anyhow::Result<()> diff --git a/mobile_config/src/main.rs b/mobile_config/src/main.rs index af5b1d82c..a904e08f7 100644 --- a/mobile_config/src/main.rs +++ b/mobile_config/src/main.rs @@ -73,7 +73,7 @@ impl Daemon { let listen_addr = settings.listen_addr()?; - let (key_cache_updater, key_cache) = KeyCache::new(settings, &pool).await?; + let (key_cache_updater, key_cache) = KeyCache::from_settings(settings, &pool).await?; let admin_svc = AdminService::new(settings, key_cache.clone(), key_cache_updater, pool.clone())?; diff --git a/mobile_config/tests/gateway_service.rs b/mobile_config/tests/gateway_service.rs new file mode 100644 index 000000000..8a818baad --- /dev/null +++ b/mobile_config/tests/gateway_service.rs @@ -0,0 +1,93 @@ +use helium_crypto::{KeyTag, Keypair, PublicKey, Sign}; +use helium_proto::services::mobile_config::{self as proto, GatewayClient}; +use mobile_config::{ + gateway_service::GatewayService, + key_cache::{CacheKeys, KeyCache}, + KeyRole, +}; +use prost::Message; +use sqlx::PgPool; +use tokio::net::TcpListener; +use tonic::{transport, Code}; + +#[sqlx::test] +async fn gateway_info_authorization_errors(pool: PgPool) -> anyhow::Result<()> { + // NOTE(mj): The information we're requesting does not exist in the DB for + // this test. But we're only interested in Authization Errors. + + let admin_key = make_keypair(); // unlimited access + let gw_key = make_keypair(); // access to self + let unknown_key = make_keypair(); // no access + let server_key = make_keypair(); // signs responses + + // Let the OS assign a port + let listener = TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; + + // Start the gateway server + let keys = CacheKeys::from_iter([(admin_key.public_key().to_owned(), KeyRole::Administrator)]); + let (_key_cache_tx, key_cache) = KeyCache::new(keys); + let gws = GatewayService::new(key_cache, pool.clone(), server_key); + let _handle = tokio::spawn( + transport::Server::builder() + .add_service(proto::GatewayServer::new(gws)) + .serve_with_incoming(tokio_stream::wrappers::TcpListenerStream::new(listener)), + ); + + // Connect with the assigned address + let mut client = GatewayClient::connect(format!("http://{addr}")).await?; + + // Request information about ourselves + let req = make_signed_info_request(gw_key.public_key(), &gw_key); + let err = client.info(req).await.expect_err("testing expects error"); + assert_ne!( + err.code(), + Code::PermissionDenied, + "gateway can request infomation about itself" + ); + + // Request gateway info as administrator + let req = make_signed_info_request(gw_key.public_key(), &admin_key); + let err = client.info(req).await.expect_err("testing expects error"); + assert_ne!( + err.code(), + Code::PermissionDenied, + "admins have full access" + ); + + // Request gateway from unknown key + let req = make_signed_info_request(gw_key.public_key(), &unknown_key); + let err = client.info(req).await.expect_err("testing expects errors"); + assert_eq!( + err.code(), + Code::PermissionDenied, + "unknown keys are denied" + ); + + // Request self with a different signer + let mut req = make_signed_info_request(gw_key.public_key(), &gw_key); + req.signature = vec![]; + req.signature = admin_key.sign(&req.encode_to_vec()).unwrap(); + let err = client.info(req).await.expect_err("testing expects errors"); + assert_eq!( + err.code(), + Code::PermissionDenied, + "signature must match signer" + ); + + Ok(()) +} + +fn make_keypair() -> Keypair { + Keypair::generate(KeyTag::default(), &mut rand::rngs::OsRng) +} + +fn make_signed_info_request(address: &PublicKey, signer: &Keypair) -> proto::GatewayInfoReqV1 { + let mut req = proto::GatewayInfoReqV1 { + address: address.to_vec(), + signer: signer.public_key().to_vec(), + signature: vec![], + }; + req.signature = signer.sign(&req.encode_to_vec()).unwrap(); + req +}