Skip to content

Commit 52a0a54

Browse files
authored
feat: track per-device credential rotation convergence (NVIDIA#2846)
Introduce the bookkeeping the future credential-rotation engine needs: two tables (`sitewide_credential_rotation`, `device_credential_rotation`) plus a one-time backfill recording every already-ingested device at v0, and a `db::credential_rotation` writer that idempotently records a device as converged to the current site-wide target. Wire convergence recording at the points where NICo actually owns a credential on the device, each committed atomically with the surrounding write/transition: * bmc -- when the per-device BMC secret is written (site-explorer set_bmc_root_credentials) * host_uefi -- alongside the bios_password_set_time stamp (API handler and machine-controller UEFI setup) * dpu_uefi -- after uefi_setup(dpu) succeeds in the DPU init flow * lockdown_ikm -- when a SuperNIC card is confirmed locked (dpa-manager handle_locking), keyed by NIC MAC * nvos -- wired but gated off until REQ-6 (set-from-factory) lands; today NICo only copies the operator credential into Vault Add a dedicated, versioned site-wide lockdown IKM credential key (`nic_lockdown_ikm/site/root/v{N}`) seeded from the BMC root, decoupling NIC lockdown key derivation from the BMC root so the two rotate independently. Make a known BMC MAC a hard precondition for UEFI password operations, failing up front instead of after driving the device. ## Related issues <!-- Refer to existing GitHub issues here --> ## Type of Change <!-- Check one that best describes this PR --> - [x] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [ ] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes <!-- If checked, describe the breaking changes and migration steps --> <!-- Breaking changes are not generally permitted, please discuss on a GitHub discussion or with the development team if you believe you need to break a backward compatibility guarantee --> - [ ] **This PR contains breaking changes** ## Testing <!-- How was this tested? Check all that apply --> - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) ## Additional Notes <!-- Any additional context, deployment notes, or reviewer guidance -->
1 parent 54a2b7a commit 52a0a54

24 files changed

Lines changed: 1532 additions & 141 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/api-core/src/dpa/lockdown.rs

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ use sha2::Sha256;
2525
use sqlx::PgPool;
2626

2727
// CURRENT_LOCKDOWN_IKM_VERSION is the site-wide lockdown IKM version the
28-
// lock/unlock flow currently derives keys from. We will leave it hardcoded to 0 until
29-
// we introduce rotation logic.
28+
// lock/unlock flow derives keys from, and the version recorded as each card's
29+
// convergence target. Hardcoded to 0 until the rotation engine lands: rotating
30+
// the IKM (v0 -> v1) is what advances this, and that logic will own making newly
31+
// ingested NICs lock under the new IKM while already-locked cards migrate.
3032
pub const CURRENT_LOCKDOWN_IKM_VERSION: u32 = 0;
3133

3234
// LOCKDOWN_KEY_LENGTH is the max length of the supported
@@ -144,22 +146,26 @@ fn lockdown_ikm_key(version: u32) -> CredentialKey {
144146
}
145147
}
146148

147-
// fetch_kdf_secret fetches the IKM for the KDF from the
148-
// dedicated site-wide lockdown credential, decoupled from the BMC root so the
149-
// two can be rotated independently.
149+
// fetch_kdf_secret fetches the IKM for the KDF from the dedicated site-wide
150+
// lockdown credential, decoupled from the BMC root so the two can be rotated
151+
// independently.
152+
//
153+
// Returns the IKM version it resolved alongside the secret so the caller can
154+
// durably record the exact version a card is locked under, rather than
155+
// re-reading the (mutable) site-wide target later. Today the version is
156+
// `CURRENT_LOCKDOWN_IKM_VERSION`; the rotation engine will own advancing it.
150157
async fn fetch_kdf_secret(
151158
credential_reader: &dyn CredentialReader,
152-
) -> Result<String, eyre::Report> {
153-
let ikm_key = lockdown_ikm_key(CURRENT_LOCKDOWN_IKM_VERSION);
159+
) -> Result<(u32, String), eyre::Report> {
160+
let version = CURRENT_LOCKDOWN_IKM_VERSION;
161+
let ikm_key = lockdown_ikm_key(version);
154162
let credentials = credential_reader
155163
.get_credentials(&ikm_key)
156164
.await?
157-
.ok_or_else(|| {
158-
eyre::eyre!("lockdown IKM v{CURRENT_LOCKDOWN_IKM_VERSION} not found; site not seeded")
159-
})?;
165+
.ok_or_else(|| eyre::eyre!("lockdown IKM v{version} not found; site not seeded"))?;
160166
let Credentials::UsernamePassword { password, .. } = credentials;
161167

162-
Ok(password)
168+
Ok((version, password))
163169
}
164170

