From 5b5dca32e95b0aa36e65da4393de7df5a80c4805 Mon Sep 17 00:00:00 2001 From: Tomas Touceda Date: Mon, 9 Aug 2021 14:38:06 -0300 Subject: [PATCH] Add more checks to observers running queries (#1589) * Add more checks to observers running queries * Fix test * Use proper authorize policy instead of doing it by hand --- changes/issue-1515-observer-query-issues | 1 + server/service/service_campaigns.go | 21 +++++-- server/service/service_osquery_test.go | 76 +++++++++++++++++++++++- 3 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 changes/issue-1515-observer-query-issues diff --git a/changes/issue-1515-observer-query-issues b/changes/issue-1515-observer-query-issues new file mode 100644 index 0000000000..0246e35e86 --- /dev/null +++ b/changes/issue-1515-observer-query-issues @@ -0,0 +1 @@ +* Observers can run queries that admins create, but not create their own. Improves errors returned and checks. diff --git a/server/service/service_campaigns.go b/server/service/service_campaigns.go index b3da2c7c87..94ef5b8570 100644 --- a/server/service/service_campaigns.go +++ b/server/service/service_campaigns.go @@ -53,27 +53,36 @@ func (svc Service) NewDistributedQueryCampaign(ctx context.Context, queryString } var query *fleet.Query + var err error if queryID != nil { - query, err := svc.ds.Query(*queryID) + query, err = svc.ds.Query(*queryID) if err != nil { return nil, err } queryString = query.Query } else { + if err := svc.authz.Authorize(ctx, &fleet.Query{}, fleet.ActionWrite); err != nil { + return nil, err + } query = &fleet.Query{ Name: fmt.Sprintf("distributed_%s_%d", vc.Email(), time.Now().Unix()), Query: queryString, Saved: false, AuthorID: ptr.Uint(vc.UserID()), } + err := query.ValidateSQL() + if err != nil { + return nil, err + } + query, err = svc.ds.NewQuery(query) + if err != nil { + return nil, errors.Wrap(err, "new query") + } } - if err := query.ValidateSQL(); err != nil { + + if err := svc.authz.Authorize(ctx, query, fleet.ActionRun); err != nil { return nil, err } - query, err := svc.ds.NewQuery(query) - if err != nil { - return nil, errors.Wrap(err, "new query") - } filter := fleet.TeamFilter{User: vc.User, IncludeObserver: query.ObserverCanRun} diff --git a/server/service/service_osquery_test.go b/server/service/service_osquery_test.go index 8f116b22c5..d106cc5512 100644 --- a/server/service/service_osquery_test.go +++ b/server/service/service_osquery_test.go @@ -1190,7 +1190,8 @@ func TestNewDistributedQueryCampaign(t *testing.T) { lq.On("RunQuery", "21", "select year, month, day, hour, minutes, seconds from time", []uint{1, 3, 5}).Return(nil) viewerCtx := viewer.NewContext(context.Background(), viewer.Viewer{ User: &fleet.User{ - ID: 0, + ID: 0, + GlobalRole: ptr.String(fleet.RoleAdmin), }, }) q := "select year, month, day, hour, minutes, seconds from time" @@ -1895,3 +1896,76 @@ func TestDistributedQueriesReloadsHostIfDetailsAreIn(t *testing.T) { assert.Nil(t, err) assert.True(t, ds.HostFuncInvoked) } + +func TestObserversCanOnlyRunDistributedCampaigns(t *testing.T) { + ds := new(mock.Store) + rs := &mock.QueryResultStore{ + HealthCheckFunc: func() error { + return nil + }, + } + lq := &live_query.MockLiveQuery{} + mockClock := clock.NewMockClock() + svc := newTestServiceWithClock(ds, rs, lq, mockClock) + + ds.AppConfigFunc = func() (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + + ds.NewDistributedQueryCampaignFunc = func(camp *fleet.DistributedQueryCampaign) (*fleet.DistributedQueryCampaign, error) { + return camp, nil + } + ds.QueryFunc = func(id uint) (*fleet.Query, error) { + return &fleet.Query{ + ID: 42, + Name: "query", + Query: "select 1;", + ObserverCanRun: false, + }, nil + } + viewerCtx := viewer.NewContext(context.Background(), viewer.Viewer{ + User: &fleet.User{ID: 0, GlobalRole: ptr.String(fleet.RoleObserver)}}) + + q := "select year, month, day, hour, minutes, seconds from time" + ds.NewActivityFunc = func(user *fleet.User, activityType string, details *map[string]interface{}) error { + return nil + } + _, err := svc.NewDistributedQueryCampaign(viewerCtx, q, nil, fleet.HostTargets{HostIDs: []uint{2}, LabelIDs: []uint{1}}) + require.Error(t, err) + + _, err = svc.NewDistributedQueryCampaign(viewerCtx, "", ptr.Uint(42), fleet.HostTargets{HostIDs: []uint{2}, LabelIDs: []uint{1}}) + require.Error(t, err) + + ds.QueryFunc = func(id uint) (*fleet.Query, error) { + return &fleet.Query{ + ID: 42, + Name: "query", + Query: "select 1;", + ObserverCanRun: true, + }, nil + } + + ds.LabelQueriesForHostFunc = func(host *fleet.Host, cutoff time.Time) (map[string]string, error) { + return map[string]string{}, nil + } + ds.SaveHostFunc = func(host *fleet.Host) error { return nil } + ds.NewDistributedQueryCampaignFunc = func(camp *fleet.DistributedQueryCampaign) (*fleet.DistributedQueryCampaign, error) { + camp.ID = 21 + return camp, nil + } + ds.NewDistributedQueryCampaignTargetFunc = func(target *fleet.DistributedQueryCampaignTarget) (*fleet.DistributedQueryCampaignTarget, error) { + return target, nil + } + ds.CountHostsInTargetsFunc = func(filter fleet.TeamFilter, targets fleet.HostTargets, now time.Time) (fleet.TargetMetrics, error) { + return fleet.TargetMetrics{}, nil + } + ds.HostIDsInTargetsFunc = func(filter fleet.TeamFilter, targets fleet.HostTargets) ([]uint, error) { + return []uint{1, 3, 5}, nil + } + ds.NewActivityFunc = func(user *fleet.User, activityType string, details *map[string]interface{}) error { + return nil + } + lq.On("RunQuery", "21", "select 1;", []uint{1, 3, 5}).Return(nil) + _, err = svc.NewDistributedQueryCampaign(viewerCtx, "", ptr.Uint(42), fleet.HostTargets{HostIDs: []uint{2}, LabelIDs: []uint{1}}) + require.NoError(t, err) +}