Various fixes for in-app upgrades

This commit is contained in:
David Lönnhager 2025-05-15 11:59:03 +02:00 committed by Sebastian Holmin
parent f6a57a9432
commit e43feef72c
7 changed files with 139 additions and 89 deletions

View File

@ -22,6 +22,15 @@ pub struct AppVersionResponse {
pub latest_beta: Option<AppVersion>,
}
/// Reply from `/app/releases/<platform>.json` endpoint
pub struct AppVersionResponse2 {
/// Information about available versions for the current target
pub version_info: VersionInfo,
/// Index of the metadata version used to sign the response.
/// Used to prevent replay/downgrade attacks.
pub metadata_version: usize,
}
impl AppVersionProxy {
/// Maximum size of `version_check_2` response
const SIZE_LIMIT: usize = 1024 * 1024;
@ -57,7 +66,7 @@ impl AppVersionProxy {
architecture: mullvad_update::format::Architecture,
rollout: f32,
lowest_metadata_version: usize,
) -> impl Future<Output = Result<(VersionInfo, usize), rest::Error>> + use<> {
) -> impl Future<Output = Result<AppVersionResponse2, rest::Error>> + use<> {
let service = self.handle.service.clone();
let path = format!("app/releases/{platform}.json");
let request = self.handle.factory.get(&path);
@ -80,12 +89,12 @@ impl AppVersionProxy {
};
let metadata_version = response.signed.metadata_version;
Ok((
VersionInfo::try_from_response(&params, response.signed)
Ok(AppVersionResponse2 {
version_info: VersionInfo::try_from_response(&params, response.signed)
.map_err(Arc::new)
.map_err(rest::Error::FetchVersions)?,
metadata_version,
))
})
}
}
}

View File

