fix(sync): bind browser data to one account owner

This commit is contained in:
2026-07-10 10:46:52 -04:00
parent 46eac43326
commit 3be13ca295
16 changed files with 678 additions and 24 deletions
+11 -1
View File
@@ -1,7 +1,7 @@
use serde::Deserialize;
use super::{SyncApiClient, read_json_from_response};
use crate::error::SyncClientError;
use crate::{device_api::is_subject_id, error::SyncClientError};
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
@@ -17,6 +17,16 @@ struct AuthErrorDocument {
}
impl SyncApiClient {
pub fn authenticated_user_id(&self) -> Result<String, SyncClientError> {
let document = self.list_devices()?;
if document.version != 1 || !is_subject_id(&document.user_id) {
return Err(SyncClientError::DeviceTrust {
reason: "authenticated user identity is invalid",
});
}
Ok(document.user_id)
}
pub fn sign_out(&self) -> Result<(), SyncClientError> {
let endpoint = self.endpoint("/api/session/logout");
let response = self
@@ -12,6 +12,40 @@ use crate::{
type TestServer = JoinHandle<std::io::Result<()>>;
#[test]
fn authenticated_user_id_uses_the_read_only_device_list() -> Result<(), Box<dyn Error>> {
let (base_url, server) = spawn_authenticated_server(
"GET /api/devices HTTP/1.1\r\n",
"200 OK",
r#"{"version":1,"user_id":"user-01","devices":[]}"#,
)?;
let client = SyncApiClient::new(
ApiClientConfig::custom(base_url, "auto"),
BearerToken::new("a".repeat(64))?,
)?;
assert_eq!(client.authenticated_user_id()?, "user-01");
join_server(server)?;
for body in [
r#"{"version":2,"user_id":"user-01","devices":[]}"#,
r#"{"version":1,"user_id":"x","devices":[]}"#,
] {
let (base_url, server) =
spawn_authenticated_server("GET /api/devices HTTP/1.1\r\n", "200 OK", body)?;
let client = SyncApiClient::new(
ApiClientConfig::custom(base_url, "auto"),
BearerToken::new("a".repeat(64))?,
)?;
assert!(matches!(
client.authenticated_user_id(),
Err(crate::SyncClientError::DeviceTrust { .. })
));
join_server(server)?;
}
Ok(())
}
#[test]
fn sign_out_posts_the_bearer_and_validates_success() -> Result<(), Box<dyn Error>> {
let (base_url, server) = spawn_logout_server("200 OK", r#"{"version":1,"signed_out":true}"#)?;
+1 -1
View File
@@ -241,7 +241,7 @@ fn challenge_value<'a>(line: &'a str, name: &'static str) -> Result<&'a str, Syn
Ok(value)
}
fn is_subject_id(value: &str) -> bool {
pub(crate) fn is_subject_id(value: &str) -> bool {
(3..=128).contains(&value.len())
&& value.bytes().all(|byte| byte.is_ascii_alphanumeric() || b"._:-".contains(&byte))
}
+9
View File
@@ -20,6 +20,15 @@ pub enum SyncClientError {
#[error("Authenticated session changed during reconciliation")]
SessionChanged,
#[error("Cloud Sync owner is unclaimed; sign in again to bind this browser data")]
SyncOwnerUnclaimed,
#[error("This browser data belongs to a different Ely account")]
SyncOwnerMismatch,
#[error("Cloud Sync owner storage is unavailable: {0}")]
SyncOwnerStorage(String),
#[error("HTTP request failed for {endpoint}: {source}")]
Http {
endpoint: String,
+2
View File
@@ -30,6 +30,7 @@ pub mod encryption;
pub mod error;
pub mod key_store;
pub mod snapshot;
mod sync_owner;
pub mod vault;
mod vault_bootstrap;
@@ -53,6 +54,7 @@ pub use snapshot::{
AuthenticatedSnapshot, AuthenticatedSnapshotHead, SnapshotDownload, SnapshotPayload,
SnapshotUploadRequest,
};
pub use sync_owner::SyncOwnerStore;
pub use vault::{
ACCOUNT_KEY_WRAP_SUITE, ACCOUNT_KEY_WRAP_VERSION, SyncVaultDocument, VaultContext,
WrappedAccountKey,
+308
View File
@@ -0,0 +1,308 @@
use std::{
io::{self, ErrorKind, Read, Write},
path::{Component, Path, PathBuf},
};
use cap_fs_ext::{
DirExt, FollowSymlinks, MetadataExt as CrossPlatformMetadataExt, OpenOptionsFollowExt,
};
use cap_std::{
ambient_authority,
fs::{Dir, File, OpenOptions, Permissions},
};
use uuid::Uuid;
use crate::{SyncClientError, device_api::is_subject_id};
const OWNER_FILE: &str = ".sync-owner-user-id";
const OWNER_TEMP_PREFIX: &str = ".sync-owner-tmp-";
const MAX_SUBJECT_ID_BYTES: usize = 128;
#[derive(Clone, Debug)]
pub struct SyncOwnerStore {
root: PathBuf,
}
impl SyncOwnerStore {
pub fn new(browser_data_root: &Path) -> Self {
Self { root: browser_data_root.to_path_buf() }
}
pub fn claim(&self, user_id: &str) -> Result<(), SyncClientError> {
validate_subject_id(user_id)?;
let directory = open_root(&self.root, true)?
.ok_or_else(|| storage("browser data root is unavailable"))?;
if let Some(owner) = read_owner(&directory)? {
return verify_owner(&owner, user_id);
}
persist_owner(&directory, user_id)
}
pub fn verify(&self, user_id: &str) -> Result<(), SyncClientError> {
validate_subject_id(user_id)?;
let Some(directory) = open_root(&self.root, false)? else {
return Err(SyncClientError::SyncOwnerUnclaimed);
};
let owner = read_owner(&directory)?.ok_or(SyncClientError::SyncOwnerUnclaimed)?;
verify_owner(&owner, user_id)
}
}
fn persist_owner(directory: &Dir, user_id: &str) -> Result<(), SyncClientError> {
let temporary = format!("{OWNER_TEMP_PREFIX}{}", Uuid::now_v7().simple());
let mut options = private_open_options();
options.write(true).create_new(true);
let mut file = directory.open_with(&temporary, &options).map_err(storage_io)?;
validate_private_file(&file)?;
set_private_file_permissions(&file)?;
let write_result = file
.write_all(user_id.as_bytes())
.and_then(|()| file.write_all(b"\n"))
.and_then(|()| file.sync_all());
drop(file);
if let Err(error) = write_result {
let _ = remove_temporary(directory, &temporary);
return Err(storage_io(error));
}
match publish_owner(directory, &temporary) {
Ok(()) => sync_directory(directory)?,
Err(error) if error.kind() == ErrorKind::AlreadyExists => {
remove_temporary(directory, &temporary)?;
}
Err(error) => {
let _ = remove_temporary(directory, &temporary);
return Err(storage_io(error));
}
}
let owner = read_owner(directory)?.ok_or_else(|| storage("sync owner disappeared"))?;
verify_owner(&owner, user_id)
}
fn read_owner(directory: &Dir) -> Result<Option<String>, SyncClientError> {
let mut options = private_open_options();
options.read(true);
let file = match directory.open_with(OWNER_FILE, &options) {
Ok(file) => file,
Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None),
Err(error) => return Err(storage_io(error)),
};
validate_private_file(&file)?;
let mut bytes = Vec::new();
file.into_std()
.take((MAX_SUBJECT_ID_BYTES + 2) as u64)
.read_to_end(&mut bytes)
.map_err(storage_io)?;
if bytes.len() > MAX_SUBJECT_ID_BYTES + 1 || bytes.last() != Some(&b'\n') {
return Err(storage("sync owner record is invalid"));
}
bytes.pop();
let owner = String::from_utf8(bytes).map_err(|_| storage("sync owner record is invalid"))?;
validate_subject_id(&owner)?;
Ok(Some(owner))
}
fn open_root(path: &Path, create: bool) -> Result<Option<Dir>, SyncClientError> {
let mut root = PathBuf::new();
let mut names = Vec::new();
for component in path.components() {
match component {
Component::Prefix(prefix) => root.push(prefix.as_os_str()),
Component::RootDir => root.push(std::path::MAIN_SEPARATOR_STR),
Component::Normal(name) => names.push(name.to_os_string()),
Component::CurDir => {}
Component::ParentDir => return Err(storage("browser data root traversal is invalid")),
}
}
if root.as_os_str().is_empty() {
return Err(storage("browser data root must be absolute"));
}
let mut directory = Dir::open_ambient_dir(root, ambient_authority()).map_err(storage_io)?;
#[cfg(windows)]
let strict_component = names.len().saturating_sub(4);
#[cfg(windows)]
let mut require_nofollow = false;
#[cfg(windows)]
let mut component_index = 0;
#[cfg(not(windows))]
let mut require_nofollow = directory_requires_nofollow(&directory)?;
for name in names {
#[cfg(windows)]
{
require_nofollow |= component_index >= strict_component;
component_index += 1;
}
if create {
match directory.create_dir(&name) {
Ok(()) => {}
Err(error) if error.kind() == ErrorKind::AlreadyExists => {}
Err(error) => return Err(storage_io(error)),
}
}
let opened = match directory.open_dir_nofollow(&name) {
Ok(directory) => directory,
Err(_) if !require_nofollow => match directory.open_dir(&name) {
Ok(directory) => directory,
Err(error) if !create && error.kind() == ErrorKind::NotFound => return Ok(None),
Err(error) => return Err(storage_io(error)),
},
Err(error) if !create && error.kind() == ErrorKind::NotFound => return Ok(None),
Err(error) => return Err(storage_io(error)),
};
#[cfg(not(windows))]
{
require_nofollow |= directory_requires_nofollow(&opened)?;
}
directory = opened;
}
validate_private_directory(&directory)?;
if create {
set_private_directory_permissions(&directory)?;
}
Ok(Some(directory))
}
fn private_open_options() -> OpenOptions {
let mut options = OpenOptions::new();
options.follow(FollowSymlinks::No);
#[cfg(unix)]
{
use cap_std::fs::OpenOptionsExt;
options.mode(0o600);
}
options
}
fn validate_private_directory(directory: &Dir) -> Result<(), SyncClientError> {
let metadata = directory.dir_metadata().map_err(storage_io)?;
if !metadata.is_dir() {
return Err(storage("browser data root is invalid"));
}
#[cfg(unix)]
{
use cap_std::fs::MetadataExt;
if metadata.uid() != rustix::process::geteuid().as_raw() {
return Err(storage("browser data root ownership is invalid"));
}
}
Ok(())
}
fn validate_private_file(file: &File) -> Result<(), SyncClientError> {
let metadata = file.metadata().map_err(storage_io)?;
if !metadata.is_file() || CrossPlatformMetadataExt::nlink(&metadata) != 1 {
return Err(storage("sync owner record is invalid"));
}
#[cfg(unix)]
{
use cap_std::fs::MetadataExt;
if metadata.uid() != rustix::process::geteuid().as_raw() || metadata.mode() & 0o077 != 0 {
return Err(storage("sync owner record permissions are invalid"));
}
}
Ok(())
}
#[cfg(unix)]
fn directory_requires_nofollow(directory: &Dir) -> Result<bool, SyncClientError> {
use cap_std::fs::MetadataExt;
let metadata = directory.dir_metadata().map_err(storage_io)?;
Ok(metadata.uid() == rustix::process::geteuid().as_raw() || metadata.mode() & 0o022 != 0)
}
#[cfg(not(any(unix, windows)))]
fn directory_requires_nofollow(_directory: &Dir) -> Result<bool, SyncClientError> {
Ok(true)
}
fn set_private_directory_permissions(directory: &Dir) -> Result<(), SyncClientError> {
#[cfg(unix)]
directory
.set_permissions(".", Permissions::from_std(std::fs::Permissions::from_mode(0o700)))
.map_err(storage_io)?;
Ok(())
}
fn set_private_file_permissions(file: &File) -> Result<(), SyncClientError> {
#[cfg(unix)]
file.set_permissions(Permissions::from_std(std::fs::Permissions::from_mode(0o600)))
.map_err(storage_io)?;
Ok(())
}
#[cfg(any(
target_vendor = "apple",
target_os = "linux",
target_os = "android",
target_os = "redox"
))]
fn publish_owner(directory: &Dir, temporary: &str) -> io::Result<()> {
use std::os::fd::AsFd;
rustix::fs::renameat_with(
directory.as_fd(),
temporary,
directory.as_fd(),
OWNER_FILE,
rustix::fs::RenameFlags::NOREPLACE,
)
.map_err(Into::into)
}
#[cfg(windows)]
fn publish_owner(directory: &Dir, temporary: &str) -> io::Result<()> {
directory.rename(temporary, directory, OWNER_FILE)
}
#[cfg(not(any(
target_vendor = "apple",
target_os = "linux",
target_os = "android",
target_os = "redox",
windows
)))]
fn publish_owner(directory: &Dir, temporary: &str) -> io::Result<()> {
directory.hard_link(temporary, directory, OWNER_FILE)?;
directory.remove_file(temporary)
}
fn remove_temporary(directory: &Dir, temporary: &str) -> Result<(), SyncClientError> {
match directory.remove_file(temporary) {
Ok(()) => Ok(()),
Err(error) if error.kind() == ErrorKind::NotFound => Ok(()),
Err(error) => Err(storage_io(error)),
}
}
#[cfg(unix)]
fn sync_directory(directory: &Dir) -> Result<(), SyncClientError> {
directory.try_clone().map_err(storage_io)?.into_std_file().sync_all().map_err(storage_io)
}
#[cfg(not(unix))]
fn sync_directory(_directory: &Dir) -> Result<(), SyncClientError> {
Ok(())
}
fn validate_subject_id(value: &str) -> Result<(), SyncClientError> {
if is_subject_id(value) {
return Ok(());
}
Err(storage("sync owner user identifier is invalid"))
}
fn verify_owner(owner: &str, user_id: &str) -> Result<(), SyncClientError> {
if owner == user_id { Ok(()) } else { Err(SyncClientError::SyncOwnerMismatch) }
}
fn storage_io(error: io::Error) -> SyncClientError {
storage(error.to_string())
}
fn storage(message: impl Into<String>) -> SyncClientError {
SyncClientError::SyncOwnerStorage(message.into())
}
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
@@ -0,0 +1,80 @@
use ely_sync_client::{SyncClientError, SyncOwnerStore};
#[test]
fn browser_data_root_keeps_one_sync_owner() -> Result<(), Box<dyn std::error::Error>> {
let directory = tempfile::tempdir()?;
let store = SyncOwnerStore::new(directory.path());
assert!(matches!(store.verify("user-01"), Err(SyncClientError::SyncOwnerUnclaimed)));
store.claim("user-01")?;
store.claim("user-01")?;
SyncOwnerStore::new(directory.path()).verify("user-01")?;
assert!(matches!(store.verify("user-02"), Err(SyncClientError::SyncOwnerMismatch)));
Ok(())
}
#[test]
fn malformed_sync_owner_fails_closed() -> Result<(), Box<dyn std::error::Error>> {
let directory = tempfile::tempdir()?;
write_owner(directory.path(), "invalid owner id!")?;
let store = SyncOwnerStore::new(directory.path());
assert!(matches!(store.verify("user-01"), Err(SyncClientError::SyncOwnerStorage(_))));
Ok(())
}
#[test]
fn concurrent_claims_publish_exactly_one_owner() -> Result<(), Box<dyn std::error::Error>> {
let directory = tempfile::tempdir()?;
let root = std::sync::Arc::new(directory.path().to_path_buf());
let barrier = std::sync::Arc::new(std::sync::Barrier::new(2));
let threads = ["user-A", "user-B"].map(|user_id| {
let root = root.clone();
let barrier = barrier.clone();
std::thread::spawn(move || {
barrier.wait();
SyncOwnerStore::new(&root).claim(user_id).is_ok()
})
});
let outcomes = threads
.into_iter()
.map(|thread| thread.join().map_err(|_| "owner claim thread panicked"))
.collect::<Result<Vec<_>, _>>()?;
assert_eq!(outcomes.iter().filter(|outcome| **outcome).count(), 1);
let owner = std::fs::read_to_string(directory.path().join(".sync-owner-user-id"))?;
assert!(matches!(owner.as_str(), "user-A\n" | "user-B\n"));
Ok(())
}
#[cfg(unix)]
#[test]
fn linked_owner_records_fail_closed() -> Result<(), Box<dyn std::error::Error>> {
use std::os::unix::fs::symlink;
let directory = tempfile::tempdir()?;
let external = directory.path().join("external-owner");
write_private_file(&external, "user-01\n")?;
symlink(&external, directory.path().join(".sync-owner-user-id"))?;
assert!(matches!(
SyncOwnerStore::new(directory.path()).verify("user-01"),
Err(SyncClientError::SyncOwnerStorage(_))
));
Ok(())
}
fn write_owner(root: &std::path::Path, value: &str) -> Result<(), std::io::Error> {
write_private_file(&root.join(".sync-owner-user-id"), value)
}
fn write_private_file(path: &std::path::Path, value: &str) -> Result<(), std::io::Error> {
std::fs::write(path, value)?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?;
}
Ok(())
}