diff --git a/Cargo.lock b/Cargo.lock index c9c60e4298..2107724c62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5010,6 +5010,8 @@ dependencies = [ name = "nym-common-models" version = "0.1.0" dependencies = [ + "anyhow", + "bincode", "nym-crypto", "serde", ] diff --git a/common/models/Cargo.toml b/common/models/Cargo.toml index 123f3f7fd2..3192a83b53 100644 --- a/common/models/Cargo.toml +++ b/common/models/Cargo.toml @@ -11,5 +11,7 @@ rust-version.workspace = true readme.workspace = true [dependencies] -serde = { workspace = true, features = ["derive"] } +anyhow = { workspace = true } +bincode = { workspace = true } nym-crypto = { path = "../crypto", features = ["asymmetric", "serde"] } +serde = { workspace = true, features = ["derive"] } diff --git a/common/models/src/ns_api.rs b/common/models/src/ns_api.rs index f0c37c91af..6f4b00a888 100644 --- a/common/models/src/ns_api.rs +++ b/common/models/src/ns_api.rs @@ -1,4 +1,4 @@ -use nym_crypto::asymmetric::ed25519::{PublicKey, Signature}; +use nym_crypto::asymmetric::ed25519::{PublicKey, Signature, SignatureError}; use serde::{Deserialize, Serialize}; pub mod get_testrun { @@ -14,16 +14,88 @@ pub mod get_testrun { pub payload: Payload, pub signature: Signature, } + + impl SignedRequest for GetTestrunRequest { + type Payload = Payload; + + fn public_key(&self) -> &PublicKey { + &self.payload.agent_public_key + } + + fn signature(&self) -> &Signature { + &self.signature + } + + fn payload(&self) -> &Self::Payload { + &self.payload + } + } } #[derive(Debug, Clone, Deserialize, Serialize)] pub struct TestrunAssignment { pub testrun_id: i64, + pub assigned_at_utc: i64, pub gateway_identity_key: String, } -#[derive(Debug, Clone, Deserialize, Serialize)] -pub struct SubmitResults { - pub message: String, - pub signature: Signature, +pub mod submit_results { + use super::*; + #[derive(Debug, Clone, Deserialize, Serialize)] + pub struct Payload { + pub probe_result: String, + pub agent_public_key: PublicKey, + pub assigned_at_utc: i64, + } + + #[derive(Debug, Clone, Deserialize, Serialize)] + pub struct SubmitResults { + pub payload: Payload, + pub signature: Signature, + } + + impl SignedRequest for SubmitResults { + type Payload = Payload; + + fn public_key(&self) -> &PublicKey { + &self.payload.agent_public_key + } + + fn signature(&self) -> &Signature { + &self.signature + } + + fn payload(&self) -> &Self::Payload { + &self.payload + } + } +} + +pub trait SignedRequest { + type Payload: serde::Serialize; + + fn public_key(&self) -> &PublicKey; + fn signature(&self) -> &Signature; + fn payload(&self) -> &Self::Payload; +} + +pub trait VerifiableRequest: SignedRequest { + type Error: From + From; + + fn verify_signature(&self) -> Result<(), Self::Error> { + bincode::serialize(self.payload()) + .map_err(Self::Error::from) + .and_then(|serialized| { + self.public_key() + .verify(serialized, self.signature()) + .map_err(Self::Error::from) + }) + } +} + +impl VerifiableRequest for T +where + T: SignedRequest, +{ + type Error = anyhow::Error; } diff --git a/nym-node-status-agent/src/cli/run_probe.rs b/nym-node-status-agent/src/cli/run_probe.rs index 17c7d2a36a..3ffd90d46c 100644 --- a/nym-node-status-agent/src/cli/run_probe.rs +++ b/nym-node-status-agent/src/cli/run_probe.rs @@ -1,5 +1,5 @@ use anyhow::{bail, Context}; -use nym_common_models::ns_api::{get_testrun, SubmitResults, TestrunAssignment}; +use nym_common_models::ns_api::{get_testrun, submit_results, TestrunAssignment}; use nym_crypto::asymmetric::ed25519::{PrivateKey, Signature}; use std::fmt::Display; use tracing::instrument; @@ -27,7 +27,7 @@ pub(crate) async fn run_probe( let log = probe.run_and_get_log(&Some(testrun.gateway_identity_key)); ns_api_client - .submit_results(testrun.testrun_id, log) + .submit_results(testrun.testrun_id, log, testrun.assigned_at_utc) .await?; } else { tracing::info!("No testruns available, exiting") @@ -92,19 +92,22 @@ impl Client { }) } - #[instrument(level = "debug", skip(self, probe_outcome))] + #[instrument(level = "debug", skip(self, probe_result))] pub(crate) async fn submit_results( &self, testrun_id: i64, - probe_outcome: String, + probe_result: String, + assigned_at_utc: i64, ) -> anyhow::Result<()> { let target_url = self.api_with_subpath(Some(testrun_id)); - let signature = self.sign_message(&probe_outcome)?; - let submit_results = SubmitResults { - message: probe_outcome, - signature, + let payload = submit_results::Payload { + probe_result, + agent_public_key: self.auth_key.public_key(), + assigned_at_utc, }; + let signature = self.sign_message(&payload)?; + let submit_results = submit_results::SubmitResults { payload, signature }; let res = self .client diff --git a/nym-node-status-api/src/db/models.rs b/nym-node-status-api/src/db/models.rs index 08bd913d7e..4062e1451d 100644 --- a/nym-node-status-api/src/db/models.rs +++ b/nym-node-status-api/src/db/models.rs @@ -2,7 +2,6 @@ use crate::{ http::{self, models::SummaryHistory}, monitor::NumericalCheckedCast, }; -use nym_crypto::asymmetric::ed25519::PublicKey; use nym_node_requests::api::v1::node::models::NodeDescription; use serde::{Deserialize, Serialize}; use strum_macros::{EnumString, FromRepr}; @@ -314,14 +313,6 @@ pub struct TestRunDto { pub last_assigned_utc: Option, } -impl TestRunDto { - pub(crate) fn assigned_agent_key(&self) -> Option { - self.assigned_agent - .as_ref() - .and_then(|value| PublicKey::from_base58_string(value).ok()) - } -} - #[derive(Debug, Clone, strum_macros::Display, EnumString, FromRepr, PartialEq)] #[repr(u8)] pub(crate) enum TestRunStatus { diff --git a/nym-node-status-api/src/db/queries/testruns.rs b/nym-node-status-api/src/db/queries/testruns.rs index 275e39f90e..465444c8ec 100644 --- a/nym-node-status-api/src/db/queries/testruns.rs +++ b/nym-node-status-api/src/db/queries/testruns.rs @@ -8,35 +8,6 @@ use chrono::Duration; use nym_crypto::asymmetric::ed25519::PublicKey; use sqlx::{pool::PoolConnection, Sqlite}; -pub(crate) async fn get_in_progress_testrun_by_id( - conn: &mut PoolConnection, - testrun_id: i64, -) -> anyhow::Result { - sqlx::query_as!( - TestRunDto, - r#"SELECT - id as "id!", - gateway_id as "gateway_id!", - status as "status!", - created_utc as "created_utc!", - ip_address as "ip_address!", - log as "log!", - assigned_agent, - last_assigned_utc - FROM testruns - WHERE - id = ? - AND - status = ? - ORDER BY created_utc"#, - testrun_id, - TestRunStatus::InProgress as i64, - ) - .fetch_one(conn.as_mut()) - .await - .map_err(|e| anyhow::anyhow!("Couldn't retrieve testrun {testrun_id}: {e}")) -} - pub(crate) async fn testrun_in_progress_assigned_to_agent( conn: &mut PoolConnection, agent_key: &PublicKey, @@ -109,12 +80,14 @@ pub(crate) async fn assign_oldest_testrun( agent_key: PublicKey, ) -> anyhow::Result> { let agent_key = agent_key.to_base58_string(); + let now = now_utc().timestamp(); // find & mark as "In progress" in the same transaction to avoid race conditions let returning = sqlx::query!( r#"UPDATE testruns SET status = ?, - assigned_agent = ? + assigned_agent = ?, + last_assigned_utc = ? WHERE rowid = ( SELECT rowid @@ -129,6 +102,7 @@ pub(crate) async fn assign_oldest_testrun( "#, TestRunStatus::InProgress as i64, agent_key, + now, TestRunStatus::Queued as i64, ) .fetch_optional(conn.as_mut()) @@ -151,6 +125,7 @@ pub(crate) async fn assign_oldest_testrun( Ok(Some(TestrunAssignment { testrun_id: testrun.id, gateway_identity_key: gw_identity.gateway_identity_key, + assigned_at_utc: now, })) } else { Ok(None) diff --git a/nym-node-status-api/src/http/api/testruns.rs b/nym-node-status-api/src/http/api/testruns.rs index 369ac6c4aa..bc965186d0 100644 --- a/nym-node-status-api/src/http/api/testruns.rs +++ b/nym-node-status-api/src/http/api/testruns.rs @@ -4,8 +4,7 @@ use axum::{ extract::{Path, State}, Router, }; -use nym_common_models::ns_api::{get_testrun, SubmitResults}; -use nym_crypto::asymmetric::ed25519::{PublicKey, Signature}; +use nym_common_models::ns_api::{get_testrun, submit_results, VerifiableRequest}; use reqwest::StatusCode; use crate::db::models::TestRunStatus; @@ -88,47 +87,41 @@ async fn request_testrun( async fn submit_testrun( Path(submitted_testrun_id): Path, State(state): State, - Json(probe_results): Json, + Json(submitted_result): Json, ) -> HttpResult { + authenticate(&submitted_result, &state)?; + let db = state.db_pool(); let mut conn = db .acquire() .await .map_err(HttpError::internal_with_logging)?; - let submitted_testrun = - queries::testruns::get_in_progress_testrun_by_id(&mut conn, submitted_testrun_id) - .await - .map_err(|e| { - tracing::warn!("testrun_id {} not found: {}", submitted_testrun_id, e); - HttpError::not_found(submitted_testrun_id) - })?; - let agent_pubkey = submitted_testrun - .assigned_agent_key() - .ok_or_else(HttpError::unauthorized)?; - + let submitter_pubkey = submitted_result.payload.agent_public_key; let assigned_testrun = - queries::testruns::testrun_in_progress_assigned_to_agent(&mut conn, &agent_pubkey) + queries::testruns::testrun_in_progress_assigned_to_agent(&mut conn, &submitter_pubkey) .await .map_err(|err| { - tracing::warn!("No testruns in progress for agent {agent_pubkey}: {err}"); + tracing::warn!("No testruns in progress for agent {submitter_pubkey}: {err}"); HttpError::invalid_input("Invalid testrun submitted") })?; if submitted_testrun_id != assigned_testrun.id { tracing::warn!( "Agent {} submitted testrun {} but {} was expected", - agent_pubkey, + submitter_pubkey, submitted_testrun_id, assigned_testrun.id ); return Err(HttpError::invalid_input("Invalid testrun submitted")); } - - verify_message( - &agent_pubkey, - &probe_results.message, - &probe_results.signature, - )?; + if Some(submitted_result.payload.assigned_at_utc) != assigned_testrun.last_assigned_utc { + tracing::warn!( + "Submitted testrun timestamp mismatch: {} != {:?}, rejecting", + submitted_result.payload.assigned_at_utc, + assigned_testrun.last_assigned_utc + ); + return Err(HttpError::invalid_input("Invalid testrun submitted")); + } let gw_identity = db::queries::select_gateway_identity(&mut conn, assigned_testrun.gateway_id) .await @@ -140,10 +133,10 @@ async fn submit_testrun( })?; tracing::debug!( "Agent {} submitted testrun {} for gateway {} ({} bytes)", - agent_pubkey, + submitter_pubkey, submitted_testrun_id, gw_identity, - &probe_results.message.len(), + &submitted_result.payload.probe_result.len(), ); queries::testruns::update_testrun_status( @@ -156,11 +149,11 @@ async fn submit_testrun( queries::testruns::update_gateway_last_probe_log( &mut conn, assigned_testrun.gateway_id, - &probe_results.message, + &submitted_result.payload.probe_result, ) .await .map_err(HttpError::internal_with_logging)?; - let result = get_result_from_log(&probe_results.message); + let result = get_result_from_log(&submitted_result.payload.probe_result); queries::testruns::update_gateway_last_probe_result( &mut conn, assigned_testrun.gateway_id, @@ -183,35 +176,20 @@ async fn submit_testrun( // TODO dz this should be middleware #[tracing::instrument(level = "debug", skip_all)] -fn authenticate(request: &get_testrun::GetTestrunRequest, state: &AppState) -> HttpResult<()> { - if !state.is_registered(&request.payload.agent_public_key) { +fn authenticate(request: &impl VerifiableRequest, state: &AppState) -> HttpResult<()> { + if !state.is_registered(request.public_key()) { tracing::warn!("Public key not registered with NS API, rejecting"); return Err(HttpError::unauthorized()); }; - verify_message( - &request.payload.agent_public_key, - &request.payload, - &request.signature, - ) - .inspect_err(|_| tracing::warn!("Signature verification failed, rejecting"))?; + request.verify_signature().map_err(|_| { + tracing::warn!("Signature verification failed, rejecting"); + HttpError::unauthorized() + })?; Ok(()) } -fn verify_message(public_key: &PublicKey, payload: &T, signature: &Signature) -> HttpResult<()> -where - T: serde::Serialize, -{ - bincode::serialize(payload) - .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 { let re = regex::Regex::new(r"\n\{\s").unwrap(); let result: Vec<_> = re.splitn(log, 2).collect(); diff --git a/nym-node-status-api/src/http/error.rs b/nym-node-status-api/src/http/error.rs index 4ce1ca1ce1..ed37966742 100644 --- a/nym-node-status-api/src/http/error.rs +++ b/nym-node-status-api/src/http/error.rs @@ -40,12 +40,6 @@ impl HttpError { status: axum::http::StatusCode::SERVICE_UNAVAILABLE, } } - pub(crate) fn not_found(msg: impl Display) -> Self { - Self { - message: serde_json::json!({"message": msg.to_string()}).to_string(), - status: axum::http::StatusCode::NOT_FOUND, - } - } } impl axum::response::IntoResponse for HttpError {