Skip to content

Commit 3ee0bcb

Browse files
TQ: Support for ZFS Key Rotation (#9737)
When Trust Quorum commits a new epoch, all U.2 crypt datasets must have their encryption keys rotated to use the new epoch's derived key. This change implements the key rotation flow triggered by epoch commits. ## Trust Quorum Integration - Add watch channel to `NodeTaskHandle` for epoch change notifications - Initialize channel with current committed epoch on startup - Notify subscribers via `send_if_modified()` when epoch changes ## Config Reconciler Integration - Accept `committed_epoch_rx` watch channel from trust quorum - Trigger reconciliation when epoch changes - Track per-disk encryption epoch in `ExternalDisks` - Add `rekey_for_epoch()` to coordinate key rotation: - Filter disks needing rekey (cached epoch < target OR unknown) - Derive keys for each disk via `StorageKeyRequester` - Send batch request to dataset task - Update cached epochs on success - Retry on failure via normal reconciliation retry logic ## Dataset Task Changes - Add `RekeyRequest`/`RekeyResult` types for batch rekey operations - Add `datasets_rekey()` with idempotency check (skip if already at target) - Use `Zfs::change_key()` for atomic key + epoch property update ## ZFS Utilities - Add `Zfs::change_key()` using native `-o user:property=value` support recently added to Illumos ZFS - Add `Zfs::load_key()`, `unload_key()`, `dataset_exists()` - Add `epoch` field to `DatasetProperties` - Add structured error types for key operations ## Crash Recovery - Add trial decryption recovery in `sled-storage` for datasets with missing epoch property (e.g., crash during initial creation) - Unload key before each trial attempt to handle crash-after-load-key - Set epoch property after successful recovery --------- Co-authored-by: Andrew J. Stone <andrew@oxidecomputer.com>
1 parent 6fa9641 commit 3ee0bcb

21 files changed

Lines changed: 1069 additions & 120 deletions

File tree

Cargo.lock

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

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ members = [
7575
"internal-dns/types/versions",
7676
"ipcc",
7777
"key-manager",
78+
"key-manager/types",
7879
"live-tests",
7980
"live-tests/macros",
8081
"nexus",
@@ -250,6 +251,7 @@ default-members = [
250251
"internal-dns/types/versions",
251252
"ipcc",
252253
"key-manager",
254+
"key-manager/types",
253255
"live-tests",
254256
"live-tests/macros",
255257
"nexus",
@@ -560,6 +562,7 @@ ipnetwork = { version = "0.21", features = ["schemars", "serde"] }
560562
ispf = { git = "https://github.com/oxidecomputer/ispf" }
561563
jiff = "0.2.15"
562564
key-manager = { path = "key-manager" }
565+
key-manager-types = { path = "key-manager/types" }
563566
kstat-rs = "0.2.4"
564567
libc = "0.2.174"
565568
libipcc = { git = "https://github.com/oxidecomputer/ipcc-rs", rev = "524eb8f125003dff50b9703900c6b323f00f9e1b" }

illumos-utils/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ http.workspace = true
2525
iddqd.workspace = true
2626
ipnetwork.workspace = true
2727
itertools.workspace = true
28+
key-manager-types.workspace = true
2829
libc.workspace = true
2930
macaddr.workspace = true
3031
nix.workspace = true

illumos-utils/src/zfs.rs

Lines changed: 130 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,42 @@ pub struct GetValueError {
206206
#[error("Failed to list snapshots: {0}")]
207207
pub struct ListSnapshotsError(#[from] crate::ExecutionError);
208208

209+
/// Error returned by [`Zfs::change_key`].
210+
#[derive(thiserror::Error, Debug)]
211+
#[error("Failed to change encryption key for dataset '{name}'")]
212+
pub struct ChangeKeyError {
213+
pub name: String,
214+
#[source]
215+
pub err: anyhow::Error,
216+
}
217+
218+
/// Error returned by [`Zfs::load_key`].
219+
#[derive(thiserror::Error, Debug)]
220+
#[error("Failed to load encryption key for dataset '{name}'")]
221+
pub struct LoadKeyError {
222+
pub name: String,
223+
#[source]
224+
pub err: crate::ExecutionError,
225+
}
226+
227+
/// Error returned by [`Zfs::dataset_exists`].
228+
#[derive(thiserror::Error, Debug)]
229+
#[error("Failed to check if dataset '{name}' exists")]
230+
pub struct DatasetExistsError {
231+
pub name: String,
232+
#[source]
233+
pub err: crate::ExecutionError,
234+
}
235+
236+
/// Error returned by [`Zfs::unload_key`].
237+
#[derive(thiserror::Error, Debug)]
238+
#[error("Failed to unload encryption key for dataset '{name}'")]
239+
pub struct UnloadKeyError {
240+
pub name: String,
241+
#[source]
242+
pub err: crate::ExecutionError,
243+
}
244+
209245
#[derive(Debug, thiserror::Error)]
210246
#[error(
211247
"Failed to create snapshot '{snap_name}' from filesystem '{filesystem}': {err}"
@@ -523,11 +559,19 @@ pub struct DatasetProperties {
523559
/// string so that unexpected compression formats don't prevent inventory
524560
/// from being collected.
525561
pub compression: String,
562+
/// The encryption key epoch for this dataset.
563+
///
564+
/// Only present on encrypted datasets that directly hold a key (e.g.,
565+
/// crypt datasets on U.2s). Not present on datasets that inherit
566+
/// encryption from a parent.
567+
pub epoch: Option<u64>,
526568
}
527569

528570
impl DatasetProperties {
529-
const ZFS_GET_PROPS: &'static str =
530-
"oxide:uuid,name,mounted,avail,used,quota,reservation,compression";
571+
const ZFS_GET_PROPS: &'static str = concat!(
572+
"oxide:uuid,oxide:epoch,",
573+
"name,mounted,avail,used,quota,reservation,compression",
574+
);
531575
}
532576

533577
impl TryFrom<&DatasetProperties> for SharedDatasetConfig {
@@ -648,6 +692,18 @@ impl DatasetProperties {
648692
.get("compression")
649693
.map(|(prop, _source)| prop.to_string())
650694
.ok_or_else(|| anyhow!("Missing 'compression'"))?;
695+
// The epoch property is only present on encrypted datasets.
696+
// Like oxide:uuid, we ignore inherited values.
697+
let epoch = props
698+
.get("oxide:epoch")
699+
.filter(|(prop, source)| {
700+
!source.starts_with("inherited") && *prop != "-"
701+
})
702+
.map(|(prop, _source)| {
703+
prop.parse::<u64>()
704+
.context("Failed to parse 'oxide:epoch'")
705+
})
706+
.transpose()?;
651707

652708
Ok(DatasetProperties {
653709
id,
@@ -658,6 +714,7 @@ impl DatasetProperties {
658714
quota,
659715
reservation,
660716
compression,
717+
epoch,
661718
})
662719
})
663720
.collect::<Result<Vec<_>, _>>()
@@ -1197,7 +1254,7 @@ impl Zfs {
11971254
name: &str,
11981255
mountpoint: &Mountpoint,
11991256
) -> Result<(), EnsureDatasetErrorRaw> {
1200-
let mount_info = Self::dataset_exists(name, mountpoint).await?;
1257+
let mount_info = Self::dataset_mount_info(name, mountpoint).await?;
12011258
if !mount_info.exists {
12021259
return Err(EnsureDatasetErrorRaw::DoesNotExist);
12031260
}
@@ -1246,7 +1303,7 @@ impl Zfs {
12461303
additional_options,
12471304
}: DatasetEnsureArgs<'_>,
12481305
) -> Result<(), EnsureDatasetErrorRaw> {
1249-
let dataset_info = Self::dataset_exists(name, &mountpoint).await?;
1306+
let dataset_info = Self::dataset_mount_info(name, &mountpoint).await?;
12501307

12511308
// Non-zoned datasets with an explicit mountpoint and the
12521309
// "canmount=on" property should be mounted within the global zone.
@@ -1365,9 +1422,29 @@ impl Zfs {
13651422
Ok(())
13661423
}
13671424

1368-
// Return (true, mounted) if the dataset exists, (false, false) otherwise,
1369-
// where mounted is if the dataset is mounted.
1370-
async fn dataset_exists(
1425+
/// Check if a ZFS dataset exists.
1426+
pub async fn dataset_exists(
1427+
name: &str,
1428+
) -> Result<bool, DatasetExistsError> {
1429+
let mut cmd = Command::new(ZFS);
1430+
cmd.args(&["list", "-H", name]);
1431+
match execute_async(&mut cmd).await {
1432+
Ok(_) => Ok(true),
1433+
Err(crate::ExecutionError::CommandFailure(ref info))
1434+
if info.stderr.contains("does not exist") =>
1435+
{
1436+
Ok(false)
1437+
}
1438+
Err(err) => Err(DatasetExistsError { name: name.to_string(), err }),
1439+
}
1440+
}
1441+
1442+
/// Get mount info for a dataset, validating its mountpoint.
1443+
///
1444+
/// Returns (exists=true, mounted) if the dataset exists with the expected
1445+
/// mountpoint, (exists=false, mounted=false) if it doesn't exist.
1446+
/// Returns an error if the dataset exists but has an unexpected mountpoint.
1447+
async fn dataset_mount_info(
13711448
name: &str,
13721449
mountpoint: &Mountpoint,
13731450
) -> Result<DatasetMountInfo, EnsureDatasetErrorRaw> {
@@ -1523,6 +1600,52 @@ impl Zfs {
15231600
})
15241601
}
15251602

1603+
/// Change the encryption key and set the oxide:epoch property.
1604+
///
1605+
/// This operation is used for ZFS key rotation when a new Trust Quorum
1606+
/// epoch is committed. The caller is responsible for writing the new key
1607+
/// to the dataset's keylocation before calling this, and zeroing the
1608+
/// keyfile afterward.
1609+
pub async fn change_key(
1610+
dataset: &str,
1611+
epoch: u64,
1612+
) -> Result<(), ChangeKeyError> {
1613+
let epoch_prop = format!("oxide:epoch={epoch}");
1614+
let mut cmd = Command::new(PFEXEC);
1615+
cmd.args(&[ZFS, "change-key", "-o", &epoch_prop, dataset]);
1616+
execute_async(&mut cmd).await.map_err(|e| ChangeKeyError {
1617+
name: dataset.to_string(),
1618+
err: e.into(),
1619+
})?;
1620+
Ok(())
1621+
}
1622+
1623+
/// Load the encryption key for an encrypted ZFS dataset.
1624+
///
1625+
/// This makes the dataset accessible for mounting. The key must have
1626+
/// previously been written to the dataset's keylocation.
1627+
pub async fn load_key(name: &str) -> Result<(), LoadKeyError> {
1628+
let mut cmd = Command::new(PFEXEC);
1629+
cmd.args(&[ZFS, "load-key", name]);
1630+
execute_async(&mut cmd)
1631+
.await
1632+
.map(|_| ())
1633+
.map_err(|err| LoadKeyError { name: name.to_string(), err })
1634+
}
1635+
1636+
/// Unload the encryption key for an encrypted ZFS dataset.
1637+
///
1638+
/// This is used for cleanup after failed key operations or during
1639+
/// trial decryption recovery. The dataset must not be mounted.
1640+
pub async fn unload_key(name: &str) -> Result<(), UnloadKeyError> {
1641+
let mut cmd = Command::new(PFEXEC);
1642+
cmd.args(&[ZFS, "unload-key", name]);
1643+
execute_async(&mut cmd)
1644+
.await
1645+
.map(|_| ())
1646+
.map_err(|err| UnloadKeyError { name: name.to_string(), err })
1647+
}
1648+
15261649
/// Calls "zfs get" to acquire multiple values
15271650
///
15281651
/// - `names`: The properties being acquired

key-manager/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ workspace = true
1010
[dependencies]
1111
async-trait.workspace = true
1212
hkdf.workspace = true
13+
key-manager-types.workspace = true
1314
omicron-common.workspace = true
1415
secrecy.workspace = true
1516
sha3.workspace = true

key-manager/src/lib.rs

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use std::fmt::Debug;
99

1010
use async_trait::async_trait;
1111
use hkdf::Hkdf;
12-
use secrecy::{ExposeSecret, ExposeSecretMut, SecretBox};
12+
use key_manager_types::Aes256GcmDiskEncryptionKey;
13+
pub use key_manager_types::VersionedAes256GcmDiskEncryptionKey;
14+
use secrecy::{ExposeSecret, SecretBox};
1315
use sha3::Sha3_256;
1416
use slog::{Logger, o, warn};
1517
use tokio::sync::{mpsc, oneshot};
@@ -52,27 +54,6 @@ pub enum Error {
5254
SecretRetrieval(#[from] SecretRetrieverError),
5355
}
5456

55-
/// Derived Disk Encryption key
56-
#[derive(Default)]
57-
struct Aes256GcmDiskEncryptionKey(SecretBox<[u8; 32]>);
58-
59-
/// A Disk encryption key for a given epoch to be used with ZFS datasets for
60-
/// U.2 devices
61-
pub struct VersionedAes256GcmDiskEncryptionKey {
62-
epoch: u64,
63-
key: Aes256GcmDiskEncryptionKey,
64-
}
65-
66-
impl VersionedAes256GcmDiskEncryptionKey {
67-
pub fn epoch(&self) -> u64 {
68-
self.epoch
69-
}
70-
71-
pub fn expose_secret(&self) -> &[u8; 32] {
72-
&self.key.0.expose_secret()
73-
}
74-
}
75-
7657
/// A request sent from a [`StorageKeyRequester`] to the [`KeyManager`].
7758
enum StorageKeyRequest {
7859
GetKey {
@@ -255,11 +236,11 @@ impl<S: SecretRetriever> KeyManager<S> {
255236
disk_id.model.as_bytes(),
256237
disk_id.serial.as_bytes(),
257238
],
258-
key.0.expose_secret_mut(),
239+
key.expose_secret_mut(),
259240
)
260241
.unwrap();
261242

262-
Ok(VersionedAes256GcmDiskEncryptionKey { epoch, key })
243+
Ok(VersionedAes256GcmDiskEncryptionKey::new(epoch, key))
263244
}
264245

265246
/// Return the epochs for all secrets which are loaded
@@ -309,6 +290,9 @@ pub enum SecretRetrieverError {
309290

310291
#[error("Trust quorum error: {0}")]
311292
TrustQuorum(String),
293+
294+
#[error("Timeout retrieving secret")]
295+
Timeout,
312296
}
313297

314298
/// A mechanism for retrieving a secrets to use as input key material to HKDF-
@@ -405,7 +389,7 @@ mod tests {
405389
};
406390
let epoch = 0;
407391
let key = km.disk_encryption_key(epoch, &disk_id).await.unwrap();
408-
assert_eq!(key.epoch, epoch);
392+
assert_eq!(key.epoch(), epoch);
409393

410394
// Key derivation is deterministic based on disk_id and loaded secrets
411395
let key2 = km.disk_encryption_key(epoch, &disk_id).await.unwrap();
@@ -436,8 +420,8 @@ mod tests {
436420
let epoch = 0;
437421
let key1 = km.disk_encryption_key(epoch, &id_1).await.unwrap();
438422
let key2 = km.disk_encryption_key(epoch, &id_2).await.unwrap();
439-
assert_eq!(key1.epoch, epoch);
440-
assert_eq!(key2.epoch, epoch);
423+
assert_eq!(key1.epoch(), epoch);
424+
assert_eq!(key2.epoch(), epoch);
441425
assert_ne!(key1.expose_secret(), key2.expose_secret());
442426
}
443427

key-manager/types/Cargo.toml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[package]
2+
name = "key-manager-types"
3+
version = "0.1.0"
4+
edition.workspace = true
5+
license = "MPL-2.0"
6+
7+
[lints]
8+
workspace = true
9+
10+
[dependencies]
11+
secrecy.workspace = true
12+
omicron-workspace-hack.workspace = true

0 commit comments

Comments
 (0)