kestrove/kestrove-desktop
2
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases 1 Wiki Activity

Removed devices: reset the device key and id in place, then set up again

Browse Source 
DeviceKey implements kv-ffi's DeviceSigner.reset(): it deletes the key material
(TPM key, DPAPI file, Secret Service / keychain entry, or key file) and the
device-id and store files, then creates a new key with a new id in the best
store available. The core calls it on device_revoked and retries a setup code's
ticket once, so a fresh Add-a-device code signs a removed computer straight in.
DeviceRevoked explains how to set it up again; the fingerprint shown refreshes.
Removing a device warns that it must be set up again as a new one.

Test: end to end through kv-sync's test server (new dev-dependency).
The Windows TPM/DPAPI paths could not be compiled here (no Windows SDK for ring).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
sami
2026-10-20 22:35:13 +09:00
co-authored by Claude Opus 5.5
parent c4a677fc3d
commit 86c21f3bff
5 changed files with 289 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src-tauri/Cargo.lock
Generated
+93
View File
@@ -245,6 +245,16 @@ dependencies = [
"nom",
]
[[package]]
name = "assert-json-diff"
version = "2.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "47e4f2b81832e72834d7518d8487a0396a28cc408186a2e8854c0f98011faf12"
dependencies = [
"serde",
"serde_json",
]
[[package]]
name = "async-broadcast"
version = "0.7.2"
@@ -1243,6 +1253,24 @@ dependencies = [
"windows-sys 0.61.2",
]
[[package]]
name = "deadpool"
version = "0.12.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0be2b1d1d6ec8d846f05e137292d0b89133caf95ef33695424c09568bdd39b1b"
dependencies = [
"deadpool-runtime",
"lazy_static",
"num_cpus",
"tokio",
]
[[package]]
name = "deadpool-runtime"
version = "0.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b"
[[package]]
name = "defmt"
version = "1.2.0"
@@ -1791,6 +1819,21 @@ dependencies = [
"autocfg",
]
[[package]]
name = "futures"
version = "0.3.34"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9a31d2a3fbaaeb2af2368bbdd904aa8e812d3c04a1ee10d3171f52d556e5d0a3"
dependencies = [
"futures-channel",
"futures-core",
"futures-executor",
"futures-io",
"futures-sink",
"futures-task",
"futures-util",
]
[[package]]
name = "futures-channel"
version = "0.3.34"
@@ -1798,6 +1841,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b1f9e3d69d39e4862ffed03ed071a76f9a13ba1d9109d355b0f0aa6b15e393c4"
dependencies = [
"futures-core",
"futures-sink",
]
[[package]]
@@ -1865,6 +1909,7 @@ version = "0.3.34"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0d50a92467f8ba5dd6e3ee5d4bd04d73ab2e4e1c44474a0674821dfce14b79bc"
dependencies = [
"futures-channel",
"futures-core",
"futures-io",
"futures-macro",
@@ -2550,6 +2595,12 @@ version = "1.10.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87"
[[package]]
name = "httpdate"
version = "1.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9"
[[package]]
name = "hybrid-array"
version = "0.4.15"
@@ -2573,6 +2624,7 @@ dependencies = [
"http",
"http-body",
"httparse",
"httpdate",
"itoa",
"pin-project-lite",
"smallvec",
@@ -3016,6 +3068,7 @@ dependencies = [
"image",
"keyring",
"kv-ffi",
"kv-sync",
"libc",
"p256",
"qrcode",
@@ -3205,9 +3258,16 @@ dependencies = [
"url",
"uuid",
"webpki-roots 0.26.11",
"wiremock",
"zeroize",
]
[[package]]
name = "lazy_static"
version = "1.5.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "20870f649af7073d53e38067b2a84312175d56ea15217e1b15bc83506ec50afb"
[[package]]
name = "libappindicator"
version = "0.9.0"
@@ -3565,6 +3625,16 @@ dependencies = [
"autocfg",
]
[[package]]
name = "num_cpus"
version = "1.17.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "91df4bbde75afed763b708b7eee1e8e7651e02d97f6d5dd763e89367e957b23b"
dependencies = [
"hermit-abi",
"libc",
]
[[package]]
name = "num_enum"
version = "0.7.6"
@@ -6888,6 +6958,29 @@ dependencies = [
"windows-sys 0.61.2",
]
[[package]]
name = "wiremock"
version = "0.6.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031"
dependencies = [
"assert-json-diff",
"base64 0.22.1",
"deadpool",
"futures",
"http",
"http-body-util",
"hyper",
"hyper-util",
"log",
"once_cell",
"regex",
"serde",
"serde_json",
"tokio",
"url",
]
[[package]]
name = "wit-bindgen"
version = "0.57.1"
src-tauri/Cargo.toml
+1
View File
@@ -77,6 +77,7 @@ core-foundation = "0.10"
[dev-dependencies]
tempfile = "3"
kv-sync = { path = "../../kestrove-core/crates/kv-sync", features = ["test-server"] }
tokio = { version = "1", features = ["rt-multi-thread", "macros"] }
[profile.release]
src-tauri/src/device_key.rs
+140 -37
View File
@@ -8,10 +8,14 @@
//!
//! The store that created the key is recorded next to the device id. A later launch uses only that store: if it
//! is unavailable, signing fails instead of silently enrolling a new device.
//!
//! When the server removed this device (docs/03 "Removed devices"), the core calls [`DeviceKey`]'s `reset`: the old
//! key and id are deleted and a new key with a new id is created in place, in the best store available.
use std::fmt;
use std::io;
use std::path::{Path, PathBuf};
use std::sync::{RwLock, RwLockReadGuard};
use kv_ffi::CoreError;
use p256::ecdsa::{signature::Signer as _, Signature, SigningKey};
@@ -71,16 +75,25 @@ enum Backend {
Cng(cng::Key),
}
pub struct DeviceKey {
/// One device identity: the id the server knows and its key.
struct Identity {
id: String,
public: Vec<u8>,
backend: Backend,
storage: KeyStorage,
}
pub struct DeviceKey {
data_dir: PathBuf,
/// Stores to create a key in, best first (for the first key and after a reset).
preferred: Vec<KeyStorage>,
current: RwLock<Identity>,
}
impl fmt::Debug for DeviceKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("DeviceKey").field("id", &self.id).field("storage", &self.storage).finish_non_exhaustive()
let cur = self.read();
f.debug_struct("DeviceKey").field("id", &cur.id).field("storage", &cur.storage).finish_non_exhaustive()
}
}
@@ -96,51 +109,61 @@ impl DeviceKey {
}
fn load_or_create_with(data_dir: &Path, preferred: &[KeyStorage]) -> io::Result<Self> {
let id_path = data_dir.join(ID_FILE);
let store_path = data_dir.join(STORE_FILE);
let id = read_text(&id_path)?;
let recorded = read_text(&store_path)?.and_then(|s| KeyStorage::from_tag(&s));
let identity = Self::load_identity(data_dir)?.map_or_else(|| create_identity(data_dir, preferred), Ok)?;
Ok(Self { data_dir: data_dir.to_path_buf(), preferred: preferred.to_vec(), current: RwLock::new(identity) })
}
fn load_identity(data_dir: &Path) -> io::Result<Option<Identity>> {
let id = read_text(&data_dir.join(ID_FILE))?;
let recorded = read_text(&data_dir.join(STORE_FILE))?.and_then(|s| KeyStorage::from_tag(&s));
if let (Some(id), Some(storage)) = (id, recorded) {
if let Some(key) = load(data_dir, &id, storage)? {
return Ok(key);
return Ok(Some(key));
}
// The id is known but its key is gone (keyring wiped, folder copied from another machine):
// this is a new device as far as the server is concerned.
eprintln!("kestrove: the device key in {} is missing; creating a new device key", storage.describe());
}
let id = uuid::Uuid::new_v4().to_string();
let mut last_err = io::Error::other("no key store available");
for &storage in preferred {
match create(data_dir, &id, storage) {
Ok(key) => {
private_file::write(&id_path, id.as_bytes())?;
private_file::write(&store_path, storage.tag().as_bytes())?;
return Ok(key);
}
Err(e) => {
eprintln!("kestrove: device key store {} unavailable: {e}", storage.describe());
last_err = e;
}
}
}
Err(last_err)
Ok(None)
}
fn read(&self) -> RwLockReadGuard<'_, Identity> {
self.current.read().unwrap_or_else(|e| e.into_inner())
}
pub fn storage(&self) -> KeyStorage {
self.storage
self.read().storage
}
/// Shown in settings when the key is less protected than it should be.
pub fn warning(&self) -> Option<&'static str> {
(self.storage == KeyStorage::File).then_some(
(self.storage() == KeyStorage::File).then_some(
"No system keyring was found, so this device's sync key is stored in a file only your user can read.",
)
}
pub fn id(&self) -> &str {
&self.id
pub fn id(&self) -> String {
self.read().id.clone()
}
/// The server removed this device for good: deletes the key and the device id, then creates a new key with a new
/// id. Afterwards the server sees a new device (pending approval, or approved by an enrollment ticket).
pub fn reset(&self) -> io::Result<()> {
let mut cur = self.current.write().unwrap_or_else(|e| e.into_inner());
// Delete first: the file and DPAPI stores reuse one path, so a new key written first would be deleted.
destroy(&self.data_dir, &cur)?;
for f in [ID_FILE, STORE_FILE] {
match std::fs::remove_file(self.data_dir.join(f)) {
Err(e) if e.kind() != io::ErrorKind::NotFound => return Err(e),
_ => {}
}
}
*cur = create_identity(&self.data_dir, &self.preferred)?;
Ok(())
}
}
impl Identity {
fn sign_raw(&self, message: &[u8]) -> io::Result<Vec<u8>> {
match &self.backend {
Backend::Software(k) => {
@@ -155,15 +178,58 @@ impl DeviceKey {
impl kv_ffi::DeviceSigner for DeviceKey {
fn device_id(&self) -> String {
self.id.clone()
self.id()
}
fn public_key(&self) -> Vec<u8> {
self.public.clone()
self.read().public.clone()
}
fn sign(&self, message: Vec<u8>) -> Result<Vec<u8>, CoreError> {
self.sign_raw(&message).map_err(|e| CoreError::Internal { msg: format!("device key: {e}") })
self.read().sign_raw(&message).map_err(|e| CoreError::Internal { msg: format!("device key: {e}") })
}
fn reset(&self) -> Result<(), CoreError> {
DeviceKey::reset(self).map_err(|e| CoreError::Internal { msg: format!("device key reset: {e}") })
}
}
/// A new key with a new device id in the first store that works; records both.
fn create_identity(data_dir: &Path, preferred: &[KeyStorage]) -> io::Result<Identity> {
let id = uuid::Uuid::new_v4().to_string();
let mut last_err = io::Error::other("no key store available");
for &storage in preferred {
match create(data_dir, &id, storage) {
Ok(key) => {
private_file::write(&data_dir.join(ID_FILE), id.as_bytes())?;
private_file::write(&data_dir.join(STORE_FILE), storage.tag().as_bytes())?;
return Ok(key);
}
Err(e) => {
eprintln!("kestrove: device key store {} unavailable: {e}", storage.describe());
last_err = e;
}
}
}
Err(last_err)
}
/// Deletes `identity`'s key material from its store. Missing material is not an error.
fn destroy(data_dir: &Path, identity: &Identity) -> io::Result<()> {
let remove = |p: PathBuf| match std::fs::remove_file(p) {
Err(e) if e.kind() != io::ErrorKind::NotFound => Err(e),
_ => Ok(()),
};
match identity.storage {
KeyStorage::File => remove(key_file(data_dir)),
#[cfg(not(windows))]
KeyStorage::Keychain | KeyStorage::SecretService => keyring_slot::delete(&identity.id),
#[cfg(windows)]
KeyStorage::Dpapi => remove(key_file(data_dir).with_extension("dpapi")),
#[cfg(windows)]
KeyStorage::Tpm => cng::Key::delete(&identity.id),
#[allow(unreachable_patterns)]
_ => Ok(()),
}
}
@@ -184,9 +250,9 @@ fn platform_stores() -> Vec<KeyStorage> {
}
}
fn software(id: &str, key: SigningKey, storage: KeyStorage) -> DeviceKey {
fn software(id: &str, key: SigningKey, storage: KeyStorage) -> Identity {
let public = key.verifying_key().to_encoded_point(false).as_bytes().to_vec();
DeviceKey { id: id.to_string(), public, backend: Backend::Software(key), storage }
Identity { id: id.to_string(), public, backend: Backend::Software(key), storage }
}
fn parse_scalar(bytes: &[u8]) -> io::Result<SigningKey> {
@@ -208,7 +274,7 @@ fn key_file(data_dir: &Path) -> PathBuf {
data_dir.join("device-key")
}
fn load(data_dir: &Path, id: &str, storage: KeyStorage) -> io::Result<Option<DeviceKey>> {
fn load(data_dir: &Path, id: &str, storage: KeyStorage) -> io::Result<Option<Identity>> {
let secret = match storage {
KeyStorage::File => private_file::read(&key_file(data_dir))?.map(Zeroizing::new),
#[cfg(not(windows))]
@@ -220,7 +286,7 @@ fn load(data_dir: &Path, id: &str, storage: KeyStorage) -> io::Result<Option<Dev
},
#[cfg(windows)]
KeyStorage::Tpm => {
return Ok(cng::Key::open(id)?.map(|k| DeviceKey {
return Ok(cng::Key::open(id)?.map(|k| Identity {
id: id.to_string(),
public: k.public_key().unwrap_or_default(),
backend: Backend::Cng(k),
@@ -233,12 +299,12 @@ fn load(data_dir: &Path, id: &str, storage: KeyStorage) -> io::Result<Option<Dev
secret.map(|s| Ok(software(id, parse_scalar(&s)?, storage))).transpose()
}
fn create(data_dir: &Path, id: &str, storage: KeyStorage) -> io::Result<DeviceKey> {
fn create(data_dir: &Path, id: &str, storage: KeyStorage) -> io::Result<Identity> {
#[cfg(windows)]
if storage == KeyStorage::Tpm {
let k = cng::Key::create(id)?;
let public = k.public_key()?;
return Ok(DeviceKey { id: id.to_string(), public, backend: Backend::Cng(k), storage });
return Ok(Identity { id: id.to_string(), public, backend: Backend::Cng(k), storage });
}
let secret = new_scalar();
match storage {
@@ -275,6 +341,13 @@ mod keyring_slot {
}
}
pub fn delete(id: &str) -> io::Result<()> {
match entry(id)?.delete_credential() {
Ok(()) | Err(keyring::Error::NoEntry) => Ok(()),
Err(e) => Err(io::Error::other(e)),
}
}
pub fn put(id: &str, secret: &[u8]) -> io::Result<()> {
let e = entry(id)?;
e.set_secret(secret).map_err(io::Error::other)?;
@@ -361,7 +434,7 @@ mod cng {
use sha2::{Digest, Sha256};
use windows_sys::Win32::Security::Cryptography::{
NCryptCreatePersistedKey, NCryptExportKey, NCryptFinalizeKey, NCryptFreeObject, NCryptOpenKey,
NCryptCreatePersistedKey, NCryptDeleteKey, NCryptExportKey, NCryptFinalizeKey, NCryptFreeObject, NCryptOpenKey,
NCryptOpenStorageProvider, NCryptSignHash, BCRYPT_ECCKEY_BLOB, BCRYPT_ECCPUBLIC_BLOB,
BCRYPT_ECDSA_P256_ALGORITHM, BCRYPT_ECDSA_PUBLIC_P256_MAGIC, MS_PLATFORM_CRYPTO_PROVIDER, NCRYPT_KEY_HANDLE,
NCRYPT_PROV_HANDLE, NCRYPT_SILENT_FLAG,
@@ -443,6 +516,15 @@ mod cng {
Ok(key)
}
/// Deletes the persisted TPM key for `id` (the server removed this device). A missing key is fine.
pub fn delete(id: &str) -> io::Result<()> {
let Some(key) = Self::open(id)? else { return Ok(()) };
// SAFETY: a live handle; NCryptDeleteKey also frees it, so `Drop` must not run afterwards.
let hr = unsafe { NCryptDeleteKey(key.0, NCRYPT_SILENT_FLAG) };
std::mem::forget(key);
check(hr, "deleting the TPM key")
}
/// SEC1 uncompressed public key (0x04 || X || Y).
pub fn public_key(&self) -> io::Result<Vec<u8>> {
let mut buf = [0u8; 8 + 64];
@@ -533,6 +615,27 @@ mod tests {
assert_ne!(a.device_id(), b.device_id());
}
/// docs/03 "Removed devices": a reset replaces the key and the id, on disk too, and leaves no old key behind.
#[test]
fn reset_replaces_key_and_id() {
let dir = tempfile::tempdir().unwrap();
let a = DeviceKey::load_or_create_file(dir.path()).unwrap();
let (old_id, old_pk) = (a.device_id(), a.public_key());
a.reset().unwrap();
assert_ne!(a.device_id(), old_id);
assert_ne!(a.public_key(), old_pk);
assert!(uuid::Uuid::parse_str(&a.device_id()).is_ok());
let sig = a.sign(b"after".to_vec()).unwrap();
VerifyingKey::from_sec1_bytes(&a.public_key())
.unwrap()
.verify(b"after", &Signature::from_slice(&sig).unwrap())
.unwrap();
// The next launch loads the new identity.
let b = DeviceKey::load_or_create_file(dir.path()).unwrap();
assert_eq!((b.device_id(), b.public_key()), (a.device_id(), a.public_key()));
assert_eq!(a.storage(), KeyStorage::File);
}
#[test]
#[cfg(unix)]
fn key_file_is_owner_only() {
src-tauri/src/sync.rs
+54 -2
View File
@@ -152,6 +152,9 @@ pub fn describe(e: &CoreError) -> String {
CoreError::PendingApproval => "This computer is waiting for approval on one of your other devices.".into(),
CoreError::NotConfigured => "No sync server is set up yet.".into(),
CoreError::NotAuthenticated => "Sign in to the sync server again (lock and unlock with your password).".into(),
CoreError::DeviceRevoked => "This device was removed from your account. Set it up again: lock and unlock with \
your master password, then approve this computer on another device."
.into(),
CoreError::IntegrityAlarm { vault_id, reason } => format!(
"The server sent vault data that failed verification (vault {vault_id}: {reason}). Syncing stopped; \
your local data is unchanged."
@@ -426,8 +429,11 @@ impl SyncService {
.map(|ds| ds.iter().filter(|d| !d.approved && !d.revoked && !d.current).count() as u32);
}
let err = result.as_ref().err().map(describe);
// A removed device got a new key inside the core (docs/03 "Removed devices"): show its fingerprint.
let fingerprint = self.core.device_key_fingerprint().ok();
self.update(|s| {
s.syncing = false;
s.device_fingerprint = fingerprint;
match &result {
Ok(r) => {
s.last_sync_at = Some(now_secs());
@@ -543,8 +549,11 @@ impl SyncService {
pub async fn finish_setup(&self, master_password: Zeroizing<String>) -> Result<SignIn, String> {
self.require_key()?;
let setup = lock(&self.pending_setup).clone().ok_or("Scan or paste a setup code first.")?;
let result =
self.core.login_with_setup(setup.clone(), master_password.to_string()).await.map_err(|e| describe(&e))?;
let result = self.core.login_with_setup(setup.clone(), master_password.to_string()).await;
// With a ticket the core already re-enrolled a removed device under a new key; refresh it either way.
let fingerprint = self.core.device_key_fingerprint().ok();
self.update(|s| s.device_fingerprint = fingerprint);
let result = result.map_err(|e| describe(&e))?;
self.update(|s| {
s.server_url = Some(setup.server_url.clone());
s.pins = setup.spki_pins.clone();
@@ -655,6 +664,49 @@ mod tests {
assert_eq!(again.status().device_fingerprint, svc.status().device_fingerprint);
}
/// docs/03 "Removed devices", end to end through kv-sync's test server: this computer waits for approval, is
/// denied (revoked), and a fresh "Add a device" code signs it straight in under a new key and id.
#[tokio::test]
async fn removed_computer_reenrols_with_a_fresh_add_device_code() {
use kv_sync::test_server::{TestServer, SETUP_TOKEN};
const PW: &str = "a long master password";
let server = TestServer::start().await;
let phone_dir = tempfile::tempdir().unwrap();
let phone = VaultCore::new_in_memory();
phone.create_account(PW.into()).unwrap();
phone.configure_server(server.url(), vec![]).unwrap();
let phone_key: Arc<dyn kv_ffi::DeviceSigner> =
Arc::new(DeviceKey::load_or_create_file(phone_dir.path()).unwrap());
phone.set_device_signer(phone_key, "phone".into(), PlatformDto::Ios).unwrap();
phone.register_account(SETUP_TOKEN.into()).await.unwrap();
server.state().require_approval = true;
let dir = tempfile::tempdir().unwrap();
let svc = SyncService::new(
VaultCore::new_in_memory(),
dir.path(),
DeviceKey::load_or_create_file(dir.path()).map_err(|e| e.to_string()),
Box::new(|_| {}),
);
svc.setup_from_text(&phone.setup_payload_for_kit().unwrap()).unwrap();
assert_eq!(svc.finish_setup(PW.to_string().into()).await.unwrap(), SignIn::PendingApproval);
let old_fp = svc.status().device_fingerprint;
let pending = phone.list_devices().await.unwrap().into_iter().find(|d| !d.approved).unwrap();
assert_eq!(Some(pending.key_fingerprint.clone()), old_fp);
let old_id = pending.device_id;
phone.revoke_device(old_id.clone()).await.unwrap();
svc.setup_from_text(&phone.add_device_payload().await.unwrap().payload).unwrap();
assert_eq!(svc.finish_setup(PW.to_string().into()).await.unwrap(), SignIn::Ok);
let new_id = kv_ffi::DeviceSigner::device_id(&DeviceKey::load_or_create_file(dir.path()).unwrap());
assert_ne!(new_id, old_id);
assert_ne!(svc.status().device_fingerprint, old_fp);
assert!(svc.status().logged_in);
let devices = phone.list_devices().await.unwrap();
assert!(devices.iter().any(|d| d.device_id == old_id && d.revoked));
assert!(devices.iter().any(|d| d.device_id == new_id && d.approved));
}
#[tokio::test]
async fn sync_needs_unlock() {
let dir = tempfile::tempdir().unwrap();