From 0662a6a62d55c8e92c7593b3542433ee0d549b68 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/node.rs | 2 +- mgmtd/src/bee_msg/target.rs | 28 +++++++++++++---- mgmtd/src/db/import_v7.rs | 9 ++---- mgmtd/src/db/schema/4.sql | 1 + mgmtd/src/db/target.rs | 59 ++++++++++++------------------------ shared/src/bee_msg/target.rs | 2 +- 6 files changed, 46 insertions(+), 55 deletions(-) create mode 100644 mgmtd/src/db/schema/4.sql diff --git a/mgmtd/src/bee_msg/node.rs b/mgmtd/src/bee_msg/node.rs index 338ccfb..a3b5ed6 100644 --- a/mgmtd/src/bee_msg/node.rs +++ b/mgmtd/src/bee_msg/node.rs @@ -246,7 +246,7 @@ client version < 8.0)" ); }; - db::target::insert_meta(tx, target_id, None)?; + db::target::insert_meta(tx, target_id)?; } (node, true) diff --git a/mgmtd/src/bee_msg/target.rs b/mgmtd/src/bee_msg/target.rs index a616045..3cb0d7e 100644 --- a/mgmtd/src/bee_msg/target.rs +++ b/mgmtd/src/bee_msg/target.rs @@ -127,6 +127,8 @@ impl HandleWithResponse for RegisterTarget { let (id, is_new) = ctx .db .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, @@ -134,6 +136,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)); } @@ -142,11 +162,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, )) }) @@ -155,7 +171,7 @@ impl HandleWithResponse for RegisterTarget { if is_new { log::info!("Registered new storage target with Id {id}"); } else { - log::debug!("Re-registered existing storage target with Id {id}"); + log::debug!("Re-registered existing storage target with Id {id}",); } Ok(RegisterTargetResp { id }) diff --git a/mgmtd/src/db/import_v7.rs b/mgmtd/src/db/import_v7.rs index 5f92ea7..5b784c7 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)?; } 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..7ed777b 100644 --- a/mgmtd/src/db/target.rs +++ b/mgmtd/src/db/target.rs @@ -37,17 +37,9 @@ pub(crate) fn validate_ids( /// /// BeeGFS doesn't really support meta targets at the moment, so there always must be exactly one /// meta target per meta node with their IDs being the same. -pub(crate) fn insert_meta( - tx: &Transaction, - target_id: TargetId, - alias: Option, -) -> Result<()> { +pub(crate) fn insert_meta(tx: &Transaction, target_id: TargetId) -> 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 +47,7 @@ pub(crate) fn insert_meta( insert( tx, target_id, - alias, + None, NodeTypeServer::Meta, Some(target_id.into()), )?; @@ -78,45 +70,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 +110,8 @@ pub(crate) fn insert( Some(1) } else { None - } + }, + reg_token ], )?; @@ -287,11 +271,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).unwrap_err(); + super::insert_meta(tx, 99).unwrap(); + // existing id + super::insert_meta(tx, 99).unwrap_err(); let targets: i64 = tx .query_row(sql!("SELECT COUNT(*) FROM meta_targets"), [], |row| { @@ -306,15 +289,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 3c909179eae9b94d503dc49b5507660c0d63aa9e 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/node.rs | 42 +++++++++++++++++++++++++++++++++++---- mgmtd/src/db/import_v7.rs | 2 +- mgmtd/src/db/target.rs | 14 ++++++++----- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/mgmtd/src/bee_msg/node.rs b/mgmtd/src/bee_msg/node.rs index a3b5ed6..2f69652 100644 --- a/mgmtd/src/bee_msg/node.rs +++ b/mgmtd/src/bee_msg/node.rs @@ -195,10 +195,46 @@ async fn update_node(msg: RegisterNode, ctx: &Context) -> Result { } } + let new_alias_or_reg_token = String::from_utf8(msg.node_alias)?; + let (node, is_new) = if let Some(node) = node { // Existing node, update data db::node::update(tx, node.uid, msg.port, machine_uuid)?; + // If the updated node is a meta node, check if its corresponding target has a + // registration token + if msg.node_type == NodeType::Meta { + let stored_reg_token: Option = 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 @@ -216,9 +252,7 @@ async fn update_node(msg: RegisterNode, ctx: &Context) -> Result { // updated to no longer start with a number, thus it is unlikely this // would happen unless BeeGFS 8 was mounted by a BeeGFS 7 client. - let new_alias = String::from_utf8(msg.node_alias) - .ok() - .and_then(|s| Alias::try_from(s).ok()); + let new_alias = Alias::try_from(new_alias_or_reg_token.clone()).ok(); if new_alias.is_none() { log::warn!( @@ -246,7 +280,7 @@ client version < 8.0)" ); }; - db::target::insert_meta(tx, target_id)?; + db::target::insert_meta(tx, target_id, Some(&new_alias_or_reg_token))?; } (node, true) diff --git a/mgmtd/src/db/import_v7.rs b/mgmtd/src/db/import_v7.rs index 5b784c7..3de8e85 100644 --- a/mgmtd/src/db/import_v7.rs +++ b/mgmtd/src/db/import_v7.rs @@ -144,7 +144,7 @@ fn meta_nodes(tx: &Transaction, f: &Path) -> Result<(NodeId, bool)> { ); }; - target::insert_meta(tx, target_id)?; + target::insert_meta(tx, target_id, None)?; } if root_id == 0 { diff --git a/mgmtd/src/db/target.rs b/mgmtd/src/db/target.rs index 7ed777b..7224746 100644 --- a/mgmtd/src/db/target.rs +++ b/mgmtd/src/db/target.rs @@ -37,7 +37,11 @@ pub(crate) fn validate_ids( /// /// BeeGFS doesn't really support meta targets at the moment, so there always must be exactly one /// meta target per meta node with their IDs being the same. -pub(crate) fn insert_meta(tx: &Transaction, target_id: TargetId) -> Result<()> { +pub(crate) fn insert_meta( + tx: &Transaction, + target_id: TargetId, + 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 { @@ -47,7 +51,7 @@ pub(crate) fn insert_meta(tx: &Transaction, target_id: TargetId) -> Result<()> { insert( tx, target_id, - None, + reg_token, NodeTypeServer::Meta, Some(target_id.into()), )?; @@ -271,10 +275,10 @@ mod test { #[test] fn set_get_meta() { with_test_data(|tx| { - super::insert_meta(tx, 1).unwrap_err(); - super::insert_meta(tx, 99).unwrap(); + super::insert_meta(tx, 1, None).unwrap_err(); + super::insert_meta(tx, 99, None).unwrap(); // existing id - super::insert_meta(tx, 99).unwrap_err(); + super::insert_meta(tx, 99, None).unwrap_err(); let targets: i64 = tx .query_row(sql!("SELECT COUNT(*) FROM meta_targets"), [], |row| {