From f55eb085f010a7fac31e67beeef5651c7ad8532e Mon Sep 17 00:00:00 2001 From: Dante Catalfamo <43040593+dantecatalfamo@users.noreply.github.com> Date: Fri, 27 Mar 2026 16:51:54 -0400 Subject: [PATCH] make sure to reset fleet challenge when resending android cert (#42550) **Related issue:** Resolves #41542 --- .../datastore/mysql/certificate_templates.go | 47 +++++-- .../mysql/certificate_templates_test.go | 79 ++++++++++++ .../mysql/host_certificate_templates.go | 5 +- ...tion_android_certificate_templates_test.go | 119 ++++++++++++++++-- 4 files changed, 229 insertions(+), 21 deletions(-) diff --git a/server/datastore/mysql/certificate_templates.go b/server/datastore/mysql/certificate_templates.go index 54dd213ed7..13e1ef5313 100644 --- a/server/datastore/mysql/certificate_templates.go +++ b/server/datastore/mysql/certificate_templates.go @@ -306,7 +306,8 @@ SELECT name, status, detail, - operation_type + operation_type, + certificate_template_id FROM host_certificate_templates WHERE host_uuid = ?` @@ -409,20 +410,48 @@ func (ds *Datastore) ResendHostCertificateTemplate(ctx context.Context, hostID u hosts h ON h.uuid = hct.host_uuid SET hct.uuid = UUID_TO_BIN(UUID(), true), + hct.fleet_challenge = NULL, + hct.not_valid_before = NULL, + hct.not_valid_after = NULL, + hct.serial = NULL, + hct.detail = NULL, hct.status = ? WHERE h.id = ? AND hct.certificate_template_id = ? - ` + ` - results, err := ds.writer(ctx).ExecContext(ctx, stmt, fleet.CertificateTemplatePending, hostID, templateID) - if err != nil { - return ctxerr.Wrap(ctx, err, "updating host certificate template uuid") - } + const deleteChallenge = ` + DELETE c FROM + challenges c + INNER JOIN + host_certificate_templates hct ON hct.fleet_challenge = c.challenge + INNER JOIN + hosts h ON h.uuid = hct.host_uuid + WHERE + h.id = ? AND + hct.certificate_template_id = ? + ` - affected, _ := results.RowsAffected() - if affected == 0 { - return ctxerr.Wrapf(ctx, notFound("HostCertificateTemplate"), "template %d does not exist for host %d", templateID, hostID) + if err := ds.withTx(ctx, func(tx sqlx.ExtContext) error { + _, err := tx.ExecContext(ctx, deleteChallenge, hostID, templateID) + if err != nil { + return ctxerr.Wrap(ctx, err, "deleting challenges associated with resent certificate template") + } + + results, err := tx.ExecContext(ctx, stmt, fleet.CertificateTemplatePending, hostID, templateID) + if err != nil { + return ctxerr.Wrap(ctx, err, "updating host certificate template uuid") + } + + affected, _ := results.RowsAffected() + if affected == 0 { + return ctxerr.Wrapf(ctx, notFound("HostCertificateTemplate"), "template %d does not exist for host %d", templateID, hostID) + } + + return nil + }); err != nil { + return ctxerr.Wrap(ctx, err, "resetting host certificate template for resend") } return nil diff --git a/server/datastore/mysql/certificate_templates_test.go b/server/datastore/mysql/certificate_templates_test.go index e458a55b4b..752e5433f6 100644 --- a/server/datastore/mysql/certificate_templates_test.go +++ b/server/datastore/mysql/certificate_templates_test.go @@ -10,6 +10,7 @@ import ( "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" + "github.com/jmoiron/sqlx" "github.com/stretchr/testify/require" ) @@ -1397,6 +1398,84 @@ func testResendHostCertificateTemplate(t *testing.T, ds *Datastore) { }) } + t.Run("clears validity fields and deletes challenge", func(t *testing.T) { + // Insert a delivered record + err = ds.BulkInsertHostCertificateTemplates(ctx, []fleet.HostCertificateTemplate{ + { + HostUUID: h1.UUID, + CertificateTemplateID: ct1.ID, + Status: fleet.CertificateTemplateDelivered, + OperationType: fleet.MDMOperationTypeInstall, + Name: "Template1", + }, + }) + require.NoError(t, err) + + // Create a fleet challenge via the on-demand mechanism + challenge, err := ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, h1.UUID, ct1.ID) + require.NoError(t, err) + require.NotEmpty(t, challenge) + + // Populate validity fields via UpsertCertificateStatus + certNotBefore := time.Now().UTC().Truncate(time.Second) + certNotAfter := certNotBefore.Add(365 * 24 * time.Hour) + certSerial := "AA:BB:CC:DD:EE" + certDetail := "enrollment succeeded" + err = ds.UpsertCertificateStatus(ctx, &fleet.CertificateStatusUpdate{ + HostUUID: h1.UUID, + CertificateTemplateID: ct1.ID, + Status: fleet.MDMDeliveryVerified, + NotValidBefore: &certNotBefore, + NotValidAfter: &certNotAfter, + Serial: &certSerial, + Detail: &certDetail, + OperationType: fleet.MDMOperationTypeInstall, + }) + require.NoError(t, err) + + // Verify fields are populated before resend + record, err := ds.GetHostCertificateTemplateRecord(ctx, h1.UUID, ct1.ID) + require.NoError(t, err) + require.NotNil(t, record.FleetChallenge) + require.NotNil(t, record.NotValidBefore) + require.NotNil(t, record.NotValidAfter) + require.NotNil(t, record.Serial) + require.NotNil(t, record.Detail) + originalUUID := record.UUID + + // Verify challenge exists in challenges table + var challengeCount int + err = sqlx.GetContext(ctx, ds.reader(ctx), &challengeCount, + `SELECT COUNT(*) FROM challenges WHERE challenge = ?`, challenge) + require.NoError(t, err) + require.Equal(t, 1, challengeCount) + + // Resend + err = ds.ResendHostCertificateTemplate(ctx, h1.ID, ct1.ID) + require.NoError(t, err) + + // Verify all fields are cleared + updated, err := ds.GetHostCertificateTemplateRecord(ctx, h1.UUID, ct1.ID) + require.NoError(t, err) + require.Equal(t, fleet.CertificateTemplatePending, updated.Status) + require.NotEqual(t, originalUUID, updated.UUID, "UUID should change after resend") + require.Nil(t, updated.FleetChallenge, "fleet_challenge should be cleared") + require.Nil(t, updated.NotValidBefore, "not_valid_before should be cleared") + require.Nil(t, updated.NotValidAfter, "not_valid_after should be cleared") + require.Nil(t, updated.Serial, "serial should be cleared") + require.Nil(t, updated.Detail, "detail should be cleared") + + // Verify the challenge row was deleted from the challenges table + err = sqlx.GetContext(ctx, ds.reader(ctx), &challengeCount, + `SELECT COUNT(*) FROM challenges WHERE challenge = ?`, challenge) + require.NoError(t, err) + require.Equal(t, 0, challengeCount, "challenge should be deleted from challenges table") + + // Clean up + err = ds.DeleteHostCertificateTemplate(ctx, h1.UUID, ct1.ID) + require.NoError(t, err) + }) + t.Run("returns error for non-existent host", func(t *testing.T) { err := ds.ResendHostCertificateTemplate(ctx, 99999, ct1.ID) require.Error(t, err) diff --git a/server/datastore/mysql/host_certificate_templates.go b/server/datastore/mysql/host_certificate_templates.go index f7c53fcbbc..c08dbc90f6 100644 --- a/server/datastore/mysql/host_certificate_templates.go +++ b/server/datastore/mysql/host_certificate_templates.go @@ -152,7 +152,10 @@ func (ds *Datastore) GetHostCertificateTemplateRecord(ctx context.Context, hostU detail, COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid, created_at, - updated_at + updated_at, + not_valid_before, + not_valid_after, + serial FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ? ` diff --git a/server/service/integration_android_certificate_templates_test.go b/server/service/integration_android_certificate_templates_test.go index a6624077ae..fad5a89b87 100644 --- a/server/service/integration_android_certificate_templates_test.go +++ b/server/service/integration_android_certificate_templates_test.go @@ -2,11 +2,14 @@ package service import ( "context" + "database/sql" "encoding/json" "errors" "fmt" + "io" "log/slog" "net/http" + "net/url" "strings" "testing" "time" @@ -14,8 +17,11 @@ import ( "github.com/fleetdm/fleet/v4/server/datastore/mysql" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mdm/android" + apple_mdm "github.com/fleetdm/fleet/v4/server/mdm/apple" + scepserver "github.com/fleetdm/fleet/v4/server/mdm/scep/server" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/service/contract" + scep_server "github.com/fleetdm/fleet/v4/server/service/integrationtest/scep_server" "github.com/fleetdm/fleet/v4/server/worker" "github.com/google/uuid" "github.com/jmoiron/sqlx" @@ -66,6 +72,8 @@ func (s *integrationMDMTestSuite) verifyCertificateStatusWithSubject( require.NotNil(t, profile, "Profile %s not found in host MDM profiles", certTemplateName) require.NotNil(t, profile.Status) require.Equal(t, string(expectedStatus), *profile.Status) + require.NotNil(t, profile.CertificateTemplateID, "certificate_template_id should not be nil for Android certificate profiles") + require.Equal(t, certificateTemplateID, *profile.CertificateTemplateID, "certificate_template_id should match") if expectedDetail != "" { require.Equal(t, expectedDetail, profile.Detail) } @@ -1367,12 +1375,17 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateAuthorizationForTeamUse // TestCertificateTemplateResend tests the resend endpoint for Android certificate templates: // 1. After a certificate reaches 'verified' status, calling resend resets it to 'pending' // 2. The UUID changes after resend (signals the device to re-fetch) -// 3. The reconcile cron picks up the pending template and re-delivers it +// 3. The fleet_challenge is cleared after resend and regenerated on next delivery +// 4. The reconcile cron picks up the pending template and re-delivers it +// 5. The SCEP proxy accepts the refreshed fleet challenge func (s *integrationMDMTestSuite) TestCertificateTemplateResend() { t := s.T() ctx := t.Context() enterpriseID := s.enableAndroidMDM(t) + // Start a test SCEP server so the SCEP proxy can forward requests + testSCEPServer := scep_server.StartTestSCEPServer(t) + // Create a test team teamName := t.Name() + "-team" var createTeamResp teamResponse @@ -1383,8 +1396,16 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateResend() { }, http.StatusOK, &createTeamResp) teamID := createTeamResp.Team.ID - // Create a test certificate authority - caID, _ := s.createTestCertificateAuthority(t, ctx) + // Create a certificate authority pointing to the real test SCEP server. + // Use a name without slashes since it appears in the SCEP proxy URL path. + ca, err := s.ds.NewCertificateAuthority(ctx, &fleet.CertificateAuthority{ + Type: string(fleet.CATypeCustomSCEPProxy), + Name: ptr.String(strings.ReplaceAll(t.Name(), "/", "-") + "-CA"), + URL: ptr.String(testSCEPServer.URL + "/scep"), + Challenge: ptr.String("test-challenge"), + }) + require.NoError(t, err) + caID := ca.ID // Create an enrolled Android host in the team host, orbitNodeKey := s.createEnrolledAndroidHost(t, ctx, enterpriseID, &teamID, "1") @@ -1412,24 +1433,48 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateResend() { s.verifyCertificateStatus(t, host, orbitNodeKey, certTemplateID, certTemplateName, caID, fleet.CertificateTemplateDelivered, "") - // Simulate device reporting certificate enrollment as verified + // Fetch via fleetd API to trigger on-demand fleet_challenge creation + // (challenge is created lazily on first fetch in delivered status) + resp := s.DoRawWithHeaders("GET", fmt.Sprintf("/api/fleetd/certificates/%d", certTemplateID), nil, http.StatusOK, map[string]string{ + "Authorization": fmt.Sprintf("Node key %s", orbitNodeKey), + }) + var preResendCertResp getDeviceCertificateTemplateResponse + err = json.NewDecoder(resp.Body).Decode(&preResendCertResp) + require.NoError(t, err) + _ = resp.Body.Close() + require.NotNil(t, preResendCertResp.Certificate.FleetChallenge, "fleet_challenge should be created on-demand") + originalFleetChallenge := *preResendCertResp.Certificate.FleetChallenge + + // Simulate device reporting certificate enrollment as verified, with validity info + certNotBefore := time.Now().UTC().Truncate(time.Second) + certNotAfter := certNotBefore.Add(365 * 24 * time.Hour) + certSerial := "AB:CD:EF:01:23:45" + certDetail := "enrollment succeeded" updateReq, err := json.Marshal(updateCertificateStatusRequest{ - Status: string(fleet.CertificateTemplateVerified), + Status: string(fleet.CertificateTemplateVerified), + NotValidBefore: &certNotBefore, + NotValidAfter: &certNotAfter, + Serial: &certSerial, + Detail: &certDetail, }) require.NoError(t, err) - resp := s.DoRawWithHeaders("PUT", fmt.Sprintf("/api/fleetd/certificates/%d/status", certTemplateID), updateReq, http.StatusOK, map[string]string{ + resp = s.DoRawWithHeaders("PUT", fmt.Sprintf("/api/fleetd/certificates/%d/status", certTemplateID), updateReq, http.StatusOK, map[string]string{ "Authorization": fmt.Sprintf("Node key %s", orbitNodeKey), }) _ = resp.Body.Close() - // Verify status is 'verified' + // Verify status is 'verified' and validity fields are populated s.verifyCertificateStatus(t, host, orbitNodeKey, certTemplateID, certTemplateName, caID, fleet.CertificateTemplateVerified, "") + verifiedRecord, err := s.ds.GetHostCertificateTemplateRecord(ctx, host.UUID, certTemplateID) + require.NoError(t, err) + require.NotNil(t, verifiedRecord.NotValidBefore, "not_valid_before should be set after verified") + require.NotNil(t, verifiedRecord.NotValidAfter, "not_valid_after should be set after verified") + require.NotNil(t, verifiedRecord.Serial, "serial should be set after verified") + require.NotNil(t, verifiedRecord.Detail, "detail should be set after verified") // Record UUID before resend - originalRecord, err := s.ds.GetHostCertificateTemplateRecord(ctx, host.UUID, certTemplateID) - require.NoError(t, err) - originalUUID := originalRecord.UUID + originalUUID := verifiedRecord.UUID // Call the resend endpoint s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/certificates/%d/resend", host.ID, certTemplateID), @@ -1447,11 +1492,16 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateResend() { ), 0) - // Verify status is reset to 'pending' and UUID changed + // Verify status is reset to 'pending', UUID changed, and all certificate fields cleared updatedRecord, err := s.ds.GetHostCertificateTemplateRecord(ctx, host.UUID, certTemplateID) require.NoError(t, err) require.Equal(t, fleet.CertificateTemplatePending, updatedRecord.Status) require.NotEqual(t, originalUUID, updatedRecord.UUID, "UUID should change after resend") + require.Nil(t, updatedRecord.FleetChallenge, "fleet_challenge should be cleared after resend") + require.Nil(t, updatedRecord.NotValidBefore, "not_valid_before should be cleared after resend") + require.Nil(t, updatedRecord.NotValidAfter, "not_valid_after should be cleared after resend") + require.Nil(t, updatedRecord.Serial, "serial should be cleared after resend") + require.Nil(t, updatedRecord.Detail, "detail should be cleared after resend") // Verify the host API reflects pending status s.verifyCertificateStatus(t, host, orbitNodeKey, certTemplateID, certTemplateName, caID, @@ -1464,6 +1514,53 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateResend() { s.verifyCertificateStatus(t, host, orbitNodeKey, certTemplateID, certTemplateName, caID, fleet.CertificateTemplateDelivered, "") + // Fetch via fleetd API again to trigger new fleet_challenge creation + resp = s.DoRawWithHeaders("GET", fmt.Sprintf("/api/fleetd/certificates/%d", certTemplateID), nil, http.StatusOK, map[string]string{ + "Authorization": fmt.Sprintf("Node key %s", orbitNodeKey), + }) + var postResendCertResp getDeviceCertificateTemplateResponse + err = json.NewDecoder(resp.Body).Decode(&postResendCertResp) + require.NoError(t, err) + _ = resp.Body.Close() + require.NotNil(t, postResendCertResp.Certificate.FleetChallenge, "fleet_challenge should be regenerated after resend and re-delivery") + require.NotEqual(t, originalFleetChallenge, *postResendCertResp.Certificate.FleetChallenge, "fleet_challenge should differ from the original after resend") + + // Use the SCEP proxy with the refreshed fleet_challenge to fetch CA capabilities. + // Android identifier format: {hostUUID},g{certificateTemplateID},{caName},{fleetChallenge} + newFleetChallenge := *postResendCertResp.Certificate.FleetChallenge + caName := *ca.Name + identifier := url.PathEscape(fmt.Sprintf("%s,g%d,%s,%s", host.UUID, certTemplateID, caName, newFleetChallenge)) + scepRes := s.DoRawWithHeaders("GET", apple_mdm.SCEPProxyPath+identifier, nil, http.StatusOK, nil, "operation", "GetCACaps") + body, err := io.ReadAll(scepRes.Body) + require.NoError(t, err) + _ = scepRes.Body.Close() + require.Equal(t, scepserver.DefaultCACaps, string(body)) + + // Verify the new fleet_challenge exists in the challenges table (GetCACaps doesn't consume it) + checkChallengeExists := func(challenge string, expectFound bool) { + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + var found string + err := sqlx.GetContext(ctx, q, &found, "SELECT challenge FROM challenges WHERE challenge = ?", challenge) + if errors.Is(err, sql.ErrNoRows) { + require.False(t, expectFound, "expected challenge to exist in DB but it was not found") + return nil + } + require.NoError(t, err) + require.True(t, expectFound, "found challenge in DB but expected it to be absent") + return nil + }) + } + checkChallengeExists(newFleetChallenge, true) + + // Consume the new challenge (simulates what PKIOperation does) + err = s.ds.ConsumeChallenge(ctx, newFleetChallenge) + require.NoError(t, err) + + // Verify the challenge is gone after consumption — a second consume must fail + checkChallengeExists(newFleetChallenge, false) + err = s.ds.ConsumeChallenge(ctx, newFleetChallenge) + require.Error(t, err, "consuming the same challenge twice should fail") + // Resend for a non-existent host should return 404 s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/certificates/%d/resend", 99999, certTemplateID), nil, http.StatusNotFound, &struct{}{})