diff --git a/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/helpers.rs b/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/helpers.rs index 6293361d6b3..66b9acf227b 100644 --- a/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/helpers.rs +++ b/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/helpers.rs @@ -1,12 +1,18 @@ // Copyright 2026 - Nym Technologies SA // SPDX-License-Identifier: GPL-3.0-only -use anyhow::{Context, bail}; -use nym_crypto::asymmetric::x25519; +use anyhow::{Context, anyhow, bail}; +use nym_crypto::asymmetric::{ed25519, x25519}; +use nym_crypto::hkdf; use nym_pemstore::load_key; +use sha2::Sha256; use std::path::Path; use std::sync::Arc; +/// Domain-separation label for the ed25519 client identity derived from the agent's noise key. +/// Changing it rotates every agent's identity, which then has to be re-announced on chain. +const CLIENT_IDENTITY_HKDF_LABEL: &[u8] = b"nym-network-monitor-agent-ed25519-client-identity-v1"; + /// Loads an x25519 Noise private key from a PEM file and returns the full key pair /// wrapped in an [`Arc`] for shared ownership. pub(crate) fn load_noise_key>(path: P) -> anyhow::Result> { @@ -17,3 +23,47 @@ pub(crate) fn load_noise_key>(path: P) -> anyhow::Result anyhow::Result { + let seed = hkdf::extract_then_expand::( + None, + &noise_key.private_key().to_bytes(), + Some(CLIENT_IDENTITY_HKDF_LABEL), + ed25519::SECRET_KEY_LENGTH, + ) + .map_err(|err| { + anyhow!("failed to derive the ed25519 client identity from the noise key: {err}") + })?; + + let private_key = ed25519::PrivateKey::from_bytes(&seed) + .context("the derived ed25519 client identity seed was not a valid private key")?; + Ok(private_key.into()) +} + +#[cfg(test)] +mod tests { + use super::*; + + // a known-answer vector rather than a determinism check: HKDF over fixed bytes can't be + // non-deterministic, but the label or the KDF changing silently WOULD invalidate every identity + // already announced on chain and stop gateways granting monitor sessions. pinning the output + // makes such a change fail here instead of in the field + #[test] + fn the_derived_client_identity_matches_its_known_answer() { + let noise_key: x25519::KeyPair = x25519::PrivateKey::from_secret([42u8; 32]).into(); + + let identity = derive_client_identity(&noise_key).unwrap(); + assert_eq!( + identity.public_key().to_base58_string(), + "DwMcWZ1JGq3UosmKbhCXvkZmF2tcFuiFis8u2du1hn5x" + ); + } +} diff --git a/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/mod.rs b/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/mod.rs index 7609d0310b2..1704cb00f70 100644 --- a/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/mod.rs +++ b/nym-network-monitor-v3/nym-network-monitor-agent/src/agent/mod.rs @@ -2,10 +2,11 @@ // SPDX-License-Identifier: GPL-3.0-only use crate::agent::config::NodeTesterConfig; +use crate::agent::helpers::derive_client_identity; use crate::agent::tested_node::TestedNodeDetails; use crate::agent::tester::NodeStressTester; use anyhow::Context; -use nym_crypto::asymmetric::x25519; +use nym_crypto::asymmetric::{ed25519, x25519}; use nym_network_monitor_orchestrator_requests::client::OrchestratorClient; use nym_network_monitor_orchestrator_requests::models::{ AgentAnnounceRequest, AgentMixAddresses, TestRunAssignmentRequest, @@ -33,6 +34,11 @@ pub(crate) struct NetworkMonitorAgent { /// The tester's own Noise key pair, used to authenticate the egress connection. noise_key: Arc, + + /// The ed25519 identity this agent presents when opening a gateway client session, derived from + /// [`Self::noise_key`]. It is announced on chain, so it must stay stable for as long as the + /// noise key does. + client_identity: ed25519::KeyPair, } impl NetworkMonitorAgent { @@ -42,12 +48,15 @@ impl NetworkMonitorAgent { tester_config: NodeTesterConfig, noise_key: Arc, orchestrator_client: OrchestratorClient, - ) -> Self { - NetworkMonitorAgent { + ) -> anyhow::Result { + let client_identity = derive_client_identity(&noise_key)?; + + Ok(NetworkMonitorAgent { tester_config, orchestrator_client, noise_key, - } + client_identity, + }) } /// The addresses this agent announces to the orchestrator, and thus the ones the nodes it @@ -59,7 +68,7 @@ impl NetworkMonitorAgent { } } - /// Announces this agent's details (mixnet address, noise key, protocol version) + /// Announces this agent's details (mixnet address, noise key, protocol version, client identity) /// to the orchestrator so they can be registered in the smart contract. pub(crate) async fn announce_agent(&self) -> anyhow::Result<()> { self.orchestrator_client @@ -68,6 +77,7 @@ impl NetworkMonitorAgent { x25519_noise_key: *self.noise_key.public_key(), // we're always using the latest noise version available noise_version: LATEST_NOISE_VERSION.into(), + ed25519_identity: *self.client_identity.public_key(), }) .await?; Ok(()) diff --git a/nym-network-monitor-v3/nym-network-monitor-agent/src/cli/run_agent.rs b/nym-network-monitor-v3/nym-network-monitor-agent/src/cli/run_agent.rs index 8a153db3d63..dac98e9217d 100644 --- a/nym-network-monitor-v3/nym-network-monitor-agent/src/cli/run_agent.rs +++ b/nym-network-monitor-v3/nym-network-monitor-agent/src/cli/run_agent.rs @@ -46,13 +46,13 @@ pub(crate) async fn execute(args: Args) -> anyhow::Result<()> { let external_address_v4 = SocketAddr::new(args.host_ip_v4, args.host_port); let external_address_v6 = SocketAddr::new(args.host_ip_v6, args.host_port); - // 1. build instance of the agent (loads the noise keys) + // 1. build instance of the agent (loads the noise keys and derives the client identity) let agent = NetworkMonitorAgent::new( args.common_args .build_config(external_address_v4, external_address_v6)?, noise_key, orchestrator_client, - ); + )?; // 2. announce the agent to the orchestrator // so that it would be registered in the smart contract diff --git a/nym-network-monitor-v3/nym-network-monitor-orchestrator-requests/src/models.rs b/nym-network-monitor-v3/nym-network-monitor-orchestrator-requests/src/models.rs index 0b21f312cdc..e82388be001 100644 --- a/nym-network-monitor-v3/nym-network-monitor-orchestrator-requests/src/models.rs +++ b/nym-network-monitor-v3/nym-network-monitor-orchestrator-requests/src/models.rs @@ -63,6 +63,11 @@ pub struct AgentAnnounceRequest { /// Version of the noise protocol used by the agent. pub noise_version: u8, + + /// Base-58 encoded ed25519 identity the agent presents when opening a gateway client session. + #[serde(with = "bs58_ed25519_pubkey")] + #[cfg_attr(feature = "openapi", schema(value_type = String))] + pub ed25519_identity: ed25519::PublicKey, } /// Confirmation returned to an agent after a successful announcement. diff --git a/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/api/v1/agent/mod.rs b/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/api/v1/agent/mod.rs index 2c356c22d64..0ff9fc7321b 100644 --- a/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/api/v1/agent/mod.rs +++ b/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/api/v1/agent/mod.rs @@ -67,7 +67,11 @@ async fn announce_agent( // 2. upsert the agent in the cache and learn whether it has already been announced let already_announced = state .agents - .try_announce_agent(body.mix_addresses, body.x25519_noise_key) + .try_announce_agent( + body.mix_addresses, + body.x25519_noise_key, + body.ed25519_identity, + ) .await; // 3. if the agent was already announced, skip the contract tx @@ -89,9 +93,9 @@ async fn announce_agent( mixnet_address, bs58_x25519_noise: body.x25519_noise_key.to_base58_string(), noise_version: body.noise_version, - // the agent does not announce an identity key yet; until it does, entries - // are written without one and the upsert fills it in on a later announce - bs58_ed25519_identity: None, + // both of the agent's entries carry the same identity: the gateway session + // gate is keyed on it alone, so it can't be tied to either address + bs58_ed25519_identity: Some(body.ed25519_identity.to_base58_string()), }, Vec::new(), ) diff --git a/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/state.rs b/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/state.rs index afc723bf618..a3adbab6757 100644 --- a/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/state.rs +++ b/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/http/state.rs @@ -6,7 +6,7 @@ use crate::orchestrator::prometheus::{PROMETHEUS_METRICS, PrometheusMetric}; use crate::storage::NetworkMonitorStorage; use crate::storage::models::NewTestRun; use axum::extract::FromRef; -use nym_crypto::asymmetric::x25519; +use nym_crypto::asymmetric::{ed25519, x25519}; use nym_network_monitor_orchestrator_requests::models::{ AgentMixAddresses, NymNodeData, NymNodeWithTestRun, PagedResult, Pagination, TestRunAssignment, TestRunData, TestRunInProgressData, TestRunResult, @@ -46,9 +46,9 @@ impl KnownAgents { } /// Records an announcement from the agent at `addresses`. The cache entry is upserted: a - /// missing entry is inserted, and if the cached noise key or IPv6 address differs from the - /// announced one it is overwritten and the agent is treated as not-yet-announced so the caller - /// re-runs the contract txs with the new details. + /// missing entry is inserted, and if the cached noise key, IPv6 address or ed25519 identity + /// differs from the announced one it is overwritten and the agent is treated as not-yet-announced + /// so the caller re-runs the contract txs with the new details. /// /// Returns the current `announced` flag: `true` means the agent was already announced to the /// contract and the caller should skip the contract txs; `false` means the caller should submit @@ -57,6 +57,7 @@ impl KnownAgents { &self, addresses: AgentMixAddresses, noise_key: x25519::PublicKey, + ed25519_identity: ed25519::PublicKey, ) -> bool { let mut guard = self.inner.lock().await; @@ -65,28 +66,47 @@ impl KnownAgents { let agent = entry.get_mut(); agent.last_active_at = OffsetDateTime::now_utc(); - if agent.noise_key == noise_key && agent.mix_v6 == addresses.v6 { + let details_diverged = agent.noise_key != noise_key || agent.mix_v6 != addresses.v6; + let identity_diverged = agent.ed25519_identity != ed25519_identity; + + if !details_diverged && !identity_diverged { return agent.announced; } - // the addresses and the noise key are all meant to be stable for the lifetime of - // an agent, so this is either a re-provisioned agent reusing its IPv4 address or a - // live one whose configuration changed. we can't distinguish those (announcements - // are bearer-token authenticated, not key authenticated), so the announcement is - // accepted - but a superseded IPv6 address stays authorised in the contract, which - // is worth knowing about. - warn!( - "agent at {} announced details differing from the cached ones (cached: {} / {}, announced: {} / {}) - re-announcing it to the contract", - addresses.v4, - agent.mix_v6, - agent.noise_key.to_base58_string(), - addresses.v6, - noise_key.to_base58_string(), - ); + // the addresses, the noise key and the identity are all meant to be stable for the + // lifetime of an agent, so this is either a re-provisioned agent reusing its IPv4 + // address or a live one whose configuration changed. we can't distinguish those + // (announcements are bearer-token authenticated, not key authenticated), so the + // announcement is accepted - but a superseded IPv6 address stays authorised in the + // contract, which is worth knowing about. + // + // both kinds of divergence share the one counter: the identity is derived from the + // noise key, so it can only change when the noise key does, and telling the two + // apart is a job for the logs below rather than for a second series that would read + // flat zero outside a change to the derivation itself. PROMETHEUS_METRICS.inc(PrometheusMetric::AgentDetailsChanged); + if details_diverged { + warn!( + "agent at {} announced details differing from the cached ones (cached: {} / {}, announced: {} / {}) - re-announcing it to the contract", + addresses.v4, + agent.mix_v6, + agent.noise_key.to_base58_string(), + addresses.v6, + noise_key.to_base58_string(), + ); + } + + if identity_diverged { + warn!( + "agent at {} announced an ed25519 identity differing from the cached one (cached: {}, announced: {ed25519_identity}) - re-announcing it to the contract", + addresses.v4, agent.ed25519_identity, + ); + } + agent.mix_v6 = addresses.v6; agent.noise_key = noise_key; + agent.ed25519_identity = ed25519_identity; agent.announced = false; } Entry::Vacant(entry) => { @@ -94,6 +114,7 @@ impl KnownAgents { mix_v6: addresses.v6, last_active_at: OffsetDateTime::now_utc(), noise_key, + ed25519_identity, announced: false, }); } @@ -121,29 +142,32 @@ impl KnownAgents { /// /// The contract holds one entry per socket address and nothing ties together the two entries /// belonging to a single agent, so the pairs are recovered by grouping on the noise key, which is -/// unique per agent. Records that don't form exactly one IPv4/IPv6 pair are dropped: they are -/// either authorisations predating the IPv6 announcement, or leftovers from an agent that has since -/// changed one of its addresses. Dropping them is safe because this cache exists purely to skip -/// redundant contract transactions - agents always announce before requesting work, which -/// re-creates the entry at the cost of one extra transaction. +/// unique per agent. Records that don't form exactly one IPv4/IPv6 pair carrying one identity are +/// dropped: they are either authorisations predating the IPv6 announcement or the identity key, or +/// leftovers from an agent that has since changed one of its addresses. Dropping them is safe +/// because this cache exists purely to skip redundant contract transactions - agents always announce +/// before requesting work, which re-creates the entry at the cost of one extra transaction, and the +/// contract's upsert fills in whatever the stale entry was missing. impl TryFrom> for KnownAgents { type Error = anyhow::Error; fn try_from(agents: Vec) -> Result { - let mut by_noise_key: HashMap> = HashMap::new(); + let mut by_noise_key: HashMap> = HashMap::new(); for agent in agents { by_noise_key - .entry(agent.bs58_x25519_noise) + .entry(agent.bs58_x25519_noise.clone()) .or_default() - .push(agent.mixnet_address); + .push(agent); } let mut agents_map = HashMap::new(); - for (bs58_noise_key, addresses) in by_noise_key { - let (v4_addresses, v6_addresses): (Vec<_>, Vec<_>) = - addresses.iter().copied().partition(|addr| addr.is_ipv4()); + for (bs58_noise_key, entries) in by_noise_key { + let addresses: Vec<_> = entries.iter().map(|entry| entry.mixnet_address).collect(); + let (v4_entries, v6_entries): (Vec<_>, Vec<_>) = entries + .into_iter() + .partition(|entry| entry.mixnet_address.is_ipv4()); - let ([mix_v4], [mix_v6]) = (v4_addresses.as_slice(), v6_addresses.as_slice()) else { + let ([v4_entry], [v6_entry]) = (v4_entries.as_slice(), v6_entries.as_slice()) else { error!( "the agent using noise key {bs58_noise_key} has {} authorised address(es) on chain ({addresses:?}) rather than a single ipv4/ipv6 pair - ignoring it until it re-announces itself", addresses.len() @@ -151,15 +175,37 @@ impl TryFrom> for KnownAgents { continue; }; + // an agent announces one identity under both of its addresses, so anything else is a + // half-written pair: an entry authorised before the field existed, or one address left + // behind by an agent that has since rotated its noise key + let (Some(v4_identity), Some(v6_identity)) = ( + &v4_entry.bs58_ed25519_identity, + &v6_entry.bs58_ed25519_identity, + ) else { + error!( + "the agent using noise key {bs58_noise_key} has an authorised address ({addresses:?}) with no announced ed25519 identity - ignoring it until it re-announces itself" + ); + continue; + }; + + if v4_identity != v6_identity { + error!( + "the agent using noise key {bs58_noise_key} announced different ed25519 identities under its two addresses ({v4_identity} / {v6_identity}) - ignoring it until it re-announces itself" + ); + continue; + } + let noise_key = x25519::PublicKey::from_base58_string(&bs58_noise_key)?; + let ed25519_identity = ed25519::PublicKey::from_base58_string(v4_identity)?; agents_map.insert( - *mix_v4, + v4_entry.mixnet_address, KnownAgent { - mix_v6: *mix_v6, + mix_v6: v6_entry.mixnet_address, // the on-chain authorisation timestamp says nothing about liveness, // so treat a restored entry as freshly active last_active_at: OffsetDateTime::now_utc(), noise_key, + ed25519_identity, announced: true, }, ); @@ -203,6 +249,9 @@ pub(crate) struct KnownAgent { pub(crate) last_active_at: OffsetDateTime, pub(crate) noise_key: x25519::PublicKey, + /// The ed25519 identity this agent presents when opening a gateway client session. + pub(crate) ed25519_identity: ed25519::PublicKey, + /// Whether this agent has been successfully registered in the smart contract, under both of /// its addresses. Set to `true` when restored from the chain at startup, or after a successful /// `/announce` contract transaction. @@ -529,6 +578,10 @@ mod tests { x25519::PublicKey::from(&x25519::PrivateKey::new(&mut seeded_rng([seed; 32]))) } + fn identity(seed: u8) -> ed25519::PublicKey { + *ed25519::KeyPair::new(&mut seeded_rng([seed; 32])).public_key() + } + fn addresses(port: u16) -> AgentMixAddresses { AgentMixAddresses { v4: format!("1.1.1.1:{port}").parse().unwrap(), @@ -539,6 +592,7 @@ mod tests { fn authorisation( address: SocketAddr, noise_key: x25519::PublicKey, + identity: Option, ) -> AuthorisedNetworkMonitor { AuthorisedNetworkMonitor { mixnet_address: address, @@ -546,7 +600,7 @@ mod tests { authorised_at: Timestamp::from_seconds(42), bs58_x25519_noise: noise_key.to_base58_string(), noise_version: 1, - bs58_ed25519_identity: None, + bs58_ed25519_identity: identity.map(|key| key.to_base58_string()), } } @@ -556,14 +610,14 @@ mod tests { let addresses = addresses(1789); let key = noise_key(1); - assert!(!agents.try_announce_agent(addresses, key).await); + assert!(!agents.try_announce_agent(addresses, key, identity(1)).await); assert!(!agents.get_agent(addresses).await.unwrap().announced); agents.mark_announced(addresses).await; assert!(agents.get_agent(addresses).await.unwrap().announced); // a repeated announcement is now a no-op, so the caller skips the contract txs - assert!(agents.try_announce_agent(addresses, key).await); + assert!(agents.try_announce_agent(addresses, key, identity(1)).await); } // agents deployed on the same host share its ipv4 address and are only told apart by the port, @@ -575,9 +629,13 @@ mod tests { let second = addresses(1790); assert_eq!(first.v4.ip(), second.v4.ip()); - agents.try_announce_agent(first, noise_key(1)).await; + agents + .try_announce_agent(first, noise_key(1), identity(1)) + .await; agents.mark_announced(first).await; - agents.try_announce_agent(second, noise_key(2)).await; + agents + .try_announce_agent(second, noise_key(2), identity(2)) + .await; assert!(agents.get_agent(first).await.unwrap().announced); assert!(!agents.get_agent(second).await.unwrap().announced); @@ -590,7 +648,7 @@ mod tests { let announced = addresses(1789); let key = noise_key(1); - agents.try_announce_agent(announced, key).await; + agents.try_announce_agent(announced, key, identity(1)).await; agents.mark_announced(announced).await; let other_v6 = AgentMixAddresses { @@ -611,7 +669,9 @@ mod tests { ] { let agents = KnownAgents::default(); let announced = addresses(1789); - agents.try_announce_agent(announced, noise_key(1)).await; + agents + .try_announce_agent(announced, noise_key(1), identity(1)) + .await; agents.mark_announced(announced).await; // either the v6 address or the noise key differs from what we have cached @@ -620,7 +680,7 @@ mod tests { } else { noise_key(1) }; - assert!(!agents.try_announce_agent(changed, key).await); + assert!(!agents.try_announce_agent(changed, key, identity(1)).await); let agent = agents.get_agent(changed).await.unwrap(); assert!(!agent.announced); @@ -629,28 +689,55 @@ mod tests { } } + // the identity is what a gateway keys the unmetered monitor session on, so an announcement + // carrying a new one has to reach the contract even though every other detail is unchanged. + // clearing the announced flag is what makes the caller re-authorise BOTH addresses + #[tokio::test] + async fn a_changed_identity_alone_requires_a_re_announcement() { + let agents = KnownAgents::default(); + let announced = addresses(1789); + + agents + .try_announce_agent(announced, noise_key(1), identity(1)) + .await; + agents.mark_announced(announced).await; + + assert!( + !agents + .try_announce_agent(announced, noise_key(1), identity(2)) + .await + ); + + let agent = agents.get_agent(announced).await.unwrap(); + assert!(!agent.announced); + assert_eq!(agent.ed25519_identity, identity(2)); + } + #[tokio::test] async fn on_chain_pairs_are_recovered_via_the_noise_key() { let first = addresses(1789); let second = addresses(1790); let (first_key, second_key) = (noise_key(1), noise_key(2)); + let (first_identity, second_identity) = (identity(1), identity(2)); let restored = KnownAgents::try_from(vec![ - authorisation(second.v6, second_key), - authorisation(first.v4, first_key), - authorisation(second.v4, second_key), - authorisation(first.v6, first_key), + authorisation(second.v6, second_key, Some(second_identity)), + authorisation(first.v4, first_key, Some(first_identity)), + authorisation(second.v4, second_key, Some(second_identity)), + authorisation(first.v6, first_key, Some(first_identity)), ]) .unwrap(); let restored_first = restored.get_agent(first).await.unwrap(); assert_eq!(restored_first.mix_v6, first.v6); assert_eq!(restored_first.noise_key, first_key); + assert_eq!(restored_first.ed25519_identity, first_identity); assert!(restored_first.announced); let restored_second = restored.get_agent(second).await.unwrap(); assert_eq!(restored_second.mix_v6, second.v6); assert_eq!(restored_second.noise_key, second_key); + assert_eq!(restored_second.ed25519_identity, second_identity); assert!(restored_second.announced); } @@ -661,16 +748,44 @@ mod tests { async fn unpaired_on_chain_records_are_dropped() { let agent = addresses(1789); let key = noise_key(1); + let id = Some(identity(1)); - let v4_only = KnownAgents::try_from(vec![authorisation(agent.v4, key)]).unwrap(); + let v4_only = KnownAgents::try_from(vec![authorisation(agent.v4, key, id)]).unwrap(); assert!(v4_only.get_agent(agent).await.is_none()); let stale_v6 = KnownAgents::try_from(vec![ - authorisation(agent.v4, key), - authorisation(agent.v6, key), - authorisation("[bbbb::1]:1789".parse().unwrap(), key), + authorisation(agent.v4, key, id), + authorisation(agent.v6, key, id), + authorisation("[bbbb::1]:1789".parse().unwrap(), key, id), ]) .unwrap(); assert!(stale_v6.get_agent(agent).await.is_none()); } + + // a pair that doesn't carry one identity across both entries is dropped rather than restored + // without one: an entry predating the field would otherwise be cached as announced, and the + // orchestrator would hand work to an agent whose on-chain entry can't grant a gateway session. + // the next announcement rebuilds it complete, and the contract's upsert overwrites in place + #[tokio::test] + async fn an_on_chain_pair_without_one_identity_is_dropped() { + let agent = addresses(1789); + let key = noise_key(1); + + for (v4_identity, v6_identity) in [ + // authorised before the identity field existed + (None, None), + // only one of the two addresses has been re-authorised since + (Some(identity(1)), None), + (None, Some(identity(1))), + // a half-written pair: the two entries disagree on who the agent is + (Some(identity(1)), Some(identity(2))), + ] { + let restored = KnownAgents::try_from(vec![ + authorisation(agent.v4, key, v4_identity), + authorisation(agent.v6, key, v6_identity), + ]) + .unwrap(); + assert!(restored.get_agent(agent).await.is_none()); + } + } } diff --git a/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/orchestrator/prometheus.rs b/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/orchestrator/prometheus.rs index 73ee93facd4..9d49a8f47e8 100644 --- a/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/orchestrator/prometheus.rs +++ b/nym-network-monitor-v3/nym-network-monitor-orchestrator/src/orchestrator/prometheus.rs @@ -140,7 +140,7 @@ pub enum PrometheusMetric { AgentContractAnnounceFailures, #[strum(props( - help = "The number of announcements that changed the cached noise key or ipv6 address of an already known agent (its previous authorisation may be left stale in the contract)" + help = "The number of announcements that changed the cached noise key, ipv6 address or ed25519 identity of an already known agent (its previous authorisation may be left stale in the contract)" ))] AgentDetailsChanged, diff --git a/openspec/changes/network-monitor-liveness-tests/specs/nym-network-monitor/spec.md b/openspec/changes/network-monitor-liveness-tests/specs/nym-network-monitor/spec.md index f259f8d4d84..922d1966530 100644 --- a/openspec/changes/network-monitor-liveness-tests/specs/nym-network-monitor/spec.md +++ b/openspec/changes/network-monitor-liveness-tests/specs/nym-network-monitor/spec.md @@ -171,7 +171,7 @@ The per-kind last-tested timestamp MUST be stored directly rather than read thro The agent registry MUST NOT be persisted; it lives only in the in-memory `KnownAgents` cache and is rebuilt from the contract on each startup, which means agents' announced flags reset across a restart and each agent re-announces (and is re-authorised on-chain) on its next run. -Rehydrating that cache from the contract requires recovering which pair of on-chain entries belongs to one agent. The contract stores one entry per socket address and carries no field linking an agent's two addresses, so the orchestrator MUST group the entries by their x25519 noise key, which is unique per agent (see the network-monitors-contract capability, which does NOT enforce that uniqueness). Entries that do not form exactly one ipv4/ipv6 pair MUST be dropped from the cache rather than guessed at - they are either authorisations predating the paired announcement or leftovers from an agent that has since changed an address - which is safe precisely because the cache only exists to skip redundant contract transactions, and an agent always announces before requesting work. +Rehydrating that cache from the contract requires recovering which pair of on-chain entries belongs to one agent. The contract stores one entry per socket address and carries no field linking an agent's two addresses, so the orchestrator MUST group the entries by their x25519 noise key, which is unique per agent (see the network-monitors-contract capability, which does NOT enforce that uniqueness). Entries that do not form exactly one ipv4/ipv6 pair MUST be dropped from the cache rather than guessed at - they are either authorisations predating the paired announcement or leftovers from an agent that has since changed an address - which is safe precisely because the cache only exists to skip redundant contract transactions, and an agent always announces before requesting work. The identity key is subject to the same rule: a pair whose two entries do not both carry the SAME identity MUST be dropped, an absent one being an authorisation predating the field and a disagreeing one being a half-written pair. Dropping rather than tolerating is what keeps every cached entry complete, so no consumer has to reason about a rehydrated agent whose identity is unknown, and it costs only the one redundant authorisation transaction the drop rule already accepts. #### Scenario: Each kind keeps its own staleness and rotation position - **WHEN** a node has been tested by both kinds @@ -189,6 +189,10 @@ Rehydrating that cache from the contract requires recovering which pair of on-ch - **WHEN** an agent has an on-chain entry with no counterpart of the other family under the same noise key - **THEN** it is left out of the rehydrated cache and re-created by that agent's next announcement, at the cost of one redundant authorisation transaction +#### Scenario: An on-chain pair without one identity is dropped rather than half-restored +- **WHEN** an agent's on-chain entries were written before the identity field existed, or carry identities that differ from one another +- **THEN** the pair is left out of the rehydrated cache and re-created, complete with its identity, by that agent's next announcement, with no revocation of the old entries because the authorisation save overwrites them in place + #### Scenario: Node registry and results survive a restart - **WHEN** the orchestrator restarts - **THEN** its node registry, per-kind work state, completed testruns with their signals, and per-kind submission watermarks are loaded from SQLite @@ -261,11 +265,11 @@ The staleness gate is per NODE AND KIND while the rotation is per ADDRESS, so a An agent SHALL announce a PAIR of mixnet socket addresses, one ipv4 and one ipv6, because a tested node sees whichever family it was reached over as the source of the probe traffic and gates on that source ip; authorising only one family would leave probes over the other rejected. An agent SHALL additionally announce the base58 ed25519 CLIENT IDENTITY public key it will present when opening a gateway client session, derived from its noise key rather than provisioned, so that the gateway session exemption can be keyed on a verified identity instead of a source address. -On `POST /v1/agent/announce` the orchestrator SHALL reject with a 400, before touching any state, an announcement whose addresses are not one plain ipv4 address and one ipv6 address that is not ipv4-mapped. Such a pair MUST NOT be normalised into shape, because an ipv4-mapped ipv6 address collapses onto the ipv4 one when a node canonicalises the authorised set, leaving the agent with a single authorised ingress while both the contract and the orchestrator believe it has two, and because rewriting an address would authorise something the agent never announced and will not use in its sphinx return hop. An announced identity key that is not valid base58 decoding to 32 bytes MUST likewise be rejected with a 400 at the same point. +On `POST /v1/agent/announce` the orchestrator SHALL reject with a 400, before touching any state, an announcement whose addresses are not one plain ipv4 address and one ipv6 address that is not ipv4-mapped. Such a pair MUST NOT be normalised into shape, because an ipv4-mapped ipv6 address collapses onto the ipv4 one when a node canonicalises the authorised set, leaving the agent with a single authorised ingress while both the contract and the orchestrator believe it has two, and because rewriting an address would authorise something the agent never announced and will not use in its sphinx return hop. An announced identity key that is not valid base58 decoding to 32 bytes MUST likewise be rejected before any state is touched, but NOT by a check at that same point: the announce request MUST carry the identity as a typed ed25519 public key rather than an unvalidated string, so the malformed case is rejected during request deserialisation and is unrepresentable by the time the handler runs. The address pair cannot be handled that way because its rule is a relation between two individually well-formed addresses. The consequence is that a malformed identity answers with the deserialisation rejection's status rather than this 400, which is accepted: the property the requirement exists to protect is that nothing is cached and nothing is written on-chain, and that holds more strongly when the handler is never entered at all. -It MUST then upsert the agent into its in-memory `KnownAgents` cache, keyed by the agent's ipv4 mixnet socket address with the ipv6 address and the identity key held inside the entry, and, if the agent was not already announced, MUST authorise BOTH addresses in the network-monitors contract by submitting ONE transaction carrying an `AuthoriseNetworkMonitor` message per address, each with the agent's base58 x25519 noise key, noise version, and identity key, then mark the agent announced. Both authorisations MUST travel in a single transaction so that an agent is never left with only one of its addresses authorised. A contract transaction failure MUST surface as a 500 and leave the agent un-announced; re-announcing is safe because the contract's agent save is an upsert. An agent whose announced noise key, ipv6 address, OR identity key differs from the cached one MUST have its announced flag reset so it is re-authorised, and that divergence SHOULD be surfaced (log plus counter) because a superseded ipv6 address stays authorised in the contract. This on-chain write is what ultimately causes network nodes to accept the agent's probe connections and to recognise its client sessions. +It MUST then upsert the agent into its in-memory `KnownAgents` cache, keyed by the agent's ipv4 mixnet socket address with the ipv6 address and the identity key held inside the entry, and, if the agent was not already announced, MUST authorise BOTH addresses in the network-monitors contract by submitting ONE transaction carrying an `AuthoriseNetworkMonitor` message per address, each with the agent's base58 x25519 noise key, noise version, and identity key, then mark the agent announced. Both authorisations MUST travel in a single transaction so that an agent is never left with only one of its addresses authorised. A contract transaction failure MUST surface as a 500 and leave the agent un-announced; re-announcing is safe because the contract's agent save is an upsert. An agent whose announced noise key, ipv6 address, OR identity key differs from the cached one MUST have its announced flag reset so it is re-authorised, and that divergence SHOULD be surfaced (log plus counter) because a superseded ipv6 address stays authorised in the contract. One counter covering all three is sufficient, and the identity MUST NOT get a series of its own: it is derived from the noise key, so it cannot diverge unless the noise key does, and a dedicated series would read flat zero outside a change to the derivation itself. The log MUST still name which of them diverged, since that is where the distinction is actually needed. This on-chain write is what ultimately causes network nodes to accept the agent's probe connections and to recognise its client sessions. -Because the identity is carried on an OPTIONAL contract field and the save is an upsert, agents authorised before the field existed acquire it on their next announcement with no backfill step. The orchestrator MUST NOT treat a cached entry without an identity as invalid, since it may have been rehydrated from such an entry after a restart. +Because the identity is carried on an OPTIONAL contract field and the save is an upsert keyed by socket address, agents authorised before the field existed acquire it on their next announcement with no backfill step and with no revocation of the old entry: the announcement rewrites both of the agent's entries in place. A cached entry therefore ALWAYS carries an identity, because the rehydration drop rule above leaves an identity-less on-chain entry out of the cache rather than admitting one that no consumer could use. #### Scenario: A first announcement authorises both addresses on-chain - **WHEN** a not-yet-announced agent calls `announce` @@ -281,7 +285,7 @@ Because the identity is carried on an OPTIONAL contract field and the save is an #### Scenario: A malformed identity key is rejected outright - **WHEN** an agent announces an identity key that is not valid base58 decoding to 32 bytes -- **THEN** the call is rejected with a 400 and nothing is cached or written on-chain +- **THEN** the request is rejected while being deserialised, so the handler never runs and nothing is cached or written on-chain #### Scenario: A changed identity key triggers re-authorisation - **WHEN** an already-announced agent announces an identity key that differs from the cached one diff --git a/openspec/changes/network-monitor-liveness-tests/tasks.md b/openspec/changes/network-monitor-liveness-tests/tasks.md index 2b557be9d9c..65f3f75b7c4 100644 --- a/openspec/changes/network-monitor-liveness-tests/tasks.md +++ b/openspec/changes/network-monitor-liveness-tests/tasks.md @@ -13,11 +13,11 @@ - [x] 2.5 Extend the validator-client signing helper so the authorisation message carries the identity - [x] 2.6 Contract tests: an omitted identity is accepted, a malformed one is rejected, a re-authorisation records a changed identity, and an entry serialised without the field deserialises with `None` - [x] 2.7 Add a regression test asserting that a serialised new-form `AuthoriseNetworkMonitor` still deserialises into a struct shaped like the old one, so the fleet-compatibility assumption behind this change is checked in CI rather than assumed -- [ ] 2.8 Agent side: derive the ed25519 identity from the x25519 noise private key via a labelled HKDF whose output is the ed25519 seed, and include its base58 public key in the announce request -- [ ] 2.9 Orchestrator side: carry the identity on the announce request and in both `AuthoriseNetworkMonitor` messages of the existing single transaction, reject a malformed identity with a 400 before touching state, hold it in the `KnownAgents` entry, and reset the announced flag when it diverges from the cached one -- [ ] 2.10 Tolerate a rehydrated cache entry that has no identity, since it may come from an entry authorised before the field existed -- [ ] 2.11 Add a counter for identity divergence alongside the existing agent-details-changed counter -- [ ] 2.12 Unit-test that an announcement with a malformed identity is rejected before any cache write or contract call, and that a changed identity re-authorises both addresses +- [x] 2.8 Agent side: derive the ed25519 identity from the x25519 noise private key via a labelled HKDF whose output is the ed25519 seed, and include its base58 public key in the announce request +- [x] 2.9 Orchestrator side: carry the identity on the announce request and in both `AuthoriseNetworkMonitor` messages of the existing single transaction, reject a malformed identity before touching state, hold it in the `KnownAgents` entry, and reset the announced flag when it diverges from the cached one +- [x] 2.10 Drop a rehydrated on-chain pair that does not carry one identity across both of its entries, rather than admitting a cache entry no consumer could use. Superseded the original "tolerate an identity-less entry": the rehydration already drops an unpairable entry and lets the next announcement rebuild it, and the contract's save is an upsert keyed by socket address, so an entry predating the field is overwritten in place with no revocation step. Keeping the cached identity non-optional also stops the orchestrator handing work to an agent whose on-chain entry cannot grant a gateway session +- [x] 2.11 Count identity divergence on the existing agent-details-changed counter, and distinguish it from a noise-key or address change in the log rather than in a second series. Superseded the original "add a counter for identity divergence": the identity is derived from the noise key, so it cannot diverge without the noise key diverging, and the rehydration drop rule from 2.10 removed the one case (an identity-less entry restored after a restart) that would have given a dedicated series real traffic +- [x] 2.12 Unit-test that a changed identity alone resets the announced flag so both addresses are re-authorised, that a rehydrated on-chain pair not carrying one shared identity is dropped (absent on either entry, or disagreeing across the two), and that the agent's derivation matches a pinned known-answer vector, so a change to the HKDF label or algorithm fails in CI rather than silently invalidating every identity already announced on chain. The malformed-identity half of the original wording has no seam left to test: the announce request carries a typed `ed25519::PublicKey`, so a malformed value is rejected during deserialisation and is unrepresentable in the handler - testing it would only exercise `serde` ## 3. Shared request/response types