165171
// ensure_lockdown_ikm_seeded idempotently seeds the dedicated site-wide
@@ -225,16 +231,32 @@ pub async fn ensure_lockdown_ikm_seeded(
225231
}
226232
}
227233

234+
// SupernicLockdownKey is a derived lockdown key together with the site-wide
235+
// lockdown IKM version it was derived from. The version travels with the key so
236+
// the lock flow can durably record the exact version the card is locked under.
237+
pub struct SupernicLockdownKey {
238+
// The 16-character hex lockdown key sent to the device.
239+
pub key: String,
240+
// The site-wide lockdown IKM version `key` was derived from.
241+
pub ikm_version: u32,
242+
}
243+
228244
// build_supernic_lockdown_key builds a single lockdown key using
229245
// the latest KdfContextVersion. Use this for locking a card.
246+
//
247+
// Returns the derived key together with the IKM version it used (see
248+
// `SupernicLockdownKey`). The unlock flow can ignore the version; the lock flow
249+
// persists it so the recorded convergence version matches what actually locked
250+
// the card.
230251
pub async fn build_supernic_lockdown_key(
231252
db_reader: &PgPool,
232253
dpa_interface_id: DpaInterfaceId,
233254
credential_reader: &dyn CredentialReader,
234-
) -> Result<String, eyre::Report> {
255+
) -> Result<SupernicLockdownKey, eyre::Report> {
235256
let ctx = build_kdf_context(db_reader, dpa_interface_id).await?;
236-
let secret = fetch_kdf_secret(credential_reader).await?;
237-
build_lockdown_key(secret.as_bytes(), &ctx, KdfContextVersion::V1)
257+
let (ikm_version, secret) = fetch_kdf_secret(credential_reader).await?;
258+
let key = build_lockdown_key(secret.as_bytes(), &ctx, KdfContextVersion::V1)?;
259+
Ok(SupernicLockdownKey { key, ikm_version })
238260
}
239261

240262
#[cfg(test)]
@@ -439,7 +461,8 @@ mod tests {
439461
.await
440462
.unwrap();
441463

442-
let secret = fetch_kdf_secret(&store).await.unwrap();
464+
let (version, secret) = fetch_kdf_secret(&store).await.unwrap();
465+
assert_eq!(version, CURRENT_LOCKDOWN_IKM_VERSION);
443466
assert_eq!(secret, "ikm-pass");
444467
}
445468

