fix(config): abort a persistent start when the identity key path cannot be examined

`Path::exists` reports false both for a key file that is absent and for one
whose metadata cannot be read, so a key symlinked onto a volume that did not
mount, or one in a directory the daemon cannot search, read as a first boot.
The node then generated a fresh identity, failed to store it, and carried on
under an npub that every peer whose allowlist names the old one refuses.

Presence is now decided by `symlink_metadata`, where only `NotFound` counts
as an absence. A dangling symlink reads as present, so the read that follows
aborts the start with the path named, and any other lookup failure aborts
directly. The legacy system-directory fallback applies the same test. A key
file that is genuinely absent still generates and persists a new identity on
first boot.
This commit is contained in:
Johnathan Corgan
2026-09-17 20:28:39 +00:00
parent b459ca029b
commit 51353faaaa
2 changed files with 169 additions and 13 deletions
+13
View File
@@ -79,6 +79,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
values the open-discovery tutorial described never occurred, and the
tutorial no longer lists them. The response shape is unchanged.
#### Identity & config
- A persistent node whose identity key path cannot be examined now refuses to
start instead of coming up under a new identity. `Path::exists` reports false
both for a key that is absent and for one whose metadata cannot be read, so a
key symlinked onto a volume that did not mount, or one in a directory the
daemon cannot search, read as a first boot: the node generated a fresh
identity, failed to store it, and carried on under an npub that every peer
whose allowlist names the old one refuses. Only a `NotFound` result is now
treated as an absence; any other failure to stat the path aborts the start and
names the path. A dangling symlink likewise aborts rather than being replaced.
The legacy `/etc/fips/fips.key` lookup follows the same rule.
### Changed
- The lockfile moves `chacha20` from 0.10.1 to 0.10.2, because 0.10.1 is yanked.
+156 -13
View File
@@ -120,15 +120,44 @@ const LEGACY_SYSTEM_CONFIG_DIR: &str = "/etc/fips";
/// - `key_path` sits in `system_dir`, so an operator using `./fips.yaml` or a
/// user config is never redirected to a system key
/// - a key does exist at `legacy_dir`
fn legacy_key_fallback(key_path: &Path, system_dir: &Path, legacy_dir: &Path) -> Option<PathBuf> {
if system_dir == legacy_dir || key_path.exists() {
return None;
///
/// Returns an error when either location cannot be examined, which the caller
/// aborts on: a lookup that failed is not evidence that no key is there.
fn legacy_key_fallback(
key_path: &Path,
system_dir: &Path,
legacy_dir: &Path,
) -> Result<Option<PathBuf>, ConfigError> {
if system_dir == legacy_dir || key_file_present(key_path)? {
return Ok(None);
}
if key_path.parent() != Some(system_dir) {
return None;
return Ok(None);
}
let legacy = legacy_dir.join(KEY_FILENAME);
legacy.exists().then_some(legacy)
Ok(key_file_present(&legacy)?.then_some(legacy))
}
/// Report whether an identity key file is present, distinguishing a genuine
/// absence from a lookup that could not be made.
///
/// `Path::exists` answers false to both, which is what a persistent start must
/// not do: a key symlinked onto a volume that did not mount, or one in a
/// directory the daemon may not search, would read as a first boot and the
/// node would generate and run under a new identity that every peer
/// allowlisting its old npub refuses. `symlink_metadata` reports a symlink
/// itself as present, so the read that follows fails and aborts the start,
/// and any other lookup error is returned for the caller to abort on. Only
/// `NotFound` is an absence, which is the first-boot case.
fn key_file_present(path: &Path) -> Result<bool, ConfigError> {
match path.symlink_metadata() {
Ok(_) => Ok(true),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false),
Err(e) => Err(ConfigError::KeyPathUnreadable {
path: path.to_path_buf(),
source: e,
}),
}
}
/// Derive the public key file path from a config file path.
@@ -508,6 +537,10 @@ pub fn write_pub_file(path: &Path, npub: &str) -> Result<(), ConfigError> {
/// 2. Persistent key file (`fips.key`) — reused across restarts
/// 3. Generate new — creates keypair, writes `fips.key` and `fips.pub`
///
/// A key file that exists but cannot be read, including one whose metadata
/// the daemon cannot look up at all, aborts the start. Only a key file that
/// is genuinely absent reaches step 3.
///
/// - **`nsec` set explicitly**: always uses that, regardless of `persistent`.
///
/// Returns the nsec string (bech32 or hex) to be used for identity creation.
@@ -538,8 +571,10 @@ pub fn resolve_identity(
let pub_path = pub_file_path(&config_ref);
if config.node.identity.persistent {
// Persistent mode: load existing key file or generate-and-persist
if key_path.exists() {
// Persistent mode: load existing key file or generate-and-persist.
// A key path the daemon cannot examine aborts the start here rather
// than falling through to generation.
if key_file_present(&key_path)? {
// Held in a guard, not a bare `String`: if the parse below fails,
// the `?` returns and a bare local would be freed uncleared.
let nsec = Zeroizing::new(read_key_file(&key_path)?);
@@ -566,7 +601,7 @@ pub fn resolve_identity(
&key_path,
Path::new(SYSTEM_CONFIG_DIR),
Path::new(LEGACY_SYSTEM_CONFIG_DIR),
) {
)? {
// Guarded for the same reason as the current-path read above.
let nsec = Zeroizing::new(read_key_file(&legacy)?);
let identity = Identity::from_secret_str(&nsec)?;
@@ -746,6 +781,12 @@ pub enum ConfigError {
#[error("refusing to write key file through a symlink: {path}")]
KeyPathIsSymlink { path: PathBuf },
#[error("cannot determine whether the identity key file {path} exists: {source}")]
KeyPathUnreadable {
path: PathBuf,
source: std::io::Error,
},
#[error("identity error: {0}")]
Identity(#[from] IdentityError),
@@ -1629,7 +1670,7 @@ node:
let key_path = system.join(KEY_FILENAME);
assert_eq!(
legacy_key_fallback(&key_path, &system, &legacy),
legacy_key_fallback(&key_path, &system, &legacy).unwrap(),
Some(legacy_key),
"a key stranded at the legacy path must be adopted, not regenerated"
);
@@ -1643,7 +1684,10 @@ node:
write_stub_key(&legacy);
let key_path = write_stub_key(&system);
assert_eq!(legacy_key_fallback(&key_path, &system, &legacy), None);
assert_eq!(
legacy_key_fallback(&key_path, &system, &legacy).unwrap(),
None
);
}
#[test]
@@ -1655,7 +1699,7 @@ node:
write_stub_key(&dir);
let absent = dir.join("nonexistent").join(KEY_FILENAME);
assert_eq!(legacy_key_fallback(&absent, &dir, &dir), None);
assert_eq!(legacy_key_fallback(&absent, &dir, &dir).unwrap(), None);
}
#[test]
@@ -1670,7 +1714,10 @@ node:
write_stub_key(&legacy);
let key_path = elsewhere.join(KEY_FILENAME);
assert_eq!(legacy_key_fallback(&key_path, &system, &legacy), None);
assert_eq!(
legacy_key_fallback(&key_path, &system, &legacy).unwrap(),
None
);
}
#[test]
@@ -1682,7 +1729,36 @@ node:
std::fs::create_dir_all(&system).unwrap();
let key_path = system.join(KEY_FILENAME);
assert_eq!(legacy_key_fallback(&key_path, &system, &legacy), None);
assert_eq!(
legacy_key_fallback(&key_path, &system, &legacy).unwrap(),
None
);
}
#[cfg(unix)]
#[test]
fn a_legacy_key_whose_metadata_cannot_be_read_is_present_not_absent() {
// A dangling symlink is the case that matters in the field: a key
// symlinked onto a volume that did not mount. Reading it as an
// absence sends a persistent node on to generate a new identity.
let root = TempDir::new().unwrap();
let legacy = root.path().join("etc/fips");
let system = root.path().join("usr/local/etc/fips");
std::fs::create_dir_all(&legacy).unwrap();
std::fs::create_dir_all(&system).unwrap();
let legacy_key = legacy.join(KEY_FILENAME);
std::os::unix::fs::symlink(
root.path().join("unmounted").join(KEY_FILENAME),
&legacy_key,
)
.unwrap();
let key_path = system.join(KEY_FILENAME);
assert_eq!(
legacy_key_fallback(&key_path, &system, &legacy).unwrap(),
Some(legacy_key),
"a legacy key the daemon cannot stat must be reported present, so the read aborts"
);
}
#[test]
@@ -2055,6 +2131,73 @@ node:
assert_eq!(resolved.nsec, resolved2.nsec);
}
#[cfg(unix)]
#[test]
fn persistent_start_aborts_when_the_key_path_is_a_dangling_symlink() {
// The key is symlinked onto a volume that did not mount. The node
// must not read that as a first boot and take a new identity, which
// every peer whose allowlist names the old npub would then refuse.
let temp_dir = TempDir::new().unwrap();
let config_path = temp_dir.path().join("fips.yaml");
let key_path = temp_dir.path().join("fips.key");
let unmounted = temp_dir.path().join("unmounted").join("fips.key");
fs::write(&config_path, "node:\n identity:\n persistent: true\n").unwrap();
std::os::unix::fs::symlink(&unmounted, &key_path).unwrap();
let config = Config::load_file(&config_path).unwrap();
// `ResolvedIdentity` carries the secret and has no `Debug`, so the
// failure is matched rather than unwrapped.
let Err(err) = resolve_identity(&config, std::slice::from_ref(&config_path)) else {
panic!("a key path that cannot be read must abort the start, not generate a new key");
};
assert!(
err.to_string().contains(&key_path.display().to_string()),
"the diagnostic must name the key path, got {err}"
);
assert!(
key_path
.symlink_metadata()
.unwrap()
.file_type()
.is_symlink(),
"the symlink itself must be left in place"
);
assert!(
!unmounted.exists(),
"nothing may be written through the symlink"
);
assert!(
!temp_dir.path().join("fips.pub").exists(),
"an aborted start writes neither key file"
);
}
#[cfg(unix)]
#[test]
fn persistent_start_aborts_when_the_key_path_cannot_be_examined() {
// A key path whose parent is not a directory fails the lookup with an
// error that is not an absence, the same shape as a directory the
// daemon may not search, and unlike a permission case it behaves the
// same for root.
let temp_dir = TempDir::new().unwrap();
let blocked = temp_dir.path().join("blocked");
fs::write(&blocked, "not a directory\n").unwrap();
let config_path = blocked.join("fips.yaml");
let mut config = Config::new();
config.node.identity.persistent = true;
let Err(err) = resolve_identity(&config, std::slice::from_ref(&config_path)) else {
panic!("a key path that cannot be examined must abort the start");
};
assert!(
err.to_string().contains(&blocked.display().to_string()),
"the diagnostic must name the key path, got {err}"
);
}
#[test]
fn test_to_yaml_empty_nsec_omitted() {
let config = Config::new();