Set app download cache to a read-only directory

Notably, this means that the loader must run as a privileged user

9 files changed, 106 insertions, 8 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 81108146fb..e9d1acf67a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2996,6 +2996,7 @@ dependencies = [
  "insta",
  "json-canon",
  "mockito",
+ "mullvad-paths",
  "mullvad-version",
  "rand 0.8.5",
  "reqwest",
diff --git a/installer-downloader/src/controller.rs b/installer-downloader/src/controller.rs
index 773ed1d3bd..c5aebaa371 100644
--- a/installer-downloader/src/controller.rs
+++ b/installer-downloader/src/controller.rs
@@ -141,6 +141,7 @@ where
                     let Some(app_url) = version_info.stable.urls.first() else {
                         return;
                     };
+                    let app_version = version_info.stable.version;
                     let app_sha256 = version_info.stable.sha256;
                     let app_size = version_info.stable.size;
 
@@ -152,6 +153,7 @@ where
                     self_.show_download_progress();
 
                     let downloader = A::from(UiAppDownloaderParameters {
+                        app_version,
                         app_url: app_url.to_owned(),
                         app_size,
                         app_progress: UiProgressUpdater::new(self_.queue()),
diff --git a/installer-downloader/src/ui_downloader.rs b/installer-downloader/src/ui_downloader.rs
index 19dfb96b2a..5658297082 100644
--- a/installer-downloader/src/ui_downloader.rs
+++ b/installer-downloader/src/ui_downloader.rs
@@ -37,6 +37,19 @@ impl<Delegate: AppDelegate, Downloader: AppDownloader + Send + 'static>
 impl<Delegate: AppDelegate, Downloader: AppDownloader + Send + 'static> AppDownloader
     for UiAppDownloader<Delegate, Downloader>
 {
+    async fn create_cache_dir(&mut self) -> Result<(), app::DownloadError> {
+        match self.downloader.create_cache_dir().await {
+            Ok(()) => Ok(()),
+            Err(err) => {
+                self.queue.queue_main(move |self_| {
+                    self_.set_download_text("ERROR: Failed to create cache directory.");
+                });
+
+                Err(err)
+            }
+        }
+    }
+
     async fn download_executable(&mut self) -> Result<(), app::DownloadError> {
         match self.downloader.download_executable().await {
             Ok(()) => {
diff --git a/installer-downloader/tests/controller.rs b/installer-downloader/tests/controller.rs
index 6576512375..af46452e89 100644
--- a/installer-downloader/tests/controller.rs
+++ b/installer-downloader/tests/controller.rs
@@ -65,6 +65,10 @@ pub struct FakeAppDownloader<const EXE_SUCCEED: bool, const VERIFY_SUCCEED: bool
 impl<const EXE_SUCCEED: bool, const VERIFY_SUCCEED: bool> AppDownloader
     for FakeAppDownloader<EXE_SUCCEED, VERIFY_SUCCEED>
 {
+    async fn create_cache_dir(&mut self) -> Result<(), DownloadError> {
+        Ok(())
+    }
+
     async fn download_executable(&mut self) -> Result<(), DownloadError> {
         self.params.app_progress.set_url(&self.params.app_url);
         self.params.app_progress.set_progress(0.);
diff --git a/mullvad-update/Cargo.toml b/mullvad-update/Cargo.toml
index 20b561776a..4c1cc1d3b9 100644
--- a/mullvad-update/Cargo.toml
+++ b/mullvad-update/Cargo.toml
@@ -27,6 +27,7 @@ sha2 = "0.10"
 tokio = { version = "1", features = ["full"] }
 async-trait = "0.1"
 
+mullvad-paths = { path = "../mullvad-paths" }
 mullvad-version = { path = "../mullvad-version", features = ["serde"] }
 
 # features required by binaries
diff --git a/mullvad-update/src/app.rs b/mullvad-update/src/app.rs
index c6cfb1eb45..f8b0ec3414 100644
--- a/mullvad-update/src/app.rs
+++ b/mullvad-update/src/app.rs
@@ -9,6 +9,7 @@ use crate::{
 
 #[derive(Debug)]
 pub enum DownloadError {
+    CreateDir(anyhow::Error),
     FetchSignature(anyhow::Error),
     FetchApp(anyhow::Error),
     Verification(anyhow::Error),
@@ -17,6 +18,7 @@ pub enum DownloadError {
 /// Parameters required to construct an [AppDownloader].
 #[derive(Clone)]
 pub struct AppDownloaderParameters<AppProgress> {
+    pub app_version: mullvad_version::Version,
     pub app_url: String,
     pub app_size: usize,
     pub app_progress: AppProgress,
@@ -26,6 +28,9 @@ pub struct AppDownloaderParameters<AppProgress> {
 /// See the [module-level documentation](self).
 #[async_trait::async_trait]
 pub trait AppDownloader: Send {
+    /// Create download directory.
+    async fn create_cache_dir(&mut self) -> Result<(), DownloadError>;
+
     /// Download the app binary.
     async fn download_executable(&mut self) -> Result<(), DownloadError>;
 
@@ -35,6 +40,7 @@ pub trait AppDownloader: Send {
 
 /// Download the app and signature, and verify the app's signature
 pub async fn install_and_upgrade(mut downloader: impl AppDownloader) -> Result<(), DownloadError> {
+    downloader.create_cache_dir().await?;
     downloader.download_executable().await?;
     downloader.verify().await
 }
@@ -42,14 +48,12 @@ pub async fn install_and_upgrade(mut downloader: impl AppDownloader) -> Result<(
 #[derive(Clone)]
 pub struct HttpAppDownloader<AppProgress> {
     params: AppDownloaderParameters<AppProgress>,
-    // TODO: set permissions
-    tmp_dir: PathBuf,
+    cache_dir: Option<PathBuf>,
 }
 
 impl<AppProgress> HttpAppDownloader<AppProgress> {
     pub fn new(params: AppDownloaderParameters<AppProgress>) -> Self {
-        let tmp_dir = std::env::temp_dir();
-        Self { params, tmp_dir }
+        Self { params, cache_dir: None }
     }
 }
 
@@ -63,9 +67,16 @@ impl<AppProgress: ProgressUpdater> From<AppDownloaderParameters<AppProgress>>
 
 #[async_trait::async_trait]
 impl<AppProgress: ProgressUpdater> AppDownloader for HttpAppDownloader<AppProgress> {
+    async fn create_cache_dir(&mut self) -> Result<(), DownloadError> {
+        let dir = crate::dir::update_directory().await.map_err(DownloadError::CreateDir)?;
+        self.cache_dir = Some(dir);
+        Ok(())
+    }
+
     async fn download_executable(&mut self) -> Result<(), DownloadError> {
+        let bin_path = self.bin_path().expect("Performed after 'create_cache_dir'");
         fetch::get_to_file(
-            self.bin_path(),
+            bin_path,
             &self.params.app_url,
             &mut self.params.app_progress,
             fetch::SizeHint::Exact(self.params.app_size),
@@ -75,7 +86,7 @@ impl<AppProgress: ProgressUpdater> AppDownloader for HttpAppDownloader<AppProgre
     }
 
     async fn verify(&mut self) -> Result<(), DownloadError> {
-        let bin_path = self.bin_path();
+        let bin_path = self.bin_path().expect("Performed after 'create_cache_dir'");
         let hash = self.hash_sha256();
         Sha256Verifier::verify(bin_path, *hash)
             .await
@@ -84,8 +95,14 @@ impl<AppProgress: ProgressUpdater> AppDownloader for HttpAppDownloader<AppProgre
 }
 
 impl<AppProgress> HttpAppDownloader<AppProgress> {
-    fn bin_path(&self) -> PathBuf {
-        self.tmp_dir.join("temp.exe")
+    fn bin_path(&self) -> Option<PathBuf> {
+        #[cfg(windows)]
+        let bin_filename = format!("{}.exe", self.params.app_version);
+
+        #[cfg(unix)]
+        let bin_filename = self.params.app_version.to_string();
+
+        self.cache_dir.as_ref().map(|dir| dir.join(bin_filename))
     }
 
     fn hash_sha256(&self) -> &[u8; 32] {
diff --git a/mullvad-update/src/dir.rs b/mullvad-update/src/dir.rs
new file mode 100644
index 0000000000..a7e7b81e11
--- /dev/null
+++ b/mullvad-update/src/dir.rs
@@ -0,0 +1,57 @@
+//! This provides a secure directory suitable for storing updates, with admin-only write access.
+
+use std::path::PathBuf;
+use tokio::fs;
+
+use anyhow::Context;
+
+/// Name of subdirectory in the cache directory
+const CACHE_DIRNAME: &str = "updates";
+
+/// This returns a directory suitable for storing updates. Only admins have write access.
+///
+/// This function is a bit racey, as the directory is created before being restricted.
+/// This is acceptable as long as the checksum of each file is verified before being used.
+pub async fn update_directory() -> anyhow::Result<PathBuf> {
+    let dir = tokio::task::spawn_blocking(|| mullvad_paths::cache_dir())
+        .await
+        .unwrap()?
+        .join(CACHE_DIRNAME);
+
+    #[cfg(windows)]
+    {
+        let dir_clone = dir.clone();
+        tokio::task::spawn_blocking(move || {
+            mullvad_paths::windows::create_privileged_directory(&dir_clone)
+        })
+        .await
+        .unwrap()
+        .context("Failed to create cache directory")?;
+    }
+
+    #[cfg(unix)]
+    {
+        use std::{fs::Permissions, os::unix::fs::PermissionsExt};
+        use tokio::fs;
+
+        fs::create_dir_all(&dir)
+            .await
+            .context("Failed to create cache directory")?;
+        fs::set_permissions(&dir, Permissions::from_mode(0o700))
+            .await
+            .context("Failed to set cache directory permissions")?;
+    }
+
+    Ok(dir)
+}
+
+/// Remove all files from the update directory
+pub async fn cleanup_update_directory() -> anyhow::Result<()> {
+
+    let dir = update_directory().await?;
+
+    // It's fine to remove the directory in its entirety, since `update_directory` recreates it.
+    fs::remove_dir_all(dir).await?;
+
+    Ok(())
+}
diff --git a/mullvad-update/src/fetch.rs b/mullvad-update/src/fetch.rs
index 1093915a7a..9b13fd9ed1 100644
--- a/mullvad-update/src/fetch.rs
+++ b/mullvad-update/src/fetch.rs
@@ -37,6 +37,8 @@ pub enum SizeHint {
 /// Download `url` to `file`. If the file already exists, this appends to it, as long
 /// as the file pointed to by `url` is larger than it.
 ///
+/// Make sure that `file` is stored in a secure directory.
+///
 /// # Arguments
 /// - `progress_updater` - This interface is notified of download progress.
 /// - `size_hint` - File size restrictions.
diff --git a/mullvad-update/src/lib.rs b/mullvad-update/src/lib.rs
index 27184e59d5..89fbdee2ed 100644
--- a/mullvad-update/src/lib.rs
+++ b/mullvad-update/src/lib.rs
@@ -2,6 +2,7 @@
 
 pub mod api;
 pub mod app;
+pub mod dir;
 pub mod fetch;
 pub mod verify;