From bf6e506c5086bcf768df6f1067283b152579886c Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Thu, 11 Jul 2024 11:40:48 -0300 Subject: [PATCH] bypass not found mdm commands (#20369) for #20367 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/20367-command-uuid | 1 + server/mdm/nanomdm/service/nanomdm/service.go | 16 +++++++++++++++- server/mdm/nanomdm/storage/mysql/queue.go | 17 +++++++++++++++++ server/service/integration_mdm_test.go | 9 +++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 changes/20367-command-uuid diff --git a/changes/20367-command-uuid b/changes/20367-command-uuid new file mode 100644 index 0000000000..cd1d7f06a2 --- /dev/null +++ b/changes/20367-command-uuid @@ -0,0 +1 @@ +* Avoid returning a 500 status code if a host sends a command response with an invalid command uuid. diff --git a/server/mdm/nanomdm/service/nanomdm/service.go b/server/mdm/nanomdm/service/nanomdm/service.go index 8f206673f9..85f943b536 100644 --- a/server/mdm/nanomdm/service/nanomdm/service.go +++ b/server/mdm/nanomdm/service/nanomdm/service.go @@ -232,7 +232,21 @@ func (s *Service) CommandAndReportResults(r *mdm.Request, results *mdm.CommandRe logger.Info(logs...) err := s.store.StoreCommandReport(r, results) if err != nil { - return nil, fmt.Errorf("storing command report: %w", err) + // allow not found commands, this is an edge case only + // valid for migrations, other response codes confuse the + // mdmclient, and this gives us the opportunity to answer + // with more commands + if !service.IsNotFound(err) { + return nil, fmt.Errorf("storing command report: %w", err) + } + + logger.Info( + "msg", "host reported status with invalid command uuid", + "command_uuid", results.CommandUUID, + "request_type", results.RequestType, + "status", results.Status, + "error_chain", results.ErrorChain, + ) } cmd, err := s.store.RetrieveNextCommand(r, results.Status == "NotNow") if err != nil { diff --git a/server/mdm/nanomdm/storage/mysql/queue.go b/server/mdm/nanomdm/storage/mysql/queue.go index a04dee76b7..a67262e4f8 100644 --- a/server/mdm/nanomdm/storage/mysql/queue.go +++ b/server/mdm/nanomdm/storage/mysql/queue.go @@ -119,6 +119,23 @@ func (m *MySQLStorage) StoreCommandReport(r *mdm.Request, result *mdm.CommandRes if result.Status == "Idle" { return nil } + + // ensure there's a matching command + matchingRow := m.db.QueryRowContext( + r.Context, + `SELECT 1 FROM nano_commands WHERE command_uuid = ?`, + result.CommandUUID, + ) + var matchingCount int + if err := matchingRow.Scan(&matchingCount); err != nil { + return err + } + // this should be already handed by the error value in Scan above, but + // just to be safe + if matchingCount == 0 { + return sql.ErrNoRows + } + if m.rm && result.Status != "NotNow" { return m.deleteCommandTx(r, result) } diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 90a45e1837..fc014d2525 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -9250,6 +9250,15 @@ func (s *integrationMDMTestSuite) TestConnectedToFleetWithoutCheckout() { require.False(t, *hostResp.Host.MDM.ConnectedToFleet) } +func (s *integrationMDMTestSuite) TestInvalidCommandUUID() { + t := s.T() + _, device := createHostThenEnrollMDM(s.ds, s.server.URL, t) + s.runWorker() + cmd, err := device.Acknowledge("foo") + require.NoError(t, err) + require.NotNil(t, cmd) +} + func (s *integrationMDMTestSuite) TestEnrollAfterDEPSyncIOSIPadOS() { t := s.T() ctx := context.Background()