From f3df2394e65db6fa59181521fcd3e85063094204 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Mon, 29 Jan 2024 18:38:10 -0600 Subject: [PATCH] When writing to logging destination fails, fleet server now issues a 4xx error instead of 500. (#16420) --- ...017-convert-logging-destination-err-to-4xx | 1 + server/service/osquery.go | 17 +++++- server/service/osquery_test.go | 57 +++++++++++++++++++ server/service/transport_error.go | 2 + 4 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 changes/5017-convert-logging-destination-err-to-4xx diff --git a/changes/5017-convert-logging-destination-err-to-4xx b/changes/5017-convert-logging-destination-err-to-4xx new file mode 100644 index 0000000000..ea19d67319 --- /dev/null +++ b/changes/5017-convert-logging-destination-err-to-4xx @@ -0,0 +1 @@ +When writing to logging destination fails, fleet server now issues a 4xx error instead of 500. diff --git a/server/service/osquery.go b/server/service/osquery.go index 9519e49995..72e080490f 100644 --- a/server/service/osquery.go +++ b/server/service/osquery.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "regexp" "strconv" "strings" @@ -29,7 +30,7 @@ import ( type osqueryError struct { message string nodeInvalid bool - + statusCode int fleet.ErrorWithUUID } @@ -46,6 +47,10 @@ func (e *osqueryError) NodeInvalid() bool { return e.nodeInvalid } +func (e *osqueryError) Status() int { + return e.statusCode +} + func newOsqueryErrorWithInvalidNode(msg string) *osqueryError { return &osqueryError{ message: msg, @@ -1539,7 +1544,10 @@ func (svc *Service) SubmitStatusLogs(ctx context.Context, logs []json.RawMessage svc.authz.SkipAuthorization(ctx) if err := svc.osqueryLogWriter.Status.Write(ctx, logs); err != nil { - return newOsqueryError("error writing status logs: " + err.Error()) + osqueryErr := newOsqueryError("error writing status logs: " + err.Error()) + // Attempting to write a large amount of data is the most likely explanation for this error. + osqueryErr.statusCode = http.StatusRequestEntityTooLarge + return osqueryErr } return nil } @@ -1616,11 +1624,14 @@ func (svc *Service) SubmitResultLogs(ctx context.Context, logs []json.RawMessage } if err := svc.osqueryLogWriter.Result.Write(ctx, filteredLogs); err != nil { - return newOsqueryError( + osqueryErr := newOsqueryError( "error writing result logs " + "(if the logging destination is down, you can reduce frequency/size of osquery logs by " + "increasing logger_tls_period and decreasing logger_tls_max_lines): " + err.Error(), ) + // Attempting to write a large amount of data is the most likely explanation for this error. + osqueryErr.statusCode = http.StatusRequestEntityTooLarge + return osqueryErr } return nil } diff --git a/server/service/osquery_test.go b/server/service/osquery_test.go index faacefd796..e4e579212a 100644 --- a/server/service/osquery_test.go +++ b/server/service/osquery_test.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "net/http" "os" "reflect" "sort" @@ -840,6 +841,62 @@ func TestSubmitResultLogsToQueryResultsDoesNotCountNullDataRows(t *testing.T) { assert.True(t, ds.OverwriteQueryResultRowsFuncInvoked) } +type failingLogger struct { +} + +func (n *failingLogger) Write(context.Context, []json.RawMessage) error { + return errors.New("some error") +} + +func TestSubmitResultLogsFail(t *testing.T) { + ds := new(mock.Store) + svc, ctx := newTestService(t, ds, nil, nil) + + host := fleet.Host{ + ID: 999, + } + ctx = hostctx.NewContext(ctx, &host) + + // Hack to get at the service internals and modify the writer + serv := ((svc.(validationMiddleware)).Service).(*Service) + + testLogger := &failingLogger{} + serv.osqueryLogWriter = &OsqueryLogger{Result: testLogger} + + logs := []string{ + `{"name":"pack/Global/system_info","hostIdentifier":"some_uuid","calendarTime":"Fri Sep 30 17:55:15 2016 UTC","unixTime":1475258115,"decorations":{"host_uuid":"some_uuid","username":"zwass"},"columns":{"cpu_brand":"Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz","hostname":"hostimus","physical_memory":"17179869184"},"action":"added"}`, + } + + logJSON := fmt.Sprintf("[%s]", strings.Join(logs, ",")) + var results []json.RawMessage + err := json.Unmarshal([]byte(logJSON), &results) + require.NoError(t, err) + + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + ds.QueryByNameFunc = func(ctx context.Context, teamID *uint, name string) (*fleet.Query, error) { + return &fleet.Query{ + ID: 1, + DiscardData: false, + AutomationsEnabled: true, + Name: name, + }, nil + } + ds.ResultCountForQueryFunc = func(ctx context.Context, queryID uint) (int, error) { + return 0, nil + } + ds.OverwriteQueryResultRowsFunc = func(ctx context.Context, rows []*fleet.ScheduledQueryResultRow) error { + return nil + } + + // Expect an error when unable to write to logging destination. + err = svc.SubmitResultLogs(ctx, results) + require.Error(t, err) + assert.Equal(t, http.StatusRequestEntityTooLarge, err.(*osqueryError).Status()) + +} + func TestGetQueryNameAndTeamIDFromResult(t *testing.T) { tests := []struct { input string diff --git a/server/service/transport_error.go b/server/service/transport_error.go index 026aa8187c..4de130f6c9 100644 --- a/server/service/transport_error.go +++ b/server/service/transport_error.go @@ -132,6 +132,8 @@ func encodeError(ctx context.Context, err error, w http.ResponseWriter) { if e.NodeInvalid() { w.WriteHeader(http.StatusUnauthorized) errMap["node_invalid"] = true + } else if e.Status() != 0 { + w.WriteHeader(e.Status()) } else { // TODO: osqueryError is not always the result of an internal error on // our side, it is also used to represent a client error (invalid data,