Update ProgramData Directory permissions

releasenotes/notes/update-programdata-permissions-6271744e48907d71.yaml

@@ -0,0 +1,11 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+security:
+  - |
+    Removes Datadog user's full control of the Datadog data directory on windows.

test/new-e2e/tests/windows/install-test/assert.go

@@ -16,9 +16,10 @@ import (
 	windows "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common"
 	windowsAgent "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/agent"
 
+	"testing"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
 // AssertInstalledUserInRegistry checks the registry for the installed user and domain

test/new-e2e/tests/windows/install-test/installtester.go

@@ -466,6 +466,7 @@ func (t *Tester) testInstalledFilePermissions(tt *testing.T, ddAgentUserIdentity
 		path             string
 		expectedSecurity func(t *testing.T) windows.ObjectSecurity
 	}{
+		//ConfigRoot is only owned by SYSTEM and Administrators
 		{
 			name: "ConfigRoot",
 			path: t.expectedConfigRoot,
@@ -478,11 +479,27 @@ func (t *Tester) testInstalledFilePermissions(tt *testing.T, ddAgentUserIdentity
 				expected.Access = append(expected.Access,
 					windows.NewExplicitAccessRuleWithFlags(
 						ddAgentUserIdentity,
-						windows.FileFullControl,
+						windows.FileReadAndExecute|windows.SYNCHRONIZE,
 						windows.AccessControlTypeAllow,
 						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
 						windows.PropagationFlagsNone,
 					),
+					windows.NewExplicitAccessRuleWithFlags(
+						ddAgentUserIdentity,
+						windows.FileWrite|windows.SYNCHRONIZE,
+						windows.AccessControlTypeAllow,
+						windows.InheritanceFlagsNone,
+						windows.PropagationFlagsNone,
+					),
+					// add creator owner permissions
+					windows.NewExplicitAccessRuleWithFlags(
+						windows.GetIdentityForSID("S-1-3-0"),
+						windows.FileFullControl,
+						windows.AccessControlTypeAllow,
+						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
+						// yes this flag is wrong, but go has the wrong value for it
+						windows.PropagationFlagsNoPropagate,
+					),
 				)
 				return expected
 			},
@@ -497,11 +514,17 @@ func (t *Tester) testInstalledFilePermissions(tt *testing.T, ddAgentUserIdentity
 					return expected
 				}
 				expected.Access = append(expected.Access,
-					windows.NewInheritedAccessRule(
+					windows.NewExplicitAccessRule(
 						ddAgentUserIdentity,
 						windows.FileFullControl,
 						windows.AccessControlTypeAllow,
 					),
+					// extra inherited rule for ddagentuser
+					windows.NewInheritedAccessRule(
+						ddAgentUserIdentity,
+						windows.FileReadAndExecute|windows.SYNCHRONIZE,
+						windows.AccessControlTypeAllow,
+					),
 				)
 				return expected
 			},
@@ -515,15 +538,53 @@ func (t *Tester) testInstalledFilePermissions(tt *testing.T, ddAgentUserIdentity
 				if windows.IsIdentityLocalSystem(ddAgentUserIdentity) {
 					return expected
 				}
-				expected.Access = append(expected.Access,
+				expected.Access = []windows.AccessRule{
+					windows.NewInheritedAccessRuleWithFlags(
+						windows.GetIdentityForSID(windows.LocalSystemSID),
+						windows.FileFullControl,
+						windows.AccessControlTypeAllow,
+						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
+						windows.PropagationFlagsNoPropagate,
+					),
+					windows.NewInheritedAccessRuleWithFlags(
+						windows.GetIdentityForSID(windows.AdministratorsSID),
+						windows.FileFullControl,
+						windows.AccessControlTypeAllow,
+						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
+						windows.PropagationFlagsNone,
+					),
+					windows.NewExplicitAccessRuleWithFlags(
+						ddAgentUserIdentity,
+						windows.FileFullControl,
+						windows.AccessControlTypeAllow,
+						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
+						windows.PropagationFlagsNone,
+					),
+					// extra inherited rule for ddagentuser
 					windows.NewInheritedAccessRuleWithFlags(
 						ddAgentUserIdentity,
+						windows.FileReadAndExecute|windows.SYNCHRONIZE,
+						windows.AccessControlTypeAllow,
+						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
+						windows.PropagationFlagsNone,
+					),
+					// add creator owner permissions
+					windows.NewInheritedAccessRuleWithFlags(
+						windows.GetIdentityForSID("S-1-3-0"),
 						windows.FileFullControl,
 						windows.AccessControlTypeAllow,
 						windows.InheritanceFlagsContainer|windows.InheritanceFlagsObject,
+						windows.PropagationFlagsNoPropagate,
+					),
+					// create owner inherited permissions
+					windows.NewInheritedAccessRuleWithFlags(
+						windows.GetIdentityForSID(windows.LocalSystemSID),
+						windows.FileFullControl,
+						windows.AccessControlTypeAllow,
 						windows.PropagationFlagsNone,
+						windows.InheritanceFlagsNone,
 					),
-				)
+				}
 				return expected
 			},
 		},

tools/windows/DatadogAgentInstaller/CustomActions/ConfigureUserCustomActions.cs

@@ -446,6 +446,103 @@ private void GrantAgentAccessPermissions()
             }
         }
 
