Verify write permissions on settings policy folder

Cargo.toml

@@ -266,11 +266,14 @@ urlencoding = { version = "2.1" }
 which = { version = "8.0.4" }
 # dsc-lib
 ipnetwork = { version = "0.21" }
-# WindowsUpdate, windows_service
+# WindowsUpdate, windows_service, dsc-lib (ACL checks)
 windows = { version = "0.62", features = [
     "Win32_Foundation",
     "Win32_NetworkManagement_WindowsFirewall",
+    "Win32_Security",
+    "Win32_Security_Authorization",
     "Win32_System_Com",
+    "Win32_System_Memory",
     "Win32_System_Ole",
     "Win32_System_Services",
     "Win32_System_Variant",

dsc/tests/dsc_settings_policy_security.tests.ps1

@@ -0,0 +1,94 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+
+Describe 'tests for policy folder security validation' {
+    BeforeDiscovery {
+        $isElevated = if ($IsWindows) {
+            ([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole(
+                [Security.Principal.WindowsBuiltInRole]::Administrator)
+        } else {
+            (id -u) -eq 0
+        }
+    }
+
+    Context 'Windows policy folder with insecure ACL emits warning' -Skip:(!$IsWindows -or !$isElevated) {
+        BeforeAll {
+            $script:policyDirPath = Join-Path $env:ProgramData "dsc"
+            $script:policyFilePath = Join-Path $script:policyDirPath "dsc.settings.json"
+            $script:existedBefore = Test-Path $script:policyDirPath
+
+            if ($script:existedBefore) {
+                $script:originalAcl = Get-Acl -Path $script:policyDirPath
+            } else {
+                New-Item -ItemType Directory -Path $script:policyDirPath -Force | Out-Null
+            }
+
+            # Write a simple policy settings file
+            @{ tracing = @{ level = "TRACE" } } | ConvertTo-Json -Depth 5 | Set-Content -Path $script:policyFilePath
+
+            # Grant Everyone write access to make the folder insecure
+            icacls $script:policyDirPath /grant "Everyone:(OI)(CI)(W)" /T | Out-Null
+        }
+
+        AfterAll {
+            Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue
+
+            if ($script:existedBefore) {
+                Set-Acl -Path $script:policyDirPath -AclObject $script:originalAcl
+            } else {
+                Remove-Item -Recurse -Force -Path $script:policyDirPath -ErrorAction SilentlyContinue
+            }
+        }
+
+        It 'Should emit a warning about insecure policy folder' {
+            dsc -l warn resource list 2> $TestDrive/tracing.txt
+            "$TestDrive/tracing.txt" | Should -FileContentMatch "is not secure, settings file will not be used"
+        }
+
+        It 'Should not exit with an error code' {
+            dsc -l warn resource list 2> $TestDrive/tracing.txt
+            $LASTEXITCODE | Should -Be 0
+        }
+    }
+
+    Context 'Linux policy folder with insecure permissions emits warning' -Skip:($IsWindows -or !$isElevated) {
+        BeforeAll {
+            $script:policyDirPath = "/etc/dsc"
+            $script:policyFilePath = Join-Path $script:policyDirPath "dsc.settings.json"
+            $script:existedBefore = Test-Path $script:policyDirPath
+
+            if ($script:existedBefore) {
+                $script:originalMode = (stat -c '%a' $script:policyDirPath)
+            } else {
+                New-Item -ItemType Directory -Path $script:policyDirPath -Force | Out-Null
+            }
+
+            # Write a simple policy settings file
+            @{ tracing = @{ level = "TRACE" } } | ConvertTo-Json -Depth 5 | Set-Content -Path $script:policyFilePath
+
+            # Make the folder world-writable (insecure)
+            chmod 777 $script:policyDirPath
+        }
+
+        AfterAll {
+            if ($script:existedBefore) {
+                chmod $script:originalMode $script:policyDirPath
+            }
+
+            Remove-Item -Path $script:policyFilePath -ErrorAction SilentlyContinue
+            if (-not $script:existedBefore) {
+                Remove-Item -Recurse -Force -Path $script:policyDirPath -ErrorAction SilentlyContinue
+            }
+        }
+
+        It 'Should emit a warning about insecure policy folder' {
+            dsc -l warn resource list 2> $TestDrive/tracing.txt
+            "$TestDrive/tracing.txt" | Should -FileContentMatch "is not secure, settings file will not be used"
+        }
+
+        It 'Should not exit with an error code' {
+            dsc -l warn resource list 2> $TestDrive/tracing.txt
+            $LASTEXITCODE | Should -Be 0
+        }
+    }
+}

lib/dsc-lib/Cargo.toml

@@ -54,6 +54,7 @@ tree-sitter-dscexpression = { workspace = true }
 
 [target.'cfg(windows)'.dependencies]
 registry = { workspace = true }
+windows = { workspace = true }
 
 [dev-dependencies]
 serde_yaml = { workspace = true }

lib/dsc-lib/locales/en-us.toml

@@ -931,6 +931,9 @@ settingNotFound = "Setting '%{name}' not found"
 failedToAbsolutizePath = "Failed to absolutize path '%{path}'"
 executableNotFoundInWorkingDirectory = "Executable '%{executable}' not found with working directory '%{cwd}'"
 executableNotFound = "Executable '%{executable}' not found"
+policyFolderNotSecure = "Policy folder '%{path}' is not secure, settings file will not be used. Required permissions: %{required}"
+policyFolderNotSecureLinux = "Only root should have write access (no group/other write bits)"
+policyFolderNotSecureWindows = "Only SYSTEM and Administrators should have write access"
 
 [types.date_version]
 invalidDay = "day `%{day}` for month `%{month}` is invalid - must be between `01` and `%{max_days}`"

lib/dsc-lib/src/util.rs

@@ -11,7 +11,7 @@ use std::{
     path::{Path, PathBuf},
     env,
 };
-use tracing::debug;
+use tracing::{debug, warn};
 use which::which;
 
 pub struct DscSettingValue {
@@ -144,12 +144,13 @@ pub fn get_setting(value_name: &str) -> Result<DscSettingValue, DscError> {
     }
 
     // Third, get setting from the policy
-    settings_file_path = PathBuf::from(get_settings_policy_file_path());
-    if let Ok(v) = load_value_from_json(&settings_file_path, value_name) {
-        result.policy = v;
-        debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy()));
-    } else {
-        debug!("{}", t!("util.notFoundSetting", name = value_name, path = settings_file_path.to_string_lossy()));
+    if let Some(settings_file_path) = get_settings_policy_file_path() {
+        if let Ok(v) = load_value_from_json(&settings_file_path, value_name) {
+            result.policy = v;
+            debug!("{}", t!("util.foundSetting", name = value_name, path = settings_file_path.to_string_lossy()));
+        } else {
+            debug!("{}", t!("util.notFoundSetting", name = value_name, path = settings_file_path.to_string_lossy()));
+        }
     }
 
     if (result.setting == serde_json::Value::Null) && (result.policy == serde_json::Value::Null) {
@@ -199,20 +200,161 @@ pub fn get_exe_path() -> Result<PathBuf, DscError> {
 }
 
 #[cfg(target_os = "windows")]
-fn get_settings_policy_file_path() -> String
+fn get_settings_policy_file_path() -> Option<PathBuf>
 {
     // $env:ProgramData+"\dsc\dsc.settings.json"
-    // This location is writable only by admins, but readable by all users
-    let Ok(local_program_data_path) = std::env::var("ProgramData") else { return String::new(); };
-    Path::new(&local_program_data_path).join("dsc").join("dsc.settings.json").display().to_string()
+    let Ok(local_program_data_path) = std::env::var("ProgramData") else { return None; };
+    let dsc_folder = Path::new(&local_program_data_path).join("dsc");
+    let settings_path = dsc_folder.join("dsc.settings.json").display().to_string();
+
+    if !dsc_folder.exists() {
+        return Some(PathBuf::from(settings_path));
+    }
+
+    if !verify_windows_acl(&dsc_folder) {
+        let required = t!("util.policyFolderNotSecureWindows", path = dsc_folder.display());
+        warn!("{}", t!("util.policyFolderNotSecure", path = dsc_folder.display(), required = required));
+        return None;
+    }
+
+    Some(PathBuf::from(settings_path))
+}
+
+/// Returns `true` if only SYSTEM and Administrators have write access; `false` otherwise.
+#[cfg(target_os = "windows")]
+fn verify_windows_acl(dsc_folder: &Path) -> bool {
+    use windows::Win32::Security::{
+        Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT},
+        IsWellKnownSid, GetAce,
+        WinBuiltinAdministratorsSid, WinLocalSystemSid,
+        ACL, ACCESS_ALLOWED_ACE, PSECURITY_DESCRIPTOR,
+    };
+    use windows::Win32::Foundation::{LocalFree, HLOCAL};
+    use windows::core::PCWSTR;
+    use std::ptr;
+
+    const FILE_WRITE_DATA: u32 = 0x0002;
+    const FILE_APPEND_DATA: u32 = 0x0004;
+    const WRITE_DAC: u32 = 0x0004_0000;
+    const WRITE_OWNER: u32 = 0x0008_0000;
+    const GENERIC_WRITE: u32 = 0x4000_0000;
+    const GENERIC_ALL: u32 = 0x1000_0000;
+    const WRITE_MASK: u32 = FILE_WRITE_DATA | FILE_APPEND_DATA | WRITE_DAC | WRITE_OWNER | GENERIC_WRITE | GENERIC_ALL;
+
+    const INHERIT_ONLY_ACE: u8 = 0x08;
+
+    let folder_wide: Vec<u16> = dsc_folder
+        .to_string_lossy()
+        .encode_utf16()
+        .chain(std::iter::once(0))
+        .collect();
+
+    let mut p_sd: PSECURITY_DESCRIPTOR = PSECURITY_DESCRIPTOR(ptr::null_mut());
+    let mut p_dacl: *mut ACL = ptr::null_mut();
+
+    let result = unsafe {
+        GetNamedSecurityInfoW(
+            PCWSTR(folder_wide.as_ptr()),
+            SE_FILE_OBJECT,
+            windows::Win32::Security::DACL_SECURITY_INFORMATION,
+            None,
+            None,
+            Some(&mut p_dacl),
+            None,
+            &mut p_sd,
+        )
+    };
+
+    // Fail closed: if we can't read the security descriptor, treat as insecure
+    if result.is_err() {
+        return false;
+    }
+
+    // A NULL DACL means full access to everyone
+    if p_dacl.is_null() {
+        unsafe { let _ = LocalFree(Some(HLOCAL(p_sd.0.cast()))); }
+        return false;
+    }
+
+    let ace_count = unsafe { (*p_dacl).AceCount };
+
+    for i in 0..ace_count {
+        let mut ace_ptr: *mut core::ffi::c_void = ptr::null_mut();
+        let ok = unsafe { GetAce(p_dacl, i as u32, &mut ace_ptr) };
+        if ok.is_err() {
+            // Fail closed: if we can't read an ACE, treat as insecure
+            unsafe { let _ = LocalFree(Some(HLOCAL(p_sd.0.cast()))); }
+            return false;
+        }
+
+        let header = unsafe { &*(ace_ptr as *const windows::Win32::Security::ACE_HEADER) };
+        // Only check ACCESS_ALLOWED_ACE_TYPE (0)
+        if header.AceType != 0 {
+            continue;
+        }
+
+        // Skip inherit-only ACEs as they don't apply to this folder
+        if (header.AceFlags & INHERIT_ONLY_ACE) != 0 {
+            continue;
+        }
+
+        let ace = unsafe { &*(ace_ptr as *const ACCESS_ALLOWED_ACE) };
+        if (ace.Mask & WRITE_MASK) == 0 {
+            continue;
+        }
+
+        // Get pointer to SID within the ACE (SidStart field)
+        let sid_ptr = &ace.SidStart as *const u32 as *const core::ffi::c_void;
+        let psid = windows::Win32::Security::PSID(sid_ptr as *mut core::ffi::c_void);
+
+        let is_system = unsafe { IsWellKnownSid(psid, WinLocalSystemSid).as_bool() };
+        let is_admin = unsafe { IsWellKnownSid(psid, WinBuiltinAdministratorsSid).as_bool() };
+
+        if !is_system && !is_admin {
+            unsafe { let _ = LocalFree(Some(HLOCAL(p_sd.0.cast()))); }
+            return false;
+        }
+    }
+
+    unsafe { let _ = LocalFree(Some(HLOCAL(p_sd.0.cast()))); }
+    true
 }
 
 #[cfg(not(target_os = "windows"))]
-fn get_settings_policy_file_path() -> String
+fn get_settings_policy_file_path() -> Option<PathBuf>
 {
+    use std::os::unix::fs::MetadataExt;
+
     // "/etc/dsc/dsc.settings.json"
-    // This location is writable only by admins, but readable by all users
-    Path::new("/etc").join("dsc").join("dsc.settings.json").display().to_string()
+    // This location is writable only by root, but readable by all users
+    let dsc_folder = Path::new("/etc").join("dsc");
+    let settings_path = dsc_folder.join("dsc.settings.json");
+
+    if !dsc_folder.exists() {
+        return Some(settings_path);
+    }
+
+    // Fail closed: if we can't read metadata, treat as insecure
+    let Ok(metadata) = fs::metadata(&dsc_folder) else {
+        warn!("{}", t!("util.policyFolderNotSecure", path = dsc_folder.display(), required = t!("util.policyFolderNotSecureLinux")));
+        return None;
+    };
+
+    let mode = metadata.mode();
+    let uid = metadata.uid();
+
+    // Verify owner is root
+    // Verify no group or other write bits are set
+    let group_write = mode & 0o020;
+    let other_write = mode & 0o002;
+
+    if uid != 0 || group_write != 0 || other_write != 0 {
+        let required = t!("util.policyFolderNotSecureLinux");
+        warn!("{}", t!("util.policyFolderNotSecure", path = dsc_folder.display(), required = required));
+        return None;
+    }
+
+    Some(settings_path)
 }
 
 /// Generates a resource ID from the specified type and name.