goblin: make password-change identity re-encryption all-or-nothing and rebind the live service
reencrypt_all was best-effort per file: a partial failure left some identities on the old password and some on the new, a split the user cannot open with one password. Make it all-or-nothing: stage every re-encryption to a sibling temp file and only commit (rename into place) once every identity re-encrypts cleanly; any phase-1 failure deletes the temps and leaves every identity on the old password. Surface the failure: change_password now returns a hard error when the re-encryption fails instead of logging and reporting success. Keep the running NostrService in sync after a successful change by refreshing its in-memory identity blobs to the new password, so same-session gated ops (which re-unlock with the new password) work without a wallet reopen. The decrypted keys are unchanged by a password change, so listening and sending are untouched. Adds an all-or-nothing test.
This commit is contained in:
2ro
@@ -245,6 +245,22 @@ impl NostrService {
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-encrypt every in-memory held identity's ncryptsec from `old` to `new`,
|
||||
/// keeping the running service in sync with a wallet-password change without a
|
||||
/// teardown. A password change does not alter the decrypted keys, so sending
|
||||
/// and listening keep working untouched; this only refreshes the encrypted
|
||||
/// blobs the in-memory copies carry, so a same-session gated op (which
|
||||
/// re-unlocks the stored identity with the NEW password) and any later
|
||||
/// identity-file save both use the new password rather than the stale old one.
|
||||
/// Best-effort per copy: a copy that fails to re-encrypt is left as is (a
|
||||
/// gated op on it may then need the app reopened), never a hard error.
|
||||
pub fn reencrypt_in_memory(&self, old: &str, new: &str) {
|
||||
for h in self.recv.write().iter_mut() {
|
||||
let _ = h.identity.reencrypt(old, new);
|
||||
}
|
||||
let _ = self.identity.write().reencrypt(old, new);
|
||||
}
|
||||
|
||||
/// Instant, purely-local identity switch: re-point the active keys/identity to
|
||||
/// a held identity already unlocked and already listening. No password, no
|
||||
/// teardown, no catch-up. `false` if `hex` is not held.
|
||||
|
||||
+98
-24
@@ -286,40 +286,66 @@ impl HeldIdentities {
|
||||
}
|
||||
|
||||
/// Re-encrypt every held identity's ncryptsec from `old` to `new`, in place
|
||||
/// on disk. Used by the wallet-password change so all front doors follow the
|
||||
/// one password. Best-effort per file; returns the first error encountered
|
||||
/// after attempting the rest.
|
||||
/// on disk, ALL-OR-NOTHING. Used by the wallet-password change so all front
|
||||
/// doors follow the one password. A partial move (some identities on the new
|
||||
/// password, some still on the old) would be a split state the user cannot
|
||||
/// recover with a single password, so this stages every re-encryption to a
|
||||
/// sibling temp file first and only commits (renames into place) once EVERY
|
||||
/// identity has re-encrypted cleanly. Any phase-1 failure deletes the temps
|
||||
/// and leaves every identity untouched on the old password.
|
||||
pub fn reencrypt_all(
|
||||
&self,
|
||||
nostr_dir: &PathBuf,
|
||||
old: &str,
|
||||
new: &str,
|
||||
) -> Result<(), HeldError> {
|
||||
let mut first_err = None;
|
||||
let suffix = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_nanos())
|
||||
.unwrap_or(0);
|
||||
// Phase 1: decrypt+re-encrypt every identity and stage it to a temp file
|
||||
// beside its target. Nothing is committed yet, so a failure here (wrong old
|
||||
// password, unreadable file, encrypt error) aborts the WHOLE change with
|
||||
// every identity still untouched on the old password, never a split.
|
||||
let mut staged: Vec<(PathBuf, PathBuf)> = Vec::new();
|
||||
let cleanup = |staged: &[(PathBuf, PathBuf)]| {
|
||||
for (tmp, _) in staged {
|
||||
let _ = fs::remove_file(tmp);
|
||||
}
|
||||
};
|
||||
for entry in &self.identities {
|
||||
let abs = entry.abs_path(nostr_dir);
|
||||
match NostrIdentity::load_at(&abs) {
|
||||
Some(mut id) => {
|
||||
if let Err(e) = id.reencrypt(old, new) {
|
||||
first_err.get_or_insert(HeldError::Io(e.to_string()));
|
||||
continue;
|
||||
}
|
||||
if let Err(e) = id.save_at(&abs) {
|
||||
first_err.get_or_insert(HeldError::Io(e.to_string()));
|
||||
}
|
||||
}
|
||||
None => {
|
||||
first_err.get_or_insert(HeldError::Io(format!(
|
||||
"identity file unreadable: {}",
|
||||
entry.path
|
||||
)));
|
||||
}
|
||||
let Some(mut id) = NostrIdentity::load_at(&abs) else {
|
||||
cleanup(&staged);
|
||||
return Err(HeldError::Io(format!(
|
||||
"identity file unreadable: {}",
|
||||
entry.path
|
||||
)));
|
||||
};
|
||||
if let Err(e) = id.reencrypt(old, new) {
|
||||
cleanup(&staged);
|
||||
return Err(HeldError::Io(e.to_string()));
|
||||
}
|
||||
let tmp =
|
||||
abs.with_extension(format!("reencrypt-tmp-{}-{}", std::process::id(), suffix));
|
||||
if let Err(e) = id.save_at(&tmp) {
|
||||
cleanup(&staged);
|
||||
return Err(HeldError::Io(e.to_string()));
|
||||
}
|
||||
staged.push((tmp, abs));
|
||||
}
|
||||
// Phase 2: every identity re-encrypted cleanly; commit by renaming each
|
||||
// temp over its target. This window is small; a rename that still fails
|
||||
// returns an explicit error rather than pretending the change succeeded.
|
||||
for (tmp, abs) in &staged {
|
||||
if let Err(e) = fs::rename(tmp, abs) {
|
||||
return Err(HeldError::Io(format!(
|
||||
"committing re-encrypted identity {}: {e}",
|
||||
abs.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
match first_err {
|
||||
Some(e) => Err(e),
|
||||
None => Ok(()),
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -540,6 +566,54 @@ mod tests {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reencrypt_all_is_all_or_nothing() {
|
||||
// Two held identities, one with a corrupted ncryptsec so its decrypt (and
|
||||
// thus the whole re-encryption) fails. The batch must fail AND leave the
|
||||
// other identity untouched on the OLD password (no partial move).
|
||||
let dir = tmpdir("allornothing");
|
||||
let (legacy, _) = NostrIdentity::create_random("old").unwrap();
|
||||
legacy.save(&dir).unwrap();
|
||||
let legacy_hex = legacy.pubkey_hex().unwrap();
|
||||
let (mut idx, _) = HeldIdentities::load_or_migrate(&dir, &legacy).unwrap();
|
||||
let (second, _) = NostrIdentity::create_random("old").unwrap();
|
||||
let second_hex = idx.add(&dir, &second).unwrap();
|
||||
|
||||
// Corrupt the second identity's ncryptsec on disk.
|
||||
let second_abs = idx.entry(&second_hex).unwrap().abs_path(&dir);
|
||||
let mut broken = NostrIdentity::load_at(&second_abs).unwrap();
|
||||
broken.ncryptsec = "ncryptsec1qqqqqbroken".to_string();
|
||||
broken.save_at(&second_abs).unwrap();
|
||||
|
||||
// All-or-nothing: the batch fails...
|
||||
assert!(
|
||||
idx.reencrypt_all(&dir, "old", "new").is_err(),
|
||||
"a failing identity must abort the whole re-encryption"
|
||||
);
|
||||
// ...and the untouched legacy identity still opens under the OLD password,
|
||||
// never having been committed to the new one.
|
||||
let legacy_still = idx.entry(&legacy_hex).unwrap().load(&dir).unwrap();
|
||||
assert!(
|
||||
legacy_still.unlock("old").is_ok(),
|
||||
"untouched identity must still open under the old password"
|
||||
);
|
||||
assert!(
|
||||
legacy_still.unlock("new").is_err(),
|
||||
"untouched identity must not have moved to the new password"
|
||||
);
|
||||
// No temp files left behind after the aborted batch.
|
||||
let leftovers: Vec<_> = std::fs::read_dir(&dir)
|
||||
.unwrap()
|
||||
.filter_map(|e| e.ok())
|
||||
.filter(|e| e.file_name().to_string_lossy().contains("reencrypt-tmp"))
|
||||
.collect();
|
||||
assert!(
|
||||
leftovers.is_empty(),
|
||||
"aborted batch must clean up its temps"
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_nsec_adds_as_held_identity_and_unlocks() {
|
||||
// The multi-identity import path: a bare nsec becomes a held identity via
|
||||
|
||||
+17
-8
@@ -2277,15 +2277,24 @@ impl Wallet {
|
||||
}
|
||||
// The grin seed password changed; re-encrypt EVERY held nostr identity's
|
||||
// ncryptsec from old to new through the same NIP-49 path, so all front
|
||||
// doors follow the one wallet password. Best-effort and non-fatal: a
|
||||
// failure is logged (never swallowed as success), and the grin password
|
||||
// change already committed above. Takes effect for the running service on
|
||||
// the next wallet open (it reloads the active identity from disk).
|
||||
// doors follow the one wallet password. This is ALL-OR-NOTHING: a partial
|
||||
// move would leave some identities on the old password and some on the new
|
||||
// (a split the user cannot open with one password), so on any failure we
|
||||
// surface a hard error rather than silently continuing. Every identity is
|
||||
// then left untouched on the old password.
|
||||
let nostr_dir = self.get_config().get_nostr_path();
|
||||
if let Some(index) = HeldIdentities::load(&nostr_dir)
|
||||
&& let Err(e) = index.reencrypt_all(&nostr_dir, &old, &new)
|
||||
{
|
||||
error!("nostr: re-encrypting held identities after password change: {e}");
|
||||
if let Some(index) = HeldIdentities::load(&nostr_dir) {
|
||||
index.reencrypt_all(&nostr_dir, &old, &new).map_err(|e| {
|
||||
error!("nostr: re-encrypting held identities after password change: {e}");
|
||||
Error::GenericError(format!("nostr identity re-encryption failed: {e}"))
|
||||
})?;
|
||||
// Keep the running service in sync so same-session gated ops (which
|
||||
// re-unlock with the new password) work without a wallet reopen. The
|
||||
// decrypted keys are unchanged by a password change, so this only
|
||||
// refreshes the in-memory encrypted blobs.
|
||||
if let Some(svc) = self.nostr_service() {
|
||||
svc.reencrypt_in_memory(&old, &new);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user