Uniform request authentication

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"] }

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<bincode::Error> + From<SignatureError>;
+
+    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<T> VerifiableRequest for T
+where
+    T: SignedRequest,
+{
+    type Error = anyhow::Error;
 }

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

nym-node-status-api/src/db/queries/testruns.rs

@@ -109,12 +109,14 @@ pub(crate) async fn assign_oldest_testrun(
     agent_key: PublicKey,
 ) -> anyhow::Result<Option<TestrunAssignment>> {
     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 +131,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 +154,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)

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, SignedRequest, 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<i64>,
     State(state): State<AppState>,
-    Json(probe_results): Json<SubmitResults>,
+    Json(submitted_result): Json<submit_results::SubmitResults>,
 ) -> HttpResult<StatusCode> {
+    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<T>(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();