diff --git a/changes/26688-error-on-signed-profiles b/changes/26688-error-on-signed-profiles new file mode 100644 index 0000000000..a45ef0e00b --- /dev/null +++ b/changes/26688-error-on-signed-profiles @@ -0,0 +1 @@ +* Error when uploading signed profiles instead of when trying to deliver them. \ No newline at end of file diff --git a/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/ProfileUploader/helpers.tsx b/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/ProfileUploader/helpers.tsx index c64b3e0d59..84d1922408 100644 --- a/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/ProfileUploader/helpers.tsx +++ b/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/ProfileUploader/helpers.tsx @@ -86,6 +86,31 @@ const generateUserChannelLearnMoreErrMsg = (errMsg: string) => { ); }; +/** + * Helper function to take whatever message is from the API and strip out the Learn More link and format it accordingly. + */ +const generateGenericLearnMoreErrMsg = (errMsg: string) => { + if (errMsg.includes(" Learn more: https://")) { + const message = errMsg.substring( + 0, + errMsg.indexOf(" Learn more: https://") + ); + const link = errMsg.substring(errMsg.indexOf("https://")); + return ( + <> + {message}{" "} + + + ); + } + return errMsg; +}; + /** We want to add some additional messageing to some of the error messages so * we add them in this function. Otherwise, we'll just return the error message from the * API. @@ -231,5 +256,9 @@ export const getErrorMessage = (err: AxiosResponse) => { return generateUserChannelLearnMoreErrMsg(apiReason); } + if (apiReason.includes("Configuration profiles can't be signed")) { + return generateGenericLearnMoreErrMsg(apiReason); + } + return `${apiReason}` || DEFAULT_ERROR_MESSAGE; }; diff --git a/go.mod b/go.mod index 94deecfabb..23e1e68aa5 100644 --- a/go.mod +++ b/go.mod @@ -57,7 +57,6 @@ require ( github.com/foxboron/go-tpm-keyfiles v0.0.0-20250520203025-c3c3a4ec1653 github.com/getsentry/sentry-go v0.18.0 github.com/ghodss/yaml v1.0.0 - github.com/github/smimesign v0.2.0 github.com/go-git/go-git/v5 v5.13.0 github.com/go-ini/ini v1.67.0 github.com/go-json-experiment/json v0.0.0-20250517221953-25912455fbc8 @@ -339,7 +338,6 @@ require ( go.opentelemetry.io/proto/otlp v1.7.0 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/time v0.11.0 // indirect - golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect google.golang.org/genproto v0.0.0-20241118233622-e639e219e697 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250603155806-513f23925822 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250603155806-513f23925822 // indirect diff --git a/go.sum b/go.sum index 4ac74acb81..a0a77004f3 100644 --- a/go.sum +++ b/go.sum @@ -193,7 +193,6 @@ github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyY github.com/cenkalti/backoff/v5 v5.0.2 h1:rIfFVxEf1QsI7E1ZHfp/B4DF/6QBAUhmgkxc0H7Zss8= github.com/cenkalti/backoff/v5 v5.0.2/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= -github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261/go.mod h1:GJKEexRPVJrBSOjoqN5VNOIKJ5Q3RViH6eu3puDRwx4= github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= @@ -334,8 +333,6 @@ github.com/getsentry/sentry-go v0.18.0 h1:MtBW5H9QgdcJabtZcuJG80BMOwaBpkRDZkxRkN github.com/getsentry/sentry-go v0.18.0/go.mod h1:Kgon4Mby+FJ7ZWHFUAZgVaIa8sxHtnRJRLTXZr51aKQ= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= -github.com/github/smimesign v0.2.0 h1:Hho4YcX5N1I9XNqhq0fNx0Sts8MhLonHd+HRXVGNjvk= -github.com/github/smimesign v0.2.0/go.mod h1:iZiiwNT4HbtGRVqCQu7uJPEZCuEE5sfSSttcnePkDl4= github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c= github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU= github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= @@ -692,7 +689,6 @@ github.com/pandatix/nvdapi v0.6.4 h1:gix57FcQtOklCUgFrJzJhRblYj+2DN9jxZP6oqtme+A github.com/pandatix/nvdapi v0.6.4/go.mod h1:DVYxPq0JRERgYzFmwTMknAtH4kB8v9KG+z40JWFRClk= github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc= github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= -github.com/pborman/getopt v0.0.0-20180811024354-2b5b3bfb099b/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pelletier/go-toml/v2 v2.2.3 h1:YmeHyLY8mFWbdkNWwpr+qIL2bEqT0o95WSdkNHvL12M= github.com/pelletier/go-toml/v2 v2.2.3/go.mod h1:MfCQTFTvCcUyyvvwm1+G6H/jORL20Xlb6rzQu9GuUkc= @@ -1081,8 +1077,6 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 h1:+cNy6SZtPcJQH3LJVLOSmiC7MMxXNOb3PU/VUEz+EhU= -golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90= google.golang.org/api v0.215.0 h1:jdYF4qnyczlEz2ReWIsosNLDuzXyvFHJtI5gcr0J7t0= google.golang.org/api v0.215.0/go.mod h1:fta3CVtuJYOEdugLNWm6WodzOS8KdFckABwN4I40hzY= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= diff --git a/server/fleet/apple_mdm_test.go b/server/fleet/apple_mdm_test.go index add3bae2dc..ac0be40bbe 100644 --- a/server/fleet/apple_mdm_test.go +++ b/server/fleet/apple_mdm_test.go @@ -87,7 +87,7 @@ func TestMDMAppleConfigProfile(t *testing.T) { return signedBytes }(), - shouldFail: false, + shouldFail: true, }, } diff --git a/server/mdm/apple/mobileconfig/mobileconfig.go b/server/mdm/apple/mobileconfig/mobileconfig.go index b3f20da01a..0dd88d48b3 100644 --- a/server/mdm/apple/mobileconfig/mobileconfig.go +++ b/server/mdm/apple/mobileconfig/mobileconfig.go @@ -10,9 +10,6 @@ import ( "github.com/fleetdm/fleet/v4/server/mdm" "github.com/fleetdm/fleet/v4/server/variables" - // we are using this package as we were having issues with pasrsing signed apple - // mobileconfig profiles with the pcks7 package we were using before. - cms "github.com/github/smimesign/ietf-cms" "howett.net/plist" ) @@ -86,24 +83,14 @@ type Parsed struct { PayloadScope string } -func (mc Mobileconfig) isSignedProfile() bool { +func (mc Mobileconfig) IsSignedProfile() bool { + trimmed := bytes.TrimSpace(mc) + if bytes.HasPrefix(trimmed, []byte("${FLEET_")) || bytes.HasPrefix(trimmed, []byte("$FLEET_")) { + return false // Not a signed profile since it only contains secret variable. + } return !bytes.HasPrefix(bytes.TrimSpace(mc), []byte(" section. mcBytes = variables.ProfileDataVariableRegex.ReplaceAll(mcBytes, []byte("")) - if mc.isSignedProfile() { - profileData, err := getSignedProfileData(mc) - if err != nil { - return nil, err - } - mcBytes = profileData - if variables.ContainsBytes(mcBytes) { - return nil, errors.New("a signed profile cannot contain Fleet variables ($FLEET_VAR_*)") - } + if mc.IsSignedProfile() { + return nil, errors.New("signed profiles are not supported") } var p Parsed if _, err := plist.Unmarshal(mcBytes, &p); err != nil { @@ -166,15 +146,8 @@ func (mc Mobileconfig) payloadSummary() ([]payloadSummary, error) { mcBytes := mc // Remove Fleet variables expected in section. mcBytes = variables.ProfileDataVariableRegex.ReplaceAll(mcBytes, []byte("")) - if mc.isSignedProfile() { - profileData, err := getSignedProfileData(mc) - if err != nil { - return nil, err - } - mcBytes = profileData - if variables.ContainsBytes(mcBytes) { - return nil, errors.New("a signed profile cannot contain Fleet variables ($FLEET_VAR_*)") - } + if mc.IsSignedProfile() { + return nil, errors.New("signed profiles are not supported") } // unmarshal the values we need from the top-level object diff --git a/server/mdm/apple/profile_matcher_test.go b/server/mdm/apple/profile_matcher_test.go index 2f2d290811..ea5d2e13e9 100644 --- a/server/mdm/apple/profile_matcher_test.go +++ b/server/mdm/apple/profile_matcher_test.go @@ -266,7 +266,7 @@ func TestPreassignProfileValidation(t *testing.T) { HostUUID: "1234", Profile: []byte(`abcd`), }, - "mobileconfig is not XML nor PKCS7", + "signed profiles are not supported", }, { "invalid profile type", diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 42a505db1e..f1e49f682a 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -380,6 +380,11 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, d return nil, ctxerr.Wrap(ctx, err, "check macOS MDM enabled") } + err := CheckProfileIsNotSigned(data) + if err != nil { + return nil, ctxerr.Wrap(ctx, err) + } + var teamName string if teamID >= 1 { tm, err := svc.EnterpriseOverrides.TeamByIDOrName(ctx, &teamID, nil) @@ -494,6 +499,19 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, d return newCP, nil } +// CheckProfileIsNotSigned checks if the provided profile data is a signed profile. +// If it is signed, it returns a BadRequestError indicating that signed profiles +// are not allowed. If the profile is not signed, it returns nil. +func CheckProfileIsNotSigned(data []byte) error { + mc := mobileconfig.Mobileconfig(data) + if mc.IsSignedProfile() { + return &fleet.BadRequestError{ + Message: "Couldn't add. Configuration profiles can't be signed. Fleet wil sign the profile for you. Learn more: https://fleetdm.com/learn-more-about/unsigning-configuration-profiles", + } + } + return nil +} + func validateConfigProfileFleetVariables(contents string, lic *fleet.LicenseInfo, groupedCAs *fleet.GroupedCertificateAuthorities) (map[string]struct{}, error) { fleetVars := variables.FindKeepDuplicates(contents) if len(fleetVars) == 0 { @@ -2587,6 +2605,12 @@ func (svc *Service) BatchSetMDMAppleProfiles(ctx context.Context, tmID *uint, tm fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), "maximum configuration profile file size is 1 MB"), ) } + + err := CheckProfileIsNotSigned(prof) + if err != nil { + return ctxerr.Wrap(ctx, err) + } + // Check for secrets in profile name before expansion if err := fleet.ValidateNoSecretsInProfileName(prof); err != nil { return ctxerr.Wrap(ctx, diff --git a/server/service/client.go b/server/service/client.go index 4e21088750..7e08952b7c 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -23,6 +23,7 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mdm" + "github.com/fleetdm/fleet/v4/server/mdm/apple/mobileconfig" "github.com/fleetdm/fleet/v4/server/ptr" kithttp "github.com/go-kit/kit/transport/http" ) @@ -349,6 +350,21 @@ func getProfilesContents(baseDir string, macProfiles, windowsProfiles, androidPr return nil, fmt.Errorf("applying custom settings: %w", err) } + ext := filepath.Ext(filePath) + // by default, use the file name (for macOS mobileconfig profiles, we'll switch to + // their PayloadDisplayName when we parse the profile below) + name := strings.TrimSuffix(filepath.Base(filePath), ext) + // for validation errors, we want to include the platform and file name in the error message + prefixErrMsg := fmt.Sprintf("Couldn't edit %s_settings.custom_settings (%s%s)", platform, name, ext) + + if platform == "macos" && (ext == ".mobileconfig" || ext == ".xml") { + // Parse and check for signed mobile configs before expanding. + mc := mobileconfig.Mobileconfig(fileContents) + if mc.IsSignedProfile() { + return nil, fmt.Errorf("%s: %s", prefixErrMsg, "Configuration profiles can't be signed. Fleet will sign the profile for you. Learn more: https://fleetdm.com/learn-more-about/unsigning-configuration-profiles") + } + } + if expandEnv { // Secrets are handled earlier in the flow when config files are initially read fileContents, err = spec.ExpandEnvBytesIgnoreSecrets(fileContents) @@ -357,13 +373,6 @@ func getProfilesContents(baseDir string, macProfiles, windowsProfiles, androidPr } } - ext := filepath.Ext(filePath) - // by default, use the file name (for macOS mobileconfig profiles, we'll switch to - // their PayloadDisplayName when we parse the profile below) - name := strings.TrimSuffix(filepath.Base(filePath), ext) - // for validation errors, we want to include the platform and file name in the error message - prefixErrMsg := fmt.Sprintf("Couldn't edit %s_settings.custom_settings (%s%s)", platform, name, ext) - // validate macOS profiles if platform == "macos" { switch ext { diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index 08630842bd..b2ac532688 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -3443,7 +3443,7 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() { "apple.mobileconfig", []byte("\x00\x01\x02"), s.token, nil) res = s.DoRawWithHeaders("POST", "/api/latest/fleet/configuration_profiles", body.Bytes(), http.StatusBadRequest, headers) errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "mobileconfig is not XML nor PKCS7 parseable") + require.Contains(t, errMsg, "Configuration profiles can't be signed. Fleet wil sign the profile for you.") // Apple/Android invalid json declaration body, headers = generateNewProfileMultipartRequest(t,