+        private void AddDatadogUserToDataFolder()
+        {
+            var dataDirectory = _session.Property("APPLICATIONDATADIRECTORY");
+
+            FileSystemSecurity fileSystemSecurity;
+            try
+            {
+                fileSystemSecurity = _fileSystemServices.GetAccessControl(dataDirectory, AccessControlSections.All);
+            }
+            catch (Exception e)
+            {
+                _session.Log($"Failed to get ACLs on {dataDirectory}: {e}");
+                throw;
+            }
+            // ddagentuser Read and execute permissions, enable child inheritance of this ACE
+            fileSystemSecurity.AddAccessRule(new FileSystemAccessRule(
+                _ddAgentUserSID,
+                FileSystemRights.ReadAndExecute | FileSystemRights.Synchronize,
+                InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit,
+                PropagationFlags.None,
+                AccessControlType.Allow));
+
+            // datadog write on this folder
+            fileSystemSecurity.AddAccessRule(new FileSystemAccessRule(
+                _ddAgentUserSID,
+                FileSystemRights.WriteData | FileSystemRights.AppendData | FileSystemRights.WriteAttributes | FileSystemRights.WriteExtendedAttributes | FileSystemRights.Synchronize,
+                InheritanceFlags.None,
+                PropagationFlags.None,
+                AccessControlType.Allow));
+
+            // add full control to CREATOR OWNER
+            fileSystemSecurity.AddAccessRule(new FileSystemAccessRule(
+                new SecurityIdentifier(WellKnownSidType.CreatorOwnerSid, null),
+                FileSystemRights.FullControl,
+                InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit,
+                PropagationFlags.InheritOnly,
+                AccessControlType.Allow));
+            try
+            {
+                UpdateAndLogAccessControl(dataDirectory, fileSystemSecurity);
+            }
+            catch (Exception e)
+            {
+                _session.Log($"Failed to set ACLs on {dataDirectory}: {e}");
+                throw;
+            }
+
+        }
+
+        private void RemoveDatadogUserFromDataFolder(SecurityIdentifier sid)
+        {
+            var dataDirectory = _session.Property("APPLICATIONDATADIRECTORY");
+
+            FileSystemSecurity fileSystemSecurity;
+            try
+            {
+                fileSystemSecurity = _fileSystemServices.GetAccessControl(dataDirectory, AccessControlSections.All);
+            }
+            catch (Exception e)
+            {
+                _session.Log($"Failed to get ACLs on {dataDirectory}: {e}");
+                throw;
+            }
+
+            // Remove ddagentuser from data folder
+            fileSystemSecurity.RemoveAccessRule(new FileSystemAccessRule(
+                sid,
+                FileSystemRights.ReadAndExecute | FileSystemRights.Synchronize,
+                InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit,
+                PropagationFlags.None,
+                AccessControlType.Allow));
+            fileSystemSecurity.RemoveAccessRule(new FileSystemAccessRule(
+                sid,
+                FileSystemRights.Write,
+                InheritanceFlags.None,
+                PropagationFlags.None,
+                AccessControlType.Allow));
+
+            // remove full control to CREATOR OWNER
+            fileSystemSecurity.RemoveAccessRule(new FileSystemAccessRule(
+                new SecurityIdentifier(WellKnownSidType.CreatorOwnerSid, null),
+                FileSystemRights.FullControl,
+                InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit,
+                PropagationFlags.InheritOnly,
+                AccessControlType.Allow));
+
+            try
+            {
+                UpdateAndLogAccessControl(dataDirectory, fileSystemSecurity);
+            }
+            catch (Exception e)
+            {
+                _session.Log($"Failed to set ACLs on {dataDirectory}: {e}");
+                throw;
+            }
+        }
+
         private void ConfigureFilePermissions()
         {
             try
@@ -471,6 +568,7 @@ private void ConfigureFilePermissions()
 
                 if (_ddAgentUserSID != new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null))
                 {
+                    AddDatadogUserToDataFolder();
                     GrantAgentAccessPermissions();
                 }
             }
@@ -579,14 +677,19 @@ public static ActionResult ConfigureUserRollback(Session session)
 
         private List<string> PathsWithAgentAccess()
         {
-            return new List<string>
-            {
-                // agent needs to be able to write logs/
-                // agent GUI needs to be able to edit config
-                // agent needs to be able to write to run/
-                // agent needs to be able to create auth_token
-                _session.Property("APPLICATIONDATADIRECTORY"),
-            };
+            var configRoot = _session.Property("APPLICATIONDATADIRECTORY");
+
+            return new List<string> {
+                Path.Combine(configRoot, "conf.d"),
+                Path.Combine(configRoot, "checks.d"),
+                Path.Combine(configRoot, "run"),
+                Path.Combine(configRoot, "logs"),
+                Path.Combine(configRoot, "datadog.yaml"),
+                Path.Combine(configRoot, "system-probe.yaml"),
+                Path.Combine(configRoot, "auth_token"),
+                Path.Combine(configRoot, "install_info"),
+                Path.Combine(configRoot, "python-cache"),
+            }; ;
         }
 
         /// <summary>
@@ -679,6 +782,8 @@ public ActionResult UninstallUser()
                             _session.Log($"Failed to remove {ddAgentUserName} from {filePath}: {e}");
                         }
                     }
+                    //remove access to root folder
+                    RemoveDatadogUserFromDataFolder(securityIdentifier);
                 }
 
                 // We intentionally do NOT delete the ddagentuser account.