diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go index 239e6ef203..8afd309d2e 100644 --- a/server/fleet/mdm.go +++ b/server/fleet/mdm.go @@ -35,6 +35,11 @@ const ( // It provides clearer semantics when used in function signatures and data structures. type FleetVarName string +// Includes $FLEET_VAR prefix +func (n FleetVarName) WithPrefix() string { + return fmt.Sprintf("$FLEET_VAR_%s", n) +} + const ( // FleetVarNDESSCEPChallenge and other variables are used as $FLEET_VAR_. // For example: $FLEET_VAR_NDES_SCEP_CHALLENGE diff --git a/server/fleet/microsoft_mdm.go b/server/fleet/microsoft_mdm.go index 01f3912bd3..5d7c3dc5dd 100644 --- a/server/fleet/microsoft_mdm.go +++ b/server/fleet/microsoft_mdm.go @@ -1012,6 +1012,10 @@ type SyncMLCmd struct { // AddCommands is a catch-all for any nested commands, // which can be found under elements. AddCommands []SyncMLCmd `xml:"Add,omitempty"` + + // ExecCommands is a catch-all for any nested commands, + // which can be found under elements. + ExecCommands []SyncMLCmd `xml:"Exec,omitempty"` } // ParseWindowsMDMCommand parses the raw XML as a single Windows MDM command. diff --git a/server/fleet/windows_mdm.go b/server/fleet/windows_mdm.go index 9061c436a2..1a28ed1f57 100644 --- a/server/fleet/windows_mdm.go +++ b/server/fleet/windows_mdm.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "slices" "strings" "time" @@ -79,9 +80,12 @@ func (m *MDMWindowsConfigProfile) ValidateUserProvided(enableCustomOSUpdates boo // we don't need to validate the required nesting // structure (Target>Item>LocURI) so we don't need to track all the tags. var inValidNode bool + var inExec bool var inLocURI bool var inComment bool + windowSCEPProfileValidator := newWindowsSCEPProfileValidator() + for { tok, err := dec.Token() if err != nil { @@ -105,8 +109,8 @@ func (m *MDMWindowsConfigProfile) ValidateUserProvided(enableCustomOSUpdates boo case xml.StartElement: // Top-level comments should be followed by or elements if inComment { - if !inValidNode && t.Name.Local != "Replace" && t.Name.Local != "Add" { - return errors.New("Windows configuration profiles can only have or top level elements after comments") + if !inValidNode && t.Name.Local != "Replace" && t.Name.Local != "Add" && t.Name.Local != "Exec" { + return errors.New("Windows configuration profiles can only have , or top level elements after comments") } inValidNode = true inComment = false @@ -115,15 +119,18 @@ func (m *MDMWindowsConfigProfile) ValidateUserProvided(enableCustomOSUpdates boo switch t.Name.Local { case "Replace", "Add": inValidNode = true + case "Exec": + inValidNode = true + inExec = true case "LocURI": if !inValidNode { - return errors.New("Windows configuration profiles can only have or top level elements.") + return errors.New("Windows configuration profiles can only have , or top level elements.") } inLocURI = true default: if !inValidNode { - return errors.New("Windows configuration profiles can only have or top level elements.") + return errors.New("Windows configuration profiles can only have , or top level elements.") } } @@ -131,12 +138,26 @@ func (m *MDMWindowsConfigProfile) ValidateUserProvided(enableCustomOSUpdates boo switch t.Name.Local { case "Replace", "Add": inValidNode = false + case "Exec": + inValidNode = false + inExec = false case "LocURI": inLocURI = false } case xml.CharData: if inLocURI { + if inExec { + if err := windowSCEPProfileValidator.validateExecLocURI(string(t)); err != nil { + return err + } + continue + } + + if err := windowSCEPProfileValidator.validateLocURI(string(t)); err != nil { + return err + } + if err := validateFleetProvidedLocURI(string(t), enableCustomOSUpdates); err != nil { return err } @@ -144,6 +165,10 @@ func (m *MDMWindowsConfigProfile) ValidateUserProvided(enableCustomOSUpdates boo } } + if err := windowSCEPProfileValidator.finalizeValidation(); err != nil { + return err + } + return nil } @@ -173,6 +198,131 @@ func validateFleetProvidedLocURI(locURI string, enableCustomOSUpdates bool) erro return nil } +// The following list of SCEP LocURIs is based on the documentation at +// https://learn.microsoft.com/en-us/windows/client-management/mdm/clientcertificateinstall-csp#devicescep +// Where going through all items, only for those with (Add or Replace) under "Supported operations" are included, +// and then based on it being marked Optional, or Required in the description. + +// A list containg all valid SCEP Profile LocURIs, a combination of optional and required to validate for non-SCEP LocURIs. +var validSCEPProfileLocURIs = slices.Concat([]string{ + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/AADKeyIdentifierList", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/ContainerName", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/CustomTextToShowInPrompt", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/KeyProtection", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/RetryCount", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/RetryDelay", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/SubjectAlternativeNames", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/TemplateName", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/ValidPeriod", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/ValidPeriodUnits", FleetVarSCEPWindowsCertificateID.WithPrefix()), +}, requiredSCEPProfileLocURIs) + +var requiredSCEPProfileLocURIs = []string{ + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/CAThumbprint", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/Challenge", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/EKUMapping", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/HashAlgorithm", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/KeyLength", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/KeyUsage", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/ServerURL", FleetVarSCEPWindowsCertificateID.WithPrefix()), + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/SubjectName", FleetVarSCEPWindowsCertificateID.WithPrefix()), +} + +var validExecSCEPProfileLocURIs = []string{ + fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s/Install/Enroll", FleetVarSCEPWindowsCertificateID.WithPrefix()), +} + +type windowsSCEPProfileValidator struct { + totalLocURIs int + totalExecLocURIs int + foundLocURIs map[string]bool + foundExecLocURIs map[string]bool +} + +func newWindowsSCEPProfileValidator() *windowsSCEPProfileValidator { + return &windowsSCEPProfileValidator{ + foundLocURIs: make(map[string]bool), + foundExecLocURIs: make(map[string]bool), + } +} + +func (v *windowsSCEPProfileValidator) isSCEPProfile() bool { + return len(v.foundLocURIs) > 0 || (v.totalExecLocURIs > 0 && len(v.foundExecLocURIs) > 0) +} + +func (v *windowsSCEPProfileValidator) validateLocURI(locURI string) error { + sanitizedLocURI := strings.TrimSpace(locURI) + + // If we see a LocURI with SCEP prefix, but no Fleet Var we fail early. + if v.isSCEPLocURIWithoutFleetVar(sanitizedLocURI) { + return fmt.Errorf("You must use %q after \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/\".", FleetVarSCEPWindowsCertificateID.WithPrefix()) + } + + if slices.Contains(validSCEPProfileLocURIs, sanitizedLocURI) { + v.foundLocURIs[sanitizedLocURI] = true + } + + v.totalLocURIs++ + return nil +} + +func (v *windowsSCEPProfileValidator) validateExecLocURI(locURI string) error { + sanitizedLocURI := strings.TrimSpace(locURI) + + // If we see a LocURI with SCEP prefix, but no Fleet Var we fail early. + if v.isSCEPLocURIWithoutFleetVar(sanitizedLocURI) { + return fmt.Errorf("You must use %q after \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/\".", FleetVarSCEPWindowsCertificateID.WithPrefix()) + } + + if slices.Contains(validExecSCEPProfileLocURIs, sanitizedLocURI) { + v.foundExecLocURIs[sanitizedLocURI] = true + } + + v.totalExecLocURIs++ + return nil +} + +// isSCEPLocURIWithoutFleetVar checks that the provided locURI starts with the SCEP prefix +// and that it includes the required Fleet Var for SCEP Windows Certificate ID. +// Skips any locURI that does not start with the SCEP prefix. +func (v windowsSCEPProfileValidator) isSCEPLocURIWithoutFleetVar(locURI string) bool { + if strings.HasPrefix(locURI, "./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/") && + !strings.HasPrefix(locURI, fmt.Sprintf("./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/%s", FleetVarSCEPWindowsCertificateID.WithPrefix())) { + return true + } + return false +} + +func (v *windowsSCEPProfileValidator) finalizeValidation() error { + if !v.isSCEPProfile() { + // Cheeky validation here, to only allow Exec elements in SCEP profiles. + if v.totalExecLocURIs > 0 { + return errors.New("Only SCEP profiles can include elements.") + } + return nil // Not a SCEP profile, nothing to validate here. + } + + // Verify that we do not have any non-scep loc URIs present + if v.totalLocURIs != len(v.foundLocURIs) { + return errors.New("Only options that have starting with \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/\" can be added to SCEP profile.") + } + + // Check that at least one Exec LocURI is present and it matches the only one we have in the array. + if len(v.foundExecLocURIs) != 1 && !v.foundExecLocURIs[validExecSCEPProfileLocURIs[0]] { + return errors.New("Couldn't add. \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID/Install/Enroll\" must be included within . Please add and try again.") + } + + // Check that all required LocURIs are present + for _, requiredLocURI := range requiredSCEPProfileLocURIs { + if !v.foundLocURIs[requiredLocURI] { + return fmt.Errorf("%q is missing. Please add and try again", requiredLocURI) + } + } + + return nil +} + type MDMWindowsProfilePayload struct { ProfileUUID string `db:"profile_uuid"` ProfileName string `db:"profile_name"` diff --git a/server/fleet/windows_mdm_test.go b/server/fleet/windows_mdm_test.go index 694e690d08..7c45c5ceb1 100644 --- a/server/fleet/windows_mdm_test.go +++ b/server/fleet/windows_mdm_test.go @@ -1,6 +1,7 @@ package fleet import ( + "fmt" "testing" "github.com/fleetdm/fleet/v4/server/mdm" @@ -41,7 +42,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "Add top level element", @@ -159,7 +160,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "XML with Replace and Atomic", @@ -177,7 +178,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "XML with Replace and Delete", @@ -195,7 +196,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "XML with Replace and Exec", @@ -213,7 +214,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Only SCEP profiles can include elements.", }, { name: "XML with Replace and Get", @@ -231,7 +232,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "XML with Replace and Results", @@ -249,7 +250,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "XML with Replace and Status", @@ -267,7 +268,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "XML with elements not defined in the protocol", @@ -285,7 +286,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements.", + wantErr: "Windows configuration profiles can only have , or top level elements.", }, { name: "invalid XML with mismatched tags", @@ -484,7 +485,7 @@ func TestValidateUserProvided(t *testing.T) { `), }, - wantErr: "Windows configuration profiles can only have or top level elements after comments", + wantErr: "Windows configuration profiles can only have , or top level elements after comments", }, { name: "XML with nested root element in data", @@ -596,6 +597,114 @@ func TestValidateUserProvided(t *testing.T) { }, wantErr: "", }, + { + name: "SCEP profile with other LocURIs", + profile: MDMWindowsConfigProfile{ + SyncML: []byte(` + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID + + + + + Custom/URI + + + `), + }, + wantErr: "Only options that have starting with \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/\" can be added to SCEP profile.", + }, + { + name: "SCEP profile without Exec block", + profile: MDMWindowsConfigProfile{ + SyncML: []byte(` + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID + + + `), + }, + wantErr: "\"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID/Install/Enroll\" must be included within . Please add and try again.", + }, + { + name: "SCEP profile with Exec block, but worng LocURI ", + profile: MDMWindowsConfigProfile{ + SyncML: []byte(` + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID + + + + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID/Random/Scep/LocURI + + + + `), + }, + wantErr: "Couldn't add. \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID/Install/Enroll\" must be included within . Please add and try again.", + }, + { + name: fmt.Sprintf("SCEP profile with missing $FLEET_VAR_%s after SCEP LocURI", FleetVarSCEPWindowsCertificateID), + profile: MDMWindowsConfigProfile{ + SyncML: []byte(` + + 12 + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/bogus-id-that-is-not-fleet-var/Install/CAThumbprint + + + chr + + 0DE4135C02E5E3C040FE1353E204D8B6F331F47A + + + `), + }, + wantErr: fmt.Sprintf("You must use \"$FLEET_VAR_%s\" after \"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/\".", FleetVarSCEPWindowsCertificateID), + }, + { + name: "SCEP Profile with missing required LocURI", + profile: MDMWindowsConfigProfile{ + SyncML: []byte(` + + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID + + + + + + + ./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID/Install/Enroll + + + + `), + }, + wantErr: fmt.Sprintf("\"./Device/Vendor/MSFT/ClientCertificateInstall/SCEP/$FLEET_VAR_%s/Install/CAThumbprint\" is missing.", FleetVarSCEPWindowsCertificateID), + }, + { + name: "Only SCEP profiles can have Exec elements", + profile: MDMWindowsConfigProfile{ + SyncML: []byte(` + + + + ./Device/Vendor/CustomExecTargetLocURI + + + + `), + }, + wantErr: "Only SCEP profiles can include elements.", + }, } for _, tt := range tests { diff --git a/server/mdm/microsoft/profile_verifier_test.go b/server/mdm/microsoft/profile_verifier_test.go index 3570615da5..30f52a0662 100644 --- a/server/mdm/microsoft/profile_verifier_test.go +++ b/server/mdm/microsoft/profile_verifier_test.go @@ -909,6 +909,23 @@ func TestPreprocessWindowsProfileContentsForVerification(t *testing.T) { func TestPreprocessWindowsProfileContentsForDeployment(t *testing.T) { ds := new(mock.Store) + scimUser := &fleet.ScimUser{ + UserName: "test@idp.com", + GivenName: ptr.String("First"), + FamilyName: ptr.String("Last"), + Department: ptr.String("Department"), + Groups: []fleet.ScimUserGroup{ + { + ID: 1, + DisplayName: "Group One", + }, + { + ID: 2, + DisplayName: "Group Two", + }, + }, + } + baseSetup := func() { ds.GetGroupedCertificateAuthoritiesFunc = func(ctx context.Context, includeSecrets bool) (*fleet.GroupedCertificateAuthorities, error) { if ds.GetAllCertificateAuthoritiesFunc == nil { @@ -934,24 +951,10 @@ func TestPreprocessWindowsProfileContentsForDeployment(t *testing.T) { ds.HostIDsByIdentifierFunc = func(ctx context.Context, filter fleet.TeamFilter, hostnames []string) ([]uint, error) { return []uint{42}, nil } + ds.ScimUserByHostIDFunc = func(ctx context.Context, hostID uint) (*fleet.ScimUser, error) { if hostID == 42 { - return &fleet.ScimUser{ - UserName: "test@idp.com", - GivenName: ptr.String("First"), - FamilyName: ptr.String("Last"), - Department: ptr.String("Department"), - Groups: []fleet.ScimUserGroup{ - { - ID: 1, - DisplayName: "Group One", - }, - { - ID: 2, - DisplayName: "Group Two", - }, - }, - }, nil + return scimUser, nil } return nil, fmt.Errorf("no scim user for host id %d", hostID) @@ -1081,6 +1084,32 @@ func TestPreprocessWindowsProfileContentsForDeployment(t *testing.T) { profileContents: `./Device/TestUser: $FLEET_VAR_HOST_END_USER_IDP_USERNAME - $FLEET_VAR_HOST_END_USER_IDP_USERNAME_LOCAL_PART - $FLEET_VAR_HOST_END_USER_IDP_GROUPS - $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT - $FLEET_VAR_HOST_END_USER_IDP_FULL_NAME`, expectedContents: `./Device/TestUser: test@idp.com - test - Group One,Group Two - Department - First Last`, }, + { + name: "missing groups on idp user", + hostUUID: "no-groups-idp", + profileContents: `./Device/TestUser: $FLEET_VAR_HOST_END_USER_IDP_GROUPS`, + expectError: true, + processingError: "There are no IdP groups for this host. Fleet couldn't populate $FLEET_VAR_HOST_END_USER_IDP_GROUPS.", + setup: func() { + scimUser.Groups = []fleet.ScimUserGroup{} + ds.ScimUserByHostIDFunc = func(ctx context.Context, hostID uint) (*fleet.ScimUser, error) { + return scimUser, nil + } + }, + }, + { + name: "missing department on idp user", + hostUUID: "no-department-idp", + profileContents: `./Device/TestUser: $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT`, + expectError: true, + processingError: "There is no IdP department for this host. Fleet couldn't populate $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT.", + setup: func() { + scimUser.Department = nil + ds.ScimUserByHostIDFunc = func(ctx context.Context, hostID uint) (*fleet.ScimUser, error) { + return scimUser, nil + } + }, + }, } params := PreprocessingParameters{ diff --git a/server/mdm/profiles/profile_variables.go b/server/mdm/profiles/profile_variables.go index fc82f8e288..3b931e9336 100644 --- a/server/mdm/profiles/profile_variables.go +++ b/server/mdm/profiles/profile_variables.go @@ -143,9 +143,9 @@ func getHostEndUserIDPUser(ctx context.Context, ds fleet.Datastore, return nil, false, ctxerr.Wrap(ctx, err, "get end users for host") } - noGroupsErr := fmt.Sprintf("There is no IdP groups for this host. Fleet couldn’t populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPGroups) - noDepartmentErr := fmt.Sprintf("There is no IdP department for this host. Fleet couldn’t populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPDepartment) - noFullnameErr := fmt.Sprintf("There is no IdP full name for this host. Fleet couldn’t populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPFullname) + noGroupsErr := fmt.Sprintf("There are no IdP groups for this host. Fleet couldn't populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPGroups) + noDepartmentErr := fmt.Sprintf("There is no IdP department for this host. Fleet couldn't populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPDepartment) + noFullnameErr := fmt.Sprintf("There is no IdP full name for this host. Fleet couldn't populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPFullname) if len(users) > 0 && users[0].IdpUserName != "" { idpUser := users[0] diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 8e81323487..bbd20cb8b6 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -5216,7 +5216,7 @@ func TestPreprocessProfileContentsEndUserIDP(t *testing.T) { }, assert: func(output string) { assert.Len(t, targets, 0) // target is not present - assert.Contains(t, updatedProfile.Detail, "There is no IdP groups for this host. Fleet couldn’t populate $FLEET_VAR_HOST_END_USER_IDP_GROUPS.") + assert.Contains(t, updatedProfile.Detail, "There are no IdP groups for this host. Fleet couldn't populate $FLEET_VAR_HOST_END_USER_IDP_GROUPS.") }, }, { @@ -5233,7 +5233,7 @@ func TestPreprocessProfileContentsEndUserIDP(t *testing.T) { }, assert: func(output string) { assert.Len(t, targets, 0) // target is not present - assert.Contains(t, updatedProfile.Detail, "There is no IdP department for this host. Fleet couldn’t populate $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT.") + assert.Contains(t, updatedProfile.Detail, "There is no IdP department for this host. Fleet couldn't populate $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT.") }, }, { @@ -5250,7 +5250,7 @@ func TestPreprocessProfileContentsEndUserIDP(t *testing.T) { }, assert: func(output string) { assert.Len(t, targets, 0) // target is not present - assert.Contains(t, updatedProfile.Detail, "There is no IdP groups for this host. Fleet couldn’t populate $FLEET_VAR_HOST_END_USER_IDP_GROUPS.") + assert.Contains(t, updatedProfile.Detail, "There are no IdP groups for this host. Fleet couldn't populate $FLEET_VAR_HOST_END_USER_IDP_GROUPS.") }, }, { @@ -5295,7 +5295,7 @@ func TestPreprocessProfileContentsEndUserIDP(t *testing.T) { }, assert: func(output string) { assert.Len(t, targets, 0) // target is not present - assert.Contains(t, updatedProfile.Detail, "There is no IdP department for this host. Fleet couldn’t populate $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT.") + assert.Contains(t, updatedProfile.Detail, "There is no IdP department for this host. Fleet couldn't populate $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT.") }, }, { @@ -5381,7 +5381,7 @@ func TestPreprocessProfileContentsEndUserIDP(t *testing.T) { } }, assert: func(output string) { - assert.Contains(t, updatedProfile.Detail, fmt.Sprintf("There is no IdP full name for this host. Fleet couldn’t populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPFullname)) + assert.Contains(t, updatedProfile.Detail, fmt.Sprintf("There is no IdP full name for this host. Fleet couldn't populate $FLEET_VAR_%s.", fleet.FleetVarHostEndUserIDPFullname)) assert.Len(t, targets, 0) }, }, diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index 014c48fb66..e96418336b 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -7547,13 +7547,13 @@ func (s *integrationMDMTestSuite) TestWindowsProfilesWithFleetVariables() { Name: "TestMixed", Contents: syncml.ForTestWithData([]syncml.TestCommand{ {Verb: "Replace", LocURI: "./Device/Vendor/MSFT/DMClient/Provider/ProviderID/UserSCEP_/SCEP/HostID", Data: "$FLEET_VAR_HOST_UUID"}, - {Verb: "Replace", LocURI: "./Device/Vendor/MSFT/DMClient/Provider/ProviderID/UserSCEP_/SCEP/Email", Data: "$FLEET_VAR_HOST_END_USER_EMAIL_IDP"}, + {Verb: "Replace", LocURI: "./Device/Vendor/MSFT/DMClient/Provider/ProviderID/UserSCEP_/SCEP/Email", Data: "$FLEET_VAR_BOGUS"}, }), }, }, teamID: &tm.ID, wantStatus: http.StatusUnprocessableEntity, - wantErrContains: "Fleet variable $FLEET_VAR_HOST_END_USER_EMAIL_IDP is not supported in Windows profiles", + wantErrContains: "Fleet variable $FLEET_VAR_BOGUS is not supported in Windows profiles", }, { name: "HOST_UUID variable accepted globally", diff --git a/server/service/mdm.go b/server/service/mdm.go index 2c932904b7..46a1a719ec 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -1796,8 +1796,15 @@ func (svc *Service) NewMDMWindowsConfigProfile(ctx context.Context, teamID uint, // fleetVarsSupportedInWindowsProfiles lists the Fleet variables that are // supported in Windows configuration profiles. +// except prefix variables var fleetVarsSupportedInWindowsProfiles = []fleet.FleetVarName{ fleet.FleetVarHostUUID, + fleet.FleetVarSCEPWindowsCertificateID, + fleet.FleetVarHostEndUserIDPUsername, + fleet.FleetVarHostEndUserIDPUsernameLocalPart, + fleet.FleetVarHostEndUserIDPFullname, + fleet.FleetVarHostEndUserIDPDepartment, + fleet.FleetVarHostEndUserIDPGroups, } func validateWindowsProfileFleetVariables(contents string, lic *fleet.LicenseInfo) ([]string, error) { @@ -1813,7 +1820,9 @@ func validateWindowsProfileFleetVariables(contents string, lic *fleet.LicenseInf // Check if all found variables are supported for _, varName := range foundVars { - if !slices.Contains(fleetVarsSupportedInWindowsProfiles, fleet.FleetVarName(varName)) { + if !slices.Contains(fleetVarsSupportedInWindowsProfiles, fleet.FleetVarName(varName)) && + !strings.HasPrefix(varName, string(fleet.FleetVarCustomSCEPChallengePrefix)) && + !strings.HasPrefix(varName, string(fleet.FleetVarCustomSCEPProxyURLPrefix)) { return nil, fleet.NewInvalidArgumentError("profile", fmt.Sprintf("Fleet variable $FLEET_VAR_%s is not supported in Windows profiles.", varName)) } } diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index 11e3c1d75a..0d05e08b83 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -1289,7 +1289,7 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) { {"mdm not enabled", 0, ``, false, "Windows MDM isn't turned on."}, {"duplicate profile name", 0, `duplicate`, true, "configuration profile with this name already exists"}, {"multiple Replace", 0, `ab`, true, ""}, - {"Replace and non-Replace", 0, `ab`, true, "Windows configuration profiles can only have or top level elements."}, + {"Replace and non-Replace", 0, `ab`, true, "Windows configuration profiles can only have , or top level elements."}, { "BitLocker profile", 0, `./Device/Vendor/MSFT/BitLocker/AllowStandardUserEncryption`, true, @@ -1305,7 +1305,7 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) { {"team mdm not enabled", 1, ``, false, "Windows MDM isn't turned on."}, {"team duplicate profile name", 1, `duplicate`, true, "configuration profile with this name already exists"}, {"team multiple Replace", 1, `ab`, true, ""}, - {"team Replace and non-Replace", 1, `ab`, true, "Windows configuration profiles can only have or top level elements."}, + {"team Replace and non-Replace", 1, `ab`, true, "Windows configuration profiles can only have , or top level elements."}, { "team BitLocker profile", 1, `./Device/Vendor/MSFT/BitLocker/AllowStandardUserEncryption`, true, @@ -2500,11 +2500,11 @@ func TestValidateWindowsProfileFleetVariables(t *testing.T) { ./Device/Vendor/MSFT/Policy/Config/System/AllowLocation - $FLEET_VAR_HOST_UUID-$FLEET_VAR_HOST_END_USER_EMAIL_IDP + $FLEET_VAR_HOST_UUID-$FLEET_VAR_BOGUS_VAR `, wantErr: true, - errContains: "Fleet variable $FLEET_VAR_HOST_END_USER_EMAIL_IDP is not supported in Windows profiles", + errContains: "Fleet variable $FLEET_VAR_BOGUS_VAR is not supported in Windows profiles", }, { name: "unknown Fleet variable", diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index 787eed0ff4..e4039bb776 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -2438,6 +2438,14 @@ func buildCommandFromProfileBytes(profileBytes []byte, commandUUID string) (*fle } } + // generate a CmdID for any nested + for i := range cmd.ExecCommands { + cmd.ExecCommands[i].CmdID = mdm_types.CmdID{ + Value: uuid.NewString(), + IncludeFleetComment: true, + } + } + rawCommand, err := xml.Marshal(cmd) if err != nil { return nil, fmt.Errorf("marshalling command: %w", err) diff --git a/server/service/microsoft_mdm_test.go b/server/service/microsoft_mdm_test.go index 97d24f709b..7ed24053ac 100644 --- a/server/service/microsoft_mdm_test.go +++ b/server/service/microsoft_mdm_test.go @@ -388,7 +388,7 @@ func TestBuildCommandFromProfileBytes(t *testing.T) { ) // build and generate a second command with the same syncml - cmd, err = buildCommandFromProfileBytes(rawSyncML, "uuid-2") + cmd, err = buildCommandFromProfileBytes(syncMLForTestWithExec("foo/bar"), "uuid-2") require.Nil(t, err) require.Equal(t, "uuid-2", cmd.CommandUUID) require.Empty(t, cmd.TargetLocURI) @@ -400,7 +400,7 @@ func TestBuildCommandFromProfileBytes(t *testing.T) { // generated xml contains additional comments about CmdID require.Equal( t, - fmt.Sprintf(`uuid-2%sfoo/bar%sfoo/bar`, syncTwo.ReplaceCommands[0].CmdID.Value, syncTwo.AddCommands[0].CmdID.Value), + fmt.Sprintf(`uuid-2%sfoo/bar%sfoo/bar%sfoo/bar`, syncTwo.ReplaceCommands[0].CmdID.Value, syncTwo.AddCommands[0].CmdID.Value, syncTwo.ExecCommands[0].CmdID.Value), string(cmd.RawCommand), ) @@ -426,6 +426,31 @@ func syncMLForTest(locURI string) []byte { `, locURI, locURI)) } +func syncMLForTestWithExec(locURI string) []byte { + return []byte(fmt.Sprintf(` + + + + %s + + + + + + + %s + + + + + + + %s + + +`, locURI, locURI, locURI)) +} + func TestReconcileWindowsProfilesWithFleetVariableError(t *testing.T) { ctx := context.Background() ds := new(mock.Store)