From 2aef3c5c1ee12047adcb9d74c09012ddfd45e61a Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Fri, 8 May 2026 11:17:28 -0600 Subject: [PATCH 01/11] Apple ACME profile validation + accept CERTIFICATE_RENEWAL_ID variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ...0639-acme-validation-and-renewal-id-rename | 2 + server/fleet/errors.go | 4 +- server/fleet/mdm.go | 27 ++++-- server/fleet/mdm_test.go | 22 +++++ server/mdm/apple/profile_processor.go | 13 +-- server/mdm/microsoft/profile_variables.go | 6 +- server/service/apple_mdm.go | 89 +++++++++++++++++-- server/service/apple_mdm_test.go | 84 ++++++++++++++++- server/service/mdm_profiles.go | 27 +++--- 9 files changed, 236 insertions(+), 38 deletions(-) create mode 100644 changes/40639-acme-validation-and-renewal-id-rename diff --git a/changes/40639-acme-validation-and-renewal-id-rename b/changes/40639-acme-validation-and-renewal-id-rename new file mode 100644 index 00000000000..4c97248d168 --- /dev/null +++ b/changes/40639-acme-validation-and-renewal-id-rename @@ -0,0 +1,2 @@ +* Reject Apple ACME profile uploads that don't include a renewal-ID marker variable in the cert Subject CN or OU. Without the marker the cert can't be linked back to its profile and won't auto-renew. +* Add `$FLEET_VAR_CERTIFICATE_RENEWAL_ID` as the preferred Apple/Windows profile variable for the renewal-ID marker. The legacy `$FLEET_VAR_SCEP_RENEWAL_ID` is still accepted for back-compat with existing profiles. diff --git a/server/fleet/errors.go b/server/fleet/errors.go index e2aae7f39ce..8b53da057f3 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -531,8 +531,8 @@ const ( // Error message variables var ( - NDESSCEPVariablesMissingErrMsg = fmt.Sprintf("SCEP profile for NDES certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", FleetVarNDESSCEPChallenge, FleetVarNDESSCEPProxyURL, FleetVarSCEPRenewalID) - SCEPRenewalIDWithoutURLChallengeErrMsg = "Variable \"$FLEET_VAR_" + string(FleetVarSCEPRenewalID) + "\" can't be used if variables for SCEP URL and Challenge are not specified." + NDESSCEPVariablesMissingErrMsg = fmt.Sprintf("SCEP profile for NDES certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", FleetVarNDESSCEPChallenge, FleetVarNDESSCEPProxyURL, FleetVarCertificateRenewalID) + SCEPRenewalIDWithoutURLChallengeErrMsg = "Variable \"$FLEET_VAR_" + string(FleetVarCertificateRenewalID) + "\" can't be used if variables for SCEP URL and Challenge are not specified." ) const ( diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go index 9fe3bde0156..d808e9a1d85 100644 --- a/server/fleet/mdm.go +++ b/server/fleet/mdm.go @@ -76,9 +76,14 @@ const ( FleetVarHostPlatform FleetVarName = "HOST_PLATFORM" // Certificate authority variables - FleetVarNDESSCEPChallenge FleetVarName = "NDES_SCEP_CHALLENGE" - FleetVarNDESSCEPProxyURL FleetVarName = "NDES_SCEP_PROXY_URL" - FleetVarSCEPRenewalID FleetVarName = "SCEP_RENEWAL_ID" + FleetVarNDESSCEPChallenge FleetVarName = "NDES_SCEP_CHALLENGE" + FleetVarNDESSCEPProxyURL FleetVarName = "NDES_SCEP_PROXY_URL" + FleetVarSCEPRenewalID FleetVarName = "SCEP_RENEWAL_ID" + // FleetVarCertificateRenewalID is the preferred name for the renewal-ID + // marker variable as of 4.86 (PR #44069). The legacy SCEP_RENEWAL_ID name + // remains accepted for back-compat with profiles authored against earlier + // docs. Both substitute to "fleet-" + profile_uuid. + FleetVarCertificateRenewalID FleetVarName = "CERTIFICATE_RENEWAL_ID" FleetVarDigiCertDataPrefix FleetVarName = "DIGICERT_DATA_" FleetVarDigiCertPasswordPrefix FleetVarName = "DIGICERT_PASSWORD_" // nolint:gosec // G101: Potential hardcoded credentials FleetVarCustomSCEPChallengePrefix FleetVarName = "CUSTOM_SCEP_CHALLENGE_" @@ -97,7 +102,7 @@ const ( func HasCAVariables(fleetVars []string) bool { for _, v := range fleetVars { if v == string(FleetVarNDESSCEPChallenge) || v == string(FleetVarNDESSCEPProxyURL) || - v == string(FleetVarSCEPRenewalID) || v == string(FleetVarSCEPWindowsCertificateID) || + v == string(FleetVarSCEPRenewalID) || v == string(FleetVarCertificateRenewalID) || v == string(FleetVarSCEPWindowsCertificateID) || strings.HasPrefix(v, string(FleetVarDigiCertDataPrefix)) || strings.HasPrefix(v, string(FleetVarDigiCertPasswordPrefix)) || strings.HasPrefix(v, string(FleetVarCustomSCEPChallengePrefix)) || strings.HasPrefix(v, string(FleetVarCustomSCEPProxyURLPrefix)) || strings.HasPrefix(v, string(FleetVarSmallstepSCEPChallengePrefix)) || strings.HasPrefix(v, string(FleetVarSmallstepSCEPProxyURLPrefix)) { @@ -118,9 +123,17 @@ var ( FleetVarNDESSCEPProxyURLRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarNDESSCEPProxyURL)) FleetVarHostEndUserIDPFullnameRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarHostEndUserIDPFullname)) FleetVarSCEPRenewalIDRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarSCEPRenewalID)) - FleetVarHostUUIDRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarHostUUID)) - FleetVarHostPlatformRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarHostPlatform)) - FleetVarSCEPWindowsCertificateIDRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarSCEPWindowsCertificateID)) + FleetVarCertificateRenewalIDRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarCertificateRenewalID)) + // FleetVarRenewalIDRegexp matches either the preferred CERTIFICATE_RENEWAL_ID + // or the legacy SCEP_RENEWAL_ID name. Use this for validation checks where + // either form satisfies the requirement. + FleetVarRenewalIDRegexp = regexp.MustCompile(fmt.Sprintf( + `(\$FLEET_VAR_%[1]s)|(\${FLEET_VAR_%[1]s})|(\$FLEET_VAR_%[2]s)|(\${FLEET_VAR_%[2]s})`, + FleetVarCertificateRenewalID, FleetVarSCEPRenewalID, + )) + FleetVarHostUUIDRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarHostUUID)) + FleetVarHostPlatformRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarHostPlatform)) + FleetVarSCEPWindowsCertificateIDRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%[1]s})`, FleetVarSCEPWindowsCertificateID)) // Fleet variable replacement failed errors HostEndUserEmailIDPVariableReplacementFailedError = fmt.Sprintf("There is no IdP email for this host. "+ diff --git a/server/fleet/mdm_test.go b/server/fleet/mdm_test.go index 1a1e1e87146..bb79152906b 100644 --- a/server/fleet/mdm_test.go +++ b/server/fleet/mdm_test.go @@ -569,6 +569,7 @@ func TestHasCAVariables(t *testing.T) { {"NDES challenge", []string{string(fleet.FleetVarHostUUID), string(fleet.FleetVarNDESSCEPChallenge)}, true}, {"NDES proxy URL", []string{string(fleet.FleetVarNDESSCEPProxyURL)}, true}, {"SCEP renewal", []string{string(fleet.FleetVarSCEPRenewalID)}, true}, + {"Certificate renewal (preferred)", []string{string(fleet.FleetVarCertificateRenewalID)}, true}, {"DigiCert data", []string{string(fleet.FleetVarDigiCertDataPrefix) + "my_ca"}, true}, {"DigiCert password", []string{string(fleet.FleetVarDigiCertPasswordPrefix) + "my_ca"}, true}, {"Custom SCEP challenge", []string{string(fleet.FleetVarCustomSCEPChallengePrefix) + "my_ca"}, true}, @@ -587,6 +588,27 @@ func TestHasCAVariables(t *testing.T) { } } +func TestFleetVarRenewalIDRegexp(t *testing.T) { + cases := []struct { + input string + want bool + }{ + {"$FLEET_VAR_CERTIFICATE_RENEWAL_ID", true}, + {"${FLEET_VAR_CERTIFICATE_RENEWAL_ID}", true}, + {"$FLEET_VAR_SCEP_RENEWAL_ID", true}, + {"${FLEET_VAR_SCEP_RENEWAL_ID}", true}, + {"prefix $FLEET_VAR_CERTIFICATE_RENEWAL_ID suffix", true}, + {"$FLEET_VAR_OTHER_VAR", false}, + {"static-value", false}, + {"", false}, + } + for _, tc := range cases { + t.Run(tc.input, func(t *testing.T) { + require.Equal(t, tc.want, fleet.FleetVarRenewalIDRegexp.MatchString(tc.input)) + }) + } +} + func TestFilterMacOSOnlyProfilesFromIOSIPadOS(t *testing.T) { for _, tc := range []struct { profiles []*fleet.MDMAppleProfilePayload diff --git a/server/mdm/apple/profile_processor.go b/server/mdm/apple/profile_processor.go index e23d0ae5452..d3fb4bf487e 100644 --- a/server/mdm/apple/profile_processor.go +++ b/server/mdm/apple/profile_processor.go @@ -204,7 +204,7 @@ func preprocessProfileContents( // In the future we should expand variablesUpdatedAt logic to include non-CA variables as // well for _, fleetVar := range fleetVars { - if fleetVar == string(fleet.FleetVarSCEPRenewalID) || + if fleetVar == string(fleet.FleetVarSCEPRenewalID) || fleetVar == string(fleet.FleetVarCertificateRenewalID) || fleetVar == string(fleet.FleetVarNDESSCEPChallenge) || fleetVar == string(fleet.FleetVarNDESSCEPProxyURL) || fleetVar == string(fleet.FleetVarHostUUID) || strings.HasPrefix(fleetVar, string(fleet.FleetVarSmallstepSCEPChallengePrefix)) || strings.HasPrefix(fleetVar, string(fleet.FleetVarSmallstepSCEPProxyURLPrefix)) || strings.HasPrefix(fleetVar, string(fleet.FleetVarDigiCertPasswordPrefix)) || strings.HasPrefix(fleetVar, string(fleet.FleetVarDigiCertDataPrefix)) || @@ -230,7 +230,8 @@ func preprocessProfileContents( case fleetVar == string(fleet.FleetVarHostEndUserEmailIDP) || fleetVar == string(fleet.FleetVarHostHardwareSerial) || fleetVar == string(fleet.FleetVarHostPlatform) || fleetVar == string(fleet.FleetVarHostEndUserIDPUsername) || fleetVar == string(fleet.FleetVarHostEndUserIDPUsernameLocalPart) || - fleetVar == string(fleet.FleetVarHostEndUserIDPGroups) || fleetVar == string(fleet.FleetVarHostEndUserIDPDepartment) || fleetVar == string(fleet.FleetVarSCEPRenewalID) || + fleetVar == string(fleet.FleetVarHostEndUserIDPGroups) || fleetVar == string(fleet.FleetVarHostEndUserIDPDepartment) || + fleetVar == string(fleet.FleetVarSCEPRenewalID) || fleetVar == string(fleet.FleetVarCertificateRenewalID) || fleetVar == string(fleet.FleetVarHostEndUserIDPFullname) || fleetVar == string(fleet.FleetVarHostUUID): // No extra validation needed for these variables @@ -398,10 +399,12 @@ func preprocessProfileContents( // Insert the SCEP URL into the profile contents hostContents = profiles.ReplaceNDESSCEPProxyURLVariable(appConfig.MDMUrl(), hostUUID, profUUID, hostContents) - case fleetVar == string(fleet.FleetVarSCEPRenewalID): - // Insert the SCEP renewal ID into the SCEP Payload CN or OU + case fleetVar == string(fleet.FleetVarSCEPRenewalID), fleetVar == string(fleet.FleetVarCertificateRenewalID): + // Insert the renewal ID into the SCEP/ACME Payload CN or OU. + // Both legacy SCEP_RENEWAL_ID and the preferred + // CERTIFICATE_RENEWAL_ID substitute to the same value. fleetRenewalID := "fleet-" + profUUID - hostContents = profiles.ReplaceFleetVariableInXML(fleet.FleetVarSCEPRenewalIDRegexp, hostContents, fleetRenewalID) + hostContents = profiles.ReplaceFleetVariableInXML(fleet.FleetVarRenewalIDRegexp, hostContents, fleetRenewalID) case strings.HasPrefix(fleetVar, string(fleet.FleetVarCustomSCEPChallengePrefix)): replacedContents, replacedVariable, err := profiles.ReplaceCustomSCEPChallengeVariable(ctx, logger, fleetVar, customSCEPCAs, hostContents) diff --git a/server/mdm/microsoft/profile_variables.go b/server/mdm/microsoft/profile_variables.go index eb2ca978364..ec4e4797ace 100644 --- a/server/mdm/microsoft/profile_variables.go +++ b/server/mdm/microsoft/profile_variables.go @@ -121,8 +121,10 @@ func preprocessWindowsProfileContents(deps ProfilePreprocessDependencies, params switch { case fleetVar == string(fleet.FleetVarSCEPWindowsCertificateID): result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarSCEPWindowsCertificateIDRegexp, result, params.ProfileUUID) - case fleetVar == string(fleet.FleetVarSCEPRenewalID): - result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarSCEPRenewalIDRegexp, result, "fleet-"+params.ProfileUUID) + case fleetVar == string(fleet.FleetVarSCEPRenewalID), fleetVar == string(fleet.FleetVarCertificateRenewalID): + // Both legacy SCEP_RENEWAL_ID and the preferred CERTIFICATE_RENEWAL_ID + // substitute to the same value. + result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarRenewalIDRegexp, result, "fleet-"+params.ProfileUUID) case strings.HasPrefix(fleetVar, string(fleet.FleetVarCustomSCEPChallengePrefix)): caName := strings.TrimPrefix(fleetVar, string(fleet.FleetVarCustomSCEPChallengePrefix)) err := profiles.IsCustomSCEPConfigured(deps.Context, deps.CustomSCEPCAs, caName, fleetVar, func(errMsg string) error { diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index daafa9a4617..dcd14e71afa 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -71,7 +71,8 @@ const ( var fleetVarsSupportedInAppleConfigProfiles = []fleet.FleetVarName{ fleet.FleetVarNDESSCEPChallenge, fleet.FleetVarNDESSCEPProxyURL, fleet.FleetVarHostEndUserEmailIDP, fleet.FleetVarHostHardwareSerial, fleet.FleetVarHostEndUserIDPUsername, fleet.FleetVarHostEndUserIDPUsernameLocalPart, - fleet.FleetVarHostEndUserIDPGroups, fleet.FleetVarHostEndUserIDPDepartment, fleet.FleetVarHostEndUserIDPFullname, fleet.FleetVarSCEPRenewalID, + fleet.FleetVarHostEndUserIDPGroups, fleet.FleetVarHostEndUserIDPDepartment, fleet.FleetVarHostEndUserIDPFullname, + fleet.FleetVarSCEPRenewalID, fleet.FleetVarCertificateRenewalID, fleet.FleetVarHostUUID, fleet.FleetVarHostPlatform, } @@ -510,6 +511,16 @@ func CheckProfileIsNotSigned(data []byte) error { } func validateConfigProfileFleetVariables(contents string, lic *fleet.LicenseInfo, groupedCAs *fleet.GroupedCertificateAuthorities) ([]string, error) { + // ACME payloads must carry the renewal-ID marker in Subject CN/OU so the + // non-proxied renewal mechanism can later link the issued cert back to the + // installing profile. This check runs before the Fleet-variable early + // return because an ACME profile with no other Fleet variables would + // otherwise sneak through and never auto-renew. Fast-path on a substring + // match avoids parsing for the common (non-ACME) case. + if err := additionalACMEValidation(contents); err != nil { + return nil, err + } + fleetVars := variables.Find(contents) if len(fleetVars) == 0 { return nil, nil @@ -649,8 +660,8 @@ func additionalCustomSCEPValidation(contents string, customSCEPVars *CustomSCEPV } foundCAs = append(foundCAs, ca) } - if !fleet.FleetVarSCEPRenewalIDRegexp.MatchString(scepPayloadContent.CommonName) && !fleet.FleetVarSCEPRenewalIDRegexp.MatchString(scepPayloadContent.OrganizationalUnit) { - return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarSCEPRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} + if !fleet.FleetVarRenewalIDRegexp.MatchString(scepPayloadContent.CommonName) && !fleet.FleetVarRenewalIDRegexp.MatchString(scepPayloadContent.OrganizationalUnit) { + return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} } if len(foundCAs) < len(customSCEPVars.CAs()) { for _, ca := range customSCEPVars.CAs() { @@ -702,8 +713,8 @@ func additionalSmallstepValidation(contents string, smallstepVars *SmallstepVars } foundCAs = append(foundCAs, ca) } - if !fleet.FleetVarSCEPRenewalIDRegexp.MatchString(scepPayloadContent.CommonName) && !fleet.FleetVarSCEPRenewalIDRegexp.MatchString(scepPayloadContent.OrganizationalUnit) { - return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarSCEPRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} + if !fleet.FleetVarRenewalIDRegexp.MatchString(scepPayloadContent.CommonName) && !fleet.FleetVarRenewalIDRegexp.MatchString(scepPayloadContent.OrganizationalUnit) { + return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} } if len(foundCAs) < len(smallstepVars.CAs()) { for _, ca := range smallstepVars.CAs() { @@ -715,6 +726,70 @@ func additionalSmallstepValidation(contents string, smallstepVars *SmallstepVars return nil } +// acmePayloadForValidation captures just the fields needed to enforce the +// renewal-ID marker requirement. ACME payload shape differs from SCEP: Subject +// is a sibling of PayloadType rather than nested under PayloadContent. +type acmePayloadForValidation struct { + PayloadType string `plist:"PayloadType"` + Subject [][][]string `plist:"Subject"` +} + +type acmeProfileForValidation struct { + PayloadContent []acmePayloadForValidation `plist:"PayloadContent"` +} + +// additionalACMEValidation checks that any com.apple.security.acme payload in +// the profile carries the renewal-ID marker variable in the cert Subject's CN +// or OU. Either the preferred $FLEET_VAR_CERTIFICATE_RENEWAL_ID or the legacy +// $FLEET_VAR_SCEP_RENEWAL_ID is accepted. Without the marker, the cert issued +// by the device's ACME exchange cannot be linked back to its profile and will +// never be auto-renewed. +func additionalACMEValidation(contents string) error { + if !strings.Contains(contents, "com.apple.security.acme") { + return nil + } + // Strip variables embedded in elements so the plist unmarshal + // doesn't choke on them (consistent with unmarshalSCEPProfile). + contents = variables.ProfileDataVariableRegex.ReplaceAllString(contents, "") + var acmeProf acmeProfileForValidation + if err := plist.Unmarshal([]byte(contents), &acmeProf); err != nil { + return &fleet.BadRequestError{Message: fmt.Sprintf("Failed to parse ACME payload with Fleet variables: %s", err.Error())} + } + for _, payload := range acmeProf.PayloadContent { + if payload.PayloadType != "com.apple.security.acme" { + continue + } + var commonName, orgUnit strings.Builder + for _, rdn := range payload.Subject { + for _, kv := range rdn { + if len(kv) != 2 { + continue + } + switch kv[0] { + case "CN": + if commonName.Len() > 0 { + commonName.WriteByte(',') + } + commonName.WriteString(kv[1]) + case "OU": + if orgUnit.Len() > 0 { + orgUnit.WriteByte(',') + } + orgUnit.WriteString(kv[1]) + } + } + } + if !fleet.FleetVarRenewalIDRegexp.MatchString(commonName.String()) && + !fleet.FleetVarRenewalIDRegexp.MatchString(orgUnit.String()) { + return &fleet.BadRequestError{ + Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + + " must be in the ACME certificate's organizational unit (OU).", + } + } + } + return nil +} + func checkThatOnlyOneSCEPPayloadIsPresent(scepProf SCEPProfileContent) (SCEPPayloadContent, error) { scepPayloadsFound := 0 var scepPayloadContent SCEPPayloadContent @@ -824,8 +899,8 @@ func additionalNDESValidation(contents string, ndesVars *NDESVarsFound) error { return err } - if !fleet.FleetVarSCEPRenewalIDRegexp.MatchString(scepPayloadContent.CommonName) && !fleet.FleetVarSCEPRenewalIDRegexp.MatchString(scepPayloadContent.OrganizationalUnit) { - return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarSCEPRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} + if !fleet.FleetVarRenewalIDRegexp.MatchString(scepPayloadContent.CommonName) && !fleet.FleetVarRenewalIDRegexp.MatchString(scepPayloadContent.OrganizationalUnit) { + return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} } // Check for the exact match on challenge and URL diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 139d7898596..0c8db5a761d 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -7061,7 +7061,7 @@ func TestValidateConfigProfileFleetVariables(t *testing.T) { profile: customSCEPForValidationWithoutRenewalID("$FLEET_VAR_CUSTOM_SCEP_CHALLENGE_scepName", "$FLEET_VAR_CUSTOM_SCEP_PROXY_URL_scepName", "$FLEET_VAR_SCEP_RENEWAL_ID", "com.apple.security.scep"), - errMsg: "Variable $FLEET_VAR_SCEP_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", + errMsg: "Variable $FLEET_VAR_CERTIFICATE_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", }, { name: "Custom SCEP profile is not scep", @@ -7140,7 +7140,7 @@ func TestValidateConfigProfileFleetVariables(t *testing.T) { profile: customSCEPForValidationWithoutRenewalID("$FLEET_VAR_NDES_SCEP_CHALLENGE", "$FLEET_VAR_NDES_SCEP_PROXY_URL", "$FLEET_VAR_SCEP_RENEWAL_ID", "com.apple.security.scep"), - errMsg: "Variable $FLEET_VAR_SCEP_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", + errMsg: "Variable $FLEET_VAR_CERTIFICATE_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", }, { name: "NDES profile is not scep", @@ -7232,13 +7232,13 @@ func TestValidateConfigProfileFleetVariables(t *testing.T) { profile: customSCEPForValidationWithoutRenewalID("$FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_smallstepName", "$FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_smallstepName", "$FLEET_VAR_SCEP_RENEWAL_ID", "com.apple.security.scep"), - errMsg: "Variable $FLEET_VAR_SCEP_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", + errMsg: "Variable $FLEET_VAR_CERTIFICATE_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", }, { name: "Smallstep renewal ID in both CN and OU", profile: customSCEPWithOURenewalIDForValidation("${FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_smallstepName}", "${FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_smallstepName}", "Name $FLEET_VAR_SCEP_RENEWAL_ID", "com.apple.security.scep"), - errMsg: "Variable $FLEET_VAR_SCEP_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", + errMsg: "Variable $FLEET_VAR_CERTIFICATE_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", }, { name: "Smallstep challenge is not a fleet variable", @@ -7280,6 +7280,82 @@ func TestValidateConfigProfileFleetVariables(t *testing.T) { } } +func TestValidateConfigProfileFleetVariablesACMEAndRenewalIDRename(t *testing.T) { + t.Parallel() + premiumLic := &fleet.LicenseInfo{Tier: fleet.TierPremium} + groupedCAs := &fleet.GroupedCertificateAuthorities{} + + const acmeProfile = ` + + + + PayloadContent + + + PayloadType + com.apple.security.acme + PayloadIdentifier + com.test.acme + PayloadUUID + 11111111-2222-3333-4444-555555555555 + DirectoryURL + https://acme.example.com/directory + Subject + + CNdevice-cn + OU%s + + + + PayloadIdentifier + com.test.profile + PayloadType + Configuration + PayloadUUID + aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee + PayloadVersion + 1 + +` + + t.Run("ACME profile with preferred CERTIFICATE_RENEWAL_ID is accepted", func(t *testing.T) { + profile := fmt.Sprintf(acmeProfile, "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) + require.NoError(t, err) + }) + + t.Run("ACME profile with legacy SCEP_RENEWAL_ID is accepted", func(t *testing.T) { + profile := fmt.Sprintf(acmeProfile, "$FLEET_VAR_SCEP_RENEWAL_ID") + _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) + require.NoError(t, err) + }) + + t.Run("ACME profile missing renewal-ID marker is rejected", func(t *testing.T) { + profile := fmt.Sprintf(acmeProfile, "static-ou-value") + _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) + require.Error(t, err) + require.Contains(t, err.Error(), "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + require.Contains(t, err.Error(), "ACME certificate") + require.NotContains(t, err.Error(), "SCEP_RENEWAL_ID", + "error message must not advertise the legacy variable name") + }) + + t.Run("Non-ACME profile without Fleet vars is unaffected", func(t *testing.T) { + const wifiProfile = ` + + + + PayloadTypeConfiguration + PayloadIdentifiercom.test.wifi + PayloadUUIDaaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee + PayloadVersion1 + +` + _, err := validateConfigProfileFleetVariables(wifiProfile, premiumLic, groupedCAs) + require.NoError(t, err) + }) +} + func TestValidateDeclarationFleetVariables(t *testing.T) { t.Parallel() diff --git a/server/service/mdm_profiles.go b/server/service/mdm_profiles.go index 8fd7747166e..d88c8ab38eb 100644 --- a/server/service/mdm_profiles.go +++ b/server/service/mdm_profiles.go @@ -193,7 +193,7 @@ func (cs *CustomSCEPVarsFound) ErrorMessage() string { } if !cs.renewalIdFound || len(cs.challengeCA) == 0 || len(cs.urlCA) == 0 { - return fmt.Sprintf("SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", fleet.FleetVarCustomSCEPChallengePrefix, fleet.FleetVarCustomSCEPProxyURLPrefix, fleet.FleetVarSCEPRenewalID) + return fmt.Sprintf("SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", fleet.FleetVarCustomSCEPChallengePrefix, fleet.FleetVarCustomSCEPProxyURLPrefix, fleet.FleetVarCertificateRenewalID) } for ca := range cs.challengeCA { @@ -296,7 +296,7 @@ func (cs *SmallstepVarsFound) ErrorMessage() string { return fleet.SCEPRenewalIDWithoutURLChallengeErrMsg } if !cs.renewalIdFound || len(cs.challengeCA) == 0 || len(cs.urlCA) == 0 { - return fmt.Sprintf("SCEP profile for Smallstep certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", fleet.FleetVarSmallstepSCEPChallengePrefix, fleet.FleetVarSmallstepSCEPProxyURLPrefix, fleet.FleetVarSCEPRenewalID) + return fmt.Sprintf("SCEP profile for Smallstep certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", fleet.FleetVarSmallstepSCEPChallengePrefix, fleet.FleetVarSmallstepSCEPProxyURLPrefix, fleet.FleetVarCertificateRenewalID) } for ca := range cs.challengeCA { if _, ok := cs.urlCA[ca]; !ok { @@ -454,10 +454,11 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f case k == string(fleet.FleetVarNDESSCEPChallenge): caFound = true ndesVars, ok = ndesVars.SetChallenge() - case k == string(fleet.FleetVarSCEPRenewalID): + case k == string(fleet.FleetVarSCEPRenewalID), k == string(fleet.FleetVarCertificateRenewalID): caFound = true - // This is kind of a goofy way of doing things but essentially, since custom SCEP, NDES, and Smallstep - // share the renewal ID Fleet variable, we need to set the + // Custom SCEP, NDES, and Smallstep all share the renewal-ID + // Fleet variable. The legacy SCEP_RENEWAL_ID and the preferred + // CERTIFICATE_RENEWAL_ID names are interchangeable here. customSCEPVars, ok = customSCEPVars.SetRenewalID() if ok { @@ -473,9 +474,10 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f return &fleet.BadRequestError{Message: fmt.Sprintf("Fleet variable $FLEET_VAR_%s does not exist.", k)} } - if k == string(fleet.FleetVarSCEPRenewalID) { - // Special message for renewal ID - return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_SCEP_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU)."} + if k == string(fleet.FleetVarSCEPRenewalID) || k == string(fleet.FleetVarCertificateRenewalID) { + // Special message for renewal ID — surface the preferred name + // in the error regardless of which form the user authored. + return &fleet.BadRequestError{Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + " must be in the SCEP certificate's organizational unit (OU)."} } return &fleet.BadRequestError{Message: fmt.Sprintf("Fleet variable $FLEET_VAR_%s is already present in configuration profile.", k)} @@ -505,9 +507,12 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if smallstepVars.RenewalOnly() { smallstepVars = nil } - // If only the renewal ID variable appeared without any of its associated variables, return an error. It is shared - // by the 3 CA types but is only allowed when CA vars are in use - if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil { + // If only the renewal ID variable appeared without any of its associated SCEP variables, + // return an error — UNLESS the profile contains an ACME payload, where the renewal-ID + // variable stands alone (no SCEP URL/Challenge) and validation lives in + // additionalACMEValidation upstream. + if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && + !strings.Contains(profileContents, "com.apple.security.acme") { return &fleet.BadRequestError{Message: fleet.SCEPRenewalIDWithoutURLChallengeErrMsg} } } From c227e4f25710648f927f640974d6401c69889c78 Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Mon, 11 May 2026 13:32:17 -0600 Subject: [PATCH 02/11] Update tests to expect CERTIFICATE_RENEWAL_ID in error messages 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. --- server/service/integration_mdm_profiles_test.go | 2 +- server/service/mdm_profiles_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index c3dad91f0e2..6062d9e3518 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -8318,7 +8318,7 @@ func testWindowsSCEPProfile(s *integrationMDMTestSuite, windowsScepProfile []byt }}, http.StatusBadRequest) errMsg = extractServerErrorText(resp.Body) - require.Contains(t, errMsg, "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_SCEP_RENEWAL_ID variables") + require.Contains(t, errMsg, "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables") s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ diff --git a/server/service/mdm_profiles_test.go b/server/service/mdm_profiles_test.go index d61add72596..0980565ce77 100644 --- a/server/service/mdm_profiles_test.go +++ b/server/service/mdm_profiles_test.go @@ -91,20 +91,20 @@ func TestValidateProfileCertificateAuthorityVariables(t *testing.T) { { name: "Custom SCEP challenge missing", profile: customSCEPForValidation("challenge", "$FLEET_VAR_CUSTOM_SCEP_PROXY_URL_scepName", "Name", "com.apple.security.scep"), - errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_SCEP_RENEWAL_ID variables.", + errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables.", }, { name: "Custom SCEP url missing", profile: customSCEPForValidation("$FLEET_VAR_CUSTOM_SCEP_CHALLENGE_scepName", "https://bozo.com", "Name", "com.apple.security.scep"), - errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_SCEP_RENEWAL_ID variables.", + errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables.", }, { name: "Custom SCEP renewal ID missing", profile: strings.Replace(customSCEPForValidation("$FLEET_VAR_CUSTOM_SCEP_CHALLENGE_scepName", "$FLEET_VAR_CUSTOM_SCEP_PROXY_URL_scepName", "Name", "com.apple.security.scep"), "$FLEET_VAR_SCEP_RENEWAL_ID", "", 1), - errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_SCEP_RENEWAL_ID variables.", + errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables.", }, { name: "Custom SCEP challenge and url CA names don't match", @@ -160,13 +160,13 @@ func TestValidateProfileCertificateAuthorityVariables(t *testing.T) { { name: "Smallstep challenge missing", profile: customSCEPForValidation("challenge", "$FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_smallstepName", "Name", "com.apple.security.scep"), - errMsg: "Smallstep certificate authority requires: $FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_, $FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_, and $FLEET_VAR_SCEP_RENEWAL_ID variables.", + errMsg: "Smallstep certificate authority requires: $FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_, $FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables.", }, { name: "Smallstep url missing", profile: customSCEPForValidation("$FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_smallstepName", "https://bozo.com", "Name", "com.apple.security.scep"), - errMsg: "Smallstep certificate authority requires: $FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_, $FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_, and $FLEET_VAR_SCEP_RENEWAL_ID variables.", + errMsg: "Smallstep certificate authority requires: $FLEET_VAR_SMALLSTEP_SCEP_CHALLENGE_, $FLEET_VAR_SMALLSTEP_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables.", }, { name: "Smallstep challenge and url CA names don't match", From f512a2bc5809854f21fb5ca5d327401d76e4bc6b Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 06:03:52 -0600 Subject: [PATCH 03/11] Add back-compat test for legacy $FLEET_VAR_SCEP_RENEWAL_ID 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. --- server/service/mdm_profiles_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/service/mdm_profiles_test.go b/server/service/mdm_profiles_test.go index 0980565ce77..785aa1d6955 100644 --- a/server/service/mdm_profiles_test.go +++ b/server/service/mdm_profiles_test.go @@ -106,6 +106,18 @@ func TestValidateProfileCertificateAuthorityVariables(t *testing.T) { "com.apple.security.scep"), "$FLEET_VAR_SCEP_RENEWAL_ID", "", 1), errMsg: "SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_, $FLEET_VAR_CUSTOM_SCEP_PROXY_URL_, and $FLEET_VAR_CERTIFICATE_RENEWAL_ID variables.", }, + { + // This variable was renamed but needs to still validate + // for back-compat. + name: "Custom SCEP accepts legacy $FLEET_VAR_SCEP_RENEWAL_ID", + profile: customSCEPForValidation( + "$FLEET_VAR_CUSTOM_SCEP_CHALLENGE_scepName", + "$FLEET_VAR_CUSTOM_SCEP_PROXY_URL_scepName", + "Name", + "com.apple.security.scep", + ), + errMsg: "", + }, { name: "Custom SCEP challenge and url CA names don't match", profile: customSCEPForValidation("$FLEET_VAR_CUSTOM_SCEP_CHALLENGE_scepName", "$FLEET_VAR_CUSTOM_SCEP_PROXY_URL_scepName2", From cc011da5eaeed656a2b718d1a737ddddc1b9056d Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 06:27:41 -0600 Subject: [PATCH 04/11] remove changes file --- changes/40639-acme-validation-and-renewal-id-rename | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 changes/40639-acme-validation-and-renewal-id-rename diff --git a/changes/40639-acme-validation-and-renewal-id-rename b/changes/40639-acme-validation-and-renewal-id-rename deleted file mode 100644 index 4c97248d168..00000000000 --- a/changes/40639-acme-validation-and-renewal-id-rename +++ /dev/null @@ -1,2 +0,0 @@ -* Reject Apple ACME profile uploads that don't include a renewal-ID marker variable in the cert Subject CN or OU. Without the marker the cert can't be linked back to its profile and won't auto-renew. -* Add `$FLEET_VAR_CERTIFICATE_RENEWAL_ID` as the preferred Apple/Windows profile variable for the renewal-ID marker. The legacy `$FLEET_VAR_SCEP_RENEWAL_ID` is still accepted for back-compat with existing profiles. From b397e9ed4afa82a50fa2ce0c824458d4f72f4739 Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 06:31:25 -0600 Subject: [PATCH 05/11] simplify comment --- server/fleet/mdm.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go index d808e9a1d85..dbfaab3b7fe 100644 --- a/server/fleet/mdm.go +++ b/server/fleet/mdm.go @@ -76,13 +76,9 @@ const ( FleetVarHostPlatform FleetVarName = "HOST_PLATFORM" // Certificate authority variables - FleetVarNDESSCEPChallenge FleetVarName = "NDES_SCEP_CHALLENGE" - FleetVarNDESSCEPProxyURL FleetVarName = "NDES_SCEP_PROXY_URL" - FleetVarSCEPRenewalID FleetVarName = "SCEP_RENEWAL_ID" - // FleetVarCertificateRenewalID is the preferred name for the renewal-ID - // marker variable as of 4.86 (PR #44069). The legacy SCEP_RENEWAL_ID name - // remains accepted for back-compat with profiles authored against earlier - // docs. Both substitute to "fleet-" + profile_uuid. + FleetVarNDESSCEPChallenge FleetVarName = "NDES_SCEP_CHALLENGE" + FleetVarNDESSCEPProxyURL FleetVarName = "NDES_SCEP_PROXY_URL" + FleetVarSCEPRenewalID FleetVarName = "SCEP_RENEWAL_ID" // deprecated in favor of FleetVarCertificateRenewalID, but remains for back-compat FleetVarCertificateRenewalID FleetVarName = "CERTIFICATE_RENEWAL_ID" FleetVarDigiCertDataPrefix FleetVarName = "DIGICERT_DATA_" FleetVarDigiCertPasswordPrefix FleetVarName = "DIGICERT_PASSWORD_" // nolint:gosec // G101: Potential hardcoded credentials From e62a4b6317a9b7d75b171c9665f3968c0948c34b Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 06:52:18 -0600 Subject: [PATCH 06/11] remove legacy name support in acme --- server/service/apple_mdm.go | 4 ++-- server/service/apple_mdm_test.go | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index dcd14e71afa..bfdc2a3d9df 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -779,8 +779,8 @@ func additionalACMEValidation(contents string) error { } } } - if !fleet.FleetVarRenewalIDRegexp.MatchString(commonName.String()) && - !fleet.FleetVarRenewalIDRegexp.MatchString(orgUnit.String()) { + if !fleet.FleetVarCertificateRenewalIDRegexp.MatchString(commonName.String()) && + !fleet.FleetVarCertificateRenewalIDRegexp.MatchString(orgUnit.String()) { return &fleet.BadRequestError{ Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + " must be in the ACME certificate's organizational unit (OU).", diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 0c8db5a761d..af9528eb6fa 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -7324,10 +7324,14 @@ func TestValidateConfigProfileFleetVariablesACMEAndRenewalIDRename(t *testing.T) require.NoError(t, err) }) - t.Run("ACME profile with legacy SCEP_RENEWAL_ID is accepted", func(t *testing.T) { + t.Run("ACME profile with legacy SCEP_RENEWAL_ID is rejected", func(t *testing.T) { + // ACME is a net-new validation surface; the legacy variable name + // is rejected even though SCEP profiles accept it. profile := fmt.Sprintf(acmeProfile, "$FLEET_VAR_SCEP_RENEWAL_ID") _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) - require.NoError(t, err) + require.Error(t, err) + require.Contains(t, err.Error(), "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + require.Contains(t, err.Error(), "ACME certificate") }) t.Run("ACME profile missing renewal-ID marker is rejected", func(t *testing.T) { From f49c8caba675638d28e33ada9dd8999a1933177f Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 06:56:23 -0600 Subject: [PATCH 07/11] use acme const --- server/service/apple_mdm.go | 4 ++-- server/service/mdm_profiles.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index bfdc2a3d9df..d1f01cd52d7 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -745,7 +745,7 @@ type acmeProfileForValidation struct { // by the device's ACME exchange cannot be linked back to its profile and will // never be auto-renewed. func additionalACMEValidation(contents string) error { - if !strings.Contains(contents, "com.apple.security.acme") { + if !strings.Contains(contents, mobileconfig.ACMEPayloadType) { return nil } // Strip variables embedded in elements so the plist unmarshal @@ -756,7 +756,7 @@ func additionalACMEValidation(contents string) error { return &fleet.BadRequestError{Message: fmt.Sprintf("Failed to parse ACME payload with Fleet variables: %s", err.Error())} } for _, payload := range acmeProf.PayloadContent { - if payload.PayloadType != "com.apple.security.acme" { + if payload.PayloadType != mobileconfig.ACMEPayloadType { continue } var commonName, orgUnit strings.Builder diff --git a/server/service/mdm_profiles.go b/server/service/mdm_profiles.go index d88c8ab38eb..80d46ef3f33 100644 --- a/server/service/mdm_profiles.go +++ b/server/service/mdm_profiles.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mdm/apple/mobileconfig" "github.com/fleetdm/fleet/v4/server/variables" ) @@ -512,7 +513,7 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f // variable stands alone (no SCEP URL/Challenge) and validation lives in // additionalACMEValidation upstream. if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && - !strings.Contains(profileContents, "com.apple.security.acme") { + !strings.Contains(profileContents, mobileconfig.ACMEPayloadType) { return &fleet.BadRequestError{Message: fleet.SCEPRenewalIDWithoutURLChallengeErrMsg} } } From 6eaf97efa022b0850aae7f0ff24564ee760d20f6 Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 08:10:47 -0600 Subject: [PATCH 08/11] concise comments --- server/service/apple_mdm.go | 24 +++++++++--------------- server/service/mdm_profiles.go | 7 +++---- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 3abeef98679..b98af8a9899 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -511,12 +511,9 @@ func CheckProfileIsNotSigned(data []byte) error { } func validateConfigProfileFleetVariables(contents string, lic *fleet.LicenseInfo, groupedCAs *fleet.GroupedCertificateAuthorities) ([]string, error) { - // ACME payloads must carry the renewal-ID marker in Subject CN/OU so the - // non-proxied renewal mechanism can later link the issued cert back to the - // installing profile. This check runs before the Fleet-variable early - // return because an ACME profile with no other Fleet variables would - // otherwise sneak through and never auto-renew. Fast-path on a substring - // match avoids parsing for the common (non-ACME) case. + // Run before the fleetVars early-return: an ACME profile may carry + // no Fleet variables besides the renewal-ID marker and would otherwise + // skip this check. if err := additionalACMEValidation(contents); err != nil { return nil, err } @@ -726,9 +723,8 @@ func additionalSmallstepValidation(contents string, smallstepVars *SmallstepVars return nil } -// acmePayloadForValidation captures just the fields needed to enforce the -// renewal-ID marker requirement. ACME payload shape differs from SCEP: Subject -// is a sibling of PayloadType rather than nested under PayloadContent. +// acmePayloadForValidation differs from SCEPPayloadContent because an ACME +// payload's Subject is a sibling of PayloadType, not nested inside it. type acmePayloadForValidation struct { PayloadType string `plist:"PayloadType"` Subject [][][]string `plist:"Subject"` @@ -738,12 +734,10 @@ type acmeProfileForValidation struct { PayloadContent []acmePayloadForValidation `plist:"PayloadContent"` } -// additionalACMEValidation checks that any com.apple.security.acme payload in -// the profile carries the renewal-ID marker variable in the cert Subject's CN -// or OU. Either the preferred $FLEET_VAR_CERTIFICATE_RENEWAL_ID or the legacy -// $FLEET_VAR_SCEP_RENEWAL_ID is accepted. Without the marker, the cert issued -// by the device's ACME exchange cannot be linked back to its profile and will -// never be auto-renewed. +// additionalACMEValidation rejects profiles whose com.apple.security.acme +// payload Subject lacks $FLEET_VAR_CERTIFICATE_RENEWAL_ID in CN or OU. +// Without the marker the cert can't be linked back to its profile and +// renewal won't fire. func additionalACMEValidation(contents string) error { if !strings.Contains(contents, mobileconfig.ACMEPayloadType) { return nil diff --git a/server/service/mdm_profiles.go b/server/service/mdm_profiles.go index 80d46ef3f33..f5695e98a09 100644 --- a/server/service/mdm_profiles.go +++ b/server/service/mdm_profiles.go @@ -508,10 +508,9 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if smallstepVars.RenewalOnly() { smallstepVars = nil } - // If only the renewal ID variable appeared without any of its associated SCEP variables, - // return an error — UNLESS the profile contains an ACME payload, where the renewal-ID - // variable stands alone (no SCEP URL/Challenge) and validation lives in - // additionalACMEValidation upstream. + // Only-renewal-ID is an error for SCEP (needs URL/Challenge companions) + // but valid for ACME — ACME profiles use the marker variable alone and + // are validated by additionalACMEValidation upstream. if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && !strings.Contains(profileContents, mobileconfig.ACMEPayloadType) { return &fleet.BadRequestError{Message: fleet.SCEPRenewalIDWithoutURLChallengeErrMsg} From f7e9849161aacbc85219450ace6067ad3485e43f Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 08:21:50 -0600 Subject: [PATCH 09/11] Use Mobileconfig.HasPayloadType for ACME bypass detection 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. --- server/service/mdm_profiles.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/server/service/mdm_profiles.go b/server/service/mdm_profiles.go index f5695e98a09..0cdccd0dbaf 100644 --- a/server/service/mdm_profiles.go +++ b/server/service/mdm_profiles.go @@ -508,11 +508,14 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if smallstepVars.RenewalOnly() { smallstepVars = nil } - // Only-renewal-ID is an error for SCEP (needs URL/Challenge companions) - // but valid for ACME — ACME profiles use the marker variable alone and - // are validated by additionalACMEValidation upstream. - if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && - !strings.Contains(profileContents, mobileconfig.ACMEPayloadType) { + // ACME profiles legitimately have only the renewal-ID variable; + // bypass the SCEP-only "needs URL/Challenge" error for them. + // Non-mobileconfig content (Windows path) parses as no-ACME. + hasACMEPayload := false + if found, err := mobileconfig.Mobileconfig(profileContents).HasPayloadType(mobileconfig.ACMEPayloadType); err == nil { + hasACMEPayload = found + } + if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && !hasACMEPayload { return &fleet.BadRequestError{Message: fleet.SCEPRenewalIDWithoutURLChallengeErrMsg} } } From 31401cff0a1d23c1c0e282bcebe4a4b87ed88f7a Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 12 May 2026 08:59:40 -0600 Subject: [PATCH 10/11] Windows SCEP profile validation accepts CERTIFICATE_RENEWAL_ID 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. --- server/service/windows_mdm_profiles.go | 24 +++++++++++++++------ server/service/windows_mdm_profiles_test.go | 14 +++++++++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/server/service/windows_mdm_profiles.go b/server/service/windows_mdm_profiles.go index 0a3268c24b0..a92206f33b4 100644 --- a/server/service/windows_mdm_profiles.go +++ b/server/service/windows_mdm_profiles.go @@ -148,6 +148,7 @@ var fleetVarsSupportedInWindowsProfiles = []fleet.FleetVarName{ fleet.FleetVarHostHardwareSerial, fleet.FleetVarSCEPWindowsCertificateID, fleet.FleetVarSCEPRenewalID, + fleet.FleetVarCertificateRenewalID, fleet.FleetVarHostEndUserIDPUsername, fleet.FleetVarHostEndUserIDPUsernameLocalPart, fleet.FleetVarHostEndUserIDPFullname, @@ -158,6 +159,18 @@ var fleetVarsSupportedInWindowsProfiles = []fleet.FleetVarName{ fleet.FleetVarNDESSCEPProxyURL, } +// subjectNameHasRenewalIDMarker reports whether a SubjectName data string +// contains the renewal-ID variable in OU=. The legacy SCEP_RENEWAL_ID name +// is accepted alongside CERTIFICATE_RENEWAL_ID for back-compat. +func subjectNameHasRenewalIDMarker(data string) bool { + for _, v := range []fleet.FleetVarName{fleet.FleetVarCertificateRenewalID, fleet.FleetVarSCEPRenewalID} { + if strings.Contains(data, "OU="+v.WithPrefix()) || strings.Contains(data, "OU="+v.WithBraces()) { + return true + } + } + return false +} + func validateWindowsProfileFleetVariables(contents string, lic *fleet.LicenseInfo, groupedCAs *fleet.GroupedCertificateAuthorities) ([]string, error) { foundVars := variables.Find(contents) if len(foundVars) == 0 { @@ -283,11 +296,9 @@ func additionalNDESValidationForWindowsProfiles(contents string, ndesVars *NDESV "Variable %q must be in the SCEP certificate's \"ServerURL\" field.", fleet.FleetVarNDESSCEPProxyURL.WithPrefix()), } } - if isSubjectName && - !strings.Contains(dataContent, "OU="+fleet.FleetVarSCEPRenewalID.WithPrefix()) && - !strings.Contains(dataContent, "OU="+fleet.FleetVarSCEPRenewalID.WithBraces()) { + if isSubjectName && !subjectNameHasRenewalIDMarker(dataContent) { return &fleet.BadRequestError{ - Message: fmt.Sprintf("SubjectName item must contain the %s variable in the OU field", fleet.FleetVarSCEPRenewalID.WithPrefix()), + Message: fmt.Sprintf("SubjectName item must contain the %s variable in the OU field", fleet.FleetVarCertificateRenewalID.WithPrefix()), } } } @@ -325,9 +336,8 @@ func additionalCustomSCEPValidationForWindowsProfiles(contents string, customSCE return errors.New("SubjectName item is missing data") } - if !strings.Contains(cmd.Data.Content, "OU="+fleet.FleetVarSCEPRenewalID.WithPrefix()) && !strings.Contains(cmd.Data.Content, "OU="+fleet.FleetVarSCEPRenewalID.WithBraces()) { - // Does not contain the renewal ID in any of it's two fleet var forms as the OU field - return fmt.Errorf("SubjectName item must contain the %s variable in the OU field", fleet.FleetVarSCEPRenewalID.WithPrefix()) + if !subjectNameHasRenewalIDMarker(cmd.Data.Content) { + return fmt.Errorf("SubjectName item must contain the %s variable in the OU field", fleet.FleetVarCertificateRenewalID.WithPrefix()) } } } diff --git a/server/service/windows_mdm_profiles_test.go b/server/service/windows_mdm_profiles_test.go index 8e9155d27d7..0e3b54f9e38 100644 --- a/server/service/windows_mdm_profiles_test.go +++ b/server/service/windows_mdm_profiles_test.go @@ -268,7 +268,19 @@ func TestAdditionalNDESValidationForWindowsProfiles(t *testing.T) { addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/ServerURL", "$FLEET_VAR_NDES_SCEP_PROXY_URL") + addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/SubjectName", "CN=test"), wantErr: true, - errContains: "SubjectName item must contain the $FLEET_VAR_SCEP_RENEWAL_ID variable in the OU field", + errContains: "SubjectName item must contain the $FLEET_VAR_CERTIFICATE_RENEWAL_ID variable in the OU field", + }, + { + name: "valid NDES profile with preferred CERTIFICATE_RENEWAL_ID", + contents: addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/Challenge", "$FLEET_VAR_NDES_SCEP_CHALLENGE") + + addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/ServerURL", "$FLEET_VAR_NDES_SCEP_PROXY_URL") + + addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/SubjectName", "CN=test,OU=$FLEET_VAR_CERTIFICATE_RENEWAL_ID"), + }, + { + name: "valid NDES profile with preferred CERTIFICATE_RENEWAL_ID (braces syntax)", + contents: addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/Challenge", "${FLEET_VAR_NDES_SCEP_CHALLENGE}") + + addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/ServerURL", "${FLEET_VAR_NDES_SCEP_PROXY_URL}") + + addItem("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/cert1/Install/SubjectName", "CN=test,OU=${FLEET_VAR_CERTIFICATE_RENEWAL_ID}"), }, { name: "nil ndes vars returns nil", From c354fbaba38458aa45cbf5a063e7321dcf345495 Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Wed, 13 May 2026 07:08:27 -0600 Subject: [PATCH 11/11] Apple non-proxied SCEP profile validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- server/service/apple_mdm.go | 48 ++++++++- server/service/apple_mdm_test.go | 168 ++++++++++++++++++++++++++++++- server/service/mdm_profiles.go | 20 ++-- 3 files changed, 224 insertions(+), 12 deletions(-) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 663d84cf5ce..522595ef3bb 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -511,12 +511,15 @@ func CheckProfileIsNotSigned(data []byte) error { } func validateConfigProfileFleetVariables(contents string, lic *fleet.LicenseInfo, groupedCAs *fleet.GroupedCertificateAuthorities) ([]string, error) { - // Run before the fleetVars early-return: an ACME profile may carry - // no Fleet variables besides the renewal-ID marker and would otherwise - // skip this check. + // Run before the fleetVars early-return: ACME and raw-SCEP profiles + // may carry no Fleet variables besides the renewal-ID marker and would + // otherwise skip this check. if err := additionalACMEValidation(contents); err != nil { return nil, err } + if err := additionalNonProxiedSCEPValidation(contents); err != nil { + return nil, err + } fleetVars := variables.Find(contents) if len(fleetVars) == 0 { @@ -775,6 +778,45 @@ func additionalACMEValidation(contents string) error { return nil } +// additionalNonProxiedSCEPValidation rejects raw SCEP profiles (those not +// using Fleet proxy CA variables) whose cert Subject OU lacks +// $FLEET_VAR_CERTIFICATE_RENEWAL_ID. Profiles that use Fleet proxy +// variables (NDES, Custom SCEP, Smallstep) are deferred to the per-CA +// validators downstream, which enforce the same OU requirement. +func additionalNonProxiedSCEPValidation(contents string) error { + if !strings.Contains(contents, mobileconfig.SCEPPayloadType) { + return nil + } + // If the profile uses any Fleet proxy CA variable, the per-CA + // validators handle it downstream. + for _, v := range variables.Find(contents) { + if v == string(fleet.FleetVarNDESSCEPChallenge) || + v == string(fleet.FleetVarNDESSCEPProxyURL) || + strings.HasPrefix(v, string(fleet.FleetVarCustomSCEPChallengePrefix)) || + strings.HasPrefix(v, string(fleet.FleetVarCustomSCEPProxyURLPrefix)) || + strings.HasPrefix(v, string(fleet.FleetVarSmallstepSCEPChallengePrefix)) || + strings.HasPrefix(v, string(fleet.FleetVarSmallstepSCEPProxyURLPrefix)) { + return nil + } + } + scepProf, err := unmarshalSCEPProfile(contents) + if err != nil { + return err + } + for _, payload := range scepProf.PayloadContent { + if payload.PayloadType != mobileconfig.SCEPPayloadType { + continue + } + if !fleet.FleetVarCertificateRenewalIDRegexp.MatchString(payload.PayloadContent.OrganizationalUnit) { + return &fleet.BadRequestError{ + Message: "Variable $FLEET_VAR_" + string(fleet.FleetVarCertificateRenewalID) + + " must be in the SCEP certificate's organizational unit (OU).", + } + } + } + return nil +} + func checkThatOnlyOneSCEPPayloadIsPresent(scepProf SCEPProfileContent) (SCEPPayloadContent, error) { scepPayloadsFound := 0 var scepPayloadContent SCEPPayloadContent diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 78cde78e504..9060895cac8 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -7187,10 +7187,14 @@ func TestValidateConfigProfileFleetVariables(t *testing.T) { errMsg: "Variable \"$FLEET_VAR_NDES_SCEP_PROXY_URL\" must be in the SCEP certificate's \"URL\" field.", }, { - name: "SCEP renewal ID without other variables", + // Raw SCEP fixture uses the legacy SCEP_RENEWAL_ID variable + // (baked into the embedded mobileconfig). The new non-proxied + // SCEP validator is a net-new surface that requires the + // preferred variable name only. + name: "raw SCEP with legacy variable name is rejected", profile: customSCEPForValidation("challenge", "url", "Name", "com.apple.security.scep"), - errMsg: fleet.SCEPRenewalIDWithoutURLChallengeErrMsg, + errMsg: "Variable $FLEET_VAR_CERTIFICATE_RENEWAL_ID must be in the SCEP certificate's organizational unit (OU).", }, { name: "NDES happy path", @@ -7429,6 +7433,166 @@ func TestValidateConfigProfileFleetVariablesACMEAndRenewalIDRename(t *testing.T) }) } +func TestAdditionalNonProxiedSCEPValidation(t *testing.T) { + t.Parallel() + premiumLic := &fleet.LicenseInfo{Tier: fleet.TierPremium} + groupedCAs := &fleet.GroupedCertificateAuthorities{} + + // Raw SCEP profile template — literal Challenge/URL, marker variable + // in Subject OU. No Fleet proxy variables. + const rawSCEPProfile = ` + + + + PayloadContent + + + PayloadTypecom.apple.security.scep + PayloadIdentifiercom.test.scep + PayloadUUID11111111-2222-3333-4444-555555555555 + PayloadContent + + Challengestatic-challenge-value + URLhttps://scep.example.com/scep + Subject + + CNdevice-cn + OU%s + + + + + PayloadIdentifiercom.test.profile + PayloadTypeConfiguration + PayloadUUIDaaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee + PayloadVersion1 + +` + + t.Run("raw SCEP with preferred CERTIFICATE_RENEWAL_ID is accepted", func(t *testing.T) { + profile := fmt.Sprintf(rawSCEPProfile, "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) + require.NoError(t, err) + }) + + t.Run("raw SCEP with legacy SCEP_RENEWAL_ID is rejected", func(t *testing.T) { + // Raw-SCEP validation is a net-new surface; the legacy variable + // is rejected even though pre-existing SCEP validators (NDES, + // Custom SCEP, Smallstep) still accept it. + profile := fmt.Sprintf(rawSCEPProfile, "$FLEET_VAR_SCEP_RENEWAL_ID") + _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) + require.Error(t, err) + require.Contains(t, err.Error(), "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + require.Contains(t, err.Error(), "SCEP certificate") + }) + + t.Run("raw SCEP missing renewal-ID marker is rejected", func(t *testing.T) { + profile := fmt.Sprintf(rawSCEPProfile, "static-ou-value") + _, err := validateConfigProfileFleetVariables(profile, premiumLic, groupedCAs) + require.Error(t, err) + require.Contains(t, err.Error(), "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + require.Contains(t, err.Error(), "SCEP certificate") + require.NotContains(t, err.Error(), "SCEP_RENEWAL_ID variables", + "error must not advertise the legacy name in the suggested-fix list") + }) + + t.Run("raw SCEP with renewal-ID in CN only is rejected", func(t *testing.T) { + // Raw SCEP validation is a net-new surface; the marker must live in + // the OU. Accepting CN placement would perpetuate the bug-shaped + // "validator says OU, code accepts either" inconsistency inherited + // from pre-existing validators. + const cnOnlyProfile = ` + + + + PayloadContent + + + PayloadTypecom.apple.security.scep + PayloadIdentifiercom.test.scep + PayloadUUID11111111-2222-3333-4444-555555555555 + PayloadContent + + Challengestatic-challenge-value + URLhttps://scep.example.com/scep + Subject + + CN$FLEET_VAR_CERTIFICATE_RENEWAL_ID + OUstatic-ou-value + + + + + PayloadIdentifiercom.test.profile + PayloadTypeConfiguration + PayloadUUIDaaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee + PayloadVersion1 + +` + _, err := validateConfigProfileFleetVariables(cnOnlyProfile, premiumLic, groupedCAs) + require.Error(t, err) + require.Contains(t, err.Error(), "$FLEET_VAR_CERTIFICATE_RENEWAL_ID") + require.Contains(t, err.Error(), "organizational unit (OU)") + }) + + t.Run("SCEP profile using Fleet proxy vars defers to per-CA validator", func(t *testing.T) { + // This profile uses NDES proxy vars but lacks a SubjectName with + // the renewal-ID marker. additionalNonProxiedSCEPValidation should + // short-circuit (return nil) and let the per-CA NDES validator + // produce its own error downstream. The end result is still an + // error, but it should be the per-CA one (referencing the NDES + // variable triple), not the raw-SCEP one. + const ndesProfile = ` + + + + PayloadContent + + + PayloadTypecom.apple.security.scep + PayloadIdentifiercom.test.ndes + PayloadUUID11111111-2222-3333-4444-555555555555 + PayloadContent + + Challenge$FLEET_VAR_NDES_SCEP_CHALLENGE + URL$FLEET_VAR_NDES_SCEP_PROXY_URL + Subject + + CNdevice-cn + OUstatic-ou + + + + + PayloadIdentifiercom.test.profile + PayloadTypeConfiguration + PayloadUUIDaaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee + PayloadVersion1 + +` + _, err := validateConfigProfileFleetVariables(ndesProfile, premiumLic, groupedCAs) + require.Error(t, err) + // The per-CA NDES validator's message — proves the raw-SCEP + // path short-circuited rather than producing its own error. + require.Contains(t, err.Error(), "NDES") + }) + + t.Run("non-SCEP profile is unaffected", func(t *testing.T) { + const wifiProfile = ` + + + + PayloadTypeConfiguration + PayloadIdentifiercom.test.wifi + PayloadUUIDaaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee + PayloadVersion1 + +` + _, err := validateConfigProfileFleetVariables(wifiProfile, premiumLic, groupedCAs) + require.NoError(t, err) + }) +} + func TestValidateDeclarationFleetVariables(t *testing.T) { t.Parallel() diff --git a/server/service/mdm_profiles.go b/server/service/mdm_profiles.go index 0cdccd0dbaf..a7686fd2eb5 100644 --- a/server/service/mdm_profiles.go +++ b/server/service/mdm_profiles.go @@ -508,14 +508,20 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if smallstepVars.RenewalOnly() { smallstepVars = nil } - // ACME profiles legitimately have only the renewal-ID variable; - // bypass the SCEP-only "needs URL/Challenge" error for them. - // Non-mobileconfig content (Windows path) parses as no-ACME. - hasACMEPayload := false - if found, err := mobileconfig.Mobileconfig(profileContents).HasPayloadType(mobileconfig.ACMEPayloadType); err == nil { - hasACMEPayload = found + // ACME and raw-SCEP profiles legitimately have only the renewal-ID + // variable; bypass the "needs URL/Challenge" error for them. Marker + // presence is enforced upstream by additionalACMEValidation and + // additionalNonProxiedSCEPValidation. Non-mobileconfig content + // (Windows path) parses as no-payload, so the error fires normally. + hasRenewableCertPayload := false + mc := mobileconfig.Mobileconfig(profileContents) + for _, pt := range []string{mobileconfig.ACMEPayloadType, mobileconfig.SCEPPayloadType} { + if found, err := mc.HasPayloadType(pt); err == nil && found { + hasRenewableCertPayload = true + break + } } - if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && !hasACMEPayload { + if ndesVars == nil && smallstepVars == nil && customSCEPVars == nil && !hasRenewableCertPayload { return &fleet.BadRequestError{Message: fleet.SCEPRenewalIDWithoutURLChallengeErrMsg} } }