From e4df6416f5c468b2eb483dde0920e54dc4259114 Mon Sep 17 00:00:00 2001 From: dynco-nym <173912580+dynco-nym@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:41:13 +0100 Subject: [PATCH] /request better authentication - moved agent API calls to Client struct --- Cargo.lock | 4 + common/models/src/ns_api.rs | 17 +- nym-node-status-agent/Cargo.toml | 3 + nym-node-status-agent/run.sh | 6 +- nym-node-status-agent/src/cli/run_probe.rs | 171 ++++++++++-------- nym-node-status-api/Cargo.toml | 1 + nym-node-status-api/launch_node_status_api.sh | 4 +- .../src/db/queries/testruns.rs | 6 +- nym-node-status-api/src/http/api/testruns.rs | 60 +++--- 9 files changed, 165 insertions(+), 107 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bbd6905ab5..c9c60e4298 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6053,12 +6053,15 @@ name = "nym-node-status-agent" version = "0.1.6" dependencies = [ "anyhow", + "bincode", + "chrono", "clap 4.5.20", "nym-bin-common", "nym-common-models", "nym-crypto", "rand", "reqwest 0.12.4", + "serde", "serde_json", "tempfile", "tokio", @@ -6073,6 +6076,7 @@ version = "0.1.6" dependencies = [ "anyhow", "axum 0.7.7", + "bincode", "chrono", "clap 4.5.20", "cosmwasm-std", diff --git a/common/models/src/ns_api.rs b/common/models/src/ns_api.rs index 306fd2ab93..f0c37c91af 100644 --- a/common/models/src/ns_api.rs +++ b/common/models/src/ns_api.rs @@ -1,6 +1,21 @@ -use nym_crypto::asymmetric::ed25519::Signature; +use nym_crypto::asymmetric::ed25519::{PublicKey, Signature}; use serde::{Deserialize, Serialize}; +pub mod get_testrun { + use super::*; + #[derive(Debug, Clone, Deserialize, Serialize)] + pub struct Payload { + pub agent_public_key: PublicKey, + pub timestamp: i64, + } + + #[derive(Debug, Clone, Deserialize, Serialize)] + pub struct GetTestrunRequest { + pub payload: Payload, + pub signature: Signature, + } +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct TestrunAssignment { pub testrun_id: i64, diff --git a/nym-node-status-agent/Cargo.toml b/nym-node-status-agent/Cargo.toml index 9a61cd5084..8bf6148cd2 100644 --- a/nym-node-status-agent/Cargo.toml +++ b/nym-node-status-agent/Cargo.toml @@ -16,6 +16,8 @@ readme.workspace = true [dependencies] anyhow = { workspace = true} +bincode = { workspace = true } +chrono = { workspace = true } clap = { workspace = true, features = ["derive", "env"] } nym-bin-common = { path = "../common/bin-common", features = ["models"]} nym-common-models = { path = "../common/models" } @@ -26,6 +28,7 @@ tokio-util = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter"] } reqwest = { workspace = true, features = ["json"] } +serde = { workspace = true } serde_json = { workspace = true } [dev-dependencies] diff --git a/nym-node-status-agent/run.sh b/nym-node-status-agent/run.sh index af1fa2353e..a0cf362e16 100755 --- a/nym-node-status-agent/run.sh +++ b/nym-node-status-agent/run.sh @@ -1,9 +1,9 @@ #!/bin/bash set -eu -environment="qa" +export ENVIRONMENT=${ENVIRONMENT:-"sandbox"} -probe_git_ref="0dd5dacdda92b1ddd51cd30a3399515e45613371" +probe_git_ref="nym-vpn-core-v1.0.0-rc.6" crate_root=$(dirname $(realpath "$0")) monorepo_root=$(dirname "${crate_root}") @@ -13,7 +13,7 @@ echo "gateway_probe_src=$gateway_probe_src" echo "crate_root=$crate_root" set -a -source "${monorepo_root}/envs/${environment}.env" +source "${monorepo_root}/envs/${ENVIRONMENT}.env" set +a export RUST_LOG="debug" diff --git a/nym-node-status-agent/src/cli/run_probe.rs b/nym-node-status-agent/src/cli/run_probe.rs index aee1116f4a..17c7d2a36a 100644 --- a/nym-node-status-agent/src/cli/run_probe.rs +++ b/nym-node-status-agent/src/cli/run_probe.rs @@ -1,33 +1,34 @@ use anyhow::{bail, Context}; -use nym_common_models::ns_api::{SubmitResults, TestrunAssignment}; -use nym_crypto::asymmetric::ed25519::{PrivateKey, PublicKey}; +use nym_common_models::ns_api::{get_testrun, SubmitResults, TestrunAssignment}; +use nym_crypto::asymmetric::ed25519::{PrivateKey, Signature}; +use std::fmt::Display; use tracing::instrument; use crate::cli::GwProbe; -const URL_BASE: &str = "internal/testruns"; +const INTERNAL_TESTRUNS: &str = "internal/testruns"; pub(crate) async fn run_probe( - server_address: &str, + server_ip: &str, server_port: u16, ns_api_auth_key: &str, probe_path: &str, ) -> anyhow::Result<()> { - let server_address = format!("{}:{}", server_address, server_port); - test_ns_api_conn(&server_address).await?; - let auth_key = PrivateKey::from_base58_string(ns_api_auth_key) .context("Couldn't parse auth key, exiting")?; + let ns_api_client = Client::new(server_ip, server_port, auth_key); let probe = GwProbe::new(probe_path.to_string()); let version = probe.version().await; tracing::info!("Probe version:\n{}", version); - if let Some(testrun) = request_testrun(auth_key.public_key(), &server_address).await? { + if let Some(testrun) = ns_api_client.request_testrun().await? { let log = probe.run_and_get_log(&Some(testrun.gateway_identity_key)); - submit_results(auth_key, &server_address, testrun.testrun_id, log).await?; + ns_api_client + .submit_results(testrun.testrun_id, log) + .await?; } else { tracing::info!("No testruns available, exiting") } @@ -35,84 +36,102 @@ pub(crate) async fn run_probe( Ok(()) } -async fn test_ns_api_conn(server_addr: &str) -> anyhow::Result<()> { - reqwest::get(server_addr) - .await - .map(|res| { - tracing::info!( - "Testing connection to NS API at {server_addr}: {}", - res.status() - ); - }) - .map_err(|err| anyhow::anyhow!("Couldn't connect to server on {}: {}", server_addr, err)) +struct Client { + server_address: String, + client: reqwest::Client, + auth_key: PrivateKey, } -#[instrument(level = "debug", skip_all)] -pub(crate) async fn request_testrun( - auth_key: PublicKey, - server_addr: &str, -) -> anyhow::Result> { - let target_url = format!("{}/{}", server_addr, URL_BASE); - let client = reqwest::Client::new(); - let res = client - .get(target_url) - .body(auth_key.to_base58_string()) - .send() - .await?; - let status = res.status(); - let response_text = res.text().await?; +impl Client { + pub fn new(server_ip: &str, server_port: u16, auth_key: PrivateKey) -> Self { + let server_address = format!("{}:{}", server_ip, server_port); + let client = reqwest::Client::new(); - if status.is_client_error() { - bail!("{}: {}", status, response_text); - } else if status.is_server_error() { - if matches!(status, reqwest::StatusCode::SERVICE_UNAVAILABLE) - && response_text.contains("No testruns available") - { - return Ok(None); - } else { - bail!("{}: {}", status, response_text); + Self { + server_address, + client, + auth_key, } } - serde_json::from_str(&response_text) - .map(|testrun| { - tracing::info!("Received testrun assignment: {:?}", testrun); - testrun - }) - .map_err(|err| { - tracing::error!("err"); - err.into() - }) -} + #[instrument(level = "debug", skip_all)] + pub(crate) async fn request_testrun(&self) -> anyhow::Result> { + let target_url = self.api_with_subpath(None::); -#[instrument(level = "debug", skip(auth_key, probe_outcome))] -pub(crate) async fn submit_results( - auth_key: PrivateKey, - server_addr: &str, - testrun_id: i64, - probe_outcome: String, -) -> anyhow::Result<()> { - let target_url = format!("{}/{}/{}", server_addr, URL_BASE, testrun_id); + let payload = get_testrun::Payload { + agent_public_key: self.auth_key.public_key(), + timestamp: chrono::offset::Utc::now().timestamp(), + }; + let signature = self.sign_message(&payload)?; + let request = get_testrun::GetTestrunRequest { payload, signature }; - let results = sign_message(auth_key, probe_outcome); + let res = self.client.get(target_url).json(&request).send().await?; + let status = res.status(); + let response_text = res.text().await?; - let client = reqwest::Client::new(); - let res = client - .post(target_url) - .json(&results) - .send() - .await - .and_then(|response| response.error_for_status())?; + if status.is_client_error() { + bail!("{}: {}", status, response_text); + } else if status.is_server_error() { + if matches!(status, reqwest::StatusCode::SERVICE_UNAVAILABLE) + && response_text.contains("No testruns available") + { + return Ok(None); + } else { + bail!("{}: {}", status, response_text); + } + } - tracing::debug!("Submitted results: {})", res.status()); - Ok(()) -} + serde_json::from_str(&response_text) + .map(|testrun| { + tracing::info!("Received testrun assignment: {:?}", testrun); + testrun + }) + .map_err(|err| { + tracing::error!("err"); + err.into() + }) + } -fn sign_message(key: PrivateKey, probe_outcome: String) -> SubmitResults { - let signature = key.sign(&probe_outcome); + #[instrument(level = "debug", skip(self, probe_outcome))] + pub(crate) async fn submit_results( + &self, + testrun_id: i64, + probe_outcome: String, + ) -> anyhow::Result<()> { + let target_url = self.api_with_subpath(Some(testrun_id)); - SubmitResults { - message: probe_outcome, - signature, + let signature = self.sign_message(&probe_outcome)?; + let submit_results = SubmitResults { + message: probe_outcome, + signature, + }; + + let res = self + .client + .post(target_url) + .json(&submit_results) + .send() + .await + .and_then(|response| response.error_for_status())?; + + tracing::debug!("Submitted results: {})", res.status()); + Ok(()) + } + + fn sign_message(&self, message: &T) -> anyhow::Result + where + T: serde::Serialize, + { + let serialized = bincode::serialize(message)?; + let signed = self.auth_key.sign(&serialized); + Ok(signed) + } + + fn api_with_subpath(&self, subpath: Option) -> String { + if let Some(subpath) = subpath { + format!("{}/{}/{}", self.server_address, INTERNAL_TESTRUNS, subpath) + } else { + format!("{}/{}", self.server_address, INTERNAL_TESTRUNS) + } } } diff --git a/nym-node-status-api/Cargo.toml b/nym-node-status-api/Cargo.toml index 102f06c3bb..576239ce39 100644 --- a/nym-node-status-api/Cargo.toml +++ b/nym-node-status-api/Cargo.toml @@ -15,6 +15,7 @@ rust-version.workspace = true [dependencies] anyhow = { workspace = true } axum = { workspace = true, features = ["tokio", "macros"] } +bincode = { workspace = true } chrono = { workspace = true } clap = { workspace = true, features = ["cargo", "derive", "env", "string"] } cosmwasm-std = { workspace = true } diff --git a/nym-node-status-api/launch_node_status_api.sh b/nym-node-status-api/launch_node_status_api.sh index 83bd3dac93..26a97fcbab 100755 --- a/nym-node-status-api/launch_node_status_api.sh +++ b/nym-node-status-api/launch_node_status_api.sh @@ -19,12 +19,12 @@ Eujj4GmvwQBgHZaNSyqUbjMFSsnXWPSjEYUPgAsKmx1A, 5ZnfSGxW6EKcFxB8jftb9V3f897VpwpZtf7kCPYzB595, H9kuRd8BGjEUD8Grh5U9YUPN5ZaQmSYz8U44R72AffKM" -export ENVIRONMENT="qa.env" +export ENVIRONMENT=${ENVIRONMENT:-"sandbox"} function run_bare() { # export necessary env vars set -a - source ../envs/$ENVIRONMENT + source ../envs/${ENVIRONMENT}.env set +a export RUST_LOG=debug diff --git a/nym-node-status-api/src/db/queries/testruns.rs b/nym-node-status-api/src/db/queries/testruns.rs index 38582e2a5b..a3adf15b19 100644 --- a/nym-node-status-api/src/db/queries/testruns.rs +++ b/nym-node-status-api/src/db/queries/testruns.rs @@ -33,8 +33,10 @@ pub(crate) async fn get_in_progress_testrun_by_id( TestRunStatus::InProgress as i64, ) .fetch_one(conn.as_mut()) - .await - .context(format!("Couldn't retrieve testrun {testrun_id}")) + .await.map_err(|e| { + anyhow::anyhow!("Couldn't retrieve testrun {testrun_id}: {e}") + }) + } pub(crate) async fn get_testruns_assigned_to_agent( diff --git a/nym-node-status-api/src/http/api/testruns.rs b/nym-node-status-api/src/http/api/testruns.rs index 5e19105e53..43122041dc 100644 --- a/nym-node-status-api/src/http/api/testruns.rs +++ b/nym-node-status-api/src/http/api/testruns.rs @@ -4,8 +4,8 @@ use axum::{ extract::{Path, State}, Router, }; -use nym_common_models::ns_api::SubmitResults; -use nym_crypto::asymmetric::ed25519::PublicKey; +use nym_common_models::ns_api::{get_testrun, SubmitResults}; +use nym_crypto::asymmetric::ed25519::{PublicKey, Signature}; use reqwest::StatusCode; use crate::db::models::TestRunStatus; @@ -32,10 +32,12 @@ pub(crate) fn routes() -> Router { #[tracing::instrument(level = "debug", skip_all)] async fn request_testrun( State(state): State, - body: String, + Json(request): Json, ) -> HttpResult> { // TODO dz log agent's network probe version - let agent_pubkey = authenticate_agent(&body, &state)?; + + authenticate(&request, &state)?; + let agent_pubkey = request.payload.agent_public_key; tracing::debug!("Agent {} requested testrun", agent_pubkey); @@ -104,12 +106,11 @@ async fn submit_testrun( return Err(HttpError::invalid_input("Invalid testrun submitted")); } - agent_pubkey - .verify(&probe_results.message, &probe_results.signature) - .map_err(|_| { - tracing::warn!("Message verification failed, rejecting"); - HttpError::unauthorized() - })?; + verify_message( + &agent_pubkey, + &probe_results.message, + &probe_results.signature, + )?; let gw_identity = db::queries::select_gateway_identity(&mut conn, assigned_testrun.gateway_id) .await @@ -162,21 +163,34 @@ async fn submit_testrun( Ok(StatusCode::CREATED) } -fn authenticate_agent(base58_pubkey: &str, state: &AppState) -> HttpResult { - let agent_pubkey = PublicKey::from_base58_string(base58_pubkey).map_err(|_| { - if base58_pubkey.is_empty() { - tracing::warn!("Auth key missing from request body, rejecting"); - } else { - tracing::warn!("Failed to deserialize key from request body, rejecting"); - } - HttpError::unauthorized() - })?; - if !state.is_registered(&agent_pubkey) { - tracing::warn!("Public key {} not registered, rejecting", agent_pubkey); +// TODO dz this should be middleware +fn authenticate(request: &get_testrun::GetTestrunRequest, state: &AppState) -> HttpResult<()> { + if !state.is_registered(&request.payload.agent_public_key) { + tracing::warn!("Public key not registered with NS API, rejecting"); return Err(HttpError::unauthorized()); - } + }; - Ok(agent_pubkey) + verify_message( + &request.payload.agent_public_key, + &request.payload, + &request.signature, + ) + .inspect_err(|_| tracing::warn!("Signature verification failed, rejecting"))?; + + Ok(()) +} + +fn verify_message(public_key: &PublicKey, message: &T, signature: &Signature) -> HttpResult<()> +where + T: serde::Serialize, +{ + bincode::serialize(message) + .map_err(HttpError::invalid_input) + .and_then(|serialized| { + public_key + .verify(serialized, signature) + .map_err(|_| HttpError::unauthorized()) + }) } fn get_result_from_log(log: &str) -> String {