Skip to content

Commit

Permalink
fix device SCEP renewal cron if a host is deleted (#19158)
Browse files Browse the repository at this point in the history
for #19149
  • Loading branch information
roperzh authored May 20, 2024
1 parent 27a55f4 commit 7559944
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 7 deletions.
1 change: 1 addition & 0 deletions changes/19149-fix-cron-scep
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Fixed an issue with SCEP renewals that could prevent commands to renew to be enqueued.
6 changes: 4 additions & 2 deletions server/datastore/mysql/mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ func (ds *Datastore) GetHostCertAssociationsToExpire(ctx context.Context, expiry
COALESCE(MAX(hm.fleet_enroll_ref), '') as enroll_reference
FROM
nano_cert_auth_associations ncaa
LEFT JOIN hosts h ON h.uuid = ncaa.id
JOIN hosts h ON h.uuid = ncaa.id
LEFT JOIN host_mdm hm ON hm.host_id = h.id
WHERE
cert_not_valid_after BETWEEN '0000-00-00' AND DATE_ADD(CURDATE(), INTERVAL ? DAY)
Expand Down Expand Up @@ -1112,7 +1112,9 @@ func (ds *Datastore) CleanSCEPRenewRefs(ctx context.Context, hostUUID string) er
stmt := `
UPDATE nano_cert_auth_associations
SET renew_command_uuid = NULL
WHERE id = ?`
WHERE id = ?
ORDER BY created_at desc
LIMIT 1`

res, err := ds.writer(ctx).ExecContext(ctx, stmt, hostUUID)
if err != nil {
Expand Down
23 changes: 23 additions & 0 deletions server/datastore/mysql/mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6199,6 +6199,29 @@ func testSCEPRenewalHelpers(t *testing.T, ds *Datastore) {
require.Equal(t, h3.UUID, assocs[2].HostUUID)
require.Equal(t, h4.UUID, assocs[3].HostUUID)

// add a new host with a very old expiriy so it shows first, verify
// that it's present before deleting it.
h5 := setHost(time.Now().AddDate(-2, -1, 0))
assocs, err = ds.GetHostCertAssociationsToExpire(ctx, 1000, 100)
require.NoError(t, err)
require.Len(t, assocs, 5)
require.Equal(t, h5.UUID, assocs[0].HostUUID)
require.Equal(t, h1.UUID, assocs[1].HostUUID)
require.Equal(t, h2.UUID, assocs[2].HostUUID)
require.Equal(t, h3.UUID, assocs[3].HostUUID)
require.Equal(t, h4.UUID, assocs[4].HostUUID)

// delete the host and verify that things work as expected
// see https://github.com/fleetdm/fleet/issues/19149
require.NoError(t, ds.DeleteHost(ctx, h5.ID))
assocs, err = ds.GetHostCertAssociationsToExpire(ctx, 1000, 100)
require.NoError(t, err)
require.Len(t, assocs, 4)
require.Equal(t, h1.UUID, assocs[0].HostUUID)
require.Equal(t, h2.UUID, assocs[1].HostUUID)
require.Equal(t, h3.UUID, assocs[2].HostUUID)
require.Equal(t, h4.UUID, assocs[3].HostUUID)

checkSCEPRenew := func(assoc fleet.SCEPIdentityAssociation, want *string) {
var got *string
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
Expand Down
27 changes: 22 additions & 5 deletions server/service/integration_mdm_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ func (s *integrationMDMTestSuite) TestLifecycleSCEPCertExpiration() {
require.NoError(t, err)
manualEnrolledDevice := mdmtest.NewTestMDMClientAppleDesktopManual(s.server.URL, desktopToken)
manualEnrolledDevice.UUID = manualHost.UUID
manualEnrolledDevice.SerialNumber = manualHost.HardwareSerial
err = manualEnrolledDevice.Enroll()
require.NoError(t, err)

Expand Down Expand Up @@ -691,15 +692,19 @@ func (s *integrationMDMTestSuite) TestLifecycleSCEPCertExpiration() {
require.NoError(t, err)
require.Nil(t, cmd)

// expire all the certs we just created
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx, `
expireCerts := func() {
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx, `
UPDATE nano_cert_auth_associations
SET cert_not_valid_after = DATE_SUB(CURDATE(), INTERVAL 1 YEAR)
WHERE id IN (?, ?, ?)
`, manualHost.UUID, automaticHost.UUID, automaticHostWithRef.UUID)
return err
})
return err
})
}

// expire all the certs we just created
expireCerts()

// generate a new config here so we can manipulate the certs.
err = RenewSCEPCertificates(ctx, logger, s.ds, &fleetCfg, s.mdmCommander)
Expand Down Expand Up @@ -759,4 +764,16 @@ func (s *integrationMDMTestSuite) TestLifecycleSCEPCertExpiration() {
cmd, err = automaticEnrolledDeviceWithRef.Idle()
require.NoError(t, err)
require.Nil(t, cmd)

// handle the case of a host being deleted, see https://github.com/fleetdm/fleet/issues/19149
expireCerts()
req := deleteHostsRequest{
IDs: []uint{manualHost.ID},
}
resp := deleteHostsResponse{}
s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusOK, &resp)
err = RenewSCEPCertificates(ctx, logger, s.ds, &fleetCfg, s.mdmCommander)
require.NoError(t, err)
checkRenewCertCommand(automaticEnrolledDevice, "")
checkRenewCertCommand(automaticEnrolledDeviceWithRef, "foo")
}

0 comments on commit 7559944

Please sign in to comment.