crates/api-core/src/errors.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,13 @@ impl From<DatabaseError> for CarbideError {
293293
DatabaseError::InvalidArgument(e) => InvalidArgument(e),
294294
DatabaseError::InvalidConfiguration(e) => InvalidConfiguration(e),
295295
DatabaseError::MissingArgument(e) => MissingArgument(e),
296+
// A corrupted/absent site-wide rotation invariant is an internal
297+
// state error, not a client-correctable one.
298+
DatabaseError::MissingSitewideRotationTarget(credential_type) => Internal {
299+
message: format!(
300+
"no site-wide rotation target for credential type: {credential_type:?}"
301+
),
302+
},
296303
DatabaseError::NetworkParseError(e) => NetworkParseError(e),
297304
DatabaseError::NetworkSegmentDelete(e) => NetworkSegmentDelete(e),
298305
DatabaseError::NetworkSegmentDuplicateMacAddress(e) => {

crates/api-core/src/handlers/credential.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,21 @@ pub(crate) async fn delete_bmc_root_credentials_by_mac(
620620
CarbideError::internal(format!("Error deleting credential for BMC: {e:?} "))
621621
})?;
622622

623+
// Drop the bmc convergence marker alongside the Vault secret it depends on:
624+
// once NICo discards the per-device BMC secret it can no longer authenticate
625+
// or rotate the device, so tracking convergence is meaningless. (The rotation
626+
// engine also joins device_credential_rotation to the live device tables, so
627+
// a row orphaned by device deletion is never acted on -- this just keeps the
628+
// table tidy at the chokepoint where the secret actually goes away.)
629+
let mut txn = api.txn_begin().await?;
630+
db::credential_rotation::delete_device_converged(
631+
&mut txn,
632+
bmc_mac_address,
633+
db::credential_rotation::CredentialRotationType::Bmc,
634+
)
635+
.await?;
636+
txn.commit().await?;
637+
623638
api.bmc_session_manager.flush_mac(bmc_mac_address).await;
624639

625640
Ok(())

crates/api-core/src/handlers/dpa.rs

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ async fn build_unlock_command(
257257
machine_id: MachineId,
258258
pci_name: &str,
259259
) -> CarbideResult<DpaCommand<'static>> {
260-
let key = crate::dpa::lockdown::build_supernic_lockdown_key(
260+
let lockdown = crate::dpa::lockdown::build_supernic_lockdown_key(
261261
&api.database_connection,
262262
sn.id,
263263
&*api.credential_manager,
@@ -271,8 +271,10 @@ async fn build_unlock_command(
271271

272272
tracing::info!(%machine_id, %pci_name, "Unlocking DPA");
273273

274+
// The unlock flow does not record convergence, so the derived IKM version is
275+
// not persisted here.
274276
Ok(DpaCommand {
275-
op: OpCode::Unlock { key },
277+
op: OpCode::Unlock { key: lockdown.key },
276278
})
277279
}
278280

@@ -414,7 +416,7 @@ async fn build_lock_command(
414416
machine_id: MachineId,
415417
pci_name: &str,
416418
) -> CarbideResult<DpaCommand<'static>> {
417-
let key = crate::dpa::lockdown::build_supernic_lockdown_key(
419+
let lockdown = crate::dpa::lockdown::build_supernic_lockdown_key(
418420
&api.database_connection,
419421
sn.id,
420422
&*api.credential_manager,
@@ -426,9 +428,37 @@ async fn build_lock_command(
426428
))
427429
})?;
428430

429-
tracing::info!(%machine_id, %pci_name, "Locking DPA");
431+
// Stage the IKM version we are about to lock the card with as the in-flight
432+
// rotation marker (`rotating_to_version`) on the card's lockdown_ikm row
433+
// *before* issuing the lock command. dpa-manager's `handle_locking` promotes
434+
// exactly this value to the convergence version when the card reports Locked
435+
// -- never the (possibly advanced) site-wide target re-read at observation
436+
// time. Staging first means we only ever issue a lock for a version we have
437+
// already recorded our intent to use; if the write fails we surface the error
438+
// and do not lock. The writer is idempotent across the per-cycle
439+
// re-derivation while Locking.
440+
let ikm_version = i32::try_from(lockdown.ikm_version).map_err(|e| CarbideError::Internal {
441+
message: format!(
442+
"lockdown IKM version {} does not fit in i32 for DPA {pci_name}: {e}",
443+
lockdown.ikm_version
444+
),
445+
})?;
446+
let mut conn = api.database_connection.acquire().await.map_err(|e| {
447+
CarbideError::GenericErrorFromReport(eyre!(
448+
"failed to acquire connection to stage lockdown IKM rotation for DPA {pci_name}: {e}"
449+
))
450+
})?;
451+
db::credential_rotation::mark_device_rotating_to_version(
452+
&mut conn,
453+
sn.mac_address,
454+
db::credential_rotation::CredentialRotationType::LockdownIkm,
455+
ikm_version,
456+
)
457+
.await?;
458+
459+
tracing::info!(%machine_id, %pci_name, ikm_version = lockdown.ikm_version, "Locking DPA");
430460
Ok(DpaCommand {
431-
op: OpCode::Lock { key },
461+
op: OpCode::Lock { key: lockdown.key },
432462
})
433463
}
434464

crates/api-core/src/handlers/machine.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,12 +516,28 @@ pub(crate) async fn admin_force_delete_machine(
516516
}
517517

518518
if machine.bios_password_set_time.is_some() {
519-
if let Err(e) = api
519+
match api
520520
.redfish_pool
521521
.clear_host_uefi_password(client.as_ref())
522522
.await
523523
{
524-
tracing::warn!(%machine_id, error = %e, "Failed to clear host UEFI password while force deleting machine");
524+
Ok(_) => {
525+
// The UEFI password was reset on the device, so the host no
526+
// longer carries the site-wide UEFI value: drop the host_uefi
527+
// convergence marker (keyed by the host BMC MAC, mirroring where
528+
// it is recorded when the password is set). Best-effort like the
529+
// clear itself -- the machine row is being deleted anyway, so a
530+
// surviving marker would be neutralized by the rotation engine's
531+
// live-device join regardless.
532+
if let Err(e) =
533+
forget_host_uefi_convergence(api, bmc_mac_address).await
534+
{
535+
tracing::warn!(%machine_id, error = %e, "Cleared host UEFI password but failed to delete its credential-rotation marker");
536+
}
537+
}
538+
Err(e) => {
539+
tracing::warn!(%machine_id, error = %e, "Failed to clear host UEFI password while force deleting machine");
540+
}
525541
}
526542

527543
// TODO (spyda): have libredfish return whether the client needs to reboot the host after clearing the host uefi password
@@ -767,6 +783,24 @@ async fn clear_bmc_credentials(api: &Api, machine: &Machine) -> Result<(), Carbi
767783
Ok(())
768784
}
769785

786+
/// Deletes the `host_uefi` credential-rotation convergence marker for a host,
787+
/// keyed by its BMC MAC. Called after force-delete resets the host UEFI password
788+
/// on the device, where the host no longer carries the site-wide UEFI value.
789+
async fn forget_host_uefi_convergence(
790+
api: &Api,
791+
bmc_mac_address: mac_address::MacAddress,
792+
) -> Result<(), CarbideError> {
793+
let mut txn = api.txn_begin().await?;
794+
db::credential_rotation::delete_device_converged(
795+
&mut txn,
796+
bmc_mac_address,
797+
db::credential_rotation::CredentialRotationType::HostUefi,
798+
)
799+
.await?;
800+
txn.commit().await?;
801+
Ok(())
802+
}
803+
770804
pub async fn get_machine_position_info(
771805
api: &Api,
772806
request: Request<rpc::MachinePositionQuery>,

crates/api-core/src/handlers/mlx_admin.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,7 @@ async fn get_device_lockdown_key(
13161316
}
13171317
})?;
13181318