@ -35,9 +35,9 @@ fn main() {
println!(r#"cargo::rustc-cfg=daita"#);
// Enable in-app upgrades on macOS and Windows
println!("cargo::rustc-check-cfg=cfg(update)");
println!("cargo::rustc-check-cfg=cfg(in_app_upgrade)");
if matches!(target_os(), Os::Windows | Os::Macos) {
println!(r#"cargo::rustc-cfg=update"#);
println!(r#"cargo::rustc-cfg=in_app_upgrade"#);
}
}

View File

@ -336,7 +336,7 @@ pub enum DaemonCommand {
/// Return whether the daemon is performing post-upgrade tasks
IsPerformingPostUpgrade(oneshot::Sender<bool>),
/// Get current version of the app
GetCurrentVersion(oneshot::Sender<String>),
GetCurrentVersion(oneshot::Sender<mullvad_version::Version>),
/// Remove settings and clear the cache
#[cfg(not(target_os = "android"))]
FactoryReset(ResponseTx<(), Error>),
@ -659,7 +659,7 @@ impl Daemon {
macos::bump_filehandle_limit();
let command_sender = daemon_command_channel.sender();
let app_upgrade_broadcast = tokio::sync::broadcast::channel(128).0; // TODO: look over bufsize
let app_upgrade_broadcast = tokio::sync::broadcast::channel(32).0;
let management_interface = ManagementInterfaceServer::start(
command_sender,
config.rpc_socket_path,
@ -1970,10 +1970,12 @@ impl Daemon {
});
}
fn on_get_current_version(&mut self, tx: oneshot::Sender<String>) {
fn on_get_current_version(&mut self, tx: oneshot::Sender<mullvad_version::Version>) {
Self::oneshot_send(
tx,
mullvad_version::VERSION.to_owned(),
mullvad_version::VERSION
.parse::<mullvad_version::Version>()
.expect("Failed to parse version"),
"get_current_version response",
);
}
@ -3230,12 +3232,12 @@ impl Daemon {
#[cfg_attr(not(in_app_upgrade), allow(clippy::unused_async))]
async fn on_app_upgrade(&self, tx: ResponseTx<(), version::Error>) {
#[cfg(update)]
#[cfg(in_app_upgrade)]
{
let result = self.version_handle.update_application().await;
Self::oneshot_send(tx, result, "on_app_upgrade response");
}
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
{
log::warn!("Ignoring app upgrade command as in-app upgrades are disabled on this OS");
Self::oneshot_send(tx, Ok(()), "on_app_upgrade response")
@ -3244,12 +3246,12 @@ impl Daemon {
#[cfg_attr(not(in_app_upgrade), allow(clippy::unused_async))]
async fn on_app_upgrade_abort(&self, tx: ResponseTx<(), version::Error>) {
#[cfg(update)]
#[cfg(in_app_upgrade)]
{
let result = self.version_handle.cancel_update().await;
Self::oneshot_send(tx, result, "on_app_upgrade_abort response");
}
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
{
log::warn!(
"Ignoring cancel app upgrade command as in-app upgrades are disabled on this OS"

View File

@ -146,7 +146,7 @@ impl ManagementService for ManagementServiceImpl {
log::debug!("get_current_version");
let (tx, rx) = oneshot::channel();
self.send_command_to_daemon(DaemonCommand::GetCurrentVersion(tx))?;
let version = self.wait_for_result(rx).await?;
let version = self.wait_for_result(rx).await?.to_string();
Ok(Response::new(version))
}

View File

@ -54,8 +54,8 @@ pub(super) struct VersionCache {
/// Whether the current (installed) version is supported or an upgrade is required
pub current_version_supported: bool,
/// The latest available versions
pub latest_version: mullvad_update::version::VersionInfo,
#[cfg(update)]
pub version_info: mullvad_update::version::VersionInfo,
#[cfg(in_app_upgrade)]
pub metadata_version: usize,
}
@ -112,7 +112,7 @@ impl VersionUpdaterInner {
self.last_app_version_info.as_ref().map(|(info, _)| info)
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
pub fn get_min_metadata_version(&self) -> usize {
self.last_app_version_info
.as_ref()
@ -123,7 +123,7 @@ impl VersionUpdaterInner {
.unwrap_or(mullvad_update::version::MIN_VERIFY_METADATA_VERSION)
}
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
pub fn get_min_metadata_version(&self) -> usize {
mullvad_update::version::MIN_VERIFY_METADATA_VERSION
}
@ -136,7 +136,7 @@ impl VersionUpdaterInner {
update: &impl Fn(VersionCache) -> BoxFuture<'static, Result<(), Error>>,
mut new_version_info: VersionCache,
) {
#[cfg(update)]
#[cfg(in_app_upgrade)]
if let Some((current_cache, _)) = self.last_app_version_info.as_ref() {
if current_cache.metadata_version == new_version_info.metadata_version {
log::trace!("Ignoring version info with same metadata version");
@ -369,11 +369,13 @@ fn do_version_check_in_background(
}
/// Combine the old version and new version endpoint
#[cfg(update)]
#[cfg(in_app_upgrade)]
fn version_check_inner(
api: &ApiContext,
min_metadata_version: usize,
) -> impl Future<Output = Result<VersionCache, Error>> {
use mullvad_api::version::{AppVersionResponse, AppVersionResponse2};
let v1_endpoint = api.version_proxy.version_check(
mullvad_version::VERSION.to_owned(),
PLATFORM,
@ -396,17 +398,26 @@ fn version_check_inner(
min_metadata_version,
);
async move {
let (v1_response, v2_response) =
tokio::try_join!(v1_endpoint, v2_endpoint).map_err(Error::Download)?;
let (
AppVersionResponse {
supported: current_version_supported,
..
},
AppVersionResponse2 {
version_info,
metadata_version,
},
) = tokio::try_join!(v1_endpoint, v2_endpoint).map_err(Error::Download)?;
Ok(VersionCache {
current_version_supported: v1_response.supported,
latest_version: v2_response.0,
metadata_version: v2_response.1,
current_version_supported,
version_info,
metadata_version,
})
}
}
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
fn version_check_inner(
api: &ApiContext,
// NOTE: This is unused when `update` is disabled
@ -433,7 +444,7 @@ fn version_check_inner(
current_version_supported: response.supported,
// Note: We're pretending that this is complete information,
// but on Android and Linux, most of the information is missing
latest_version: VersionInfo {
version_info: VersionInfo {
stable: mullvad_update::version::Version {
version: latest_stable,
changelog: "".to_owned(),
@ -494,7 +505,7 @@ async fn try_load_cache(cache_dir: &Path) -> Result<(VersionCache, SystemTime),
let cache: VersionCache = serde_json::from_str(&content).map_err(Error::Deserialize)?;
if cache_is_old(&cache.latest_version, &APP_VERSION) {
if cache_is_old(&cache.version_info, &APP_VERSION) {
return Err(Error::OutdatedVersion);
}
@ -523,7 +534,7 @@ fn dev_version_cache() -> VersionCache {
VersionCache {
current_version_supported: false,
latest_version: VersionInfo {
version_info: VersionInfo {
stable: mullvad_update::version::Version {
version: mullvad_version::VERSION.parse().unwrap(),
changelog: "".to_owned(),
@ -533,7 +544,7 @@ fn dev_version_cache() -> VersionCache {
},
beta: None,
},
#[cfg(update)]
#[cfg(in_app_upgrade)]
metadata_version: 0,
}
}
@ -757,7 +768,7 @@ mod test {
// TODO: The tests pass, but check that this is a sane fake version cache anyway
VersionCache {
current_version_supported: true,
latest_version: VersionInfo {
version_info: VersionInfo {
stable: Version {
version: "2025.5".parse::<mullvad_version::Version>().unwrap(),
urls: vec![],
@ -767,7 +778,7 @@ mod test {
},
beta: None,
},
#[cfg(update)]
#[cfg(in_app_upgrade)]
metadata_version: 0,
}
}

View File

@ -1,8 +1,9 @@
#![cfg(update)]
#![cfg(in_app_upgrade)]
use mullvad_types::version::{AppUpgradeDownloadProgress, AppUpgradeError, AppUpgradeEvent};
use mullvad_update::app::{bin_path, AppDownloader, AppDownloaderParameters, DownloadError};
use rand::seq::SliceRandom;
use std::io;
use std::path::PathBuf;
use std::time::{Duration, Instant};
use talpid_types::ErrorExt;
@ -116,23 +117,11 @@ where
let _ = event_tx.send(AppUpgradeEvent::DownloadStarting);
if let Err(err) = downloader.download_executable().await {
let _ = event_tx.send(AppUpgradeEvent::Error(AppUpgradeError::DownloadFailed));
log::error!("{}", err.display_chain());
log::info!("Cleaning up download at '{bin_path:?}'",);
#[cfg(not(test))]
tokio::fs::remove_file(&bin_path)
.await
.expect("Removing download file");
return Err(err.into());
};
let _ = event_tx.send(AppUpgradeEvent::VerifyingInstaller);
if let Err(err) = downloader.verify().await {
let _ = event_tx.send(AppUpgradeEvent::Error(AppUpgradeError::VerificationFailed));
log::error!("{}", err.display_chain());
log::info!("Cleaning up download at '{:?}'", bin_path);
#[cfg(not(test))]
tokio::fs::remove_file(&bin_path)
.await
.expect("Removing download file");
return Err(err.into());
};
let _ = event_tx.send(AppUpgradeEvent::VerifiedInstaller);
@ -156,10 +145,11 @@ async fn create_download_dir() -> Result<PathBuf> {
pub async fn clear_download_dir() -> Result<PathBuf> {
let download_dir = mullvad_paths::get_cache_dir()?.join("mullvad-update");
log::info!("Cleaning up download directory: {}", download_dir.display());
fs::remove_dir_all(&download_dir)
.await
.map_err(Error::CreateDownloadDir)?;
Ok(download_dir)
match fs::remove_dir_all(&download_dir).await {
Ok(()) => Ok(download_dir),
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(download_dir),
Err(err) => Err(Error::CreateDownloadDir(err)),
}
}
pub struct ProgressUpdater {

View File

@ -5,24 +5,24 @@ use futures::channel::{mpsc, oneshot};
use futures::stream::StreamExt;
use mullvad_api::{availability::ApiAvailability, rest::MullvadRestHandle};
use mullvad_types::version::{AppVersionInfo, SuggestedUpgrade};
#[cfg(update)]
#[cfg(in_app_upgrade)]
use mullvad_update::app::{AppDownloader, AppDownloaderParameters, HttpAppDownloader};
use mullvad_update::version::VersionInfo;
use talpid_core::mpsc::Sender;
#[cfg(update)]
#[cfg(in_app_upgrade)]
use talpid_types::ErrorExt;
use crate::management_interface::AppUpgradeBroadcast;
use crate::DaemonEventSender;
#[cfg(update)]
#[cfg(in_app_upgrade)]
use super::downloader::ProgressUpdater;
use super::{
check::{VersionCache, VersionUpdater},
Error,
};
#[cfg(update)]
#[cfg(in_app_upgrade)]
use super::downloader;
use std::mem;
@ -50,7 +50,7 @@ impl VersionRouterHandle {
result_rx.await.map_err(|_| Error::VersionRouterClosed)?
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
pub async fn update_application(&self) -> Result<()> {
let (result_tx, result_rx) = oneshot::channel();
self.tx
@ -59,7 +59,7 @@ impl VersionRouterHandle {
result_rx.await.map_err(|_| Error::VersionRouterClosed)
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
pub async fn cancel_update(&self) -> Result<()> {
let (result_tx, result_rx) = oneshot::channel();
self.tx
@ -70,17 +70,17 @@ impl VersionRouterHandle {
}
// These wrapper traits and type aliases exist to help feature gate the module
#[cfg(update)]
#[cfg(in_app_upgrade)]
trait Downloader:
AppDownloader + Send + 'static + From<AppDownloaderParameters<ProgressUpdater>>
{
}
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
trait Downloader {}
#[cfg(update)]
#[cfg(in_app_upgrade)]
type DefaultDownloader = HttpAppDownloader<ProgressUpdater>;
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
type DefaultDownloader = ();
impl Downloader for DefaultDownloader {}
@ -104,7 +104,7 @@ struct VersionRouter<S = DaemonEventSender<AppVersionInfo>, D = DefaultDownloade
/// Channels that receive responses to `get_latest_version`
version_request_channels: Vec<oneshot::Sender<Result<AppVersionInfo>>>,
/// Broadcast channel for app upgrade events
#[cfg(update)]
#[cfg(in_app_upgrade)]
app_upgrade_broadcast: AppUpgradeBroadcast,
/// Type used to spawn the downloader task, replaced when testing
_phantom: std::marker::PhantomData<D>,
@ -119,10 +119,10 @@ enum Message {
/// Check for updates
GetLatestVersion(oneshot::Sender<Result<AppVersionInfo>>),
/// Update the application
#[cfg(update)]
#[cfg(in_app_upgrade)]
UpdateApplication { result_tx: oneshot::Sender<()> },
/// Cancel the ongoing update
#[cfg(update)]
#[cfg(in_app_upgrade)]
CancelUpdate { result_tx: oneshot::Sender<()> },
}
@ -133,7 +133,7 @@ enum State {
/// Running version checker, no upgrade in progress
HasVersion { version_cache: VersionCache },
/// Download is in progress, so we don't forward version checks
#[cfg(update)]
#[cfg(in_app_upgrade)]
Downloading {
/// Version info received from `HasVersion`
version_cache: VersionCache,
@ -143,7 +143,7 @@ enum State {
downloader_handle: downloader::DownloaderHandle,
},
/// Download is complete. We have a verified binary
#[cfg(update)]
#[cfg(in_app_upgrade)]
Downloaded {
/// Version info received from `HasVersion`
version_cache: VersionCache,
@ -162,12 +162,12 @@ impl std::fmt::Display for State {
match self {
State::NoVersion => write!(f, "NoVersion"),
State::HasVersion { .. } => write!(f, "HasVersion"),
#[cfg(update)]
#[cfg(in_app_upgrade)]
State::Downloading {
upgrading_to_version,
..
} => write!(f, "Downloading '{}'", upgrading_to_version.version),
#[cfg(update)]
#[cfg(in_app_upgrade)]
State::Downloaded {
verified_installer_path,
..
@ -181,7 +181,7 @@ impl State {
match self {
State::NoVersion => None,
State::HasVersion { version_cache, .. } => Some(version_cache),
#[cfg(update)]
#[cfg(in_app_upgrade)]
State::Downloading { version_cache, .. } | State::Downloaded { version_cache, .. } => {
Some(version_cache)
}
@ -189,7 +189,7 @@ impl State {
}
}
#[cfg_attr(not(update), allow(unused_variables))]
#[cfg_attr(not(in_app_upgrade), allow(unused_variables))]
pub(crate) fn spawn_version_router(
api_handle: MullvadRestHandle,
availability_handle: ApiAvailability,
@ -204,7 +204,7 @@ pub(crate) fn spawn_version_router(
let (new_version_tx, new_version_rx) = mpsc::unbounded();
let (refresh_version_check_tx, refresh_version_check_rx) = mpsc::unbounded();
#[cfg(update)]
#[cfg(in_app_upgrade)]
let _ = downloader::clear_download_dir().await.inspect_err(|err| {
log::error!(
"{}",
@ -228,7 +228,7 @@ pub(crate) fn spawn_version_router(
version_event_sender,
new_version_rx,
version_request_channels: vec![],
#[cfg(update)]
#[cfg(in_app_upgrade)]
app_upgrade_broadcast,
refresh_version_check_tx,
_phantom: std::marker::PhantomData::<DefaultDownloader>,
@ -288,12 +288,12 @@ where
Message::GetLatestVersion(result_tx) => {
self.get_latest_version(result_tx);
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
Message::UpdateApplication { result_tx } => {
self.update_application();
let _ = result_tx.send(());
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
Message::CancelUpdate { result_tx } => {
self.cancel_upgrade();
let _ = result_tx.send(());
@ -328,7 +328,7 @@ where
app_version_info: new_app_version,
}
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
State::Downloaded { .. } | State::Downloading { .. } => {
let app_version_info = to_app_version_info(&version_cache, self.beta_program, None);
@ -367,7 +367,7 @@ where
// Always cancel download if the suggested upgrade changes
let version_cache = match mem::replace(&mut self.state, State::NoVersion) {
#[cfg(update)]
#[cfg(in_app_upgrade)]
State::Downloaded { version_cache, .. } | State::Downloading { version_cache, .. } => {
log::warn!("Switching beta after updating resulted in new suggested upgrade: {:?}, aborting", new_app_version.suggested_upgrade);
version_cache
@ -402,14 +402,14 @@ where
}
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
fn update_application(&mut self) {
use crate::version::downloader::spawn_downloader;
match mem::replace(&mut self.state, State::NoVersion) {
State::HasVersion { version_cache } => {
let Some(upgrading_to_version) =
recommended_version_upgrade(&version_cache.latest_version, self.beta_program)
recommended_version_upgrade(&version_cache.version_info, self.beta_program)
else {
// If there's no suggested upgrade, do nothing
log::debug!("Received update request without suggested upgrade");
@ -439,18 +439,32 @@ where
}
}
#[cfg(update)]
#[cfg(in_app_upgrade)]
fn cancel_upgrade(&mut self) {
use mullvad_types::version::AppUpgradeEvent;
match mem::replace(&mut self.state, State::NoVersion) {
// If we're upgrading, emit an event if a version was received during the upgrade
// Otherwise, just reset upgrade info to last known state
State::Downloaded { version_cache, .. } | State::Downloading { version_cache, .. } => {
State::Downloading { version_cache, .. } => {
self.state = State::HasVersion { version_cache };
}
State::Downloaded { version_cache, .. } => {
let app_version_info = to_app_version_info(&version_cache, self.beta_program, None);
self.state = State::HasVersion { version_cache };
// Send "Aborted" here, since there's no "Downloader" to do it for us
let _ = self.app_upgrade_broadcast.send(AppUpgradeEvent::Aborted);
// Notify the daemon and version requesters about new version
self.notify_version_requesters(app_version_info.clone());
let _ = self.version_event_sender.send(app_version_info);
}
// No-op unless we're downloading something right now
// In the `Downloaded` state, we also do nothing
state => self.state = state,
};
debug_assert!(matches!(
self.state,
State::HasVersion { .. } | State::NoVersion
@ -462,7 +476,7 @@ where
/// support in-app upgrades), then the future will never resolve as to not escape the select statement.
#[allow(clippy::unused_async, unused_variables)]
async fn wait_for_update(state: &mut State) -> Option<AppVersionInfo> {
#[cfg(update)]
#[cfg(in_app_upgrade)]
match state {
State::Downloading {
version_cache,
@ -500,7 +514,7 @@ async fn wait_for_update(state: &mut State) -> Option<AppVersionInfo> {
unreachable!()
}
}
#[cfg(not(update))]
#[cfg(not(in_app_upgrade))]
{
let () = std::future::pending().await;
unreachable!()
@ -516,7 +530,7 @@ fn to_app_version_info(
) -> AppVersionInfo {
let current_version_supported = cache.current_version_supported;
let suggested_upgrade =
recommended_version_upgrade(&cache.latest_version, beta_program).map(|version| {
recommended_version_upgrade(&cache.version_info, beta_program).map(|version| {
SuggestedUpgrade {
version: version.version,
changelog: version.changelog,
@ -549,7 +563,7 @@ fn recommended_version_upgrade(
}
}
#[cfg(all(test, update))]
#[cfg(all(test, in_app_upgrade))]
mod test {
use super::downloader::ProgressUpdater;
use futures::channel::mpsc::unbounded;
@ -688,7 +702,7 @@ mod test {
version.incremental += 1;
VersionCache {
current_version_supported: true,
latest_version: VersionInfo {
version_info: VersionInfo {
beta: None,
stable: mullvad_update::version::Version {
version,
@ -716,7 +730,7 @@ mod test {
beta.version.incremental += 1;
VersionCache {
current_version_supported: true,
latest_version: VersionInfo {
version_info: VersionInfo {
beta: Some(beta),
stable,
},
@ -854,7 +868,7 @@ mod test {
assert_eq!(version_cache, &version_cache_test);
assert_eq!(
upgrading_to_version.version,
version_cache_test.latest_version.stable.version
version_cache_test.version_info.stable.version
);
}
other => panic!("State should be Downloading, was {other:?}"),
@ -902,7 +916,7 @@ mod test {
let version_info = channels
.version_event_receiver
.try_next()
.expect("Version event sender should not be closed")
.expect("Version event channel should contain message")
.expect("Version event should be sent");
assert_eq!(
version_info
@ -912,6 +926,10 @@ mod test {
.verified_installer_path,
Some(verified_installer_path.clone())
);
channels
.version_event_receiver
.try_next()
.expect_err("Channel should not have any messages");
version_router.update_application();
assert!(
@ -925,10 +943,30 @@ mod test {
"State should be HasVersion after cancelling the upgrade"
);
assert_eq!(
app_upgrade_listener.try_recv(),
Ok(AppUpgradeEvent::Aborted),
"The `AppUpgradeEvent::Aborted` should be sent when cancelling a finished download"
);
assert_eq!(
app_upgrade_listener.try_recv(),
Err(TryRecvError::Empty),
"The `AppUpgradeEvent::Aborted` should not be sent when cancelling a finished download"
"No more events should be sent",
);
let version_info = channels
.version_event_receiver
.try_next()
.expect("Version event channel should contain message")
.expect("Version event should be sent");
assert_eq!(
version_info
.suggested_upgrade
.as_ref()
.unwrap()
.verified_installer_path,
None,
"Aborting should send a new `AppVersionInfo` without a verified installer path"
);
}
@ -939,7 +977,7 @@ mod test {
let upgrade_version = get_new_stable_version_cache();
let mut upgrade_version_newer = upgrade_version.clone();
upgrade_version_newer
.latest_version
.version_info
.stable
.version
.incremental += 1;