Fix: Verhindert Datenverlust bei Master-Key-Backend-Wechsel und -Race

resolve_master_key() erzeugte bei vorübergehend nicht erreichbarem
OS-Keyring (z. B. unter systemd --user ohne D-Bus-Secret-Service)
stillschweigend einen NEUEN Datei-Schlüssel statt den zuvor genutzten
Keyring-Schlüssel weiterzuverwenden - bereits verschlüsselte
Zugangsdaten wurden dadurch dauerhaft unentschlüsselbar. Welches
Backend (Keyring oder Datei) genutzt wird, wird jetzt beim ersten
Aufruf in einer Marker-Datei festgehalten und danach konsistent
wiederverwendet.

Zusätzlich: sowohl die Datei- als auch die Keyring-Variante von
load_or_create() gingen bei zwei gleichzeitigen ersten Aufrufen
(Race) unterschiedliche, sich widersprechende Schlüssel ein - beide
lesen jetzt den tatsächlich persistierten Schlüssel zurück, statt
blind ihren eigenen zu verwenden.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjyzpGWECyKSBWz5DtkTGz
This commit is contained in:
2026-09-15 23:05:29 +02:00
co-authored by Claude Sonnet 5
parent 2a4619c8e8
commit 706494d106
+71 -12
View File
@@ -16,24 +16,60 @@ use crate::error::{Error, Result};
const KEYRING_SERVICE: &str = "smart-mount"; const KEYRING_SERVICE: &str = "smart-mount";
const KEYRING_USERNAME: &str = "master-key"; const KEYRING_USERNAME: &str = "master-key";
const KEY_FILE_NAME: &str = "master.key"; const KEY_FILE_NAME: &str = "master.key";
const KEY_BACKEND_MARKER_NAME: &str = "master.key.backend";
const KEY_LEN: usize = 32; const KEY_LEN: usize = 32;
/// Ermittelt (und erzeugt bei Bedarf) den 256-Bit-Master-Schlüssel für die /// Ermittelt (und erzeugt bei Bedarf) den 256-Bit-Master-Schlüssel für die
/// Zugangsdaten-Verschlüsselung, siehe Modul-Dokumentation für die Fallback-Reihenfolge. /// Zugangsdaten-Verschlüsselung, siehe Modul-Dokumentation für die Fallback-Reihenfolge.
///
/// Welcher Backend (Keyring oder Schlüsseldatei) für einen Nutzer verwendet wird, wird beim
/// ersten Aufruf in einer Marker-Datei festgehalten und danach immer wieder verwendet. Ohne
/// diese Festlegung würde eine vorübergehend nicht erreichbare Keyring (z. B. `systemd --user`
/// ohne D-Bus-Secret-Service) sonst bei jedem Aufruf transparent einen *neuen* Datei-Schlüssel
/// erzeugen und damit zuvor unter dem Keyring-Schlüssel verschlüsselte Zugangsdaten unwiderruflich
/// unlesbar machen.
pub fn resolve_master_key() -> Result<[u8; 32]> { pub fn resolve_master_key() -> Result<[u8; 32]> {
if sudo_ctdra::is_run_as_root() { if sudo_ctdra::is_run_as_root() {
return file_key::load_or_create(&key_file_path()); return file_key::load_or_create(&key_file_path());
} }
match keyring_key::load_or_create() { let marker_path = key_backend_marker_path();
Ok(key) => Ok(key), match fs::read_to_string(&marker_path) {
Ok(backend) => match backend.trim() {
"keyring" => keyring_key::load_or_create()
.map_err(|reason| Error::Crypto(format!("OS keyring not available ({reason})"))),
_ => file_key::load_or_create(&key_file_path()),
},
Err(_) => match keyring_key::load_or_create() {
Ok(key) => {
write_key_backend_marker(&marker_path, "keyring");
Ok(key)
}
Err(reason) => { Err(reason) => {
logger_ctdra::warn( logger_ctdra::warn(
"crypto", "crypto",
&format!("OS keyring not available ({reason}), using key file"), &format!("OS keyring not available ({reason}), using key file"),
); );
file_key::load_or_create(&key_file_path()) let key = file_key::load_or_create(&key_file_path())?;
write_key_backend_marker(&marker_path, "file");
Ok(key)
} }
},
}
}
fn write_key_backend_marker(marker_path: &Path, backend: &str) {
if let Some(dir) = marker_path.parent() {
let _ = fs::create_dir_all(dir);
}
if let Err(e) = fs::write(marker_path, backend) {
logger_ctdra::warn(
"crypto",
&format!(
"could not persist key backend marker '{}': {e}",
marker_path.display()
),
);
} }
} }
@@ -45,6 +81,14 @@ fn key_file_path() -> PathBuf {
.unwrap_or_else(|| PathBuf::from(KEY_FILE_NAME)) .unwrap_or_else(|| PathBuf::from(KEY_FILE_NAME))
} }
fn key_backend_marker_path() -> PathBuf {
let config_path = config_ctdra::get_config_path();
config_path
.parent()
.map(|dir| dir.join(KEY_BACKEND_MARKER_NAME))
.unwrap_or_else(|| PathBuf::from(KEY_BACKEND_MARKER_NAME))
}
mod file_key { mod file_key {
use super::*; use super::*;
@@ -89,14 +133,17 @@ mod file_key {
#[cfg(not(unix))] #[cfg(not(unix))]
let mut opts = OpenOptions::new(); let mut opts = OpenOptions::new();
let mut file = opts match opts.write(true).create_new(true).open(path) {
.write(true) Ok(mut file) => {
.create_new(true)
.open(path)
.map_err(|e| Error::io(path, e))?;
file.write_all(&key).map_err(|e| Error::io(path, e))?; file.write_all(&key).map_err(|e| Error::io(path, e))?;
Ok(key) Ok(key)
} }
// Another process won the race and created the file first; use its key instead of
// silently generating our own (which would desynchronize the two processes' keys).
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => read(path),
Err(e) => Err(Error::io(path, e)),
}
}
fn fill_random(buf: &mut [u8]) -> Result<()> { fn fill_random(buf: &mut [u8]) -> Result<()> {
::getrandom::fill(buf) ::getrandom::fill(buf)
@@ -115,10 +162,22 @@ mod keyring_key {
Ok(hex_key) => decode(&hex_key), Ok(hex_key) => decode(&hex_key),
Err(keyring::Error::NoEntry) => { Err(keyring::Error::NoEntry) => {
let key = generate()?; let key = generate()?;
entry if let Err(e) = entry.set_password(&encode(&key)) {
.set_password(&encode(&key)) // A concurrent first run may have already created the entry; use its key
.map_err(|e| format!("Could not store key in keyring: {e}"))?; // instead of failing outright.
Ok(key) return match entry.get_password() {
Ok(hex_key) => decode(&hex_key),
Err(_) => Err(format!("Could not store key in keyring: {e}")),
};
}
// Re-read the entry: a concurrent writer may have overwritten ours after our
// own `set_password` succeeded. Using whichever key ultimately "won" ensures
// both processes agree on the same key instead of one silently using a key
// that was never actually persisted.
match entry.get_password() {
Ok(hex_key) => decode(&hex_key),
Err(_) => Ok(key),
}
} }
Err(e) => Err(format!("Keyring access failed: {e}")), Err(e) => Err(format!("Keyring access failed: {e}")),
} }