Fixed Android certificate enrollment failures caused by SCEP challenge expiration when devices were offline. (#38753)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #37651

Switched to issue the SCEP fleet challenge on demand instead of ahead of
time.

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Resolved Android certificate enrollment failures caused by SCEP
challenge expiration during offline periods, improving enrollment
reliability when devices lack connectivity.

* **Improvements**
* Certificate challenges are now generated on-demand when requested by
devices, rather than pre-generated, enhancing offline enrollment
support.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

server/datastore/mysql/challenges.go

@@ -15,13 +15,19 @@ import (
 // NewChallenge generates a random, base64-encoded challenge and inserts it into the challenges
 // table. It returns the generated challenge or an error if the insertion fails.
 func (ds *Datastore) NewChallenge(ctx context.Context) (string, error) {
+	return newChallenge(ctx, ds.writer(ctx))
+}
+
+// newChallenge is a helper that generates and inserts a challenge using the provided executor.
+// This allows challenge creation within transactions.
+func newChallenge(ctx context.Context, exec sqlx.ExecerContext) (string, error) {
 	key := make([]byte, 24)
 	_, err := rand.Read(key)
 	if err != nil {
 		return "", err
 	}
 	challenge := base64.URLEncoding.EncodeToString(key)
-	_, err = ds.writer(ctx).ExecContext(ctx, `INSERT INTO challenges (challenge) VALUES (?)`, challenge)
+	_, err = exec.ExecContext(ctx, `INSERT INTO challenges (challenge) VALUES (?)`, challenge)
 	if err != nil {
 		return "", err
 	}

server/datastore/mysql/host_certificate_templates.go

@@ -441,50 +441,28 @@ func (ds *Datastore) GetAndTransitionCertificateTemplatesToDelivering(
 	return result, err
 }
 
-// TransitionCertificateTemplatesToDelivered transitions templates from 'delivering' to 'delivered'
-// and sets the fleet_challenge for each template.
-func (ds *Datastore) TransitionCertificateTemplatesToDelivered(
-	ctx context.Context,
-	hostUUID string,
-	challenges map[uint]string, // certificateTemplateID -> challenge
-) error {
-	if len(challenges) == 0 {
+// TransitionCertificateTemplatesToDelivered transitions the specified templates from 'delivering' to 'delivered'.
+// The fleet_challenge is cleared so a fresh one is generated when the device fetches the certificate template via
+// GetOrCreateFleetChallengeForCertificateTemplate.
+func (ds *Datastore) TransitionCertificateTemplatesToDelivered(ctx context.Context, hostUUID string, templateIDs []uint) error {
+	if len(templateIDs) == 0 {
 		return nil
 	}
 
-	// Build UPDATE with CASE for each template's challenge.
-	// This is called once per host, so the CASE size is bounded by templates per host (small).
-	// Using a single UPDATE per host is more efficient than individual updates when processing many hosts.
-	var caseStmt strings.Builder
-	args := make([]any, 0, len(challenges)*3+1) // CASE args + hostUUID + IN args
-	caseStmt.WriteString("CASE certificate_template_id ")
-	for templateID, challenge := range challenges {
-		caseStmt.WriteString("WHEN ? THEN ? ")
-		args = append(args, templateID, challenge)
-	}
-	caseStmt.WriteString("END")
-
-	// Add hostUUID for WHERE clause
-	args = append(args, hostUUID)
-
-	// Build IN clause for template IDs
-	inPlaceholders := make([]string, 0, len(challenges))
-	for templateID := range challenges {
-		inPlaceholders = append(inPlaceholders, "?")
-		args = append(args, templateID)
-	}
-
-	query := fmt.Sprintf(`
+	query, args, err := sqlx.In(fmt.Sprintf(`
 		UPDATE host_certificate_templates
 		SET
 			status = '%s',
-			fleet_challenge = %s,
+			fleet_challenge = NULL,
 			updated_at = NOW()
 		WHERE
 			host_uuid = ? AND
 			status = '%s' AND
-			certificate_template_id IN (%s)
-	`, fleet.CertificateTemplateDelivered, caseStmt.String(), fleet.CertificateTemplateDelivering, strings.Join(inPlaceholders, ","))
+			certificate_template_id IN (?)
+	`, fleet.CertificateTemplateDelivered, fleet.CertificateTemplateDelivering), hostUUID, templateIDs)
+	if err != nil {
+		return ctxerr.Wrap(ctx, err, "build transition to delivered query")
+	}
 
 	if _, err := ds.writer(ctx).ExecContext(ctx, query, args...); err != nil {
 		return ctxerr.Wrap(ctx, err, "transition to delivered")
@@ -649,8 +627,9 @@ func (ds *Datastore) GetAndroidCertificateTemplatesForRenewal(
 }
 
 // SetAndroidCertificateTemplatesForRenewal marks the specified certificate templates for renewal
-// by setting status to 'pending', clearing validity fields, and generating a new UUID.
+// by setting status to 'pending', clearing validity fields and fleet_challenge, and generating a new UUID.
 // The new UUID signals to the Android agent that the certificate needs renewal.
+// The fleet_challenge is cleared so a fresh one is generated when the device fetches the renewed certificate.
 func (ds *Datastore) SetAndroidCertificateTemplatesForRenewal(
 	ctx context.Context,
 	templates []fleet.HostCertificateTemplateForRenewal,
@@ -677,6 +656,7 @@ func (ds *Datastore) SetAndroidCertificateTemplatesForRenewal(
 			not_valid_before = NULL,
 			not_valid_after = NULL,
 			serial = NULL,
+			fleet_challenge = NULL,
 			updated_at = NOW()
 		WHERE (host_uuid, certificate_template_id) IN (%s)
 	`, fleet.CertificateTemplatePending, placeholders.String())
@@ -687,3 +667,61 @@ func (ds *Datastore) SetAndroidCertificateTemplatesForRenewal(
 
 	return nil
 }
+
+// GetOrCreateFleetChallengeForCertificateTemplate ensures a fleet challenge exists for the given
+// host and certificate template. If a challenge already exists in host_certificate_templates,
+// it returns it. If not, it creates a new one atomically and stores it in both the challenges
+// table (for validation) and host_certificate_templates (for retrieval).
+// This method only works for templates in 'delivered' status.
+func (ds *Datastore) GetOrCreateFleetChallengeForCertificateTemplate(
+	ctx context.Context,
+	hostUUID string,
+	certificateTemplateID uint,
+) (string, error) {
+	var challenge string
+	err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
+		// Check if challenge already exists using FOR UPDATE to prevent race conditions
+		var existingChallenge sql.NullString
+		err := sqlx.GetContext(ctx, tx, &existingChallenge, fmt.Sprintf(`
+			SELECT fleet_challenge
+			FROM host_certificate_templates
+			WHERE host_uuid = ? AND certificate_template_id = ? AND status = '%s' AND operation_type = '%s'
+			FOR UPDATE
+		`, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall), hostUUID, certificateTemplateID)
+		if err != nil {
+			if errors.Is(err, sql.ErrNoRows) {
+				return ctxerr.Wrap(ctx, notFound("HostCertificateTemplate"), "template not found or not in delivered status")
+			}
+			return ctxerr.Wrap(ctx, err, "check existing challenge")
+		}
+
+		// If challenge exists and is non-empty, return it
+		if existingChallenge.Valid && existingChallenge.String != "" {
+			challenge = existingChallenge.String
+			return nil
+		}
+
+		// Create new challenge using the transaction
+		newChal, err := newChallenge(ctx, tx)
+		if err != nil {
+			return ctxerr.Wrap(ctx, err, "create challenge")
+		}
+
+		// Update host_certificate_templates with the challenge
+		if _, err := tx.ExecContext(ctx, fmt.Sprintf(`
+			UPDATE host_certificate_templates
+			SET fleet_challenge = ?, updated_at = NOW()
+			WHERE host_uuid = ? AND certificate_template_id = ? AND status = '%s' AND operation_type = '%s'
+		`, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall), newChal, hostUUID, certificateTemplateID); err != nil {
+			return ctxerr.Wrap(ctx, err, "update fleet_challenge in host_certificate_templates")
+		}
+
+		challenge = newChal
+		return nil
+	})
+
+	if err != nil {
+		return "", err
+	}
+	return challenge, nil
+}

server/datastore/mysql/host_certificate_templates_test.go

@@ -40,6 +40,7 @@ func TestHostCertificateTemplates(t *testing.T) {
 		{"CertificateTemplateReinstalledAfterTransferBackToOriginalTeam", testCertificateTemplateReinstalledAfterTransferBackToOriginalTeam},
 		{"GetAndroidCertificateTemplatesForRenewal", testGetAndroidCertificateTemplatesForRenewal},
 		{"SetAndroidCertificateTemplatesForRenewal", testSetAndroidCertificateTemplatesForRenewal},
+		{"GetOrCreateFleetChallengeForCertificateTemplate", testGetOrCreateFleetChallengeForCertificateTemplate},
 	}
 
 	for _, c := range cases {
@@ -865,15 +866,30 @@ func testCertificateTemplateFullStateMachine(t *testing.T, ds *Datastore) {
 		require.EqualValues(t, fleet.CertificateTemplateDelivering, *r.Status)
 	}
 
-	// Step 4: Transition to delivered with challenges
-	challenges := map[uint]string{
-		setup.template.ID: "challenge-abc",
-		templateTwo.ID:    "challenge-xyz",
-	}
-	err = ds.TransitionCertificateTemplatesToDelivered(ctx, "android-host", challenges)
+	// Step 4: Transition to delivered (challenges are created on-demand)
+	err = ds.TransitionCertificateTemplatesToDelivered(ctx, "android-host", []uint{setup.template.ID, templateTwo.ID})
 	require.NoError(t, err)
 
-	// Verify final state
+	// Verify delivered state (no challenges yet)
+	records, err = ds.ListCertificateTemplatesForHosts(ctx, []string{"android-host"})
+	require.NoError(t, err)
+	require.Len(t, records, 2)
+	for _, r := range records {
+		require.NotNil(t, r.Status)
+		require.EqualValues(t, fleet.CertificateTemplateDelivered, *r.Status)
+		require.Nil(t, r.FleetChallenge) // Challenge not created yet
+	}
+
+	// Step 5: Create challenges on-demand (simulating device fetch)
+	challenge1, err := ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, "android-host", setup.template.ID)
+	require.NoError(t, err)
+	require.NotEmpty(t, challenge1)
+
+	challenge2, err := ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, "android-host", templateTwo.ID)
+	require.NoError(t, err)
+	require.NotEmpty(t, challenge2)
+
+	// Verify challenges are now set
 	records, err = ds.ListCertificateTemplatesForHosts(ctx, []string{"android-host"})
 	require.NoError(t, err)
 	require.Len(t, records, 2)
@@ -882,9 +898,9 @@ func testCertificateTemplateFullStateMachine(t *testing.T, ds *Datastore) {
 		require.EqualValues(t, fleet.CertificateTemplateDelivered, *r.Status)
 		require.NotNil(t, r.FleetChallenge)
 		if r.CertificateTemplateID == setup.template.ID {
-			require.Equal(t, "challenge-abc", *r.FleetChallenge)
+			require.Equal(t, challenge1, *r.FleetChallenge)
 		} else {
-			require.Equal(t, "challenge-xyz", *r.FleetChallenge)
+			require.Equal(t, challenge2, *r.FleetChallenge)
 		}
 	}
 
@@ -1825,6 +1841,12 @@ func testSetAndroidCertificateTemplatesForRenewal(t *testing.T, ds *Datastore) {
 	insertHostCertTemplate(t, ds, host1.UUID, templateID, fleet.CertificateTemplateVerified, fleet.MDMOperationTypeInstall, &notValidBefore, &notValidAfter)
 	insertHostCertTemplate(t, ds, host2.UUID, templateID, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall, &notValidBefore, &notValidAfter)
 
+	// Set a fleet_challenge on host1 to verify it gets cleared during renewal
+	_, err = ds.writer(ctx).ExecContext(ctx,
+		`UPDATE host_certificate_templates SET fleet_challenge = 'old-challenge' WHERE host_uuid = ?`,
+		host1.UUID)
+	require.NoError(t, err)
+
 	// Get the original UUIDs
 	var originalUUIDs []struct {
 		HostUUID string `db:"host_uuid"`
@@ -1855,9 +1877,10 @@ func testSetAndroidCertificateTemplatesForRenewal(t *testing.T, ds *Datastore) {
 		NotValidBefore *string `db:"not_valid_before"`
 		NotValidAfter  *string `db:"not_valid_after"`
 		Serial         *string `db:"serial"`
+		FleetChallenge *string `db:"fleet_challenge"`
 	}
 	err = sqlx.SelectContext(ctx, ds.reader(ctx), &updatedRecords,
-		`SELECT host_uuid, status, COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid, not_valid_before, not_valid_after, serial
+		`SELECT host_uuid, status, COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid, not_valid_before, not_valid_after, serial, fleet_challenge
 		 FROM host_certificate_templates WHERE host_uuid IN (?, ?) ORDER BY host_uuid`,
 		host1.UUID, host2.UUID)
 	require.NoError(t, err)
@@ -1878,9 +1901,109 @@ func testSetAndroidCertificateTemplatesForRenewal(t *testing.T, ds *Datastore) {
 		require.Nil(t, r.NotValidBefore, "not_valid_before should be cleared")
 		require.Nil(t, r.NotValidAfter, "not_valid_after should be cleared")
 		require.Nil(t, r.Serial, "serial should be cleared")
+		// Fleet challenge should be cleared so a new one is generated on next delivery
+		require.Nil(t, r.FleetChallenge, "fleet_challenge should be cleared")
 	}
 
 	// Test empty slice doesn't error
 	err = ds.SetAndroidCertificateTemplatesForRenewal(ctx, []fleet.HostCertificateTemplateForRenewal{})
 	require.NoError(t, err)
 }
+
+func testGetOrCreateFleetChallengeForCertificateTemplate(t *testing.T, ds *Datastore) {
+	ctx := t.Context()
+
+	// Create test setup
+	team, err := ds.NewTeam(ctx, &fleet.Team{Name: "test team challenge"})
+	require.NoError(t, err)
+
+	ca, err := ds.NewCertificateAuthority(ctx, &fleet.CertificateAuthority{
+		Name: ptr.String("test ca challenge"),
+		Type: string(fleet.CAConfigCustomSCEPProxy),
+		URL:  ptr.String("http://localhost:8080/scep"),
+	})
+	require.NoError(t, err)
+
+	now := time.Now().UTC()
+	host := test.NewHost(t, ds, "host-challenge", "192.168.1.1", "host_key_challenge", uuid.NewString(), now, test.WithPlatform("android"), test.WithTeamID(team.ID))
+
+	t.Run("returns error for non-existent template", func(t *testing.T) {
+		_, err := ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, host.UUID, 99999)
+		require.Error(t, err)
+		require.True(t, fleet.IsNotFound(err))
+	})
+
+	t.Run("returns error for non-delivered status", func(t *testing.T) {
+		// Create a separate template for this test
+		pendingTemplate, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{
+			TeamID:                 team.ID,
+			Name:                   "test template pending",
+			CertificateAuthorityID: ca.ID,
+			SubjectName:            "CN=test-pending",
+		})
+		require.NoError(t, err)
+
+		// Insert a pending certificate template
+		_, err = ds.writer(ctx).ExecContext(ctx,
+			`INSERT INTO host_certificate_templates
+				(host_uuid, certificate_template_id, status, operation_type, name, uuid)
+			VALUES (?, ?, ?, ?, 'test', UUID_TO_BIN(UUID(), true))`,
+			host.UUID, pendingTemplate.ID, fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall)
+		require.NoError(t, err)
+
+		_, err = ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, host.UUID, pendingTemplate.ID)
+		require.Error(t, err)
+		require.True(t, fleet.IsNotFound(err))
+	})
+
+	t.Run("creates challenge on first call and returns same on subsequent calls", func(t *testing.T) {
+		template, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{
+			TeamID:                 team.ID,
+			Name:                   "test template challenge",
+			CertificateAuthorityID: ca.ID,
+			SubjectName:            "CN=test",
+		})
+		require.NoError(t, err)
+
+		// Insert a delivered certificate template WITHOUT a challenge
+		_, err = ds.writer(ctx).ExecContext(ctx,
+			`INSERT INTO host_certificate_templates
+				(host_uuid, certificate_template_id, status, operation_type, name, uuid, fleet_challenge)
+			VALUES (?, ?, ?, ?, 'test', UUID_TO_BIN(UUID(), true), NULL)`,
+			host.UUID, template.ID, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall)
+		require.NoError(t, err)
+
+		// First call should create a challenge
+		challenge, err := ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, host.UUID, template.ID)
+		require.NoError(t, err)
+		require.NotEmpty(t, challenge)
+		require.Len(t, challenge, 32) // Base64 encoded 24 bytes
+
+		// Verify challenge was stored in host_certificate_templates
+		var storedChallenge string
+		err = sqlx.GetContext(ctx, ds.reader(ctx), &storedChallenge,
+			`SELECT fleet_challenge FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ?`,
+			host.UUID, template.ID)
+		require.NoError(t, err)
+		require.Equal(t, challenge, storedChallenge)
+
+		// Verify challenge was also inserted into challenges table
+		var createdAt time.Time
+		err = sqlx.GetContext(ctx, ds.reader(ctx), &createdAt,
+			`SELECT created_at FROM challenges WHERE challenge = ?`, challenge)
+		require.NoError(t, err)
+		require.WithinDuration(t, time.Now(), createdAt, 5*time.Second)
+
+		// Subsequent call should return the same challenge
+		challenge2, err := ds.GetOrCreateFleetChallengeForCertificateTemplate(ctx, host.UUID, template.ID)
+		require.NoError(t, err)
+		require.Equal(t, challenge, challenge2)
+
+		// Verify only one challenge exists in challenges table
+		var count int
+		err = sqlx.GetContext(ctx, ds.reader(ctx), &count,
+			`SELECT COUNT(*) FROM challenges WHERE challenge = ?`, challenge)
+		require.NoError(t, err)
+		require.Equal(t, 1, count)
+	})
+}