Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions crates/bsk-cli/src/daemon/local_auth/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@ pub(crate) async fn accept<S: AsyncRead + AsyncWrite + Unpin>(
.as_ref()
.and_then(|p| p.get("version"))
.and_then(|v| v.as_str());
let capable = version
.and_then(|v| semver::Version::parse(v).ok())
.is_some_and(|v| v >= semver::Version::new(0, 4, 0));
state.record_unpaired(version, capable);
if !local.mode().accepts_legacy() {
state.record_unpaired(version);
reply(
ws,
&request.id,
Expand Down Expand Up @@ -71,7 +68,9 @@ pub(crate) async fn accept<S: AsyncRead + AsyncWrite + Unpin>(
let probe = Probe::parse(&first)?;
let hello = probe.params.local_auth;
let Some(initial) = local.store.select(&hello.credentials) else {
state.record_unpaired(None, true);
if !local.mode().accepts_legacy() {
state.record_unpaired(None);
}
challenge(
ws,
&probe.id,
Expand Down Expand Up @@ -306,6 +305,15 @@ async fn durable_commit(
txn: bsk_protocol::local_auth::Bytes<16>,
instance: String,
) -> Result<()> {
// Publish the paired identity before another unverified registration can
// claim it. Reuse the gate already shared by registration and revocation.
let _registry = state
.local_auth
.lifecycle
.registry_gate
.clone()
.lock_owned()
.await;
let local = state.local_auth.clone();
let replaced =
tokio::task::spawn_blocking(move || local.store.commit(&id, txn, &instance)).await??;
Expand Down
8 changes: 8 additions & 0 deletions crates/bsk-cli/src/daemon/local_auth/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ impl PairingStore {
pub fn resolve(&self, id: &str) -> Result<String> {
resolve(&self.state.lock().unwrap(), id)
}
pub fn has_paired_browser(&self, instance: &str) -> bool {
self.state
.lock()
.unwrap()
.devices
.values()
.any(|device| device.browser_id.as_deref() == Some(instance))
}
pub fn revoke(&self, id: Option<&str>) -> Result<Vec<String>> {
self.update(|state| {
let ids = match id {
Expand Down
37 changes: 21 additions & 16 deletions crates/bsk-cli/src/daemon/peer_authorization.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Local HMAC and remote TLS/Bearer authorization share the business driver.
use super::local_auth::{LocalAuth, store::DeviceStatus};
use super::local_auth::{LocalAuth, LocalAuthMode, store::DeviceStatus};
use std::sync::Arc;

pub(crate) struct LocalPeer {
Expand Down Expand Up @@ -66,22 +66,19 @@ impl PeerAuthorization {
pub async fn registration_guard(
&self,
) -> Option<(
tokio::sync::OwnedMutexGuard<()>,
Option<tokio::sync::OwnedMutexGuard<()>>,
tokio::sync::OwnedMutexGuard<()>,
)> {
if let Self::Local(peer) = self {
let device = peer.local.device_gate(&peer.device_id).lock_owned().await;
let registry = peer
.local
.lifecycle
.registry_gate
.clone()
.lock_owned()
.await;
Some((device, registry))
} else {
None
}
let (device, local) = match self {
Self::Local(peer) => (
Some(peer.local.device_gate(&peer.device_id).lock_owned().await),
&peer.local,
),
Self::Legacy(local) => (None, local),
Self::Remote(_) => return None,
};
let registry = local.lifecycle.registry_gate.clone().lock_owned().await;
Some((device, registry))
}
pub fn register(&self, client: &Arc<super::browsers::BrowserClient>) {
if let Self::Local(peer) = self {
Expand All @@ -91,7 +88,15 @@ impl PeerAuthorization {
}
}
pub fn matches_instance(&self, instance: &str) -> bool {
!matches!(self, Self::Local(peer) if peer.browser_id != instance)
match self {
Self::Local(peer) => peer.browser_id == instance,
// Revoking a key does not turn its identity into an unpaired one.
Self::Legacy(local) => {
local.mode() == LocalAuthMode::TrustLocal
|| !local.store.has_paired_browser(instance)
}
Self::Remote(_) => true,
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions crates/bsk-cli/src/daemon/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ impl DaemonState {
}

impl DaemonState {
pub fn record_unpaired(&self, version: Option<&str>, pairing_capable: bool) {
if !pairing_capable && self.local_auth.mode() == super::local_auth::LocalAuthMode::Warn {
return;
}
pub fn record_unpaired(&self, version: Option<&str>) {
let extension_version = version
.and_then(|v| semver::Version::parse(v).ok())
.map(|v| v.to_string());
Expand All @@ -109,6 +106,9 @@ impl DaemonState {
));
}
pub fn recent_unpaired(&self) -> Option<bsk_protocol::system::UnpairedAttempt> {
if self.local_auth.mode().accepts_legacy() {
return None;
}
self.last_unpaired_attempt
.lock()
.unwrap()
Expand Down
142 changes: 142 additions & 0 deletions crates/bsk-cli/tests/auto_update_handover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const MARKER: &[u8] = b"auto-update-handover-fixture";
struct ReleaseServer {
url: String,
downloads: Arc<AtomicUsize>,
hold_archive: Arc<AtomicBool>,
stop: Arc<AtomicBool>,
worker: Option<thread::JoinHandle<()>>,
}
Expand Down Expand Up @@ -58,9 +59,11 @@ impl ReleaseServer {
.unwrap();
let stop = Arc::new(AtomicBool::new(false));
let downloads = Arc::new(AtomicUsize::new(0));
let hold_archive = Arc::new(AtomicBool::new(false));
let worker = {
let stop = Arc::clone(&stop);
let downloads = Arc::clone(&downloads);
let hold_archive = Arc::clone(&hold_archive);
let archive_request = format!("GET /bsk{suffix} ");
thread::spawn(move || {
while !stop.load(Ordering::SeqCst) {
Expand All @@ -86,6 +89,9 @@ impl ReleaseServer {
}
let body = if request.starts_with(archive_request.as_bytes()) {
downloads.fetch_add(1, Ordering::SeqCst);
while hold_archive.load(Ordering::SeqCst) && !stop.load(Ordering::SeqCst) {
thread::sleep(Duration::from_millis(10));
}
thread::sleep(archive_delay);
&archive
} else {
Expand All @@ -103,6 +109,7 @@ impl ReleaseServer {
Self {
url: format!("{base}/version.json"),
downloads,
hold_archive,
stop,
worker: Some(worker),
}
Expand Down Expand Up @@ -496,6 +503,141 @@ fn auto_update_exits_only_after_the_new_daemon_serves() {
);
}

#[test]
fn l11b_revocation_during_auto_update_survives_process_handover() {
use bsk::daemon::local_auth::store::{PairingStore, Pending, now, random};
use bsk_protocol::local_auth::*;
use bsk_protocol::{ResponseBody, ResponseFrame};
use futures_util::{SinkExt, StreamExt};
use tokio_tungstenite::tungstenite::{Message, client::IntoClientRequest};

let fixture = Fixture::new(newer_bsk);
fixture.server.hold_archive.store(true, Ordering::SeqCst);
let lock = bsk::daemon::lockfile::acquire_at(&fixture.home).unwrap();
let store = PairingStore::open(&lock).unwrap();
store.pair(52800, None).unwrap();
let id = store.devices()[0].device_id.clone();
let pending = Pending {
txn: random(),
r#gen: 1,
key: random(),
created_at: now(),
};
store
.stage_rotation(&id, 0, None, pending.clone(), "a1b2c3d4")
.unwrap();
store.commit(&id, pending.txn, "a1b2c3d4").unwrap();
drop(store);
drop(lock);

let port = unused_port();
let old_pid = fixture.start_daemon(port);
fixture.wait_for("an update download in flight", || {
fixture.server.downloads.load(Ordering::SeqCst) == 1
});
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.unwrap();
let probe = || {
runtime.block_on(async {
let mut request = format!("ws://127.0.0.1:{port}/")
.into_client_request()
.unwrap();
request.headers_mut().insert(
"Origin",
"chrome-extension://abcdefghijklmnopabcdefghijklmnop"
.parse()
.unwrap(),
);
let (mut socket, _) = tokio_tungstenite::connect_async(request).await.unwrap();
let hello = Probe {
id: "handover".into(),
method: ProbeMethod::Ping,
params: ProbeParams {
local_auth: Hello {
v: V1,
client_nonce: random(),
instance_id: "a1b2c3d4".into(),
credentials: vec![Credential {
role: CredentialRole::Active,
device_id: Bytes::decode(&id).unwrap(),
r#gen: 1,
pending: None,
}],
},
},
};
socket
.send(Message::Text(serde_json::to_string(&hello).unwrap()))
.await
.unwrap();
let reply = tokio::time::timeout(Duration::from_secs(5), socket.next())
.await
.unwrap()
.unwrap()
.unwrap();
let frame: ResponseFrame = serde_json::from_str(&reply.into_text().unwrap()).unwrap();
let ResponseBody::Ok(value) = frame.body else {
panic!("expected authentication reply")
};
let response: ChallengeResponse =
serde_json::from_value(value["local_auth"].clone()).unwrap();
let _ = socket.close(None).await;
response
})
};
assert!(matches!(probe(), ChallengeResponse::Challenge { .. }));

let revoked = fixture
.command()
.args(["daemon", "revoke", &id])
.output()
.unwrap();
assert!(
revoked.status.success(),
"{}",
String::from_utf8_lossy(&revoked.stderr)
);
assert_eq!(fixture.info().unwrap()["pid"], old_pid);
fixture.server.hold_archive.store(false, Ordering::SeqCst);
fixture.wait_for("the replacement daemon to load the revocation", || {
fixture
.info()
.is_some_and(|info| info["pid"] != old_pid && info["ws_port"] == port)
});
fixture.wait_for("the revoked device's previous daemon to exit", || {
fixture.daemon_exited()
});
assert_eq!(fixture.record().unwrap()["result"], "succeeded");
assert!(matches!(
probe(),
ChallengeResponse::PairingRequired {
legacy_accepted: true,
..
}
));
let devices = fixture
.command()
.args(["daemon", "devices"])
.output()
.unwrap();
assert!(devices.status.success());
let devices: serde_json::Value = serde_json::from_slice(&devices.stdout).unwrap();
assert_eq!(devices[0]["device_id"], id);
assert_eq!(devices[0]["status"], "revoked");
let browsers = fixture
.command()
.args(["--json", "browsers"])
.output()
.unwrap();
assert!(browsers.status.success());
assert_eq!(
serde_json::from_slice::<serde_json::Value>(&browsers.stdout).unwrap(),
serde_json::json!([])
);
}

#[test]
fn a_failed_handover_restores_the_previous_executable_and_keeps_serving() {
let fixture = Fixture::new(release_whose_daemon_fails);
Expand Down
Loading
Loading