fix(agent): private key permissions on Windows

crates/agent-tunnel/Cargo.toml

@@ -17,6 +17,7 @@ test-utils = []
 [dependencies]
 # Internal crates
 agent-tunnel-proto = { path = "../agent-tunnel-proto", features = ["serde"] }
+devolutions-agent-shared = { path = "../devolutions-agent-shared" }
 devolutions-gateway-task = { path = "../devolutions-gateway-task" }
 
 # Async / runtime

crates/agent-tunnel/src/cert.rs

@@ -8,6 +8,7 @@ use std::time::Duration;
 
 use anyhow::{Context as _, bail};
 use camino::{Utf8Path, Utf8PathBuf};
+use devolutions_agent_shared::write_restricted_file;
 use picky::pem::{PemError, parse_pem, read_pem};
 use picky::x509::Cert;
 use picky_asn1_x509::{ExtensionView, GeneralName};
@@ -165,19 +166,9 @@ impl CaManager {
             // Persist to disk.
             std::fs::create_dir_all(data_dir).with_context(|| format!("create data directory {data_dir}"))?;
             std::fs::write(&cert_path, &ca_cert_pem).with_context(|| format!("write CA cert to {cert_path}"))?;
-            std::fs::write(&key_path, ca_key_pair.serialize_pem())
+            write_restricted_file(&key_path, &ca_key_pair.serialize_pem())
                 .with_context(|| format!("write CA key to {key_path}"))?;
 
-            #[cfg(unix)]
-            {
-                use std::os::unix::fs::PermissionsExt as _;
-                std::fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600))
-                    .with_context(|| format!("set permissions on {key_path}"))?;
-            }
-
-            // TODO: On Windows, set explicit DACL on the CA key file.
-            // Currently relying on ProgramData directory ACL (SYSTEM + Admins only).
-
             info!(%cert_path, "Agent tunnel CA certificate generated and saved");
 
             Ok(Arc::new(Self {
@@ -292,16 +283,9 @@ impl CaManager {
             .context("sign server certificate with CA")?;
 
         std::fs::write(&cert_path, server_cert.pem()).with_context(|| format!("write server cert to {cert_path}"))?;
-        std::fs::write(&key_path, server_key_pair.serialize_pem())
+        write_restricted_file(&key_path, &server_key_pair.serialize_pem())
             .with_context(|| format!("write server key to {key_path}"))?;
 
-        #[cfg(unix)]
-        {
-            use std::os::unix::fs::PermissionsExt as _;
-            std::fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600))
-                .with_context(|| format!("set permissions on {key_path}"))?;
-        }
-
         info!(%cert_path, %hostname, "Server certificate generated and saved");
 
         Ok((cert_path, key_path))

crates/devolutions-agent-shared/Cargo.toml

@@ -10,6 +10,7 @@ publish = false
 workspace = true
 
 [dependencies]
+anyhow = "1"
 camino = "1.1"
 cfg-if = "1"
 serde = { version = "1", features = ["derive"] }

crates/devolutions-agent-shared/src/lib.rs

@@ -2,6 +2,7 @@
 pub mod windows;
 
 mod date_version;
+mod restricted_file;
 pub mod temp_file;
 mod update_manifest;
 mod update_status;
@@ -11,6 +12,7 @@ use std::env;
 use camino::Utf8PathBuf;
 use cfg_if::cfg_if;
 pub use date_version::{DateVersion, DateVersionError};
+pub use restricted_file::write_restricted_file;
 pub use update_manifest::{
     InstalledProductUpdateInfo, ProductUpdateInfo, ProductUpdateInfoV1, UPDATE_MANIFEST_V2_MINOR_VERSION,
     UpdateManifest, UpdateManifestV1, UpdateManifestV2, UpdateProductKey, UpdateSchedule, VersionMajorV2,

crates/devolutions-agent-shared/src/restricted_file.rs

@@ -0,0 +1,80 @@
+use anyhow::Context as _;
+use camino::Utf8Path;
+
+pub fn write_restricted_file(path: &Utf8Path, contents: &str) -> anyhow::Result<()> {
+    use std::io::Write as _;
+
+    let _ = std::fs::remove_file(path);
+
+    let mut file = create_restricted_file(path)?;
+
+    file.write_all(contents.as_bytes())
+        .with_context(|| format!("write to {path}"))
+}
+
+#[cfg(not(windows))]
+fn create_restricted_file(path: &Utf8Path) -> anyhow::Result<std::fs::File> {
+    let mut options = std::fs::OpenOptions::new();
+    options.write(true).create_new(true);
+
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::OpenOptionsExt as _;
+        options.mode(0o600);
+    }
+
+    options.open(path).with_context(|| format!("create file {path}"))
+}
+
+#[cfg(windows)]
+fn create_restricted_file(path: &Utf8Path) -> anyhow::Result<std::fs::File> {
+    use win_api_wrappers::identity::sid::Sid;
+    use win_api_wrappers::raw::Win32::Security;
+    use win_api_wrappers::raw::Win32::Security::Authorization::GRANT_ACCESS;
+    use win_api_wrappers::raw::Win32::Storage::FileSystem::{
+        DELETE, FILE_ALL_ACCESS, FILE_GENERIC_READ, FILE_GENERIC_WRITE,
+    };
+    use win_api_wrappers::security::acl::{Acl, ExplicitAccess, InheritableAcl, InheritableAclKind, Trustee};
+    use win_api_wrappers::security::attributes::SecurityAttributesInit;
+    use win_api_wrappers::token::Token;
+
+    let modify = FILE_GENERIC_READ.0 | FILE_GENERIC_WRITE.0 | DELETE.0;
+
+    let entry = |access_permissions, sid| ExplicitAccess {
+        access_permissions,
+        access_mode: GRANT_ACCESS,
+        inheritance: Security::NO_INHERITANCE,
+        trustee: Trustee::Sid(sid),
+    };
+
+    let well_known = |sid_type| Sid::from_well_known(sid_type, None).context("get well-known SID");
+
+    let entries = [
+        entry(FILE_ALL_ACCESS.0, well_known(Security::WinLocalSystemSid)?),
+        entry(FILE_ALL_ACCESS.0, well_known(Security::WinBuiltinAdministratorsSid)?),
+        entry(modify, well_known(Security::WinNetworkServiceSid)?),
+        entry(
+            modify,
+            Token::current_process_token()
+                .sid_and_attributes()
+                .context("get current process token user")?
+                .sid,
+        ),
+    ];
+
+    let dacl = InheritableAcl {
+        kind: InheritableAclKind::Protected,
+        acl: Acl::new()
+            .and_then(|acl| acl.set_entries(&entries))
+            .context("build restricted DACL")?,
+    };
+
+    let attributes = SecurityAttributesInit {
+        dacl: Some(dacl),
+        ..Default::default()
+    }
+    .init();
+
+    win_api_wrappers::fs::create_file(path.as_std_path(), Some(&attributes))
+        .with_context(|| format!("create file {path}"))
+}

crates/win-api-wrappers/src/fs.rs

@@ -1,4 +1,5 @@
 use std::ffi::OsString;
+use std::fs::File;
 use std::os::windows::ffi::OsStringExt;
 use std::path::Path;
 
@@ -20,6 +21,31 @@ pub fn create_directory(path: &Path, security_attributes: Option<&SecurityAttrib
     Ok(())
 }
 
+pub fn create_file(path: &Path, security_attributes: Option<&SecurityAttributes>) -> anyhow::Result<File> {
+    use std::os::windows::io::{FromRawHandle as _, OwnedHandle};
+
+    let path = U16CString::from_os_str(path.as_os_str()).context("invalid path")?;
+
+    // SAFETY: FFI call with no outstanding preconditions.
+    let handle = unsafe {
+        FileSystem::CreateFileW(
+            path.as_pcwstr(),
+            FileSystem::FILE_GENERIC_READ.0 | FileSystem::FILE_GENERIC_WRITE.0,
+            FileSystem::FILE_SHARE_NONE,
+            security_attributes.map(|x| x.as_ptr()),
+            FileSystem::CREATE_NEW,
+            FileSystem::FILE_ATTRIBUTE_NORMAL,
+            None,
+        )
+    }
+    .context("failed to create file")?;
+
+    // SAFETY: `CreateFileW` succeeded and returned a valid file handle that we now own.
+    let handle = unsafe { OwnedHandle::from_raw_handle(handle.0) };
+
+    Ok(File::from(handle))
+}
+
 pub fn get_system32_path() -> anyhow::Result<String> {
     let mut buffer = [0u16; MAX_PATH as usize];
 

devolutions-agent/src/enrollment.rs

@@ -6,6 +6,7 @@
 use anyhow::{Context as _, Result, bail};
 use base64::Engine as _;
 use camino::{Utf8Path, Utf8PathBuf};
+use devolutions_agent_shared::write_restricted_file;
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
@@ -235,8 +236,7 @@ fn persist_enrollment_response(
     // at config save, a non-zero exit would leave partial, orphaned state behind. On any failure we
     // roll that partial state back below so the machine is left exactly as it was before enroll.
     let persist = || -> Result<()> {
-        // Write the locally-generated private key first (before cert/CA from the network).
-        std::fs::write(&client_key_path, key_pem)
+        write_restricted_file(&client_key_path, key_pem)
             .with_context(|| format!("failed to write client private key: {client_key_path}"))?;
 
         std::fs::write(&client_cert_path, &client_cert_pem)
@@ -245,12 +245,11 @@ fn persist_enrollment_response(
         std::fs::write(&gateway_ca_path, &gateway_ca_cert_pem)
             .with_context(|| format!("failed to write gateway CA certificate: {gateway_ca_path}"))?;
 
-        // Restrict permissions on cert/key files (owner-only on Unix).
         #[cfg(unix)]
         {
             use std::os::unix::fs::PermissionsExt as _;
             let restricted = std::fs::Permissions::from_mode(0o600);
-            for path in [&client_cert_path, &client_key_path, &gateway_ca_path] {
+            for path in [&client_cert_path, &gateway_ca_path] {
                 std::fs::set_permissions(path, restricted.clone())
                     .with_context(|| format!("failed to set permissions on {path}"))?;
             }