Auto-clean duplicate Okta CA SCEP cert after profile install (#46172)

**Related issue:** Resolves #42757

## Summary

Resending or renewing the Okta conditional access profile leaves an
orphaned SCEP certificate in the per-user macOS keychain, accumulating
duplicates with every renewal. This PR auto-runs an existing
keychain-cleanup script after a successful `InstallProfile` ack for the
Okta CA profile, so admins no longer have to find and run the script
manually.

## Root cause

Investigation in the issue thread isolated the trigger:

- The Okta CA `.mobileconfig` bundles `com.apple.security.scep` with
`com.apple.security.identitypreference` in a single profile (macOS
rejects the alternative — `Identity payload not found in same profile as
identity preference payload`).
- The Identity Preference payload creates a keychain-resident preference
item that keeps the *old* cert pinned across profile replacement, even
though the rewritten Identity Preference now points to the fresh SCEP
enrollment.
- EAP-TLS Wi-Fi profiles renew cleanly because they reference the cert
via SystemConfiguration (`PayloadCertificateUUID`), not the keychain —
so this isn't a generic SCEP-bundling issue.

The team decision in the issue (`@sharon-fdm`) was to delete the
duplicate certificate rather than restructure the profile. A standalone
cleanup script already shipped at
`docs/solutions/macos/scripts/delete-duplicate-scep-certificates.sh` and
was linked from the Okta CA guide; admins had to find and run it.

## Approach

Hook the existing Apple MDM `InstallProfile` ack path in
`MDMAppleCheckinAndCommandService.CommandAndReportResults`, parallel to
the existing ACME `CertificateList` follow-up. When the ack is for the
Okta CA profile and status is `verifying`, enqueue an internal host
script run that executes the cleanup script targeting the host's
per-user MDM enrollment short name.

Key properties:

- **Single hook, three paths covered.** Admin "Resend" nulls the profile
status and the reconciliation cron re-enqueues an `InstallProfile`; the
SCEP renewal cron also re-issues `InstallProfile`. Both flow through the
same ack handler this hook attaches to.
- **Idempotent.** The cleanup script no-ops when only one matching cert
is present, so triggering on initial installs (not just renewals) is
safe and removes the need to distinguish "is this a renewal".
- **Tightly gated.** Single indexed lookup keyed on `(host_uuid,
command_uuid, profile_identifier, platform='darwin')`. Other
SCEP-bearing profiles do not trigger the script. No work happens for
hosts with no per-user enrollment.
- **Internal-script semantics** (matches lock/unlock/wipe prior art).
Runs even when scripts are globally disabled. Does not appear in the
user-facing host activity feed.
- **Failure-isolated.** Enqueue errors are logged but do not break the
ack path; the renewal itself is what matters.
- **Defense in depth on the shell call.** The macOS short name is
validated against a strict regex (`^[A-Za-z0-9_][A-Za-z0-9_.-]*$`, ≤31
chars) before being interpolated, and POSIX single-quote-escaped on the
way through.

## Files

**New**
- `server/service/conditional_access_cleanup.go` — `//go:embed` of the
cleanup script, the hook helper `maybeRunOktaCACleanupScript`, the
validated shell-wrapper builder, and the POSIX single-quote escape
helper.
- `server/service/conditional_access_cleanup_test.go` — unit coverage
for username validation, shell escaping, the routing decisions of the
hook helper (mock-based), and an embed-sync assertion against the docs
copy.
-
`server/service/embedded_scripts/delete-duplicate-scep-certificates.sh`
— embed source-of-truth copy, byte-for-byte equal to the public
`docs/solutions/macos/scripts/` script.
- `changes/42757-okta-conditional-access-duplicate-scep-cert-cleanup` —
user-visible changes note.

**Datastore**
- `server/datastore/mysql/mdm.go` —
`OktaCACleanupTargetForInstallCommand`: single SQL lookup that returns
`(host_id, user_short_name, ok)` for the new hook. Returns `ok=false`
for non-Okta profiles, non-darwin hosts, or hosts without a user-channel
enrollment.
- `server/datastore/mysql/scripts.go` —
`NewInternalHostScriptExecutionRequest`: thin wrapper that routes
through the existing internal-script codepath (`isInternal=true`) used
by lock/unlock/wipe. Refactored the existing public method to share an
internal helper.

**Interface / mocks**
- `server/fleet/conditional_access_idp.go` — exported
`ConditionalAccessOktaProfileIdentifier`,
`ConditionalAccessOktaCertificateCN`, and the new `OktaCACleanupTarget`
struct, so both the template-render path and the SQL lookup can
reference the same source of truth.
- `server/fleet/datastore.go` — `OktaCACleanupTargetForInstallCommand`
and `NewInternalHostScriptExecutionRequest` added to the `Datastore`
interface.
- `server/mock/datastore_mock.go` — regenerated (additions only).

**Wiring**
- `server/service/apple_mdm.go` — call into
`maybeRunOktaCACleanupScript` from the InstallProfile
`MDMDeliveryVerifying` branch, alongside the existing ACME
`maybeQueueCertificateListForACMEProfile` follow-up. Warns on error
rather than failing the ack.
- `server/service/conditional_access_idp.go` — use the new
`fleet.ConditionalAccessOktaCertificateCN` constant when rendering the
profile template, eliminating the magic string duplication.

**Tests touched**
- `server/datastore/mysql/mdm_test.go` — integration test
`testOktaCACleanupTargetForInstallCommand` covering the happy path,
non-Okta profile, device-only enrollment, and unknown command.
- `server/datastore/mysql/scripts_test.go` —
`testNewInternalHostScriptExecutionRequest` confirming the internal flag
is set correctly and the new entry only appears under the internal-only
listing filter.
- `server/service/apple_mdm_test.go` — added the new mock stub for
`OktaCACleanupTargetForInstallCommandFunc` to
`TestMDMCommandAndReportResultsProfileHandling` so the existing test
continues to pass with the new hook in the codepath.
- `server/service/conditional_access_idp_test.go` — the rendered-profile
assertion now also pins on the shared
`ConditionalAccessOktaProfileIdentifier` and
`ConditionalAccessOktaCertificateCN` constants so the template can't
drift from the SQL lookup.
This commit is contained in:
Dante Catalfamo
2026-06-05 16:20:32 -04:00
committed by GitHub
parent 1c626f5824
commit 49db931ffb
15 changed files with 631 additions and 4 deletions
@@ -0,0 +1 @@
- Removed orphaned duplicate SCEP certificates from the per-user keychain automatically after an Okta conditional access profile is reinstalled or renewed on macOS hosts.
+39
View File
@@ -1657,6 +1657,45 @@ WHERE
return dest, nil
}
// OktaCACleanupTargetForInstallCommand returns the host ID and target
// macOS short name needed to schedule the Okta conditional access
// keychain-cleanup script after a successful InstallProfile ack. The ok
// return is false when the command's profile is not the Okta CA profile,
// the host's platform is not darwin, or the host has no per-user MDM
// enrollment short name on record. All gating is done in a single indexed
// lookup keyed on (host_uuid, command_uuid).
func (ds *Datastore) OktaCACleanupTargetForInstallCommand(ctx context.Context, hostUUID, commandUUID string) (fleet.OktaCACleanupTarget, bool, error) {
const stmt = `
SELECT
h.id AS host_id,
COALESCE((
SELECT nu.user_short_name
FROM nano_enrollments ne
INNER JOIN nano_users nu ON ne.user_id = nu.id
WHERE ne.type = 'User' AND ne.enabled = 1 AND ne.device_id = h.uuid
ORDER BY ne.created_at ASC, ne.id ASC LIMIT 1
), '') AS user_short_name
FROM host_mdm_apple_profiles hmap
JOIN hosts h ON h.uuid = hmap.host_uuid
WHERE hmap.command_uuid = ?
AND hmap.host_uuid = ?
AND hmap.profile_identifier = ?
AND h.platform = 'darwin'`
var dest fleet.OktaCACleanupTarget
err := sqlx.GetContext(ctx, ds.reader(ctx), &dest, stmt, commandUUID, hostUUID, fleet.ConditionalAccessOktaProfileIdentifier)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return dest, false, nil
}
return dest, false, ctxerr.Wrap(ctx, err, "look up Okta CA cleanup target")
}
if dest.UserShortName == "" {
return dest, false, nil
}
return dest, true, nil
}
func (ds *Datastore) ProfileHasACMEPayloadForCommand(ctx context.Context, hostUUID, commandUUID string) (fleet.ProfileACMECommandResult, error) {
const stmt = `
SELECT
+83
View File
@@ -65,6 +65,7 @@ func TestMDMShared(t *testing.T) {
{"TestCleanUpMDMManagedCertificates", testCleanUpMDMManagedCertificates},
{"TestEnqueueCommandWithName", testEnqueueCommandWithName},
{"TestProfileHasACMEPayloadForCommand", testProfileHasACMEPayloadForCommand},
{"TestOktaCACleanupTargetForInstallCommand", testOktaCACleanupTargetForInstallCommand},
{"TestRenewMDMManagedCertificatesNullType", testRenewMDMManagedCertificatesNullType},
}
@@ -6206,6 +6207,88 @@ func testProfileHasACMEPayloadForCommand(t *testing.T, ds *Datastore) {
})
}
func testOktaCACleanupTargetForInstallCommand(t *testing.T, ds *Datastore) {
ctx := t.Context()
newHost := func(t *testing.T, suffix, platform string) *fleet.Host {
t.Helper()
h, err := ds.NewHost(ctx, &fleet.Host{
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now(),
OsqueryHostID: ptr.String("okta-cleanup-osq-" + suffix),
NodeKey: ptr.String("okta-cleanup-nk-" + suffix),
UUID: "okta-cleanup-host-" + suffix,
Hostname: "okta-cleanup-" + suffix,
Platform: platform,
})
require.NoError(t, err)
return h
}
insertHostProfile := func(t *testing.T, hostUUID, identifier, commandUUID string) {
t.Helper()
// host_mdm_apple_profiles has no FK to mdm_apple_configuration_profiles,
// so we can insert the host-profile row directly with a fresh
// profile_uuid per sub-test and avoid the (team_id, identifier)
// unique-key conflict that comes from reusing the Okta CA identifier.
require.NoError(t, ds.BulkUpsertMDMAppleHostProfiles(ctx, []*fleet.MDMAppleBulkUpsertHostProfilePayload{{
ProfileUUID: uuid.NewString(),
ProfileIdentifier: identifier,
HostUUID: hostUUID,
Checksum: []byte("0123456789abcdef"),
Scope: fleet.PayloadScopeUser,
OperationType: fleet.MDMOperationTypeInstall,
CommandUUID: commandUUID,
}}))
}
t.Run("okta CA profile + per-user enrollment present: returns target", func(t *testing.T) {
host := newHost(t, "happy", "darwin")
nanoEnroll(t, ds, host, true) // creates Device + User enrollment with user_short_name = "alice"
cmdUUID := uuid.NewString()
insertHostProfile(t, host.UUID, fleet.ConditionalAccessOktaProfileIdentifier, cmdUUID)
got, ok, err := ds.OktaCACleanupTargetForInstallCommand(ctx, host.UUID, cmdUUID)
require.NoError(t, err)
require.True(t, ok)
require.Equal(t, host.ID, got.HostID)
require.Equal(t, "alice", got.UserShortName)
})
t.Run("non-Okta profile identifier: ok=false", func(t *testing.T) {
host := newHost(t, "wrong-id", "darwin")
nanoEnroll(t, ds, host, true)
cmdUUID := uuid.NewString()
insertHostProfile(t, host.UUID, "com.example.unrelated", cmdUUID)
_, ok, err := ds.OktaCACleanupTargetForInstallCommand(ctx, host.UUID, cmdUUID)
require.NoError(t, err)
require.False(t, ok)
})
t.Run("okta profile + only device-channel enrollment: ok=false", func(t *testing.T) {
host := newHost(t, "no-user-chan", "darwin")
nanoEnroll(t, ds, host, false) // Device-only enrollment, no nano_users row
cmdUUID := uuid.NewString()
insertHostProfile(t, host.UUID, fleet.ConditionalAccessOktaProfileIdentifier, cmdUUID)
_, ok, err := ds.OktaCACleanupTargetForInstallCommand(ctx, host.UUID, cmdUUID)
require.NoError(t, err)
require.False(t, ok)
})
t.Run("unknown command: ok=false, no error", func(t *testing.T) {
host := newHost(t, "no-cmd", "darwin")
nanoEnroll(t, ds, host, true)
_, ok, err := ds.OktaCACleanupTargetForInstallCommand(ctx, host.UUID, "no-such-cmd")
require.NoError(t, err)
require.False(t, ok)
})
}
func testRenewMDMManagedCertificatesNullType(t *testing.T, ds *Datastore) {
ctx := t.Context()
+14 -1
View File
@@ -36,6 +36,19 @@ var hostScriptDetailsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{
}
func (ds *Datastore) NewHostScriptExecutionRequest(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error) {
return ds.newHostScriptExecutionRequestPublic(ctx, request, false)
}
// NewInternalHostScriptExecutionRequest enqueues a host script run flagged
// as internal (fleet-initiated). Internal scripts run even when scripts
// are globally disabled and do not appear in the user-facing host activity
// feed. Use for server-driven follow-up actions (e.g. cleanup scripts
// after MDM events) rather than user-requested runs.
func (ds *Datastore) NewInternalHostScriptExecutionRequest(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error) {
return ds.newHostScriptExecutionRequestPublic(ctx, request, true)
}
func (ds *Datastore) newHostScriptExecutionRequestPublic(ctx context.Context, request *fleet.HostScriptRequestPayload, isInternal bool) (*fleet.HostScriptResult, error) {
var res *fleet.HostScriptResult
return res, ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
var err error
@@ -49,7 +62,7 @@ func (ds *Datastore) NewHostScriptExecutionRequest(ctx context.Context, request
id, _ := scRes.LastInsertId()
request.ScriptContentID = uint(id) //nolint:gosec // dismiss G115
}
res, err = ds.newHostScriptExecutionRequest(ctx, tx, request, false)
res, err = ds.newHostScriptExecutionRequest(ctx, tx, request, isInternal)
return err
})
}
+39
View File
@@ -56,6 +56,7 @@ func TestScripts(t *testing.T) {
{"BatchSetScriptActivatesNextActivity", testBatchSetScriptActivatesNextActivity},
{"CountHostScriptAttempts", testCountHostScriptAttempts},
{"ScriptModificationResetsAttemptNumber", testScriptModificationResetsAttemptNumber},
{"NewInternalHostScriptExecutionRequest", testNewInternalHostScriptExecutionRequest},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
@@ -3252,3 +3253,41 @@ func testScriptModificationResetsAttemptNumber(t *testing.T, ds *Datastore) {
require.Equal(t, int64(0), *results[1].AttemptNumber)
require.True(t, results[1].Canceled)
}
func testNewInternalHostScriptExecutionRequest(t *testing.T, ds *Datastore) {
ctx := context.Background()
res, err := ds.NewInternalHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{
HostID: 1,
ScriptContents: "echo internal",
})
require.NoError(t, err)
require.NotZero(t, res.ID)
require.Nil(t, res.UserID)
// The internal-only filter on ListPendingHostScriptExecutions surfaces
// internal scripts; the default (all-pending) listing includes them
// alongside user-initiated ones.
pendingAll, err := ds.ListPendingHostScriptExecutions(ctx, 1, false)
require.NoError(t, err)
require.Len(t, pendingAll, 1)
require.Equal(t, res.ID, pendingAll[0].ID)
pendingInternal, err := ds.ListPendingHostScriptExecutions(ctx, 1, true)
require.NoError(t, err)
require.Len(t, pendingInternal, 1)
require.Equal(t, res.ID, pendingInternal[0].ID)
// A non-internal request should NOT appear under the internal-only filter,
// confirming the new entry routed through the internal codepath.
resUser, err := ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{
HostID: 2,
ScriptContents: "echo user",
})
require.NoError(t, err)
require.NotZero(t, resUser.ID)
pendingUserViaInternal, err := ds.ListPendingHostScriptExecutions(ctx, 2, true)
require.NoError(t, err)
require.Empty(t, pendingUserViaInternal)
}
+20
View File
@@ -9,3 +9,23 @@ type ConditionalAccessIDPAssets struct{}
func (c *ConditionalAccessIDPAssets) AuthzType() string {
return "conditional_access_idp_assets"
}
// ConditionalAccessOktaProfileIdentifier is the top-level PayloadIdentifier
// of the .mobileconfig profile delivered for Okta conditional access. It is
// also the value stored in host_mdm_apple_profiles.profile_identifier for
// hosts that have received the profile, and is used to detect when an
// InstallProfile ack applies to the Okta CA profile.
const ConditionalAccessOktaProfileIdentifier = "com.fleetdm.conditional-access-okta"
// ConditionalAccessOktaCertificateCN is the Subject CN of the SCEP
// certificate issued for Okta conditional access. The duplicate-cert
// cleanup script matches certificates in the keychain by this CN.
const ConditionalAccessOktaCertificateCN = "Fleet conditional access for Okta"
// OktaCACleanupTarget identifies where to run the Okta conditional access
// keychain-cleanup script after a successful InstallProfile ack for the
// Okta CA profile.
type OktaCACleanupTarget struct {
HostID uint `db:"host_id"`
UserShortName string `db:"user_short_name"`
}
+14
View File
@@ -490,6 +490,14 @@ type Datastore interface {
// positive risk (one redundant CertificateList per false match).
ProfileHasACMEPayloadForCommand(ctx context.Context, hostUUID, commandUUID string) (ProfileACMECommandResult, error)
// OktaCACleanupTargetForInstallCommand returns the host ID and target
// macOS short name needed to schedule the Okta conditional access
// keychain-cleanup script after a successful InstallProfile ack. The
// ok return is false when the command's profile is not the Okta CA
// profile, the host's platform is not darwin, or the host has no
// per-user MDM enrollment short name on record.
OktaCACleanupTargetForInstallCommand(ctx context.Context, hostUUID, commandUUID string) (OktaCACleanupTarget, bool, error)
// AreHostsConnectedToFleetMDM checks each host MDM enrollment with
// this server and returns a map indexed by the host uuid and a boolean
// indicating if the enrollment is active.
@@ -2426,6 +2434,12 @@ type Datastore interface {
// NewHostScriptExecutionRequest creates a new host script result entry with
// just the script to run information (result is not yet available).
NewHostScriptExecutionRequest(ctx context.Context, request *HostScriptRequestPayload) (*HostScriptResult, error)
// NewInternalHostScriptExecutionRequest is like NewHostScriptExecutionRequest
// but marks the request as internal (fleet-initiated), so it runs even when
// scripts are globally disabled and does not appear in the user-facing host
// activity feed. Use for server-driven follow-up actions (e.g. cleanup
// scripts after MDM events).
NewInternalHostScriptExecutionRequest(ctx context.Context, request *HostScriptRequestPayload) (*HostScriptResult, error)
// SetHostScriptExecutionResult stores the result of a host script execution
// return nil, "", nil. action is populated if this script was an MDM action (lock/unlock/wipe/uninstall).
SetHostScriptExecutionResult(ctx context.Context, result *HostScriptResultPayload, attemptNumber *int) (hsr *HostScriptResult, action string, err error)
+24
View File
@@ -358,6 +358,8 @@ type SoftDeleteMDMHostCertificatesForUnenrolledHostsFunc func(ctx context.Contex
type ProfileHasACMEPayloadForCommandFunc func(ctx context.Context, hostUUID string, commandUUID string) (fleet.ProfileACMECommandResult, error)
type OktaCACleanupTargetForInstallCommandFunc func(ctx context.Context, hostUUID string, commandUUID string) (fleet.OktaCACleanupTarget, bool, error)
type AreHostsConnectedToFleetMDMFunc func(ctx context.Context, hosts []*fleet.Host) (map[string]bool, error)
type AggregatedMunkiVersionFunc func(ctx context.Context, teamID *uint) ([]fleet.AggregatedMunkiVersion, time.Time, error)
@@ -1436,6 +1438,8 @@ type SetOrUpdateMDMAppleDeclarationFunc func(ctx context.Context, declaration *f
type NewHostScriptExecutionRequestFunc func(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error)
type NewInternalHostScriptExecutionRequestFunc func(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error)
type SetHostScriptExecutionResultFunc func(ctx context.Context, result *fleet.HostScriptResultPayload, attemptNumber *int) (hsr *fleet.HostScriptResult, action string, err error)
type GetHostScriptExecutionResultFunc func(ctx context.Context, execID string) (*fleet.HostScriptResult, error)
@@ -2560,6 +2564,9 @@ type DataStore struct {
ProfileHasACMEPayloadForCommandFunc ProfileHasACMEPayloadForCommandFunc
ProfileHasACMEPayloadForCommandFuncInvoked bool
OktaCACleanupTargetForInstallCommandFunc OktaCACleanupTargetForInstallCommandFunc
OktaCACleanupTargetForInstallCommandFuncInvoked bool
AreHostsConnectedToFleetMDMFunc AreHostsConnectedToFleetMDMFunc
AreHostsConnectedToFleetMDMFuncInvoked bool
@@ -4177,6 +4184,9 @@ type DataStore struct {
NewHostScriptExecutionRequestFunc NewHostScriptExecutionRequestFunc
NewHostScriptExecutionRequestFuncInvoked bool
NewInternalHostScriptExecutionRequestFunc NewInternalHostScriptExecutionRequestFunc
NewInternalHostScriptExecutionRequestFuncInvoked bool
SetHostScriptExecutionResultFunc SetHostScriptExecutionResultFunc
SetHostScriptExecutionResultFuncInvoked bool
@@ -6282,6 +6292,13 @@ func (s *DataStore) ProfileHasACMEPayloadForCommand(ctx context.Context, hostUUI
return s.ProfileHasACMEPayloadForCommandFunc(ctx, hostUUID, commandUUID)
}
func (s *DataStore) OktaCACleanupTargetForInstallCommand(ctx context.Context, hostUUID string, commandUUID string) (fleet.OktaCACleanupTarget, bool, error) {
s.mu.Lock()
s.OktaCACleanupTargetForInstallCommandFuncInvoked = true
s.mu.Unlock()
return s.OktaCACleanupTargetForInstallCommandFunc(ctx, hostUUID, commandUUID)
}
func (s *DataStore) AreHostsConnectedToFleetMDM(ctx context.Context, hosts []*fleet.Host) (map[string]bool, error) {
s.mu.Lock()
s.AreHostsConnectedToFleetMDMFuncInvoked = true
@@ -10055,6 +10072,13 @@ func (s *DataStore) NewHostScriptExecutionRequest(ctx context.Context, request *
return s.NewHostScriptExecutionRequestFunc(ctx, request)
}
func (s *DataStore) NewInternalHostScriptExecutionRequest(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error) {
s.mu.Lock()
s.NewInternalHostScriptExecutionRequestFuncInvoked = true
s.mu.Unlock()
return s.NewInternalHostScriptExecutionRequestFunc(ctx, request)
}
func (s *DataStore) SetHostScriptExecutionResult(ctx context.Context, result *fleet.HostScriptResultPayload, attemptNumber *int) (hsr *fleet.HostScriptResult, action string, err error) {
s.mu.Lock()
s.SetHostScriptExecutionResultFuncInvoked = true
+9
View File
@@ -4180,6 +4180,15 @@ func (svc *MDMAppleCheckinAndCommandService) CommandAndReportResults(r *mdm.Requ
svc.logger.WarnContext(r.Context, "queue CertificateList after ACME profile install",
"err", err, "host_uuid", cmdResult.Identifier(), "command_uuid", cmdResult.CommandUUID)
}
// Okta conditional access bundles a SCEP payload with an Identity
// Preference payload, which leaves the previous cert pinned in
// the per-user keychain across renewals. After a successful ack,
// queue an internal cleanup script that removes the orphaned
// duplicate. No-op when the profile isn't the Okta CA one.
if err := svc.maybeRunOktaCACleanupScript(r.Context, cmdResult.Identifier(), cmdResult.CommandUUID); err != nil {
svc.logger.WarnContext(r.Context, "run Okta CA keychain cleanup after profile install",
"err", err, "host_uuid", cmdResult.Identifier(), "command_uuid", cmdResult.CommandUUID)
}
}
return nil, nil
case "RemoveProfile":
+3
View File
@@ -2766,6 +2766,9 @@ func TestMDMCommandAndReportResultsProfileHandling(t *testing.T) {
ds.ProfileHasACMEPayloadForCommandFunc = func(ctx context.Context, hUUID, cmdUUID string) (fleet.ProfileACMECommandResult, error) {
return fleet.ProfileACMECommandResult{Platform: "ios"}, nil
}
ds.OktaCACleanupTargetForInstallCommandFunc = func(ctx context.Context, hUUID, cmdUUID string) (fleet.OktaCACleanupTarget, bool, error) {
return fleet.OktaCACleanupTarget{}, false, nil
}
_, err := svc.CommandAndReportResults(
&mdm.Request{Context: ctx},
@@ -0,0 +1,83 @@
package service
import (
"context"
_ "embed"
"fmt"
"regexp"
"strings"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
"github.com/fleetdm/fleet/v4/server/fleet"
)
// deleteDuplicateOktaSCEPScript is the macOS shell script that removes
// orphaned duplicate SCEP certificates left in the per-user keychain after
// the Okta conditional access profile is reinstalled or renewed. The
// canonical, customer-facing copy lives at
// docs/solutions/macos/scripts/delete-duplicate-scep-certificates.sh; the
// embed_sync test asserts the two stay byte-identical.
//
//go:embed embedded_scripts/delete-duplicate-scep-certificates.sh
var deleteDuplicateOktaSCEPScript string
// macOS short names are 1-31 chars of ASCII letters, digits, and a small
// set of punctuation. We're conservative here because the value is
// interpolated into a shell command line; single-quote escaping covers the
// rest. Reject anything weird so a malformed nano_users row cannot reach
// the shell.
var validMacOSShortNameRE = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9_.-]*$`)
// buildOktaCACleanupScript wraps the embedded cleanup script with the
// positional arguments it expects (auto-confirm, target user, certificate
// CN). Returns ok=false when the username fails validation, in which case
// the caller should log and skip rather than dispatch a malformed script.
func buildOktaCACleanupScript(username string) (string, bool) {
if len(username) == 0 || len(username) > 31 || !validMacOSShortNameRE.MatchString(username) {
return "", false
}
return fmt.Sprintf("#!/bin/bash\nset -- -y -u %s %s\n%s",
shellSingleQuote(username),
shellSingleQuote(fleet.ConditionalAccessOktaCertificateCN),
deleteDuplicateOktaSCEPScript,
), true
}
// shellSingleQuote returns s wrapped in single quotes, escaping any
// embedded single quotes via the POSIX 'foo'"'"'bar' idiom.
func shellSingleQuote(s string) string {
return "'" + strings.ReplaceAll(s, "'", `'"'"'`) + "'"
}
// maybeRunOktaCACleanupScript schedules the Okta conditional access
// keychain-cleanup script on the host after a successful InstallProfile
// ack for the Okta CA profile. It silently no-ops when the command does
// not apply to the Okta CA profile, when no per-user MDM enrollment short
// name is on record, or when the short name fails validation. Errors are
// returned to the caller for logging; the caller must not fail the ack
// path on them.
func (svc *MDMAppleCheckinAndCommandService) maybeRunOktaCACleanupScript(ctx context.Context, hostUUID, commandUUID string) error {
target, ok, err := svc.ds.OktaCACleanupTargetForInstallCommand(ctx, hostUUID, commandUUID)
if err != nil {
return ctxerr.Wrap(ctx, err, "look up Okta CA cleanup target")
}
if !ok {
return nil
}
script, ok := buildOktaCACleanupScript(target.UserShortName)
if !ok {
svc.logger.DebugContext(ctx, "skip Okta CA keychain cleanup: invalid macOS username",
"host_uuid", hostUUID, "command_uuid", commandUUID)
return nil
}
if _, err := svc.ds.NewInternalHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{
HostID: target.HostID,
ScriptContents: script,
SyncRequest: false,
}); err != nil {
return ctxerr.Wrap(ctx, err, "enqueue Okta CA keychain cleanup script")
}
return nil
}
@@ -0,0 +1,149 @@
package service
import (
"context"
"errors"
"log/slog"
"os"
"strings"
"testing"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/mock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestEmbeddedCleanupScriptMatchesDocs(t *testing.T) {
docs, err := os.ReadFile("../../docs/solutions/macos/scripts/delete-duplicate-scep-certificates.sh")
require.NoError(t, err, "the public Okta CA cleanup script must exist; if you moved it, update the test path")
require.Equal(t, string(docs), deleteDuplicateOktaSCEPScript,
"embedded_scripts/delete-duplicate-scep-certificates.sh has drifted from the docs copy; keep them in sync")
}
func TestBuildOktaCACleanupScript(t *testing.T) {
cases := []struct {
name string
username string
wantOK bool
}{
{"valid simple", "alice", true},
{"valid with dot", "alice.smith", true},
{"valid with underscore prefix", "_servicedesk", true},
{"valid with dash", "alice-smith", true},
{"valid mixed", "Alice.Smith-1", true},
{"empty", "", false},
{"starts with dash", "-alice", false},
{"starts with dot", ".alice", false},
{"contains space", "alice smith", false},
{"contains slash", "alice/smith", false},
{"contains single quote", "ali'ce", false},
{"contains backtick", "ali`ce", false},
{"contains semicolon", "alice;rm -rf /", false},
{"contains shell metachar dollar", "ali$ce", false},
{"too long", strings.Repeat("a", 32), false},
{"exactly 31 chars", strings.Repeat("a", 31), true},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
got, ok := buildOktaCACleanupScript(c.username)
require.Equal(t, c.wantOK, ok)
if !c.wantOK {
assert.Empty(t, got)
return
}
assert.Contains(t, got, "set -- -y -u '"+c.username+"' "+shellSingleQuote(fleet.ConditionalAccessOktaCertificateCN))
assert.Contains(t, got, deleteDuplicateOktaSCEPScript,
"wrapped script must contain the embedded cleanup script verbatim")
})
}
}
func TestShellSingleQuote(t *testing.T) {
cases := map[string]string{
"": "''",
"foo": "'foo'",
"foo bar": "'foo bar'",
"al'ce": `'al'"'"'ce'`,
"$danger`": "'$danger`'",
}
for in, want := range cases {
t.Run(in, func(t *testing.T) {
require.Equal(t, want, shellSingleQuote(in))
})
}
}
func TestMaybeRunOktaCACleanupScript(t *testing.T) {
ctx := context.Background()
const (
hostUUID = "host-uuid"
commandUUID = "cmd-uuid"
hostID = uint(99)
shortName = "alice"
)
probeErrBoom := errors.New("boom")
cases := []struct {
name string
target fleet.OktaCACleanupTarget
targetOK bool
targetErr error
wantErr bool
wantEnqueue bool
wantInvalidLog bool
}{
{
name: "okta CA profile + valid user: enqueues",
target: fleet.OktaCACleanupTarget{HostID: hostID, UserShortName: shortName},
targetOK: true,
wantEnqueue: true,
},
{
name: "not okta CA profile: no enqueue",
targetOK: false,
},
{
name: "lookup error: propagates",
targetErr: probeErrBoom,
wantErr: true,
},
{
name: "invalid username: skipped without error",
target: fleet.OktaCACleanupTarget{HostID: hostID, UserShortName: "ali ce"},
targetOK: true,
wantInvalidLog: true,
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
ds := new(mock.Store)
ds.OktaCACleanupTargetForInstallCommandFunc = func(_ context.Context, hUUID, cmdUUID string) (fleet.OktaCACleanupTarget, bool, error) {
require.Equal(t, hostUUID, hUUID)
require.Equal(t, commandUUID, cmdUUID)
return c.target, c.targetOK, c.targetErr
}
ds.NewInternalHostScriptExecutionRequestFunc = func(_ context.Context, req *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error) {
require.Equal(t, hostID, req.HostID)
require.Contains(t, req.ScriptContents, "set -- -y -u '"+shortName+"' '"+fleet.ConditionalAccessOktaCertificateCN+"'")
require.Contains(t, req.ScriptContents, deleteDuplicateOktaSCEPScript)
require.False(t, req.SyncRequest)
require.Nil(t, req.UserID)
return &fleet.HostScriptResult{}, nil
}
svc := &MDMAppleCheckinAndCommandService{
ds: ds,
logger: slog.New(slog.DiscardHandler),
}
err := svc.maybeRunOktaCACleanupScript(ctx, hostUUID, commandUUID)
if c.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, c.wantEnqueue, ds.NewInternalHostScriptExecutionRequestFuncInvoked)
})
}
}
+1 -1
View File
@@ -392,7 +392,7 @@ func (svc *Service) ConditionalAccessGetIdPAppleProfile(ctx context.Context) (pr
CACertBase64: caCertBase64,
SCEPURL: scepURL,
Challenge: challenge,
CertificateCN: "Fleet conditional access for Okta",
CertificateCN: fleet.ConditionalAccessOktaCertificateCN,
MTLSURL: mtlsURL,
CACertUUID: caCertUUID,
SCEPPayloadUUID: scepPayloadUUID,
@@ -258,8 +258,11 @@ func TestConditionalAccessGetIdPAppleProfile(t *testing.T) {
require.Contains(t, profileStr, "com.fleetdm.conditional-access-preference")
require.Contains(t, profileStr, "com.fleetdm.chrome.certs")
// Verify certificate CN is present in the profile
require.Contains(t, profileStr, "Fleet conditional access for Okta")
// Top-level PayloadIdentifier and certificate CN must match the
// shared constants so the InstallProfile-ack cleanup hook can
// reliably gate on them.
require.Contains(t, profileStr, fleet.ConditionalAccessOktaProfileIdentifier)
require.Contains(t, profileStr, fleet.ConditionalAccessOktaCertificateCN)
// Verify the renewal-ID marker is in the SCEP payload's Subject OU
// so auto-renewal activates by default. Substituted to
@@ -0,0 +1,147 @@
#!/bin/bash
# Deletes orphaned duplicate certificates matching a given CN from the login
# keychain, keeping the most recently issued one (the one tied to the current
# profile).
#
# Usage: ./delete-scep-certs.sh [-y] [-a] [-u username] <certificate common name>
# -y Skip confirmation prompt
# -a Remove all matching certificates (including the newest)
# -u Target a specific user's login keychain (required when running as root)
#
# Example: ./delete-scep-certs.sh "Fleet conditional access for Okta"
set -e
auto_confirm=false
remove_all=false
target_user=""
while getopts "yau:" opt; do
case "$opt" in
y) auto_confirm=true ;;
a) remove_all=true ;;
u) target_user="$OPTARG" ;;
*)
echo "Usage: $0 [-y] [-a] [-u username] <certificate common name>" >&2
exit 1
;;
esac
done
shift $((OPTIND - 1))
if [ $# -eq 0 ]; then
echo "Usage: $0 [-y] [-a] [-u username] <certificate common name>" >&2
echo " -y Skip confirmation prompt" >&2
echo " -a Remove all matching certificates (including the newest)" >&2
echo " -u Target a specific user's login keychain (required when running as root)" >&2
exit 1
fi
CN="$1"
# Resolve the keychain path.
if [ -n "$target_user" ]; then
KEYCHAIN="/Users/$target_user/Library/Keychains/login.keychain-db"
if [ ! -f "$KEYCHAIN" ]; then
echo "Error: keychain not found at $KEYCHAIN" >&2
exit 1
fi
elif [ "$(id -u)" -eq 0 ]; then
echo "Error: running as root without -u flag. Specify the target user with -u <username>." >&2
exit 1
else
KEYCHAIN="login.keychain-db"
fi
# Collect SHA-1 hash and Not Before date for every matching certificate.
# Output is written to a temp file as: <epoch> <hash>
tmpfile=$(mktemp)
trap 'rm -f "$tmpfile" "$tmpfile.raw" "$tmpfile.err"' EXIT
security find-certificate -a -c "$CN" -Z -p "$KEYCHAIN" >"$tmpfile.raw" 2>"$tmpfile.err" || true
# Split the raw output into individual cert blocks and extract hash + date.
current_hash=""
current_pem=""
while IFS= read -r line; do
case "$line" in
"SHA-1 hash:"*)
current_hash=$(echo "$line" | awk '{print $NF}')
;;
"-----BEGIN CERTIFICATE-----")
current_pem="$line"$'\n'
;;
"-----END CERTIFICATE-----")
current_pem+="$line"$'\n'
not_before=$(echo "$current_pem" | openssl x509 -noout -startdate 2>/dev/null | cut -d= -f2)
epoch=$(date -j -f "%b %e %T %Y %Z" "$not_before" "+%s" 2>/dev/null || echo "0")
echo "$epoch $current_hash" >> "$tmpfile"
current_pem=""
;;
*)
if [ -n "$current_pem" ]; then
current_pem+="$line"$'\n'
fi
;;
esac
done < "$tmpfile.raw"
total=$(wc -l < "$tmpfile" | tr -d ' ')
if [ "$total" -eq 0 ]; then
echo "No certificates found matching \"$CN\""
exit 0
fi
if [ "$total" -eq 1 ] && [ "$remove_all" = false ]; then
echo "Only one certificate found matching \"$CN\", nothing to delete."
exit 0
fi
# Sort by epoch descending; the first line is the newest.
newest_hash=$(sort -rn "$tmpfile" | head -1 | awk '{print $2}')
if [ "$remove_all" = true ]; then
echo "Found $total certificate(s) matching \"$CN\""
echo " Will delete: ALL $total certificate(s):"
while read -r epoch hash; do
issued=$(date -r "$epoch" "+%Y-%m-%d %H:%M:%S" 2>/dev/null || echo "unknown")
echo " $hash (issued $issued)"
done < "$tmpfile"
else
to_delete=$((total - 1))
echo "Found $total certificate(s) matching \"$CN\""
echo " Keeping newest: $newest_hash"
echo " Will delete: $to_delete orphaned certificate(s):"
while read -r epoch hash; do
if [ "$hash" = "$newest_hash" ]; then
continue
fi
issued=$(date -r "$epoch" "+%Y-%m-%d %H:%M:%S" 2>/dev/null || echo "unknown")
echo " $hash (issued $issued)"
done < "$tmpfile"
fi
if [ "$auto_confirm" = false ]; then
printf "\nProceed? [y/N] "
read -r answer
if [ "$answer" != "y" ] && [ "$answer" != "Y" ]; then
echo "Aborted."
exit 0
fi
fi
deleted=0
while read -r epoch hash; do
if [ "$remove_all" = false ] && [ "$hash" = "$newest_hash" ]; then
continue
fi
echo "Deleting $hash"
security delete-identity -Z "$hash" "$KEYCHAIN"
deleted=$((deleted + 1))
done < "$tmpfile"
if [ "$remove_all" = true ]; then
echo "Done. Deleted all $deleted certificate(s)."
else
echo "Done. Deleted $deleted orphaned certificate(s), kept 1."
fi