From 2eba09326c22e49602042ea8fc9dbbf0ec5e9777 Mon Sep 17 00:00:00 2001 From: Rusty Bee <145002912+rustybee42@users.noreply.github.com> Date: Fri, 24 Oct 2025 13:49:34 +0200 Subject: [PATCH 1/2] fix: Use registration token to prevent double target id use On storage target registration, the node sends a "node string id" / alias which is generated locally on the node. This commit uses that as a registration token to "ensure" that a once registered target id is not reused with a different target - if the registration token is already known, the RegisterNodeMsg will fail. This restores old managements behavior. Note that the RegisterNodeMsg actually doesn't do anything on an already known target (and didn't before this commit) besides logging. It is up to the server nodes to abort on a failing RegisterNodeMsg. --- mgmtd/src/bee_msg/register_target.rs | 26 ++++++++++--- mgmtd/src/db/import_v7.rs | 9 +---- mgmtd/src/db/schema/4.sql | 1 + mgmtd/src/db/target.rs | 55 ++++++++++------------------ shared/src/bee_msg/target.rs | 2 +- 5 files changed, 44 insertions(+), 49 deletions(-) create mode 100644 mgmtd/src/db/schema/4.sql diff --git a/mgmtd/src/bee_msg/register_target.rs b/mgmtd/src/bee_msg/register_target.rs index b361bf3..bf577d6 100644 --- a/mgmtd/src/bee_msg/register_target.rs +++ b/mgmtd/src/bee_msg/register_target.rs @@ -11,6 +11,8 @@ impl HandleWithResponse for RegisterTarget { let (id, is_new) = app .write_tx(move |tx| { + let reg_token = str::from_utf8(&self.reg_token)?; + // Do not do anything if the target already exists if let Some(id) = try_resolve_num_id( tx, @@ -18,6 +20,24 @@ impl HandleWithResponse for RegisterTarget { NodeType::Storage, self.target_id.into(), )? { + let stored_reg_token: Option = tx.query_row( + sql!( + "SELECT reg_token FROM targets + WHERE target_id = ?1 AND node_type = ?2" + ), + rusqlite::params![id.num_id(), NodeType::Storage.sql_variant()], + |row| row.get(0), + )?; + + if let Some(st) = stored_reg_token + && st != reg_token + { + bail!( + "Storage target {id} has already been registered and its \ +registration token ({reg_token}) does not match the stored token ({st})" + ); + } + return Ok((id.num_id().try_into()?, false)); } @@ -26,11 +46,7 @@ impl HandleWithResponse for RegisterTarget { } Ok(( - db::target::insert_storage( - tx, - self.target_id, - Some(format!("target_{}", std::str::from_utf8(&self.alias)?).try_into()?), - )?, + db::target::insert_storage(tx, self.target_id, Some(reg_token))?, true, )) }) diff --git a/mgmtd/src/db/import_v7.rs b/mgmtd/src/db/import_v7.rs index 7917b57..6da8d85 100644 --- a/mgmtd/src/db/import_v7.rs +++ b/mgmtd/src/db/import_v7.rs @@ -143,13 +143,8 @@ fn meta_nodes(tx: &Transaction, f: &Path) -> Result<(NodeId, bool)> { "{num_id} is not a valid numeric meta node/target id (must be between 1 and 65535)", ); }; - target::insert( - tx, - target_id, - None, - NodeTypeServer::Meta, - Some(target_id.into()), - )?; + + target::insert_meta(tx, target_id, None)?; } if root_id == 0 { diff --git a/mgmtd/src/db/schema/4.sql b/mgmtd/src/db/schema/4.sql new file mode 100644 index 0000000..5cfe284 --- /dev/null +++ b/mgmtd/src/db/schema/4.sql @@ -0,0 +1 @@ +ALTER TABLE targets ADD COLUMN reg_token TEXT; diff --git a/mgmtd/src/db/target.rs b/mgmtd/src/db/target.rs index 81596d9..7224746 100644 --- a/mgmtd/src/db/target.rs +++ b/mgmtd/src/db/target.rs @@ -40,14 +40,10 @@ pub(crate) fn validate_ids( pub(crate) fn insert_meta( tx: &Transaction, target_id: TargetId, - alias: Option, + reg_token: Option<&str>, ) -> Result<()> { let target_id = if target_id == 0 { misc::find_new_id(tx, "targets", "target_id", NodeType::Meta, 1..=0xFFFF)? - } else if try_resolve_num_id(tx, EntityType::Target, NodeType::Meta, target_id.into())? - .is_some() - { - bail!(TypedError::value_exists("numeric target id", target_id)); } else { target_id }; @@ -55,7 +51,7 @@ pub(crate) fn insert_meta( insert( tx, target_id, - alias, + reg_token, NodeTypeServer::Meta, Some(target_id.into()), )?; @@ -78,45 +74,36 @@ pub(crate) fn insert_meta( pub(crate) fn insert_storage( tx: &Transaction, target_id: TargetId, - alias: Option, + reg_token: Option<&str>, ) -> Result { let target_id = if target_id == 0 { misc::find_new_id(tx, "targets", "target_id", NodeType::Storage, 1..=0xFFFF)? - } else if try_resolve_num_id(tx, EntityType::Target, NodeType::Storage, target_id.into())? - .is_some() - { - return Ok(target_id); } else { target_id }; - insert(tx, target_id, alias, NodeTypeServer::Storage, None)?; + insert(tx, target_id, reg_token, NodeTypeServer::Storage, None)?; Ok(target_id) } -pub(crate) fn insert( +fn insert( tx: &Transaction, target_id: TargetId, - alias: Option, + reg_token: Option<&str>, node_type: NodeTypeServer, // This is optional because storage targets come "unmapped" node_id: Option, ) -> Result<()> { anyhow::ensure!(target_id > 0, "A target id must be > 0"); - let alias = if let Some(alias) = alias { - alias - } else { - format!("target_{}_{}", node_type.user_str(), target_id).try_into()? - }; - + let alias = format!("target_{}_{target_id}", node_type.user_str()).try_into()?; let new_uid = entity::insert(tx, EntityType::Target, &alias)?; tx.execute( sql!( - "INSERT INTO targets (target_uid, node_type, target_id, node_id, pool_id) - VALUES (?1, ?2, ?3, ?4, ?5)" + "INSERT INTO targets (target_uid, node_type, target_id, node_id, pool_id, reg_token) + VALUES (?1, ?2, ?3, ?4, ?5, ?6)" ), params![ new_uid, @@ -127,7 +114,8 @@ pub(crate) fn insert( Some(1) } else { None - } + }, + reg_token ], )?; @@ -287,11 +275,10 @@ mod test { #[test] fn set_get_meta() { with_test_data(|tx| { - super::insert_meta(tx, 1, Some("existing_meta_target".try_into().unwrap())) - .unwrap_err(); - super::insert_meta(tx, 99, Some("new_meta_target".try_into().unwrap())).unwrap(); - // existing alias - super::insert_meta(tx, 99, Some("new_meta_target".try_into().unwrap())).unwrap_err(); + super::insert_meta(tx, 1, None).unwrap_err(); + super::insert_meta(tx, 99, None).unwrap(); + // existing id + super::insert_meta(tx, 99, None).unwrap_err(); let targets: i64 = tx .query_row(sql!("SELECT COUNT(*) FROM meta_targets"), [], |row| { @@ -306,15 +293,11 @@ mod test { #[test] fn set_get_storage_and_map() { with_test_data(|tx| { - let new_target_id = - super::insert_storage(tx, 0, Some("new_storage_target".try_into().unwrap())) - .unwrap(); - super::insert_storage(tx, 1000, Some("new_storage_target_2".try_into().unwrap())) - .unwrap(); + let new_target_id = super::insert_storage(tx, 0, Some("new_storage_target")).unwrap(); + super::insert_storage(tx, 1000, Some("new_storage_target_2")).unwrap(); - // existing alias - super::insert_storage(tx, 0, Some("new_storage_target".try_into().unwrap())) - .unwrap_err(); + // existing id + super::insert_storage(tx, 1000, Some("new_storage_target")).unwrap_err(); super::update_storage_node_mappings(tx, &[new_target_id, 1000], 1).unwrap(); diff --git a/shared/src/bee_msg/target.rs b/shared/src/bee_msg/target.rs index da806f3..62b166f 100644 --- a/shared/src/bee_msg/target.rs +++ b/shared/src/bee_msg/target.rs @@ -94,7 +94,7 @@ impl Deserializable for TargetReachabilityState { #[derive(Clone, Debug, Default, PartialEq, Eq, BeeSerde)] pub struct RegisterTarget { #[bee_serde(as = CStr<0>)] - pub alias: Vec, + pub reg_token: Vec, pub target_id: TargetId, } From 35cac3a450ff7d13aedbd5fd115ecee53825625a Mon Sep 17 00:00:00 2001 From: Rusty Bee <145002912+rustybee42@users.noreply.github.com> Date: Fri, 7 Nov 2025 10:01:51 +0100 Subject: [PATCH 2/2] fix: Use registration token to prevent double meta node/target id use This takes the previously ignored node_alias field on received heartbeats from meta nodes (only!) and stores it together with the meta target as a registration token. When the field is already filled, a mismatch will fail the heartbeat. --- mgmtd/src/bee_msg/common.rs | 42 +++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/mgmtd/src/bee_msg/common.rs b/mgmtd/src/bee_msg/common.rs index 2352ec7..6a96765 100644 --- a/mgmtd/src/bee_msg/common.rs +++ b/mgmtd/src/bee_msg/common.rs @@ -51,10 +51,46 @@ pub(super) async fn update_node(msg: RegisterNode, app: &impl App) -> Result = tx.query_row( + sql!("SELECT reg_token FROM meta_targets WHERE node_id = ?1"), + [node.num_id()], + |row| row.get(0), + )?; + + if let Some(ref t) = stored_reg_token + && t != &new_alias_or_reg_token + { + bail!( + "Meta node {} has already been registered and its \ +registration token ({}) does not match the stored token ({})", + node, + new_alias_or_reg_token, + t + ); + } else if stored_reg_token.is_none() { + tx.execute( + sql!( + "UPDATE targets SET reg_token = ?1 + WHERE node_id = ?2 AND node_type = ?3" + ), + rusqlite::params![ + new_alias_or_reg_token, + node.num_id(), + NodeType::Meta.sql_variant() + ], + )?; + } + } + (node, false) } else { // New node, do additional checks and insert data @@ -72,9 +108,7 @@ pub(super) async fn update_node(msg: RegisterNode, app: &impl App) -> Result