diff --git a/Cargo.lock b/Cargo.lock index 874292004..e2503e883 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -94,6 +94,7 @@ dependencies = [ "async-trait", "base64 0.23.0", "camino", + "devolutions-agent-shared", "devolutions-gateway-task", "hex", "ipnetwork", diff --git a/crates/agent-tunnel/Cargo.toml b/crates/agent-tunnel/Cargo.toml index a837d2301..9bf1fe7f0 100644 --- a/crates/agent-tunnel/Cargo.toml +++ b/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 diff --git a/crates/agent-tunnel/src/cert.rs b/crates/agent-tunnel/src/cert.rs index 188364703..4d605c667 100644 --- a/crates/agent-tunnel/src/cert.rs +++ b/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)) diff --git a/crates/devolutions-agent-shared/Cargo.toml b/crates/devolutions-agent-shared/Cargo.toml index cb482c9a9..3161ecaee 100644 --- a/crates/devolutions-agent-shared/Cargo.toml +++ b/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"] } diff --git a/crates/devolutions-agent-shared/src/lib.rs b/crates/devolutions-agent-shared/src/lib.rs index 85edfa6be..c75c429fe 100644 --- a/crates/devolutions-agent-shared/src/lib.rs +++ b/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, diff --git a/crates/devolutions-agent-shared/src/restricted_file.rs b/crates/devolutions-agent-shared/src/restricted_file.rs new file mode 100644 index 000000000..c2e4bc796 --- /dev/null +++ b/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 { + 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 { + 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}")) +} diff --git a/crates/win-api-wrappers/src/fs.rs b/crates/win-api-wrappers/src/fs.rs index fe7e1e46c..ca2b3014a 100644 --- a/crates/win-api-wrappers/src/fs.rs +++ b/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 { + 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 { let mut buffer = [0u16; MAX_PATH as usize]; diff --git a/devolutions-agent/src/enrollment.rs b/devolutions-agent/src/enrollment.rs index b5b348282..e623e5c07 100644 --- a/devolutions-agent/src/enrollment.rs +++ b/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}"))?; }