Error on signed configuration profiles (#33341)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #26688 

I'm not sure if the IsSignedProfile check is too aggressive and can
potentially shadow other problems with the file?

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually


## Media:
Gitops
<img width="575" height="189" alt="Screenshot 2025-09-23 at 11 48 19"
src="https://github.com/user-attachments/assets/1e7c950e-2543-4c9a-b6f0-8b546a30eb1f"
/>

API
<img width="1318" height="169" alt="Screenshot 2025-09-23 at 12 04 22"
src="https://github.com/user-attachments/assets/fc8f9171-fab9-46be-befa-dc6af82d2f7b"
/>


Frontend
<img width="779" height="89" alt="Screenshot 2025-09-23 at 12 01 59"
src="https://github.com/user-attachments/assets/78dcaf56-d344-4499-bdfa-1abb97b29b15"
/>
This commit is contained in:
Magnus Jensen 2025-09-25 14:50:48 +03:00 committed by GitHub
parent 8e5bac79ca
commit 61347155b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 82 additions and 54 deletions

View file

@ -0,0 +1 @@
* Error when uploading signed profiles instead of when trying to deliver them.

View file

@ -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}{" "}
<CustomLink
url={link}
text="Learn more"
variant="flash-message-link"
newTab
/>
</>
);
}
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<IApiError>) => {
return generateUserChannelLearnMoreErrMsg(apiReason);
}
if (apiReason.includes("Configuration profiles can't be signed")) {
return generateGenericLearnMoreErrMsg(apiReason);
}
return `${apiReason}` || DEFAULT_ERROR_MESSAGE;
};

2
go.mod
View file

@ -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

6
go.sum
View file

@ -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=

View file

@ -87,7 +87,7 @@ func TestMDMAppleConfigProfile(t *testing.T) {
return signedBytes
}(),
shouldFail: false,
shouldFail: true,
},
}

View file

@ -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("<?xml"))
}
// getSignedProfileData attempts to parse the signed mobileconfig and extract the
// profile byte data from it.
func getSignedProfileData(mc Mobileconfig) (Mobileconfig, error) {
signedData, err := cms.ParseSignedData(mc)
if err != nil {
return nil, fmt.Errorf("mobileconfig is not XML nor PKCS7 parseable: %w", err)
}
data, err := signedData.GetData()
if err != nil {
return nil, fmt.Errorf("could not get profile data from the signed mobileconfig: %w", err)
}
return Mobileconfig(data), nil
}
// ParseConfigProfile attempts to parse the Mobileconfig byte slice as a Fleet MDMAppleConfigProfile.
//
// The byte slice must be XML or PKCS7 parseable. Fleet also requires that it contains both
@ -114,15 +101,8 @@ func (mc Mobileconfig) ParseConfigProfile() (*Parsed, error) {
mcBytes := mc
// Remove Fleet variables expected in <data> 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 <data> 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

View file

@ -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",

View file

@ -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,

View file

@ -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 {

View file

@ -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,