diff --git a/changes/19808-prof b/changes/19808-prof new file mode 100644 index 0000000000..71d19f8c4b --- /dev/null +++ b/changes/19808-prof @@ -0,0 +1 @@ +* Fixed bugs on enrollment profiles when the organization name contains invalid XML characters. diff --git a/server/mdm/apple/apple_mdm.go b/server/mdm/apple/apple_mdm.go index 0275cb226e..1cf44a437c 100644 --- a/server/mdm/apple/apple_mdm.go +++ b/server/mdm/apple/apple_mdm.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "encoding/xml" "fmt" "net/url" "slices" @@ -659,11 +658,15 @@ func NewDEPClient(storage godep.ClientStorage, appCfgUpdater fleet.AppConfigUpda })) } +var funcMap = map[string]any{ + "xml": mobileconfig.XMLEscapeString, +} + // enrollmentProfileMobileconfigTemplate is the template Fleet uses to assemble a .mobileconfig enrollment profile to serve to devices. // // During a profile replacement, the system updates payloads with the same PayloadIdentifier and // PayloadUUID in the old and new profiles. -var enrollmentProfileMobileconfigTemplate = template.Must(template.New("").Parse(` +var enrollmentProfileMobileconfigTemplate = template.Must(template.New("").Funcs(funcMap).Parse(` @@ -676,7 +679,7 @@ var enrollmentProfileMobileconfigTemplate = template.Must(template.New("").Parse Key Type RSA Challenge - {{ .SCEPChallenge }} + {{ .SCEPChallenge | xml }} Key Usage 5 Keysize @@ -727,11 +730,11 @@ var enrollmentProfileMobileconfigTemplate = template.Must(template.New("").Parse PayloadDisplayName - {{ .Organization }} enrollment + {{ .Organization | xml }} enrollment PayloadIdentifier ` + FleetPayloadIdentifier + ` PayloadOrganization - {{ .Organization }} + {{ .Organization | xml }} PayloadScope System PayloadType @@ -753,13 +756,8 @@ func GenerateEnrollmentProfileMobileconfig(orgName, fleetURL, scepChallenge, top return nil, fmt.Errorf("resolve Apple MDM url: %w", err) } - var escaped strings.Builder - if err := xml.EscapeText(&escaped, []byte(scepChallenge)); err != nil { - return nil, fmt.Errorf("escape SCEP challenge for XML: %w", err) - } - var buf bytes.Buffer - if err := enrollmentProfileMobileconfigTemplate.Execute(&buf, struct { + if err := enrollmentProfileMobileconfigTemplate.Funcs(funcMap).Execute(&buf, struct { Organization string SCEPURL string SCEPChallenge string @@ -768,7 +766,7 @@ func GenerateEnrollmentProfileMobileconfig(orgName, fleetURL, scepChallenge, top }{ Organization: orgName, SCEPURL: scepURL, - SCEPChallenge: escaped.String(), + SCEPChallenge: scepChallenge, Topic: topic, ServerURL: serverURL, }); err != nil { diff --git a/server/mdm/apple/apple_mdm_test.go b/server/mdm/apple/apple_mdm_test.go index a03b5030d7..ace48cae5e 100644 --- a/server/mdm/apple/apple_mdm_test.go +++ b/server/mdm/apple/apple_mdm_test.go @@ -16,6 +16,7 @@ import ( "github.com/fleetdm/fleet/v4/server/mock" nanodep_mock "github.com/fleetdm/fleet/v4/server/mock/nanodep" "github.com/go-kit/log" + "github.com/groob/plist" "github.com/stretchr/testify/require" ) @@ -183,6 +184,79 @@ func TestAddEnrollmentRefToFleetURL(t *testing.T) { } } +func TestGenerateEnrollmentProfileMobileconfig(t *testing.T) { + type scepPayload struct { + Challenge string + URL string + } + + type enrollmentPayload struct { + PayloadType string + ServerURL string // used by the enrollment payload + PayloadContent scepPayload // scep contains a nested payload content dict + } + + type enrollmentProfile struct { + PayloadIdentifier string + PayloadContent []enrollmentPayload + } + + tests := []struct { + name string + orgName string + fleetURL string + scepChallenge string + expectError bool + }{ + { + name: "valid input with simple values", + orgName: "Fleet", + fleetURL: "https://example.com", + scepChallenge: "testChallenge", + expectError: false, + }, + { + name: "organization name and enroll secret with special characters", + orgName: `Fleet & Co. "Special" `, + fleetURL: "https://example.com", + scepChallenge: "test/&Challenge", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := GenerateEnrollmentProfileMobileconfig(tt.orgName, tt.fleetURL, tt.scepChallenge, "com.foo.bar") + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + require.NotNil(t, result) + + var profile enrollmentProfile + + require.NoError(t, plist.Unmarshal(result, &profile)) + + for _, p := range profile.PayloadContent { + switch p.PayloadType { + case "com.apple.security.scep": + scepURL, err := ResolveAppleSCEPURL(tt.fleetURL) + require.NoError(t, err) + require.Equal(t, scepURL, p.PayloadContent.URL) + require.Equal(t, tt.scepChallenge, p.PayloadContent.Challenge) + case "com.apple.mdm": + mdmURL, err := ResolveAppleMDMURL(tt.fleetURL) + require.NoError(t, err) + require.Contains(t, mdmURL, p.ServerURL) + default: + require.Failf(t, "unrecognized payload type in enrollment profile: %s", p.PayloadType) + } + } + } + }) + } +} + type notFoundError struct{} func (e notFoundError) IsNotFound() bool { return true } diff --git a/server/mdm/apple/mobileconfig/mobileconfig.go b/server/mdm/apple/mobileconfig/mobileconfig.go index 31edd44399..690b15c796 100644 --- a/server/mdm/apple/mobileconfig/mobileconfig.go +++ b/server/mdm/apple/mobileconfig/mobileconfig.go @@ -2,6 +2,7 @@ package mobileconfig import ( "bytes" + "encoding/xml" "errors" "fmt" "strings" @@ -264,3 +265,17 @@ var ( ErrEmptyPayloadContent = errors.New("empty PayloadContent") ErrEncryptedPayloadContent = errors.New("encrypted PayloadContent") ) + +// XMLEscapeString returns the escaped XML equivalent of the plain text data s. +func XMLEscapeString(s string) (string, error) { + // avoid allocation if we can. + if !strings.ContainsAny(s, "'\"&<>\t\n\r") { + return s, nil + } + var b strings.Builder + if err := xml.EscapeText(&b, []byte(s)); err != nil { + return "", err + } + + return b.String(), nil +} diff --git a/server/mdm/apple/mobileconfig/mobileconfig_test.go b/server/mdm/apple/mobileconfig/mobileconfig_test.go new file mode 100644 index 0000000000..793503a438 --- /dev/null +++ b/server/mdm/apple/mobileconfig/mobileconfig_test.go @@ -0,0 +1,36 @@ +package mobileconfig + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestXMLEscapeString(t *testing.T) { + tests := []struct { + input string + expected string + }{ + // characters that should be escaped + {"hello & world", "hello & world"}, + {"this is a ", "this is a <test>"}, + {"\"quotes\" and 'single quotes'", ""quotes" and 'single quotes'"}, + {"special chars: \t\n\r", "special chars: "}, + // no special characters + {"plain string", "plain string"}, + // string that already contains escaped characters + {"already <escaped>", "already &lt;escaped&gt;"}, + // empty string + {"", ""}, + // multiple special characters + {"A&BD\"'E\tF\nG\r", "A&B<C>D"'E F G "}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + out, err := XMLEscapeString(tt.input) + require.NoError(t, err) + require.Equal(t, tt.expected, out) + }) + } +} diff --git a/server/mdm/apple/mobileconfig/profiles.go b/server/mdm/apple/mobileconfig/profiles.go index 24bf6479ba..a2dfdf4438 100644 --- a/server/mdm/apple/mobileconfig/profiles.go +++ b/server/mdm/apple/mobileconfig/profiles.go @@ -2,6 +2,10 @@ package mobileconfig import "text/template" +var funcMap = map[string]any{ + "xml": XMLEscapeString, +} + // FleetdProfileOptions are the keys required to execute a // FleetdProfileTemplate. type FleetdProfileOptions struct { @@ -20,7 +24,7 @@ type FleetdProfileOptions struct { // // Internally, this is used by Fleet MDM to configure the installer delivered // to hosts during DEP enrollment. -var FleetdProfileTemplate = template.Must(template.New("").Option("missingkey=error").Parse(` +var FleetdProfileTemplate = template.Must(template.New("").Funcs(funcMap).Option("missingkey=error").Parse(` @@ -28,7 +32,7 @@ var FleetdProfileTemplate = template.Must(template.New("").Option("missingkey=er EnrollSecret - {{ .EnrollSecret }} + {{ .EnrollSecret | xml }} FleetURL {{ .ServerURL }} EnableScripts diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index fffda99b09..61c90f5ce1 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -308,7 +308,8 @@ func (s *integrationMDMTestSuite) SetupSuite() { APNSTopic: "com.apple.mgmt.External.10ac3ce5-4668-4e58-b69a-b2b5ce667589", } - s.scepChallenge = "scepchallenge" + // ensure all our tests support challenges with invalid XML characters + s.scepChallenge = "scepcha/>