1319-
let lockdown_key = crate::dpa::lockdown::build_supernic_lockdown_key(
1319+
let lockdown = crate::dpa::lockdown::build_supernic_lockdown_key(
13201320
&api.database_connection,
13211321
dpa_interface.id,
13221322
&*api.credential_manager,
@@ -1328,5 +1328,5 @@ async fn get_device_lockdown_key(
13281328
),
13291329
})?;
13301330

1331-
Ok(lockdown_key)
1331+
Ok(lockdown.key)
13321332
}

crates/api-core/src/handlers/uefi.rs

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,16 @@ pub(crate) async fn set_host_uefi_password(
195195
CarbideError::InvalidArgument("Specified machine does not have BMC address".into())
196196
})?;
197197

198+
// A known BMC MAC is a hard precondition for setting the UEFI password: it
199+
// keys the host_uefi rotation bookkeeping recorded below, so reject the
200+
// request up front rather than driving the device and only then discovering
201+
// we cannot track its convergence.
202+
let host_bmc_mac = snapshot.host_snapshot.bmc_info.mac.ok_or_else(|| {
203+
CarbideError::InvalidArgument(
204+
"Specified machine does not have a known BMC MAC address".into(),
205+
)
206+
})?;
207+
198208
let bmc_access_info =
199209
db::machine_interface::lookup_bmc_access_info(&mut txn, addr.ip(), Some(addr.port()))
200210
.await?;
@@ -222,14 +232,38 @@ pub(crate) async fn set_host_uefi_password(
222232
tracing::error!(%e, "Failed to run uefi_setup call");
223233
CarbideError::internal(format!("Failed redfish uefi_setup subtask: {e}"))
224234
})?;
225-
api.with_txn(|txn| db::machine::update_bios_password_set_time(&machine_id, txn).boxed())
226-
.await?
227-
.map_err(|e| {
228-
tracing::error!("Failed to update bios_password_set_time: {}", e);
229-
CarbideError::Internal {
230-
message: format!("Failed to update BIOS password timestamp: {e}"),
231-
}
232-
})?;
235+
// uefi_setup returns a BMC job_id; the password change completes
236+
// asynchronously on the device and we do not poll it here. We optimistically
237+
// stamp bios_password_set_time and, in the same transaction, record host_uefi
238+
// convergence (keyed by the host BMC MAC, mirroring the backfill) so the two
239+
// always agree -- convergence rides along with the pre-existing marker. If
240+
// the dispatched job ultimately fails on the BMC, both are inaccurate.
241+
//
242+
// TODO(credential-rotation): gate both the bios_password_set_time stamp and
243+
// the host_uefi convergence record on confirmed job_id completion (poll the
244+
// BMC job rather than trusting dispatch). Whatever confirms completion should
245+
// perform both updates together -- convergence does not need its own separate
246+
// write path or operator-facing API; it follows bios_password_set_time.
247+
api.with_txn(|txn| {
248+
async move {
249+
db::machine::update_bios_password_set_time(&machine_id, txn).await?;
250+
db::credential_rotation::record_device_converged(
251+
txn,
252+
host_bmc_mac,
253+
db::credential_rotation::CredentialRotationType::HostUefi,
254+
)
255+
.await?;
256+
Ok::<(), db::DatabaseError>(())
257+
}
258+
.boxed()
259+
})
260+
.await?
261+
.map_err(|e| {
262+
tracing::error!("Failed to update bios_password_set_time: {}", e);
263+
CarbideError::Internal {
264+
message: format!("Failed to update BIOS password timestamp: {e}"),
265+
}
266+
})?;
233267

