diff --git a/server/datastore/mysql/android_test.go b/server/datastore/mysql/android_test.go index 4cfffd2790d..94b364afa6a 100644 --- a/server/datastore/mysql/android_test.go +++ b/server/datastore/mysql/android_test.go @@ -2699,7 +2699,7 @@ func testAddDeleteAndroidAppWithConfiguration(t *testing.T, ds *Datastore) { test.CreateInsertGlobalVPPToken(t, ds) - testConfig := json.RawMessage(`{"ManagedConfiguration": {"DisableShareScreen": true, "DisableComputerAudio": true}}`) + testConfig := []byte(`{"ManagedConfiguration": {"DisableShareScreen": true, "DisableComputerAudio": true}}`) // Create android and VPP apps app1, err := ds.InsertVPPAppWithTeam(ctx, &fleet.VPPApp{ Name: "android1", BundleIdentifier: "android1", @@ -2738,7 +2738,7 @@ func testAddDeleteAndroidAppWithConfiguration(t *testing.T, ds *Datastore) { require.NotZero(t, meta2.VPPAppsTeamsID) // Edit android app - newConfig := json.RawMessage(`{"workProfileWidgets": "WORK_PROFILE_WIDGETS_ALLOWED"}`) + newConfig := []byte(`{"workProfileWidgets": "WORK_PROFILE_WIDGETS_ALLOWED"}`) app1.VPPAppTeam.Configuration = newConfig _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team1.ID) require.NoError(t, err) @@ -2750,7 +2750,7 @@ func testAddDeleteAndroidAppWithConfiguration(t *testing.T, ds *Datastore) { require.Equal(t, newConfig, meta.Configuration) // Add invalid configuration - badConfig := json.RawMessage(`"-": "-"`) + badConfig := []byte(`"-": "-"`) app1.VPPAppTeam.Configuration = badConfig _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team1.ID) require.Error(t, err) diff --git a/server/datastore/mysql/vpp_test.go b/server/datastore/mysql/vpp_test.go index f59c671a5fb..dc1ad0646b0 100644 --- a/server/datastore/mysql/vpp_test.go +++ b/server/datastore/mysql/vpp_test.go @@ -2588,7 +2588,7 @@ func testAndroidAppConfigs(t *testing.T, ds *Datastore) { require.NoError(t, err) config1 := json.RawMessage(`{"workProfileWidgets":"WORK_PROFILE_WIDGETS_ALLOWED", "managedConfiguration": {"1":1}}`) - expectedConfig1 := json.RawMessage(`{"workProfileWidgets": "WORK_PROFILE_WIDGETS_ALLOWED", "managedConfiguration": {"1": 1}}`) + expectedConfig1 := []byte(`{"workProfileWidgets": "WORK_PROFILE_WIDGETS_ALLOWED", "managedConfiguration": {"1": 1}}`) _, err = ds.SetTeamVPPApps(ctx, &team.ID, []fleet.VPPAppTeam{ {VPPAppID: app1.VPPAppID, SelfService: true, DisplayName: ptr.String("name 1")}, @@ -2611,9 +2611,9 @@ func testAndroidAppConfigs(t *testing.T, ds *Datastore) { } } - require.Equal(t, json.RawMessage(nil), assigned[app1.VPPAppID].Configuration) - require.Equal(t, json.RawMessage(nil), assigned[app2.VPPAppID].Configuration) - require.Equal(t, json.RawMessage(`{}`), assigned[app3.VPPAppID].Configuration) + require.Equal(t, []byte(nil), assigned[app1.VPPAppID].Configuration) + require.Equal(t, []byte(nil), assigned[app2.VPPAppID].Configuration) + require.Equal(t, []byte(`{}`), assigned[app3.VPPAppID].Configuration) require.Equal(t, expectedConfig1, assigned[app4.VPPAppID].Configuration) _, err = ds.SetTeamVPPApps(ctx, &team.ID, []fleet.VPPAppTeam{ @@ -2645,9 +2645,9 @@ func testAndroidAppConfigs(t *testing.T, ds *Datastore) { } } - require.Equal(t, json.RawMessage(nil), assigned[app1.VPPAppID].Configuration) - require.Equal(t, json.RawMessage(`{"managedConfiguration": 1}`), assigned[app2.VPPAppID].Configuration) - require.Equal(t, json.RawMessage(`{}`), assigned[app3.VPPAppID].Configuration) + require.Equal(t, []byte(nil), assigned[app1.VPPAppID].Configuration) + require.Equal(t, []byte(`{"managedConfiguration": 1}`), assigned[app2.VPPAppID].Configuration) + require.Equal(t, []byte(`{}`), assigned[app3.VPPAppID].Configuration) require.Equal(t, expectedConfig1, assigned[app4.VPPAppID].Configuration) // Delete all diff --git a/server/fleet/vpp.go b/server/fleet/vpp.go index ec132992a57..21e5936d083 100644 --- a/server/fleet/vpp.go +++ b/server/fleet/vpp.go @@ -1,9 +1,12 @@ package fleet import ( - "encoding/json" "fmt" + "slices" "time" + + "github.com/fleetdm/fleet/v4/server/variables" + "howett.net/plist" ) type VPPAppID struct { @@ -56,12 +59,12 @@ type VPPAppTeam struct { // app creation if AddAutoInstallPolicy is true. AddedAutomaticInstallPolicy *Policy `json:"-"` DisplayName *string `json:"display_name"` - // Configuration is a json file used to customize Android app - // behavior/settings. Applicable to Android apps only. - Configuration json.RawMessage `json:"configuration,omitempty"` - AutoUpdateEnabled *bool `json:"-"` - AutoUpdateStartTime *string `json:"-"` - AutoUpdateEndTime *string `json:"-"` + // Configuration is the managed app configuration payload. JSON for Android, + // XML for iOS / iPadOS. + Configuration []byte `json:"configuration,omitempty"` + AutoUpdateEnabled *bool `json:"-"` + AutoUpdateStartTime *string `json:"-"` + AutoUpdateEndTime *string `json:"-"` } func (v VPPAppTeam) GetPlatform() string { @@ -129,9 +132,9 @@ type VPPAppStoreApp struct { // "Browsers", etc. Categories []string `json:"categories"` DisplayName string `json:"display_name"` - // Configuration is a json file used to customize Android app - // behavior/settings. Applicable to Android apps only. - Configuration json.RawMessage `json:"configuration,omitempty"` + // Configuration is the managed app configuration payload. JSON for Android, + // XML for iOS / iPadOS. + Configuration []byte `json:"configuration,omitempty"` } // VPPAppStatusSummary represents aggregated status metrics for a VPP app. @@ -197,6 +200,88 @@ type AppStoreAppUpdatePayload struct { LabelsIncludeAll []string Categories []string DisplayName *string - Configuration json.RawMessage + Configuration []byte SoftwareAutoUpdateConfig } + +// FleetVarsSupportedInAppleAppConfig is the allow-list of Fleet variables that +// can appear in an iOS / iPadOS managed app configuration plist. Subset of the +// variables supported in Apple configuration profiles — credential variables +// (NDES, SCEP, DigiCert) don't fit the InstallApplication command shape. +var FleetVarsSupportedInAppleAppConfig = []FleetVarName{ + FleetVarHostUUID, + FleetVarHostHardwareSerial, + FleetVarHostPlatform, + FleetVarHostEndUserEmailIDP, + FleetVarHostEndUserIDPUsername, + FleetVarHostEndUserIDPUsernameLocalPart, + FleetVarHostEndUserIDPGroups, + FleetVarHostEndUserIDPDepartment, + FleetVarHostEndUserIDPFullname, +} + +// ValidateAppleAppConfiguration validates a managed app configuration payload +// for an iOS or iPadOS InstallApplication command. The payload must be an XML +// plist whose root element is a , and any Fleet variable tokens used in +// string values or keys must be drawn from FleetVarsSupportedInAppleAppConfig. +// Empty input is allowed — callers decide whether to store or clear. +func ValidateAppleAppConfiguration(config []byte) error { + if len(config) == 0 { + return nil + } + + var root map[string]any + format, err := plist.Unmarshal(config, &root) + if err != nil { + return NewInvalidArgumentError("configuration", fmt.Sprintf("invalid plist: %s", err)) + } + // Apple's MDM InstallApplication only accepts XML plist for the + // Configuration dict; reject binary, OpenStep and GNUStep formats up front + // so admins don't get surprised when devices reject the install. + if format != plist.XMLFormat { + return NewInvalidArgumentError("configuration", "configuration must be an XML plist") + } + + // Raw-bytes scan catches structural bypasses (duplicate keys, trailing + // siblings) invisible to the decoded tree. The decoded-tree walk below + // catches XML-entity-encoded tokens the regex won't match. + for _, name := range variables.Find(string(config)) { + if !slices.Contains(FleetVarsSupportedInAppleAppConfig, FleetVarName(name)) { + return NewInvalidArgumentError("configuration", fmt.Sprintf("unsupported variable $FLEET_VAR_%s", name)) + } + } + if name, ok := findUnsupportedFleetVar(root); ok { + return NewInvalidArgumentError("configuration", fmt.Sprintf("unsupported variable $FLEET_VAR_%s", name)) + } + return nil +} + +// findUnsupportedFleetVar walks v's keys and string values and returns the +// first $FLEET_VAR_* token that is not in FleetVarsSupportedInAppleAppConfig. +// The second return is false when every referenced variable is allowed. +func findUnsupportedFleetVar(v any) (string, bool) { + switch t := v.(type) { + case string: + for _, name := range variables.Find(t) { + if !slices.Contains(FleetVarsSupportedInAppleAppConfig, FleetVarName(name)) { + return name, true + } + } + case map[string]any: + for k, val := range t { + if name, ok := findUnsupportedFleetVar(k); ok { + return name, ok + } + if name, ok := findUnsupportedFleetVar(val); ok { + return name, ok + } + } + case []any: + for _, val := range t { + if name, ok := findUnsupportedFleetVar(val); ok { + return name, ok + } + } + } + return "", false +} diff --git a/server/fleet/vpp_test.go b/server/fleet/vpp_test.go new file mode 100644 index 00000000000..22efa50ac84 --- /dev/null +++ b/server/fleet/vpp_test.go @@ -0,0 +1,172 @@ +package fleet + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidateAppleAppConfiguration(t *testing.T) { + const fragment = ` + ServerURL + https://example.com + EnableTelemetry + +` + + const fullDoc = ` + + + + ServerURL + https://example.com + +` + + const nested = ` + Outer + + Inner + value + + List + + + K + v + + +` + + cases := []struct { + name string + input string + wantErr bool + errSub string + }{ + {name: "empty", input: ""}, + {name: "bare dict fragment", input: fragment}, + {name: "full plist document", input: fullDoc}, + {name: "nested dict and array of dicts", input: nested}, + {name: "garbage non-XML", input: "not a plist", wantErr: true, errSub: "invalid plist"}, + {name: "malformed XML unclosed tag", input: "foobar", wantErr: true, errSub: "invalid plist"}, + {name: "root is array", input: `x`, wantErr: true, errSub: "invalid plist"}, + {name: "root is string", input: `oops`, wantErr: true, errSub: "invalid plist"}, + { + name: "allowed variable", + input: `HostID$FLEET_VAR_HOST_UUID`, + }, + { + name: "allowed variable with braces", + input: `HostID${FLEET_VAR_HOST_UUID}`, + }, + { + name: "multiple allowed variables in one string", + input: `Khttps://x/$FLEET_VAR_HOST_UUID/$FLEET_VAR_HOST_HARDWARE_SERIAL`, + }, + { + name: "credential variable not allowed in app config", + input: `K$FLEET_VAR_NDES_SCEP_CHALLENGE`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_NDES_SCEP_CHALLENGE", + }, + { + name: "unknown variable name", + input: `K$FLEET_VAR_BOGUS_NAME`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_BOGUS_NAME", + }, + { + name: "all standard plist value types accepted", + input: ` + Sval + I42 + R3.14 + T + F + DYWJj + Ax1 +`, + }, + { + name: "ASCII control character in string value", + input: "Kx\x01y", + wantErr: true, + errSub: "invalid plist", + }, + { + name: "json null token", + input: "null", + wantErr: true, + errSub: "invalid plist", + }, + { + name: "hex-entity-encoded $ does not bypass disallow list", + input: `K$FLEET_VAR_NDES_SCEP_CHALLENGE`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_NDES_SCEP_CHALLENGE", + }, + { + name: "decimal-entity-encoded $ does not bypass disallow list", + input: `K$FLEET_VAR_NDES_SCEP_CHALLENGE`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_NDES_SCEP_CHALLENGE", + }, + { + name: "disallowed variable inside a CDATA section is caught", + input: `K`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_NDES_SCEP_CHALLENGE", + }, + { + name: "disallowed variable nested inside an array is caught", + input: `KInner$FLEET_VAR_BOGUS`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_BOGUS", + }, + { + name: "disallowed variable used as a key is caught", + input: `$FLEET_VAR_BOGUSx`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_BOGUS", + }, + { + name: "duplicate key bypass: disallowed var hidden by last-wins map semantics", + input: `K$FLEET_VAR_NDES_SCEP_CHALLENGEKsafe_value`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_NDES_SCEP_CHALLENGE", + }, + { + name: "trailing sibling bypass: disallowed var in element dropped by parser", + input: `KsafeX$FLEET_VAR_NDES_SCEP_CHALLENGE`, + wantErr: true, + errSub: "unsupported variable $FLEET_VAR_NDES_SCEP_CHALLENGE", + }, + { + name: "openstep dict format rejected", + input: `{ServerURL = "https://x.com";}`, + wantErr: true, + errSub: "must be an XML plist", + }, + { + name: "binary plist rejected", + input: "bplist00\xd1\x01\x02Q1Q2\x08\x0b\r\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0f", + wantErr: true, + errSub: "must be an XML plist", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := ValidateAppleAppConfiguration([]byte(c.input)) + if c.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), c.errSub) + var iae *InvalidArgumentError + require.ErrorAs(t, err, &iae) + return + } + require.NoError(t, err) + }) + } +} diff --git a/server/service/integration_android_software_test.go b/server/service/integration_android_software_test.go index 33e1eea4f3a..91b90c2604c 100644 --- a/server/service/integration_android_software_test.go +++ b/server/service/integration_android_software_test.go @@ -771,7 +771,7 @@ func (s *integrationMDMTestSuite) TestBatchAndroidApps() { TeamID: teamID, }, http.StatusOK, &titleResp) require.Equal(t, "app_1", *titleResp.SoftwareTitle.ApplicationID) - require.Equal(t, json.RawMessage(`{}`), titleResp.SoftwareTitle.AppStoreApp.Configuration) + require.Equal(t, []byte(`{}`), titleResp.SoftwareTitle.AppStoreApp.Configuration) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", titleApp2), &getSoftwareTitleRequest{ ID: titleApp2, @@ -800,7 +800,7 @@ func (s *integrationMDMTestSuite) TestBatchAndroidApps() { TeamID: teamID, }, http.StatusOK, &titleResp) require.Equal(t, "app_1", *titleResp.SoftwareTitle.ApplicationID) - require.Equal(t, json.RawMessage(`{}`), titleResp.SoftwareTitle.AppStoreApp.Configuration) + require.Equal(t, []byte(`{}`), titleResp.SoftwareTitle.AppStoreApp.Configuration) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", titleApp2), &getSoftwareTitleRequest{ ID: titleApp2,