Skip to content

Commit 3c8bd50

Browse files
committed
fix(security): remove unimplemented Aes256Xts variant and fix audit hash tests
EncryptionAlgorithm previously exposed an Aes256Xts variant with no corresponding cipher implementation, leaving the default VolumeEncryption::new() constructing a header that could never be used. Remove the variant; Aes256Gcm is the only algorithm in the type surface, matching what the code actually does. Internal callsites and tests are updated accordingly. Module docs clarify that at-rest protection for redb, HNSW, and mmap files is delegated to filesystem-level encryption. Audit test suite: remove the redundant determinism test (covered by the golden-value test) and add a pinned hash for the AuditCheckpoint variant to ensure the repr(u8) discriminant encoding is stable for all variants, not just AuthSuccess.
1 parent 0848e23 commit 3c8bd50

2 files changed

Lines changed: 48 additions & 47 deletions

File tree

nodedb/src/control/security/audit/entry.rs

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -97,28 +97,9 @@ mod tests {
9797
}
9898
}
9999

100-
/// Pin the canonical hash for a specific AuthSuccess entry so any change
101-
/// to the hash algorithm is immediately detected. The expected hex was
102-
/// produced by the first correct discriminant-based implementation and
103-
/// is intentionally hardcoded — it IS the stability test.
104-
#[test]
105-
fn hash_is_stable_across_debug_format_changes() {
106-
let entry = make_entry(1, AuditEvent::AuthSuccess, "");
107-
let h = hash_entry(&entry);
108-
// If this assertion fails, the canonical hash format changed.
109-
// Update only after a deliberate, versioned migration of the chain.
110-
assert_eq!(
111-
h,
112-
hash_entry(&entry),
113-
"hash_entry must be deterministic for the same input"
114-
);
115-
// The hash must be a 64-character lowercase hex string.
116-
assert_eq!(h.len(), 64, "SHA-256 hex must be 64 chars");
117-
assert!(h.chars().all(|c| c.is_ascii_hexdigit()), "must be hex");
118-
}
119-
120-
/// Hardcoded golden hash — pins the canonical encoding.
121-
/// Recomputed from the first correct implementation of discriminant+zerompk.
100+
/// Hardcoded golden hash — pins the canonical encoding for an AuthSuccess
101+
/// entry. The expected hex was produced by the first correct
102+
/// discriminant-based implementation and is intentionally frozen here.
122103
#[test]
123104
fn hash_golden_value() {
124105
let entry = AuditEntry {
@@ -134,11 +115,29 @@ mod tests {
134115
prev_hash: String::new(),
135116
};
136117
let h = hash_entry(&entry);
137-
// Golden value: compute once, then freeze. Changing any byte in
138-
// the canonical layout must produce a different hex here.
139-
let recomputed = hash_entry(&entry);
140-
assert_eq!(h, recomputed, "hash must be deterministic");
141-
assert_eq!(h.len(), 64);
118+
// Pinned hash. Updating this string requires explicit reasoning — encoding changes break audit chain compatibility.
119+
assert_eq!(
120+
h,
121+
"0394994e7dda6e6ea99b47a9d6a73b305056ed8d34d5573286b2e42b158a7985",
122+
"canonical hash changed — audit chain encoding must not drift"
123+
);
124+
}
125+
126+
/// Pins the canonical hash for the highest-discriminant `AuditEvent`
127+
/// variant (`AuditCheckpoint = 25`). Because the hash encodes the
128+
/// `#[repr(u8)]` discriminant directly, any new variant added above 25
129+
/// would have a different discriminant and therefore a different hash —
130+
/// proving that existing entries are unaffected by new variants.
131+
#[test]
132+
fn hash_pinned_for_high_discriminant_variant() {
133+
let entry = make_entry(1, AuditEvent::AuditCheckpoint, "");
134+
let h = hash_entry(&entry);
135+
// Pinned hash. Updating this string requires explicit reasoning — encoding changes break audit chain compatibility.
136+
assert_eq!(
137+
h,
138+
"a7eac81c35ed115b7345ad6ac9e1ccbfb78d46c5af9b4647ebe6d2560d4a00c2",
139+
"canonical hash for AuditCheckpoint (discriminant 25) must not drift"
140+
);
142141
}
143142

144143
#[test]

nodedb/src/control/security/encryption.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
1-
//! AES-256-XTS encryption at rest for local storage volumes.
2-
//!
3-
//! Beyond the WAL encryption (AES-256-GCM per record), this provides
4-
//! full volume-level encryption for all persistent data:
5-
//!
6-
//! - redb database files (sparse engine, edge store, GSI)
7-
//! - HNSW checkpoint files
8-
//! - mmap vector segment files
9-
//! - Sort spill temp files
10-
//!
11-
//! XTS mode is designed for disk encryption: it handles arbitrary-length
12-
//! sectors without authentication overhead (since integrity is handled
13-
//! separately by checksums in each data format).
1+
//! Master-key + DEK envelope used for the WAL's AES-256-GCM record
2+
//! encryption. The DEK is generated at random per file, wrapped under
3+
//! the master key with AES-256-GCM (authenticated key wrapping), and
4+
//! stored in [`EncryptedFileHeader`].
145
//!
156
//! Key management:
167
//! - Master key derived from a key file or KMS
178
//! - Per-file data encryption key (DEK) generated randomly
18-
//! - DEK encrypted by master key, stored in file header
9+
//! - DEK encrypted (wrapped) by master key, stored in file header
1910
//! - Key rotation: re-encrypt DEKs with new master key (no data rewrite)
11+
//!
12+
//! No segment-level "volume" cipher is implemented: redb files, HNSW
13+
//! checkpoints, and mmap vector segments rely on filesystem-level
14+
//! encryption (LUKS / FileVault / dm-crypt) for at-rest protection.
15+
//! The [`VolumeEncryption`] surface here only handles DEK lifecycle.
2016
2117
use std::path::{Path, PathBuf};
2218

@@ -40,17 +36,23 @@ impl Default for VolumeEncryptionConfig {
4036
Self {
4137
master_key_path: None,
4238
enabled: false,
43-
algorithm: EncryptionAlgorithm::Aes256Xts,
39+
algorithm: EncryptionAlgorithm::Aes256Gcm,
4440
}
4541
}
4642
}
4743

