From 6d87bf94de814587c90c734ac64536be25d65ec9 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Thu, 12 Sep 2024 13:29:34 -0300 Subject: [PATCH] Clear policy results and stats when setting an installer --- server/datastore/mysql/policies.go | 22 +++- server/datastore/mysql/policies_test.go | 118 ++++++++++++++++++ server/service/integration_enterprise_test.go | 106 +++++++++++++++- server/service/team_policies.go | 7 ++ 4 files changed, 242 insertions(+), 11 deletions(-) diff --git a/server/datastore/mysql/policies.go b/server/datastore/mysql/policies.go index f96f99289d7f..7ca49d35646a 100644 --- a/server/datastore/mysql/policies.go +++ b/server/datastore/mysql/policies.go @@ -753,9 +753,10 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs // Get the query and platforms of the current policies so that we can check if query or platform changed later, if needed type policyLite struct { - Name string `db:"name"` - Query string `db:"query"` - Platforms string `db:"platforms"` + Name string `db:"name"` + Query string `db:"query"` + Platforms string `db:"platforms"` + SoftwareInstallerID *uint `db:"software_installer_id"` } teamIDToPoliciesByName := make(map[*uint]map[string]policyLite, len(teamIDToPolicies)) for teamID, teamPolicySpecs := range teamIDToPolicies { @@ -769,10 +770,10 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs var args []interface{} var err error if teamID == nil { - query, args, err = sqlx.In("SELECT name, query, platforms FROM policies WHERE team_id IS NULL AND name IN (?)", policyNames) + query, args, err = sqlx.In("SELECT name, query, platforms, software_installer_id FROM policies WHERE team_id IS NULL AND name IN (?)", policyNames) } else { query, args, err = sqlx.In( - "SELECT name, query, platforms FROM policies WHERE team_id = ? AND name IN (?)", *teamID, policyNames, + "SELECT name, query, platforms, software_installer_id FROM policies WHERE team_id = ? AND name IN (?)", *teamID, policyNames, ) } if err != nil { @@ -838,12 +839,21 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs shouldRemoveAllPolicyMemberships bool removePolicyStats bool ) - // Figure out if the query or platform changed + // Figure out if the query, platform or software installer changed. + var softwareInstallerID *uint + if spec.SoftwareTitleID != nil { + softwareInstallerID = softwareInstallerIDs[teamID][*spec.SoftwareTitleID] + } if prev, ok := teamIDToPoliciesByName[teamID][spec.Name]; ok { switch { case prev.Query != spec.Query: shouldRemoveAllPolicyMemberships = true removePolicyStats = true + case teamID != nil && + ((prev.SoftwareInstallerID == nil && spec.SoftwareTitleID != nil) || + (prev.SoftwareInstallerID != nil && softwareInstallerID != nil && *prev.SoftwareInstallerID != *softwareInstallerID)): + shouldRemoveAllPolicyMemberships = true + removePolicyStats = true case prev.Platforms != spec.Platform: removePolicyStats = true } diff --git a/server/datastore/mysql/policies_test.go b/server/datastore/mysql/policies_test.go index c800eeee1c58..96392e494f04 100644 --- a/server/datastore/mysql/policies_test.go +++ b/server/datastore/mysql/policies_test.go @@ -4061,6 +4061,24 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) { require.NoError(t, err) team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"}) require.NoError(t, err) + newHost := func(name string, teamID *uint, platform string) *fleet.Host { + h, err := ds.NewHost(ctx, &fleet.Host{ + OsqueryHostID: ptr.String(uuid.New().String()), + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + PolicyUpdatedAt: time.Now(), + SeenTime: time.Now(), + NodeKey: ptr.String(uuid.New().String()), + UUID: uuid.New().String(), + Hostname: name, + TeamID: teamID, + Platform: platform, + }) + require.NoError(t, err) + return h + } + + host1Team1 := newHost("host1Team1", &team1.ID, "darwin") installer1ID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ InstallScript: "hello", @@ -4113,6 +4131,24 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) { installer3, err := ds.GetSoftwareInstallerMetadataByID(ctx, installer3ID) require.NoError(t, err) require.NotNil(t, installer3.TitleID) + // Another installer on team1 to test changing installers. + installer5ID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + InstallScript: "hello5", + PreInstallQuery: "SELECT 5;", + PostInstallScript: "world5", + InstallerFile: bytes.NewReader([]byte("hello5")), + StorageID: "storage5", + Filename: "file5", + Title: "file5", + Version: "1.0", + Source: "programs", + UserID: user1.ID, + TeamID: &team1.ID, + }) + require.NoError(t, err) + installer5, err := ds.GetSoftwareInstallerMetadataByID(ctx, installer5ID) + require.NoError(t, err) + require.NotNil(t, installer5.TitleID) // Installers cannot be assigned to global policies. err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{ @@ -4164,6 +4200,7 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, team1Policies, 1) require.NotNil(t, team1Policies[0].SoftwareInstallerID) + policy1Team1 := team1Policies[0] require.Equal(t, installer1.InstallerID, *team1Policies[0].SoftwareInstallerID) team2Policies, _, err := ds.ListTeamPolicies(ctx, team2.ID, fleet.ListOptions{}, fleet.ListOptions{}) require.NoError(t, err) @@ -4176,6 +4213,14 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) { require.NotNil(t, noTeamPolicies[0].SoftwareInstallerID) require.Equal(t, installer3.InstallerID, *noTeamPolicies[0].SoftwareInstallerID) + // Record policy execution on policy1Team1. + err = ds.RecordPolicyQueryExecutions(ctx, host1Team1, map[uint]*bool{ + policy1Team1.ID: ptr.Bool(false), + }, time.Now(), false) + require.NoError(t, err) + err = ds.UpdateHostPolicyCounts(ctx) + require.NoError(t, err) + // Unset software installer from "Team policy 1". err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{ { @@ -4193,6 +4238,8 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, team1Policies, 1) require.Nil(t, team1Policies[0].SoftwareInstallerID) + // Should not clear results because we've cleared not changed/set-new installer. + require.Equal(t, uint(1), team1Policies[0].FailingHostCount) // Set "Team policy 1" to a software installer on team2. err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{ @@ -4317,11 +4364,82 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) { require.Len(t, team1Policies, 1) require.NotNil(t, team1Policies[0].SoftwareInstallerID) require.Equal(t, installer1.InstallerID, *team1Policies[0].SoftwareInstallerID) + // Should clear results because we've are setting an installer. + require.Equal(t, uint(0), team1Policies[0].FailingHostCount) + countBiggerThanZero := true + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &countBiggerThanZero, + `SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`, + team1Policies[0].ID, + ) + }) + require.False(t, countBiggerThanZero) team2Policies, _, err = ds.ListTeamPolicies(ctx, team2.ID, fleet.ListOptions{}, fleet.ListOptions{}) require.NoError(t, err) require.Len(t, team2Policies, 1) require.NotNil(t, team2Policies[0].SoftwareInstallerID) require.Equal(t, installer4.InstallerID, *team2Policies[0].SoftwareInstallerID) + + // Record policy execution on policy1Team1 to test that setting the same installer won't clear results. + err = ds.RecordPolicyQueryExecutions(ctx, host1Team1, map[uint]*bool{ + policy1Team1.ID: ptr.Bool(false), + }, time.Now(), false) + require.NoError(t, err) + err = ds.UpdateHostPolicyCounts(ctx) + require.NoError(t, err) + err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{ + { + Name: "Team policy 1", + Query: "SELECT 1;", + Description: "Description 1", + Resolution: "Resolution 1", + Team: "team1", + Platform: "darwin", + SoftwareTitleID: installer1.TitleID, + }, + }) + require.NoError(t, err) + team1Policies, _, err = ds.ListTeamPolicies(ctx, team1.ID, fleet.ListOptions{}, fleet.ListOptions{}) + require.NoError(t, err) + require.Len(t, team1Policies, 1) + require.Equal(t, uint(1), team1Policies[0].FailingHostCount) + countBiggerThanZero = false + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &countBiggerThanZero, + `SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`, + team1Policies[0].ID, + ) + }) + require.True(t, countBiggerThanZero) + + // Now change the installer, should clear results. + err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{ + { + Name: "Team policy 1", + Query: "SELECT 1;", + Description: "Description 1", + Resolution: "Resolution 1", + Team: "team1", + Platform: "darwin", + SoftwareTitleID: installer5.TitleID, + }, + }) + require.NoError(t, err) + team1Policies, _, err = ds.ListTeamPolicies(ctx, team1.ID, fleet.ListOptions{}, fleet.ListOptions{}) + require.NoError(t, err) + require.Len(t, team1Policies, 1) + require.Equal(t, uint(0), team1Policies[0].FailingHostCount) + countBiggerThanZero = true + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &countBiggerThanZero, + `SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`, + team1Policies[0].ID, + ) + }) + require.False(t, countBiggerThanZero) } func testTeamPoliciesNoTeam(t *testing.T, ds *Datastore) { diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 97acc152192a..1d15444f8149 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -13553,6 +13553,35 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID) require.NoError(t, err) require.Nil(t, policy1Team1.SoftwareInstallerID) + + host1LastInstall, err := s.ds.GetHostLastInstallData(ctx, host1Team1.ID, dummyInstallerPkgInstallerID) + require.NoError(t, err) + require.Nil(t, host1LastInstall) + + // Add some results and stats that should be cleared after setting an installer again. + distributedResp := submitDistributedQueryResultsResponse{} + s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults( + host1Team1, + map[uint]*bool{ + policy1Team1.ID: ptr.Bool(false), + }, + ), http.StatusOK, &distributedResp) + err = s.ds.UpdateHostPolicyCounts(ctx) + require.NoError(t, err) + policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID) + require.NoError(t, err) + require.Equal(t, uint(0), policy1Team1.PassingHostCount) + require.Equal(t, uint(1), policy1Team1.FailingHostCount) + passes := true + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &passes, + `SELECT passes FROM policy_membership WHERE policy_id = ? AND host_id = ?`, + policy1Team1.ID, host1Team1.ID, + ) + }) + require.False(t, passes) + // Back to associating dummy_installer.pkg to policy1Team1. mtplr = modifyTeamPolicyResponse{} s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy1Team1.ID), modifyTeamPolicyRequest{ @@ -13564,18 +13593,85 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers require.NoError(t, err) require.NotNil(t, policy1Team1.SoftwareInstallerID) require.Equal(t, dummyInstallerPkgInstallerID, *policy1Team1.SoftwareInstallerID) + // Policy stats and membership should be cleared from policy1Team1. + require.Equal(t, uint(0), policy1Team1.PassingHostCount) + require.Equal(t, uint(0), policy1Team1.FailingHostCount) + countBiggerThanZero := true + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &countBiggerThanZero, + `SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`, + policy1Team1.ID, + ) + }) + require.False(t, countBiggerThanZero) - // Associate ruby.deb to policy2Team1. + // Add (again) some results and stats that should be cleared after changing an existing installer. + distributedResp = submitDistributedQueryResultsResponse{} + s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults( + host1Team1, + map[uint]*bool{ + policy1Team1.ID: ptr.Bool(false), + }, + ), http.StatusOK, &distributedResp) + err = s.ds.UpdateHostPolicyCounts(ctx) + require.NoError(t, err) + policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID) + require.NoError(t, err) + require.Equal(t, uint(0), policy1Team1.PassingHostCount) + require.Equal(t, uint(1), policy1Team1.FailingHostCount) + passes = true + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &passes, + `SELECT passes FROM policy_membership WHERE policy_id = ? AND host_id = ?`, + policy1Team1.ID, host1Team1.ID, + ) + }) + require.False(t, passes) + + // Change the installer (temporarily to test that changing an installer will clear results) + // Associate ruby.deb to policy1Team1. mtplr = modifyTeamPolicyResponse{} - s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy2Team1.ID), modifyTeamPolicyRequest{ + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy1Team1.ID), modifyTeamPolicyRequest{ ModifyPolicyPayload: fleet.ModifyPolicyPayload{ SoftwareTitleID: &rubyDebTitleID, }, }, http.StatusOK, &mtplr) - host1LastInstall, err := s.ds.GetHostLastInstallData(ctx, host1Team1.ID, dummyInstallerPkgInstallerID) + // After changing the installer, membership and stats should be cleared. + policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID) require.NoError(t, err) - require.Nil(t, host1LastInstall) + require.NotNil(t, policy1Team1.SoftwareInstallerID) + require.Equal(t, rubyDebInstallerID, *policy1Team1.SoftwareInstallerID) + // Policy stats and membership should be cleared from policy1Team1. + require.Equal(t, uint(0), policy1Team1.PassingHostCount) + require.Equal(t, uint(0), policy1Team1.FailingHostCount) + countBiggerThanZero = true + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, + &countBiggerThanZero, + `SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`, + policy1Team1.ID, + ) + }) + require.False(t, countBiggerThanZero) + + // Back to (again) associating dummy_installer.pkg to policy1Team1. + mtplr = modifyTeamPolicyResponse{} + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy1Team1.ID), modifyTeamPolicyRequest{ + ModifyPolicyPayload: fleet.ModifyPolicyPayload{ + SoftwareTitleID: &dummyInstallerPkgTitleID, + }, + }, http.StatusOK, &mtplr) + + // Associate ruby.deb to policy2Team1. + mtplr = modifyTeamPolicyResponse{} + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy2Team1.ID), modifyTeamPolicyRequest{ + ModifyPolicyPayload: fleet.ModifyPolicyPayload{ + SoftwareTitleID: &rubyDebTitleID, + }, + }, http.StatusOK, &mtplr) // We use DoJSONWithoutAuth for distributed/write because we want the requests to not have the // current user's "Authorization: Bearer " header. @@ -13584,7 +13680,7 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers // Failing policy1Team1 means an install request must be generated. // Failing policy2Team1 should not trigger a install request because it has a .deb attached to it (does not apply to macOS hosts). // Failing policy3Team1 should do nothing because it doesn't have any installers associated to it. - distributedResp := submitDistributedQueryResultsResponse{} + distributedResp = submitDistributedQueryResultsResponse{} s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults( host1Team1, map[uint]*bool{ diff --git a/server/service/team_policies.go b/server/service/team_policies.go index 74c22fe4e553..14cea750e1a0 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -493,6 +493,13 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f if err != nil { return nil, err } + // If the associated installer is changed (or it's set and the policy didn't have an associated installer) + // then we clear the results of the policy so that automation can be triggered upon failure + // (automation is currently triggered on the first failure or when it goes from passing to failure). + if softwareInstallerID != nil && (policy.SoftwareInstallerID == nil || *policy.SoftwareInstallerID != *softwareInstallerID) { + removeAllMemberships = true + removeStats = true + } policy.SoftwareInstallerID = softwareInstallerID }