ensure distributed queries can't be empty strings (#15641)

There's a bug somewhere (Fleet, osquery or both?)
that causes hosts to check-in in a loop if you send an empty query
string.

we previously fixed this for detail query overrides (see #14286, #14296)
but I'm also adding a general check as a safeguard for issues like
#15524
This commit is contained in:
Roberto Dip 2023-12-14 12:38:54 -03:00 committed by GitHub
parent 27bd63d8f0
commit 82b7156952
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 92 additions and 1 deletions

View file

@ -35,6 +35,7 @@ import (
"github.com/fleetdm/fleet/v4/server/datastore/redis/redistest"
"github.com/fleetdm/fleet/v4/server/fleet"
mdm_types "github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/live_query/live_query_mock"
servermdm "github.com/fleetdm/fleet/v4/server/mdm"
apple_mdm "github.com/fleetdm/fleet/v4/server/mdm/apple"
"github.com/fleetdm/fleet/v4/server/mdm/apple/mobileconfig"
@ -122,6 +123,7 @@ func (s *integrationMDMTestSuite) SetupSuite() {
)
mdmCommander := apple_mdm.NewMDMAppleCommander(mdmStorage, mdmPushService)
redisPool := redistest.SetupRedis(s.T(), "zz", false, false, false)
s.withServer.lq = live_query_mock.New(s.T())
var depSchedule *schedule.Schedule
var profileSchedule *schedule.Schedule
@ -135,6 +137,7 @@ func (s *integrationMDMTestSuite) SetupSuite() {
SCEPStorage: scepStorage,
MDMPusher: mdmPushService,
Pool: redisPool,
Lq: s.lq,
StartCronSchedules: []TestNewScheduleFunc{
func(ctx context.Context, ds fleet.Datastore) fleet.NewCronScheduleFunc {
return func() (fleet.CronSchedule, error) {
@ -10275,3 +10278,39 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() {
0,
)
}
func (s *integrationMDMTestSuite) TestWindowsFreshEnrollEmptyQuery() {
t := s.T()
host, _ := createWindowsHostThenEnrollMDM(s.ds, s.server.URL, t)
// make sure we don't have any profiles
s.Do(
"POST",
"/api/v1/fleet/mdm/profiles/batch",
batchSetMDMProfilesRequest{Profiles: map[string][]byte{}},
http.StatusNoContent,
)
// Ensure we can read distributed queries for the host.
err := s.ds.UpdateHostRefetchRequested(context.Background(), host.ID, true)
require.NoError(t, err)
s.lq.On("QueriesForHost", host.ID).Return(map[string]string{fmt.Sprintf("%d", host.ID): "SELECT 1 FROM osquery;"}, nil)
req := getDistributedQueriesRequest{NodeKey: *host.NodeKey}
var dqResp getDistributedQueriesResponse
s.DoJSON("POST", "/api/osquery/distributed/read", req, http.StatusOK, &dqResp)
require.NotContains(t, dqResp.Queries, "fleet_detail_query_mdm_config_profiles_windows")
// add two profiles
s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: map[string][]byte{
"N1": mobileconfigForTest("N1", "I1"),
"N2": syncMLForTest("./Foo/Bar"),
}}, http.StatusNoContent)
req = getDistributedQueriesRequest{NodeKey: *host.NodeKey}
dqResp = getDistributedQueriesResponse{}
s.DoJSON("POST", "/api/osquery/distributed/read", req, http.StatusOK, &dqResp)
require.Contains(t, dqResp.Queries, "fleet_detail_query_mdm_config_profiles_windows")
require.NotEmpty(t, dqResp.Queries, "fleet_detail_query_mdm_config_profiles_windows")
}

View file

@ -654,7 +654,19 @@ func (svc *Service) GetDistributedQueries(ctx context.Context) (queries map[stri
//
// Thus, we set the alwaysTrueQuery for all queries, except for those where we set
// an explicit discovery query (e.g. orbit_info, google_chrome_profiles).
for name := range queries {
for name, query := range queries {
// there's a bug somewhere (Fleet, osquery or both?)
// that causes hosts to check-in in a loop if you send
// an empty query string.
//
// we previously fixed this for detail query overrides (see
// #14286, #14296) but I'm also adding this here as a safeguard
// for issues like #15524
if query == "" {
delete(queries, name)
delete(discovery, name)
continue
}
discoveryQuery := discovery[name]
if discoveryQuery == "" {
discoveryQuery = alwaysTrueQuery

View file

@ -1164,6 +1164,46 @@ func TestGetDistributedQueriesMissingHost(t *testing.T) {
assert.Contains(t, err.Error(), "missing host")
}
func TestGetDistributedQueriesEmptyQuery(t *testing.T) {
mockClock := clock.NewMockClock()
ds := new(mock.Store)
lq := live_query_mock.New(t)
svc, ctx := newTestServiceWithClock(t, ds, nil, lq, mockClock)
host := &fleet.Host{
Platform: "darwin",
}
ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
return map[string]string{"empty_label_query": ""}, nil
}
ds.HostLiteFunc = func(ctx context.Context, id uint) (*fleet.Host, error) {
return host, nil
}
ds.UpdateHostFunc = func(ctx context.Context, gotHost *fleet.Host) error {
return nil
}
ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
return &fleet.AppConfig{Features: fleet.Features{EnableHostUsers: true}}, nil
}
ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
return map[string]string{"empty_policy_query": ""}, nil
}
lq.On("QueriesForHost", uint(0)).Return(map[string]string{"empty_live_query": ""}, nil)
ctx = hostctx.NewContext(ctx, host)
queries, discovery, _, err := svc.GetDistributedQueries(ctx)
require.NoError(t, err)
require.NotEmpty(t, queries)
for n, q := range queries {
require.NotEmpty(t, q, n)
}
for n, q := range discovery {
require.NotEmpty(t, q, n)
}
}
func TestLabelQueries(t *testing.T) {
mockClock := clock.NewMockClock()
ds := new(mock.Store)