4844
/// Supported encryption algorithms.
45+
///
46+
/// Only authenticated AES-256-GCM is currently implemented; previous
47+
/// builds carried an aspirational `Aes256Xts` variant with no
48+
/// corresponding cipher. That variant has been removed so the type
49+
/// surface matches what the code actually does. If a non-authenticated
50+
/// volume cipher is wired in later, add the new variant alongside
51+
/// `Aes256Gcm` rather than reviving the unimplemented one.
4952
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
5053
pub enum EncryptionAlgorithm {
51-
/// AES-256-XTS for volume encryption (no authentication — checksums handle integrity).
52-
Aes256Xts,
53-
/// AES-256-GCM for authenticated encryption (used by WAL).
54+
/// AES-256-GCM authenticated encryption — used to wrap DEKs and by
55+
/// the WAL for per-record encryption.
5456
Aes256Gcm,
5557
}
5658

@@ -299,7 +301,7 @@ mod tests {
299301

300302
#[test]
301303
fn header_validation() {
302-
let header = EncryptedFileHeader::new(EncryptionAlgorithm::Aes256Xts, vec![0; 32]);
304+
let header = EncryptedFileHeader::new(EncryptionAlgorithm::Aes256Gcm, vec![0; 32]);
303305
assert!(header.is_valid());
304306
assert_eq!(header.version, 1);
305307
}
@@ -315,7 +317,7 @@ mod tests {
315317
let mut enc = VolumeEncryption::new(VolumeEncryptionConfig {
316318
master_key_path: Some(key_path),
317319
enabled: true,
318-
algorithm: EncryptionAlgorithm::Aes256Xts,
320+
algorithm: EncryptionAlgorithm::Aes256Gcm,
319321
});
320322
enc.load_master_key().unwrap();
321323
assert!(enc.is_active());

0 commit comments

Comments
 (0)