From becb2880c9e3737e910f384584a15233cdb370f6 Mon Sep 17 00:00:00 2001 From: Narasimhan Venkadeswaran Date: Tue, 12 May 2026 14:04:12 -0700 Subject: [PATCH] switch admin password rotation --- crates/api-test-helper/src/mock_rms.rs | 25 +- crates/api/src/rack/rms_client.rs | 57 +++- .../state_controller/switch/configuring.rs | 265 +++++++++++++++--- .../src/tests/switch_state_controller/mod.rs | 249 +++++++++++++++- 4 files changed, 553 insertions(+), 43 deletions(-) diff --git a/crates/api-test-helper/src/mock_rms.rs b/crates/api-test-helper/src/mock_rms.rs index 65f0d71fee..28e593a0e4 100644 --- a/crates/api-test-helper/src/mock_rms.rs +++ b/crates/api-test-helper/src/mock_rms.rs @@ -176,6 +176,11 @@ pub struct MockRmsApi { enable_scale_up_fabric_telemetry_interface_calls: Mutex>, + // Switch password calls. + update_switch_system_password_responses: + Mutex>>, + update_switch_system_password_calls: Mutex>, + // Version (no request type). version_responses: Mutex>>, version_call_count: Mutex, @@ -255,6 +260,8 @@ impl MockRmsApi { configure_scale_up_fabric_manager_calls: Default::default(), enable_scale_up_fabric_telemetry_interface_responses: Default::default(), enable_scale_up_fabric_telemetry_interface_calls: Default::default(), + update_switch_system_password_responses: Default::default(), + update_switch_system_password_calls: Default::default(), version_responses: Default::default(), version_call_count: Default::default(), } @@ -513,6 +520,16 @@ impl MockRmsApi { rms::EnableScaleUpFabricTelemetryInterfaceResponse ); + // Switch password + impl_enqueue_inspect!( + enqueue_update_switch_system_password, + update_switch_system_password_calls, + update_switch_system_password_responses, + update_switch_system_password_calls, + rms::UpdateSwitchSystemPasswordRequest, + rms::UpdateSwitchSystemPasswordResponse + ); + // Version (special — no request type) pub async fn enqueue_version(&self, resp: Result<(), RackManagerError>) { self.version_responses.lock().await.push_back(resp); @@ -675,9 +692,13 @@ impl RmsApi for MockRmsApi { } async fn update_switch_system_password( &self, - _cmd: rms::UpdateSwitchSystemPasswordRequest, + cmd: rms::UpdateSwitchSystemPasswordRequest, ) -> Result { - Ok(rms::UpdateSwitchSystemPasswordResponse::default()) + self.update_switch_system_password_calls + .lock() + .await + .push(cmd); + pop_or_err(&mut self.update_switch_system_password_responses.lock().await) } async fn get_power_state( &self, diff --git a/crates/api/src/rack/rms_client.rs b/crates/api/src/rack/rms_client.rs index dbc82c81d7..d409630cc2 100644 --- a/crates/api/src/rack/rms_client.rs +++ b/crates/api/src/rack/rms_client.rs @@ -78,6 +78,10 @@ pub mod test_support { Arc>>, queued_set_power_state_by_device_list_responses: Arc>>>, + submitted_switch_system_password_requests: + Arc>>, + queued_switch_system_password_responses: + Arc>>>, } impl Default for RmsSim { @@ -98,6 +102,8 @@ pub mod test_support { queued_set_power_state_by_device_list_responses: Arc::new(Mutex::new( VecDeque::new(), )), + submitted_switch_system_password_requests: Arc::new(Mutex::new(Vec::new())), + queued_switch_system_password_responses: Arc::new(Mutex::new(VecDeque::new())), } } } @@ -137,6 +143,12 @@ pub mod test_support { queued_set_power_state_by_device_list_responses: self .queued_set_power_state_by_device_list_responses .clone(), + submitted_switch_system_password_requests: self + .submitted_switch_system_password_requests + .clone(), + queued_switch_system_password_responses: self + .queued_switch_system_password_responses + .clone(), } } @@ -242,6 +254,16 @@ pub mod test_support { .push_back(response); } + pub async fn queue_update_switch_system_password_response( + &self, + response: Result, + ) { + self.queued_switch_system_password_responses + .lock() + .await + .push_back(response); + } + /// Snapshot the recorded `SetPowerStateByDeviceList` requests, in /// the order they were received. pub async fn submitted_set_power_state_by_device_list_requests( @@ -252,6 +274,15 @@ pub mod test_support { .await .clone() } + + pub async fn submitted_switch_system_password_requests( + &self, + ) -> Vec { + self.submitted_switch_system_password_requests + .lock() + .await + .clone() + } } #[derive(Debug, Clone)] @@ -274,6 +305,10 @@ pub mod test_support { Arc>>, queued_set_power_state_by_device_list_responses: Arc>>>, + submitted_switch_system_password_requests: + Arc>>, + queued_switch_system_password_responses: + Arc>>>, } #[async_trait::async_trait] @@ -310,9 +345,17 @@ pub mod test_support { } async fn update_switch_system_password( &self, - _cmd: rms::UpdateSwitchSystemPasswordRequest, + cmd: rms::UpdateSwitchSystemPasswordRequest, ) -> Result { - Ok(rms::UpdateSwitchSystemPasswordResponse::default()) + self.submitted_switch_system_password_requests + .lock() + .await + .push(cmd); + self.queued_switch_system_password_responses + .lock() + .await + .pop_front() + .unwrap_or_else(|| Ok(successful_switch_system_password_response())) } async fn set_power_state( &self, @@ -572,4 +615,14 @@ pub mod test_support { })) } } + + fn successful_switch_system_password_response() -> rms::UpdateSwitchSystemPasswordResponse { + rms::UpdateSwitchSystemPasswordResponse { + response: Some(rms::NodeBatchResponse { + status: rms::ReturnCode::Success as i32, + message: "Updated switch system password on 0 of 0 devices".to_string(), + ..Default::default() + }), + } + } } diff --git a/crates/api/src/state_controller/switch/configuring.rs b/crates/api/src/state_controller/switch/configuring.rs index 8c22f64d56..98a080b5e5 100644 --- a/crates/api/src/state_controller/switch/configuring.rs +++ b/crates/api/src/state_controller/switch/configuring.rs @@ -19,6 +19,7 @@ use carbide_uuid::switch::SwitchId; use forge_secrets::credentials::{CredentialKey, Credentials}; +use librms::protos::rack_manager as rms; use model::switch::{ConfiguringState, Switch, SwitchControllerState, ValidatingState}; use crate::state_controller::state_handler::{ @@ -26,6 +27,8 @@ use crate::state_controller::state_handler::{ }; use crate::state_controller::switch::context::SwitchStateHandlerContextObjects; +const NVOS_ADMIN_USERNAME: &str = "admin"; + /// Handles the Configuring state for a switch. pub async fn handle_configuring( switch_id: &SwitchId, @@ -59,56 +62,132 @@ async fn handle_rotate_os_password( let key = CredentialKey::SwitchNvosAdmin { bmc_mac_address }; - if let Ok(Some(Credentials::UsernamePassword { .. })) = - ctx.services.credential_manager.get_credentials(&key).await - { - tracing::info!( - "Switch {:?}: NVOS admin credentials already exist in vault for BMC MAC {}", - switch_id, - bmc_mac_address - ); - return Ok(StateHandlerOutcome::transition( - SwitchControllerState::Validating { - validating_state: ValidatingState::ValidationComplete, - }, - )); - } - let mut txn = ctx.services.db_pool.begin().await?; let expected_switch = db::expected_switch::find_by_bmc_mac_address(&mut txn, bmc_mac_address).await?; + let switch_endpoint = + db::switch::find_switch_endpoints_by_ids(txn.as_mut(), std::slice::from_ref(switch_id)) + .await? + .into_iter() + .next(); txn.commit().await?; - //TODO: This logic should be replaced with the logic of rotate password let expected_switch = match expected_switch { Some(es) => es, None => { - return Ok(StateHandlerOutcome::transition( - SwitchControllerState::Error { - cause: format!("No expected switch found for BMC MAC {}", bmc_mac_address), - }, - )); + return Ok(error_transition(format!( + "No expected switch found for BMC MAC {}", + bmc_mac_address + ))); } }; - let (username, password) = match (expected_switch.nvos_username, expected_switch.nvos_password) - { - (Some(username), Some(password)) => (username, password), - _ => { + let target_password = match expected_switch.nvos_password.clone() { + Some(password) if !password.is_empty() => password, + Some(_) => { + return Ok(error_transition(format!( + "Switch {:?}: NVOS admin password is empty for BMC MAC {}", + switch_id, bmc_mac_address + ))); + } + None => { tracing::info!( - "Switch {:?}: no NVOS credentials in vault or expected switch for BMC MAC {}, skipping", + "Switch {:?}: no target NVOS admin password for BMC MAC {}, skipping", switch_id, bmc_mac_address ); - return Ok(StateHandlerOutcome::transition( - SwitchControllerState::Validating { - validating_state: ValidatingState::ValidationComplete, - }, - )); + return Ok(validating_complete_transition()); + } + }; + + let current_credentials = ctx + .services + .credential_manager + .get_credentials(&key) + .await + .map_err(|e| { + StateHandlerError::GenericError(eyre::eyre!( + "Switch {:?}: failed to read NVOS admin credentials from vault: {}", + switch_id, + e + )) + })?; + + let current_credentials = match current_credentials { + Some(Credentials::UsernamePassword { username, password }) => { + if username == NVOS_ADMIN_USERNAME && password == target_password { + tracing::info!( + "Switch {:?}: target NVOS admin credentials already exist in vault for BMC MAC {}", + switch_id, + bmc_mac_address + ); + return Ok(validating_complete_transition()); + } + + (username, password) } + None => ( + expected_switch + .nvos_username + .clone() + .unwrap_or_else(|| NVOS_ADMIN_USERNAME.to_string()), + target_password.clone(), + ), }; - let credentials = Credentials::UsernamePassword { username, password }; + if current_credentials.0.is_empty() || current_credentials.1.is_empty() { + return Ok(error_transition(format!( + "Switch {:?}: missing current NVOS credentials for BMC MAC {}", + switch_id, bmc_mac_address + ))); + } + + let rack_id = state + .rack_id + .clone() + .or_else(|| expected_switch.rack_id.clone()); + + let request = match build_update_switch_system_password_request( + switch_id, + rack_id + .as_ref() + .map(ToString::to_string) + .unwrap_or_default(), + bmc_mac_address, + expected_switch.bmc_ip_address, + switch_endpoint.as_ref(), + current_credentials, + target_password.clone(), + ) { + Ok(request) => request, + Err(cause) => return Ok(error_transition(cause)), + }; + + let Some(rms_client) = ctx.services.rms_client.as_ref() else { + return Ok(error_transition("RMS client not configured")); + }; + + let response = match rms_client.update_switch_system_password(request).await { + Ok(response) => response, + Err(error) => { + return Ok(error_transition(format!( + "Switch {:?}: failed to update NVOS admin password through RMS: {}", + switch_id, error + ))); + } + }; + + if let Err(cause) = validate_update_switch_system_password_response(&response) { + return Ok(error_transition(format!( + "Switch {:?}: failed to update NVOS admin password through RMS: {}", + switch_id, cause + ))); + } + + let credentials = Credentials::UsernamePassword { + username: NVOS_ADMIN_USERNAME.to_string(), + password: target_password, + }; ctx.services .credential_manager @@ -123,14 +202,124 @@ async fn handle_rotate_os_password( })?; tracing::info!( - "Switch {:?}: stored NVOS admin credentials from expected switch into vault for BMC MAC {}", + "Switch {:?}: rotated NVOS admin password through RMS and stored credentials in vault for BMC MAC {}", switch_id, bmc_mac_address ); - Ok(StateHandlerOutcome::transition( - SwitchControllerState::Validating { - validating_state: ValidatingState::ValidationComplete, - }, - )) + Ok(validating_complete_transition()) +} + +fn build_update_switch_system_password_request( + switch_id: &SwitchId, + rack_id: String, + bmc_mac_address: mac_address::MacAddress, + expected_bmc_ip: Option, + switch_endpoint: Option<&db::switch::SwitchEndpointRow>, + current_credentials: (String, String), + target_password: String, +) -> Result { + let bmc_ip = switch_endpoint + .map(|endpoint| endpoint.bmc_ip) + .or(expected_bmc_ip); + let mut host_interfaces = Vec::new(); + + if let Some(endpoint) = switch_endpoint + && (endpoint.nvos_ip.is_some() || endpoint.nvos_mac.is_some()) + { + host_interfaces.push(rms::NetworkInterface { + ip_address: endpoint + .nvos_ip + .map(|ip_address| ip_address.to_string()) + .unwrap_or_default(), + mac_address: endpoint + .nvos_mac + .map(|mac_address| mac_address.to_string()) + .unwrap_or_default(), + }); + } + + if bmc_ip.is_none() && host_interfaces.is_empty() { + return Err(format!( + "no BMC or NVOS endpoint found for switch {}", + switch_id + )); + } + + Ok(rms::UpdateSwitchSystemPasswordRequest { + metadata: None, + nodes: Some(rms::NodeSet { + devices: vec![rms::NewNodeInfo { + node_id: switch_id.to_string(), + rack_id, + r#type: Some(rms::NodeType::Switch as i32), + bmc_endpoint: bmc_ip.map(|ip_address| rms::BmcEndpoint { + interface: Some(rms::NetworkInterface { + ip_address: ip_address.to_string(), + mac_address: bmc_mac_address.to_string(), + }), + port: 443, + credentials: None, + }), + host_endpoint: Some(rms::HostEndpoint { + interfaces: host_interfaces, + port: 0, + credentials: Some(rms::Credentials { + auth: Some(rms::credentials::Auth::UserPass(rms::UsernamePassword { + username: current_credentials.0, + password: current_credentials.1, + })), + }), + }), + }], + }), + username: NVOS_ADMIN_USERNAME.to_string(), + password: target_password, + }) +} + +fn validate_update_switch_system_password_response( + response: &rms::UpdateSwitchSystemPasswordResponse, +) -> Result<(), String> { + let batch_response = response + .response + .as_ref() + .ok_or_else(|| "RMS response missing NodeBatchResponse".to_string())?; + + if let Some(failed_result) = batch_response.node_results.iter().find(|result| { + result.status != rms::ReturnCode::Success as i32 || !result.error_message.is_empty() + }) { + let error_message = if failed_result.error_message.is_empty() { + format!( + "RMS reported password update failure for node {}", + failed_result.node_id + ) + } else { + failed_result.error_message.clone() + }; + return Err(error_message); + } + + if batch_response.status != rms::ReturnCode::Success as i32 || batch_response.failed_nodes > 0 { + let message = if batch_response.message.is_empty() { + "RMS reported password update failure".to_string() + } else { + batch_response.message.clone() + }; + return Err(message); + } + + Ok(()) +} + +fn validating_complete_transition() -> StateHandlerOutcome { + StateHandlerOutcome::transition(SwitchControllerState::Validating { + validating_state: ValidatingState::ValidationComplete, + }) +} + +fn error_transition(cause: impl Into) -> StateHandlerOutcome { + StateHandlerOutcome::transition(SwitchControllerState::Error { + cause: cause.into(), + }) } diff --git a/crates/api/src/tests/switch_state_controller/mod.rs b/crates/api/src/tests/switch_state_controller/mod.rs index f2e7e0dda6..2da3177c06 100644 --- a/crates/api/src/tests/switch_state_controller/mod.rs +++ b/crates/api/src/tests/switch_state_controller/mod.rs @@ -18,8 +18,12 @@ use std::sync::Arc; use std::time::Duration; +use carbide_uuid::switch::SwitchId; use db::switch as db_switch; -use forge_secrets::credentials::TestCredentialManager; +use forge_secrets::credentials::{ + CredentialKey, CredentialReader, CredentialWriter, Credentials, TestCredentialManager, +}; +use librms::protos::rack_manager as rms; use model::switch::{ConfiguringState, SwitchControllerState}; use rpc::forge::forge_server::Forge; use tokio_util::sync::CancellationToken; @@ -35,6 +39,64 @@ use crate::tests::common::api_fixtures::create_test_env; mod fixtures; use fixtures::switch::{mark_switch_as_deleted, set_switch_controller_state}; +fn switch_password_response( + switch_id: &SwitchId, + status: rms::ReturnCode, + error_message: &str, +) -> rms::UpdateSwitchSystemPasswordResponse { + let success = status == rms::ReturnCode::Success; + rms::UpdateSwitchSystemPasswordResponse { + response: Some(rms::NodeBatchResponse { + status: status as i32, + message: if success { + "Updated switch system password on 1 of 1 devices".to_string() + } else { + "Updated switch system password on 0 of 1 devices".to_string() + }, + total_nodes: 1, + successful_nodes: i32::from(success), + failed_nodes: i32::from(!success), + node_results: vec![rms::NodeResult { + node_id: switch_id.to_string(), + status: status as i32, + error_message: error_message.to_string(), + }], + ..Default::default() + }), + } +} + +async fn create_configuring_switch_with_nvos_password( + env: &common::api_fixtures::TestEnv, + pool: &sqlx::PgPool, +) -> Result<(SwitchId, mac_address::MacAddress), Box> { + let switch_id = common::api_fixtures::site_explorer::new_switch( + env, + Some("Switch4".to_string()), + Some("Data Center A, Rack 1".to_string()), + ) + .await?; + + let mut txn = pool.begin().await?; + let switch = db_switch::find_by_id(txn.as_mut(), &switch_id) + .await? + .expect("switch should exist"); + let bmc_mac_address = switch + .bmc_mac_address + .expect("test switch should have a BMC MAC address"); + set_switch_controller_state( + txn.as_mut(), + &switch_id, + SwitchControllerState::Configuring { + config_state: ConfiguringState::RotateOsPassword, + }, + ) + .await?; + txn.commit().await?; + + Ok((switch_id, bmc_mac_address)) +} + #[crate::sqlx_test] async fn test_switch_state_transition_validation( pool: sqlx::PgPool, @@ -87,6 +149,191 @@ async fn test_switch_state_transition_validation( Ok(()) } +#[crate::sqlx_test] +async fn test_switch_rotate_os_password_calls_rms_and_persists_admin_credentials( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let env = create_test_env(pool.clone()).await; + let (switch_id, bmc_mac_address) = + create_configuring_switch_with_nvos_password(&env, &pool).await?; + let credential_key = CredentialKey::SwitchNvosAdmin { bmc_mac_address }; + + env.test_credential_manager + .set_credentials( + &credential_key, + &Credentials::UsernamePassword { + username: "admin".to_string(), + password: "old-pass".to_string(), + }, + ) + .await + .expect("failed to seed existing NVOS credentials"); + env.rms_sim + .queue_update_switch_system_password_response(Ok(switch_password_response( + &switch_id, + rms::ReturnCode::Success, + "", + ))) + .await; + + env.run_switch_controller_iteration().await; + + let requests = env + .rms_sim + .submitted_switch_system_password_requests() + .await; + assert_eq!(requests.len(), 1); + let request = &requests[0]; + assert_eq!(request.username, "admin"); + assert_eq!(request.password, "nvos_pass1"); + + let nodes = request.nodes.as_ref().expect("nodes should be set"); + assert_eq!(nodes.devices.len(), 1); + let device = &nodes.devices[0]; + assert_eq!(device.node_id, switch_id.to_string()); + assert_eq!(device.r#type, Some(rms::NodeType::Switch as i32)); + assert!( + device + .bmc_endpoint + .as_ref() + .and_then(|endpoint| endpoint.interface.as_ref()) + .is_some(), + "BMC endpoint interface should be populated" + ); + let host_credentials = device + .host_endpoint + .as_ref() + .and_then(|endpoint| endpoint.credentials.as_ref()) + .and_then(|credentials| credentials.auth.as_ref()) + .expect("host credentials should be set"); + match host_credentials { + rms::credentials::Auth::UserPass(user_pass) => { + assert_eq!(user_pass.username, "admin"); + assert_eq!(user_pass.password, "old-pass"); + } + rms::credentials::Auth::SessionToken(_) => { + panic!("host credentials should use username/password") + } + } + + let credentials = env + .test_credential_manager + .get_credentials(&credential_key) + .await + .expect("failed to read rotated credentials from vault") + .expect("rotated credentials should be stored in vault"); + assert_eq!( + credentials, + Credentials::UsernamePassword { + username: "admin".to_string(), + password: "nvos_pass1".to_string(), + } + ); + + let mut txn = pool.acquire().await?; + let switch = db_switch::find_by_id(&mut txn, &switch_id) + .await? + .expect("switch should exist"); + assert!(matches!( + switch.controller_state.value, + SwitchControllerState::Validating { .. } + )); + + Ok(()) +} + +#[crate::sqlx_test] +async fn test_switch_rotate_os_password_rms_failure_does_not_update_vault( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let env = create_test_env(pool.clone()).await; + let (switch_id, bmc_mac_address) = + create_configuring_switch_with_nvos_password(&env, &pool).await?; + let credential_key = CredentialKey::SwitchNvosAdmin { bmc_mac_address }; + let existing_credentials = Credentials::UsernamePassword { + username: "admin".to_string(), + password: "old-pass".to_string(), + }; + + env.test_credential_manager + .set_credentials(&credential_key, &existing_credentials) + .await + .expect("failed to seed existing NVOS credentials"); + env.rms_sim + .queue_update_switch_system_password_response(Ok(switch_password_response( + &switch_id, + rms::ReturnCode::Failure, + "mock rotation failed", + ))) + .await; + + env.run_switch_controller_iteration().await; + + let credentials = env + .test_credential_manager + .get_credentials(&credential_key) + .await + .expect("failed to read existing credentials from vault") + .expect("existing credentials should remain in vault"); + assert_eq!(credentials, existing_credentials); + + let mut txn = pool.acquire().await?; + let switch = db_switch::find_by_id(&mut txn, &switch_id) + .await? + .expect("switch should exist"); + assert!(matches!( + switch.controller_state.value, + SwitchControllerState::Error { ref cause } if cause.contains("mock rotation failed") + )); + + Ok(()) +} + +#[crate::sqlx_test] +async fn test_switch_rotate_os_password_rms_transport_error_does_not_update_vault( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let env = create_test_env(pool.clone()).await; + let (switch_id, bmc_mac_address) = + create_configuring_switch_with_nvos_password(&env, &pool).await?; + let credential_key = CredentialKey::SwitchNvosAdmin { bmc_mac_address }; + let existing_credentials = Credentials::UsernamePassword { + username: "admin".to_string(), + password: "old-pass".to_string(), + }; + + env.test_credential_manager + .set_credentials(&credential_key, &existing_credentials) + .await + .expect("failed to seed existing NVOS credentials"); + env.rms_sim + .queue_update_switch_system_password_response(Err( + librms::RackManagerError::ApiInvocationError(tonic::Status::unavailable("rms down")), + )) + .await; + + env.run_switch_controller_iteration().await; + + let credentials = env + .test_credential_manager + .get_credentials(&credential_key) + .await + .expect("failed to read existing credentials from vault") + .expect("existing credentials should remain in vault"); + assert_eq!(credentials, existing_credentials); + + let mut txn = pool.acquire().await?; + let switch = db_switch::find_by_id(&mut txn, &switch_id) + .await? + .expect("switch should exist"); + assert!(matches!( + switch.controller_state.value, + SwitchControllerState::Error { ref cause } if cause.contains("rms down") + )); + + Ok(()) +} + #[crate::sqlx_test] async fn test_switch_deletion_with_state_controller( pool: sqlx::PgPool,