234268
Ok(Response::new(rpc::SetHostUefiPasswordResponse { job_id }))
235269
}

crates/api-core/src/setup.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,10 @@ pub async fn start_api(
392392
// lockdown key without operator action. No-op once seeded or if the BMC
393393
// root is not yet configured.
394394
crate::dpa::lockdown::ensure_lockdown_ikm_seeded(&*credential_manager).await?;
395+
396+
// Initial credential-rotation bookkeeping is backfilled by the
397+
// `*_credential_rotation_backfill` data migration (see its header for the
398+
// ordering invariants), not seeded here.
395399
};
396400

397401
let common_pools =
@@ -471,6 +475,7 @@ pub async fn start_api(
471475
.rotate_switch_nvos_credentials
472476
.clone(),
473477
carbide_config.site_explorer.explore_mode,
478+
db_pool.clone(),
474479
);
475480

476481
let nvlink_config = carbide_config.nvlink_config.clone().unwrap_or_default();

crates/api-core/src/test_support/builder.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ impl TestApiBuilder {
217217
Arc::new(std::sync::atomic::AtomicBool::new(false)),
218218
// Tests use MockEndpointExplorer. So this doesn't affect anything.
219219
SiteExplorerExploreMode::NvRedfish,
220+
self.db_pool.clone(),
220221
);
221222

222223
let metric_emitter = self.metric_emitter.unwrap_or_else(|| {

0 commit comments

Comments
 (0)