From 4784eb2322b4fa9647fa5380f671483647204bec Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Tue, 24 Apr 2018 12:40:11 -0700 Subject: [PATCH] Fix distributed query status parsing for osquery 3 (#1742) A change from strings to integers broke the existing code. We now support either for current and backwards compatibility. --- server/kolide/osquery.go | 11 ++++++++++- server/launcher/launcher.go | 5 ++--- server/launcher/launcher_test.go | 4 ++-- server/mock/service_osquery.go | 4 ++-- server/service/endpoint_osquery.go | 2 +- server/service/logging_osquery.go | 2 +- server/service/service_osquery.go | 4 ++-- server/service/service_osquery_test.go | 16 ++++++++-------- server/service/transport_osquery.go | 24 ++++++++++++++++++++++-- server/service/transport_osquery_test.go | 4 ++-- 10 files changed, 52 insertions(+), 24 deletions(-) diff --git a/server/kolide/osquery.go b/server/kolide/osquery.go index 16ef4f3e4e..4b11119754 100644 --- a/server/kolide/osquery.go +++ b/server/kolide/osquery.go @@ -17,7 +17,7 @@ type OsqueryService interface { // for) should be returned. Returning 0 for this will not activate the // feature. GetDistributedQueries(ctx context.Context) (queries map[string]string, accelerate uint, err error) - SubmitDistributedQueryResults(ctx context.Context, results OsqueryDistributedQueryResults, statuses map[string]string) (err error) + SubmitDistributedQueryResults(ctx context.Context, results OsqueryDistributedQueryResults, statuses map[string]OsqueryStatus) (err error) SubmitStatusLogs(ctx context.Context, logs []json.RawMessage) (err error) SubmitResultLogs(ctx context.Context, logs []json.RawMessage) (err error) } @@ -26,6 +26,15 @@ type OsqueryService interface { // osquery distributed query. type OsqueryDistributedQueryResults map[string][]map[string]string +// OsqueryStatus represents osquery status codes (0 = success, nonzero = +// failure) +type OsqueryStatus int + +const ( + // StatusOK is the success code returned by osquery + StatusOK OsqueryStatus = 0 +) + // QueryContent is the format of a query stanza in an osquery configuration. type QueryContent struct { Query string `json:"query"` diff --git a/server/launcher/launcher.go b/server/launcher/launcher.go index ca59e78d80..be8c3e9447 100644 --- a/server/launcher/launcher.go +++ b/server/launcher/launcher.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "strconv" "github.com/go-kit/kit/log" "github.com/kolide/osquery-go/plugin/distributed" @@ -114,10 +113,10 @@ func (svc *launcherWrapper) PublishResults(ctx context.Context, nodeKey string, } osqueryResults := make(kolide.OsqueryDistributedQueryResults, len(results)) - statuses := make(map[string]string, len(results)) + statuses := make(map[string]kolide.OsqueryStatus, len(results)) for _, result := range results { - statuses[result.QueryName] = strconv.Itoa(result.Status) + statuses[result.QueryName] = kolide.OsqueryStatus(result.Status) osqueryResults[result.QueryName] = result.Rows } diff --git a/server/launcher/launcher_test.go b/server/launcher/launcher_test.go index b84dfb8405..32dc6b5b24 100644 --- a/server/launcher/launcher_test.go +++ b/server/launcher/launcher_test.go @@ -66,7 +66,7 @@ func TestLauncherPublishResults(t *testing.T) { tls.SubmitDistributedQueryResultsFunc = func( ctx context.Context, results kolide.OsqueryDistributedQueryResults, - statuses map[string]string) (err error) { + statuses map[string]kolide.OsqueryStatus) (err error) { assert.Equal(t, results["query"][0], result) return nil } @@ -141,7 +141,7 @@ func newTLSService(t *testing.T) *mock.TLSService { SubmitDistributedQueryResultsFunc: func( ctx context.Context, results kolide.OsqueryDistributedQueryResults, - statuses map[string]string, + statuses map[string]kolide.OsqueryStatus, ) (err error) { return }, diff --git a/server/mock/service_osquery.go b/server/mock/service_osquery.go index cb97a46bcb..f0458be0ef 100644 --- a/server/mock/service_osquery.go +++ b/server/mock/service_osquery.go @@ -19,7 +19,7 @@ type GetClientConfigFunc func(ctx context.Context) (config *kolide.OsqueryConfig type GetDistributedQueriesFunc func(ctx context.Context) (queries map[string]string, accelerate uint, err error) -type SubmitDistributedQueryResultsFunc func(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]string) (err error) +type SubmitDistributedQueryResultsFunc func(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]kolide.OsqueryStatus) (err error) type SubmitStatusLogsFunc func(ctx context.Context, logs []json.RawMessage) (err error) @@ -68,7 +68,7 @@ func (s *TLSService) GetDistributedQueries(ctx context.Context) (queries map[str return s.GetDistributedQueriesFunc(ctx) } -func (s *TLSService) SubmitDistributedQueryResults(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]string) (err error) { +func (s *TLSService) SubmitDistributedQueryResults(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]kolide.OsqueryStatus) (err error) { s.SubmitDistributedQueryResultsFuncInvoked = true return s.SubmitDistributedQueryResultsFunc(ctx, results, statuses) } diff --git a/server/service/endpoint_osquery.go b/server/service/endpoint_osquery.go index c3c75e7622..ca0320b8f2 100644 --- a/server/service/endpoint_osquery.go +++ b/server/service/endpoint_osquery.go @@ -93,7 +93,7 @@ func makeGetDistributedQueriesEndpoint(svc kolide.Service) endpoint.Endpoint { type submitDistributedQueryResultsRequest struct { NodeKey string `json:"node_key"` Results kolide.OsqueryDistributedQueryResults `json:"queries"` - Statuses map[string]string `json:"statuses"` + Statuses map[string]kolide.OsqueryStatus `json:"statuses"` } type submitDistributedQueryResultsResponse struct { diff --git a/server/service/logging_osquery.go b/server/service/logging_osquery.go index 089316774b..023b63bc01 100644 --- a/server/service/logging_osquery.go +++ b/server/service/logging_osquery.go @@ -87,7 +87,7 @@ func (mw loggingMiddleware) GetDistributedQueries(ctx context.Context) (map[stri return queries, accelerate, err } -func (mw loggingMiddleware) SubmitDistributedQueryResults(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]string) error { +func (mw loggingMiddleware) SubmitDistributedQueryResults(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]kolide.OsqueryStatus) error { var ( err error ) diff --git a/server/service/service_osquery.go b/server/service/service_osquery.go index f117d2796c..8ec8bd02d8 100644 --- a/server/service/service_osquery.go +++ b/server/service/service_osquery.go @@ -657,7 +657,7 @@ func (svc service) ingestDistributedQuery(host kolide.Host, name string, rows [] return nil } -func (svc service) SubmitDistributedQueryResults(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]string) error { +func (svc service) SubmitDistributedQueryResults(ctx context.Context, results kolide.OsqueryDistributedQueryResults, statuses map[string]kolide.OsqueryStatus) error { host, ok := hostctx.FromContext(ctx) if !ok { @@ -678,7 +678,7 @@ func (svc service) SubmitDistributedQueryResults(ctx context.Context, results ko // osquery docs say any nonzero (string) value for // status indicates a query error status, ok := statuses[query] - failed := ok && status != "0" + failed := (ok && status != kolide.StatusOK) err = svc.ingestDistributedQuery(host, query, rows, failed) default: err = osqueryError{message: "unknown query prefix: " + query} diff --git a/server/service/service_osquery_test.go b/server/service/service_osquery_test.go index 8570f0bfc1..635ec266be 100644 --- a/server/service/service_osquery_test.go +++ b/server/service/service_osquery_test.go @@ -293,7 +293,7 @@ func TestLabelQueries(t *testing.T) { map[string][]map[string]string{ hostLabelQueryPrefix + "1": {{"col1": "val1"}}, }, - map[string]string{}, + map[string]kolide.OsqueryStatus{}, ) assert.Nil(t, err) @@ -330,7 +330,7 @@ func TestLabelQueries(t *testing.T) { hostLabelQueryPrefix + "2": {{"col1": "val1"}}, hostLabelQueryPrefix + "3": {}, }, - map[string]string{}, + map[string]kolide.OsqueryStatus{}, ) assert.Nil(t, err) @@ -551,7 +551,7 @@ func TestDetailQueriesWithEmptyStrings(t *testing.T) { require.Nil(t, err) // Verify that results are ingested properly - svc.SubmitDistributedQueryResults(ctx, results, map[string]string{}) + svc.SubmitDistributedQueryResults(ctx, results, map[string]kolide.OsqueryStatus{}) // Make sure the result saved to the datastore host, err = ds.AuthenticateHost(nodeKey) @@ -591,7 +591,7 @@ func TestDetailQueriesWithEmptyStrings(t *testing.T) { // Advance clock and queries should exist again mockClock.AddTime(1*time.Hour + 1*time.Minute) - err = svc.SubmitDistributedQueryResults(ctx, kolide.OsqueryDistributedQueryResults{}, map[string]string{}) + err = svc.SubmitDistributedQueryResults(ctx, kolide.OsqueryDistributedQueryResults{}, map[string]kolide.OsqueryStatus{}) require.Nil(t, err) host, err = ds.AuthenticateHost(nodeKey) require.Nil(t, err) @@ -719,7 +719,7 @@ func TestDetailQueries(t *testing.T) { require.Nil(t, err) // Verify that results are ingested properly - svc.SubmitDistributedQueryResults(ctx, results, map[string]string{}) + svc.SubmitDistributedQueryResults(ctx, results, map[string]kolide.OsqueryStatus{}) // Make sure the result saved to the datastore host, err = ds.AuthenticateHost(nodeKey) @@ -759,7 +759,7 @@ func TestDetailQueries(t *testing.T) { // Advance clock and queries should exist again mockClock.AddTime(1*time.Hour + 1*time.Minute) - err = svc.SubmitDistributedQueryResults(ctx, kolide.OsqueryDistributedQueryResults{}, map[string]string{}) + err = svc.SubmitDistributedQueryResults(ctx, kolide.OsqueryDistributedQueryResults{}, map[string]kolide.OsqueryStatus{}) require.Nil(t, err) host, err = ds.AuthenticateHost(nodeKey) require.Nil(t, err) @@ -887,7 +887,7 @@ func TestDistributedQueries(t *testing.T) { // this test. time.Sleep(10 * time.Millisecond) - err = svc.SubmitDistributedQueryResults(ctx, results, map[string]string{}) + err = svc.SubmitDistributedQueryResults(ctx, results, map[string]kolide.OsqueryStatus{}) require.Nil(t, err) // Now the distributed query should be completed and not returned @@ -951,7 +951,7 @@ func TestOrphanedQueryCampaign(t *testing.T) { // Submit results ctx = hostctx.NewContext(context.Background(), *host) - err = svc.SubmitDistributedQueryResults(ctx, results, map[string]string{}) + err = svc.SubmitDistributedQueryResults(ctx, results, map[string]kolide.OsqueryStatus{}) require.Nil(t, err) // The campaign should be set to completed because it is orphaned diff --git a/server/service/transport_osquery.go b/server/service/transport_osquery.go index b17216a720..c6fdba155f 100644 --- a/server/service/transport_osquery.go +++ b/server/service/transport_osquery.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "net/http" + "strconv" "github.com/kolide/fleet/server/kolide" "github.com/pkg/errors" @@ -54,7 +55,7 @@ func decodeSubmitDistributedQueryResultsRequest(ctx context.Context, r *http.Req type distributedQueryResultsShim struct { NodeKey string `json:"node_key"` Results map[string]json.RawMessage `json:"queries"` - Statuses map[string]string `json:"statuses"` + Statuses map[string]interface{} `json:"statuses"` } var shim distributedQueryResultsShim @@ -73,10 +74,29 @@ func decodeSubmitDistributedQueryResultsRequest(ctx context.Context, r *http.Req results[query] = queryResults } + // Statuses were represented by strings in osquery < 3.0 and now + // integers in osquery > 3.0. Massage to string for compatibility with + // the service definition. + statuses := map[string]kolide.OsqueryStatus{} + for query, status := range shim.Statuses { + switch s := status.(type) { + case string: + sint, err := strconv.Atoi(s) + if err != nil { + return nil, errors.Wrap(err, "parse status to int") + } + statuses[query] = kolide.OsqueryStatus(sint) + case float64: + statuses[query] = kolide.OsqueryStatus(s) + default: + return nil, errors.Errorf("query status should be string or number, got %T", s) + } + } + req := submitDistributedQueryResultsRequest{ NodeKey: shim.NodeKey, Results: results, - Statuses: shim.Statuses, + Statuses: statuses, } return req, nil diff --git a/server/service/transport_osquery_test.go b/server/service/transport_osquery_test.go index 594d5a7be8..cae0ac0fd2 100644 --- a/server/service/transport_osquery_test.go +++ b/server/service/transport_osquery_test.go @@ -97,7 +97,7 @@ func TestDecodeSubmitDistributedQueryResultsRequest(t *testing.T) { }, "id3": {}, }, params.Results) - assert.Equal(t, map[string]string{"id1": "0", "id3": "1"}, params.Statuses) + assert.Equal(t, map[string]kolide.OsqueryStatus{"id1": 0, "id3": 1}, params.Statuses) }).Methods("POST") // Note we explicitly test the case that requires using the shim @@ -115,7 +115,7 @@ func TestDecodeSubmitDistributedQueryResultsRequest(t *testing.T) { ], "id3": "" }, - "statuses": {"id1": "0", "id3": "1"} + "statuses": {"id1": 0, "id3": "1"} }`)) router.ServeHTTP(