Windows SCEP profile validation accepts CERTIFICATE_RENEWAL_ID (PR 7/9)#45237
Merged
Conversation
Two related changes that prepare Apple profile uploads for the non-proxied renewal flow: 1. Add `$FLEET_VAR_CERTIFICATE_RENEWAL_ID` as the preferred name for the renewal-ID marker variable (per docs PR #44069). The legacy `$FLEET_VAR_SCEP_RENEWAL_ID` is still accepted by validation and substitution for back-compat with existing profiles. Both names substitute to the same value: "fleet-" + profile_uuid. 2. Add additionalACMEValidation: any profile containing a com.apple.security.acme payload must include either renewal-ID variable in the cert Subject's CN or OU, otherwise the upload is rejected. Without the marker the resulting cert can't be linked back to its profile and would never auto-renew. The shared SCEP-renewal-ID-without-URL/Challenge guard in mdm_profiles.go skips its check when an ACME payload is present — ACME profiles use the marker variable alone (no SCEP URL/Challenge companions). Validation error messages reference only the new CERTIFICATE_RENEWAL_ID name to steer new authoring toward it.
The validation error messages in mdm_profiles.go switched to reference the preferred CERTIFICATE_RENEWAL_ID name. The unit and integration test assertions still required the legacy SCEP_RENEWAL_ID text. Updating them to match.
Decision 2.7 requires profile validation to keep accepting the pre-rename variable name. The existing failure-case tests don't guard that contract because they all exercise error paths. Add a happy-path case asserting that a Custom SCEP profile authored with $FLEET_VAR_SCEP_RENEWAL_ID validates successfully when all other required variables are present.
…leetdm/fleet into pr-2.3-apple-validation-rename
The renewal-ID-only bypass in validateProfileCertificateAuthorityVariables checked the profile body with strings.Contains, which would false-positive on "com.apple.security.acme" appearing in a description, identifier, or custom string field. Switch to the existing Mobileconfig.HasPayloadType helper for a structured plist parse. Parse failure (e.g., Windows-path content that isn't a mobileconfig) is treated as no-ACME, so the SCEP-only error fires normally on that path.
Extend the SubjectName OU= check in additionalNDESValidationForWindowsProfiles and additionalCustomSCEPValidationForWindowsProfiles to accept either the preferred $FLEET_VAR_CERTIFICATE_RENEWAL_ID or the legacy $FLEET_VAR_SCEP_RENEWAL_ID. Error messages reference only the preferred variable name. New constant added to fleetVarsSupportedInWindowsProfiles.
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
WalkthroughWindows MDM profile validation now recognizes the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 40639-cert-renew #45237 +/- ##
===================================================
Coverage ? 66.75%
===================================================
Files ? 2730
Lines ? 218570
Branches ? 10703
===================================================
Hits ? 145913
Misses ? 59447
Partials ? 13210
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…aw-scep-validation
…aw-scep-validation
Base automatically changed from
pr-2.3-apple-validation-rename
to
40639-cert-renew
May 12, 2026 22:10
…aw-scep-validation # Conflicts: # server/fleet/mdm.go # server/service/apple_mdm.go # server/service/apple_mdm_test.go
Reject raw-SCEP profiles (com.apple.security.scep payload, no Fleet proxy variables) whose Subject OU lacks $FLEET_VAR_CERTIFICATE_RENEWAL_ID. Profiles using Fleet proxy variables (NDES/Custom SCEP/Smallstep) keep their existing per-CA validators. Net-new validator: OU placement only, preferred variable name only — no back-compat owed on a surface that didn't previously enforce renewal-ID at all. Also broadens the validateConfigProfileFleetVariables bypass to recognize both ACME and SCEP payloads via mobileconfig.HasPayloadType.
…aw-scep-validation
…o pr-2.4-windows-validation
mostlikelee
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
sgress454
approved these changes
May 14, 2026
Contributor
sgress454
left a comment
sgress454
left a comment
There was a problem hiding this comment.
looks good, did a quick search in the feature branch for FleetVarSCEPRenewalID and it looks like all the other relevant cases are covered 👍
mostlikelee
added a commit
that referenced
this pull request
May 14, 2026
… 1.4) Status sweep: - PR 2.1 (#44344), PR 2.2 (#44345), PR 2.3 (#45043), PR 2.3b (#45364), PR 2.4 (#45237) all merged — task lists in §5/§6/§7/§7b/§8 now reflect completion. Items deferred to PR 2.5 (release notes) stay open. - §10.1 annotated with the 2026-05-14 local end-to-end validation result. Hydrant verification still required for customer readiness. New refinement — Decision 1.4 and §7c (PR 2.3c): - Surfaced during local validation: a user-installed Root CA ended up with origin=mdm because MDM CertificateList returned the entire keychain on InstallProfile ack, beating the next osquery sync. - Decision: osquery sync downgrades existing origin=mdm rows to origin=osquery when it also observes them. One-way (mdm → osquery, never reverse). - Rationale: osquery sees a strict superset of MDM CertificateList, so observation by both implies the cert is in the device keychain by some path — not evidence of MDM delivery. Origin should reflect "did Fleet deliver this?", not "who saw it first?" - Renewal correctness is unaffected; matcher keys on the fleet-<profile_uuid> marker, not on origin.
2 tasks
mostlikelee
added a commit
that referenced
this pull request
May 15, 2026
Adds end-to-end coverage for the public profile-upload API surface under Decision 2.6 (marker is opt-in). Four tests: - TestACMEProfileUploadAcceptsAllMarkerPlacements: 4-case matrix (preferred/legacy/no-marker × OU/CN placement) confirms acceptance across the board for com.apple.security.acme profiles. - TestRawSCEPProfileUploadAcceptsAllMarkerPlacements: same matrix for com.apple.security.scep payloads without Fleet proxy variables. - TestConditionalAccessProfileUploadsCleanly: renders Fleet's generated Conditional Access SCEP profile and confirms clean upload via custom OS settings. - TestWindowsSCEPProfilePreferredVariableAccepted: covers PR #45237's pre-existing-surface back-compat (NDES / Custom SCEP proxy SCEP accept the preferred variable name).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #44347
What this PR does
Extends the SubjectName OU= check in both Windows SCEP validators (NDES, Custom SCEP) to accept either
\$FLEET_VAR_CERTIFICATE_RENEWAL_ID(preferred) or the legacy\$FLEET_VAR_SCEP_RENEWAL_ID. Validation error messages reference only the preferred name.Why this is needed
Pre-existing Windows SCEP validation only accepted
\$FLEET_VAR_SCEP_RENEWAL_ID. The variable was renamed customer-facing to\$FLEET_VAR_CERTIFICATE_RENEWAL_ID(docs PR #44069); without this change, Windows would reject customers who follow the new docs.Why preferred + legacy here, preferred-only on Apple
Per Decision 2.7's net-new-vs-pre-existing rule:
\$FLEET_VAR_CERTIFICATE_RENEWAL_ID.Checklist for submitter
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)Testing
Summary by CodeRabbit
CERTIFICATE_RENEWAL_IDFleet variable in Windows MDM configuration profiles, alongside the existing legacy SCEP renewal variable.