From c9bdae8fb3eb0a39c1b6e6a0fce631d58715465b Mon Sep 17 00:00:00 2001 From: Dante Catalfamo <43040593+dantecatalfamo@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:14:12 -0500 Subject: [PATCH] Embedded secrets validation (#24624) #24549 Co-authored-by: Victor Lyuboslavsky --- changes/24549-validate-script-profle-secrets | 1 + cmd/fleetctl/testing_utils.go | 3 + ee/server/service/setup_experience.go | 4 + pkg/spec/spec.go | 6 +- server/datastore/mysql/secret_variables.go | 77 +++++++++++++++ .../datastore/mysql/secret_variables_test.go | 96 +++++++++++++++++++ server/fleet/datastore.go | 8 ++ server/fleet/fleet_vars_test.go | 6 +- server/fleet/secrets.go | 23 ++++- server/mock/datastore_mock.go | 24 +++++ server/service/apple_mdm.go | 39 +++++++- server/service/apple_mdm_test.go | 6 ++ server/service/integration_enterprise_test.go | 17 ++++ server/service/integration_mdm_dep_test.go | 5 + .../service/integration_mdm_profiles_test.go | 60 +++++++++++- server/service/mdm.go | 33 ++++++- server/service/mdm_test.go | 21 ++++ server/service/scripts.go | 18 ++++ server/service/scripts_test.go | 9 ++ server/service/setup_experience_test.go | 6 ++ 20 files changed, 448 insertions(+), 14 deletions(-) create mode 100644 changes/24549-validate-script-profle-secrets diff --git a/changes/24549-validate-script-profle-secrets b/changes/24549-validate-script-profle-secrets new file mode 100644 index 000000000000..fdf7ea4a416e --- /dev/null +++ b/changes/24549-validate-script-profle-secrets @@ -0,0 +1 @@ +- Validate fleet secrets embedded into scripts and profiles on ingestion diff --git a/cmd/fleetctl/testing_utils.go b/cmd/fleetctl/testing_utils.go index 0c4507b359e3..19bb8bdaeff3 100644 --- a/cmd/fleetctl/testing_utils.go +++ b/cmd/fleetctl/testing_utils.go @@ -152,6 +152,9 @@ func runServerWithMockedDS(t *testing.T, opts ...*service.TestServerOpts) (*http ds.ApplyYaraRulesFunc = func(context.Context, []fleet.YaraRule) error { return nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } var cachedDS fleet.Datastore if len(opts) > 0 && opts[0].NoCacheDatastore { diff --git a/ee/server/service/setup_experience.go b/ee/server/service/setup_experience.go index 7bece3103e03..c11b1ac05f92 100644 --- a/ee/server/service/setup_experience.go +++ b/ee/server/service/setup_experience.go @@ -76,6 +76,10 @@ func (svc *Service) SetSetupExperienceScript(ctx context.Context, teamID *uint, ScriptContents: string(b), } + if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{script.ScriptContents}); err != nil { + return fleet.NewInvalidArgumentError("script", err.Error()) + } + // setup experience is only supported for macOS currently so we need to override the file // extension check in the general script validation if filepath.Ext(script.Name) != ".sh" { diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 08eed07042bc..ce7eb4b81744 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -186,10 +186,10 @@ func expandEnv(s string, failOnSecret bool) (string, error) { case strings.HasPrefix(env, fleet.ServerVarPrefix): // Don't expand fleet vars -- they will be expanded on the server return "", false - case strings.HasPrefix(env, fleet.FLEET_SECRET_PREFIX): + case strings.HasPrefix(env, fleet.ServerSecretPrefix): if failOnSecret { err = multierror.Append(err, fmt.Errorf("environment variables with %q prefix are only allowed in profiles and scripts: %q", - fleet.FLEET_SECRET_PREFIX, env)) + fleet.ServerSecretPrefix, env)) } return "", false } @@ -231,7 +231,7 @@ func LookupEnvSecrets(s string, secretsMap map[string]string) error { } var err *multierror.Error _ = fleet.MaybeExpand(s, func(env string) (string, bool) { - if strings.HasPrefix(env, fleet.FLEET_SECRET_PREFIX) { + if strings.HasPrefix(env, fleet.ServerSecretPrefix) { // lookup the secret and save it, but don't replace v, ok := os.LookupEnv(env) if !ok { diff --git a/server/datastore/mysql/secret_variables.go b/server/datastore/mysql/secret_variables.go index fa8d1f6fca41..50ff7955d215 100644 --- a/server/datastore/mysql/secret_variables.go +++ b/server/datastore/mysql/secret_variables.go @@ -68,3 +68,80 @@ func (ds *Datastore) GetSecretVariables(ctx context.Context, names []string) ([] return secretVariables, nil } + +func (ds *Datastore) ExpandEmbeddedSecrets(ctx context.Context, document string) (string, error) { + embeddedSecrets := fleet.ContainsPrefixVars(document, fleet.ServerSecretPrefix) + + secrets, err := ds.GetSecretVariables(ctx, embeddedSecrets) + if err != nil { + return "", ctxerr.Wrap(ctx, err, "expanding embedded secrets") + } + + secretMap := make(map[string]string, len(secrets)) + + for _, secret := range secrets { + secretMap[secret.Name] = secret.Value + } + + missingSecrets := []string{} + + for _, wantSecret := range embeddedSecrets { + if _, ok := secretMap[wantSecret]; !ok { + missingSecrets = append(missingSecrets, wantSecret) + } + } + + if len(missingSecrets) > 0 { + return "", fleet.MissingSecretsError{MissingSecrets: missingSecrets} + } + + expanded := fleet.MaybeExpand(document, func(s string) (string, bool) { + if !strings.HasPrefix(s, fleet.ServerSecretPrefix) { + return "", false + } + val, ok := secretMap[strings.TrimPrefix(s, fleet.ServerSecretPrefix)] + return val, ok + }) + + return expanded, nil +} + +func (ds *Datastore) ValidateEmbeddedSecrets(ctx context.Context, documents []string) error { + wantSecrets := make(map[string]struct{}) + haveSecrets := make(map[string]struct{}) + + for _, document := range documents { + vars := fleet.ContainsPrefixVars(document, fleet.ServerSecretPrefix) + for _, v := range vars { + wantSecrets[v] = struct{}{} + } + } + + wantSecretsList := make([]string, 0, len(wantSecrets)) + for wantSecret := range wantSecrets { + wantSecretsList = append(wantSecretsList, wantSecret) + } + + dbSecrets, err := ds.GetSecretVariables(ctx, wantSecretsList) + if err != nil { + return ctxerr.Wrap(ctx, err, "validating document embedded secrets") + } + + for _, dbSecret := range dbSecrets { + haveSecrets[dbSecret.Name] = struct{}{} + } + + missingSecrets := []string{} + + for wantSecret := range wantSecrets { + if _, ok := haveSecrets[wantSecret]; !ok { + missingSecrets = append(missingSecrets, wantSecret) + } + } + + if len(missingSecrets) > 0 { + return &fleet.MissingSecretsError{MissingSecrets: missingSecrets} + } + + return nil +} diff --git a/server/datastore/mysql/secret_variables_test.go b/server/datastore/mysql/secret_variables_test.go index 72eaa5ab3030..8936442b58f0 100644 --- a/server/datastore/mysql/secret_variables_test.go +++ b/server/datastore/mysql/secret_variables_test.go @@ -17,6 +17,8 @@ func TestSecretVariables(t *testing.T) { fn func(t *testing.T, ds *Datastore) }{ {"UpsertSecretVariables", testUpsertSecretVariables}, + {"ValidateEmbeddedSecrets", testValidateEmbeddedSecrets}, + {"ExpandEmbeddedSecrets", testExpandEmbeddedSecrets}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -70,3 +72,97 @@ func testUpsertSecretVariables(t *testing.T, ds *Datastore) { assert.Equal(t, secretMap[results[0].Name], results[0].Value) } + +func testValidateEmbeddedSecrets(t *testing.T, ds *Datastore) { + noSecrets := ` +This document contains to fleet secrets. +$FLEET_VAR_XX $HOSTNAME ${SOMETHING_ELSE} +` + + validSecret := ` +This document contains a valid ${FLEET_SECRET_VALID}. +Another $FLEET_SECRET_ALSO_VALID. +` + + invalidSecret := ` +This document contains a secret not stored in the database. +Hello doc${FLEET_SECRET_INVALID}. $FLEET_SECRET_ALSO_INVALID +` + ctx := context.Background() + secretMap := map[string]string{ + "VALID": "testValue1", + "ALSO_VALID": "testValue2", + } + + secrets := make([]fleet.SecretVariable, 0, len(secretMap)) + for name, value := range secretMap { + secrets = append(secrets, fleet.SecretVariable{Name: name, Value: value}) + } + + err := ds.UpsertSecretVariables(ctx, secrets) + require.NoError(t, err) + + err = ds.ValidateEmbeddedSecrets(ctx, []string{noSecrets}) + require.NoError(t, err) + + err = ds.ValidateEmbeddedSecrets(ctx, []string{validSecret}) + require.NoError(t, err) + + err = ds.ValidateEmbeddedSecrets(ctx, []string{noSecrets, validSecret}) + require.NoError(t, err) + + err = ds.ValidateEmbeddedSecrets(ctx, []string{invalidSecret}) + require.ErrorContains(t, err, "$FLEET_SECRET_INVALID") + require.ErrorContains(t, err, "$FLEET_SECRET_ALSO_INVALID") + + err = ds.ValidateEmbeddedSecrets(ctx, []string{noSecrets, validSecret, invalidSecret}) + require.ErrorContains(t, err, "$FLEET_SECRET_INVALID") + require.ErrorContains(t, err, "$FLEET_SECRET_ALSO_INVALID") +} + +func testExpandEmbeddedSecrets(t *testing.T, ds *Datastore) { + noSecrets := ` +This document contains to fleet secrets. +$FLEET_VAR_XX $HOSTNAME ${SOMETHING_ELSE} +` + + validSecret := ` +This document contains a valid ${FLEET_SECRET_VALID}. +Another $FLEET_SECRET_ALSO_VALID. +` + validSecretExpanded := ` +This document contains a valid testValue1. +Another testValue2. +` + + invalidSecret := ` +This document contains a secret not stored in the database. +Hello doc${FLEET_SECRET_INVALID}. $FLEET_SECRET_ALSO_INVALID +` + + ctx := context.Background() + secretMap := map[string]string{ + "VALID": "testValue1", + "ALSO_VALID": "testValue2", + } + + secrets := make([]fleet.SecretVariable, 0, len(secretMap)) + for name, value := range secretMap { + secrets = append(secrets, fleet.SecretVariable{Name: name, Value: value}) + } + + err := ds.UpsertSecretVariables(ctx, secrets) + require.NoError(t, err) + + expanded, err := ds.ExpandEmbeddedSecrets(ctx, noSecrets) + require.NoError(t, err) + require.Equal(t, noSecrets, expanded) + + expanded, err = ds.ExpandEmbeddedSecrets(ctx, validSecret) + require.NoError(t, err) + require.Equal(t, validSecretExpanded, expanded) + + _, err = ds.ExpandEmbeddedSecrets(ctx, invalidSecret) + require.ErrorContains(t, err, "$FLEET_SECRET_INVALID") + require.ErrorContains(t, err, "$FLEET_SECRET_ALSO_INVALID") +} diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 8d93dfd1fd6b..a12608785ca0 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1883,6 +1883,14 @@ type Datastore interface { // GetSecretVariables retrieves secret variables from the database. GetSecretVariables(ctx context.Context, names []string) ([]SecretVariable, error) + + // ValidateEmbeddedSecrets parses fleet secrets from a list of + // documents and checks that they exist in the database. + ValidateEmbeddedSecrets(ctx context.Context, documents []string) error + + // ExpandEmbeddedSecrets expands the fleet secrets in a + // document using the secrets stored in the datastore. + ExpandEmbeddedSecrets(ctx context.Context, document string) (string, error) } // MDMAppleStore wraps nanomdm's storage and adds methods to deal with diff --git a/server/fleet/fleet_vars_test.go b/server/fleet/fleet_vars_test.go index 0876799ec22c..992ba7eb4e7b 100644 --- a/server/fleet/fleet_vars_test.go +++ b/server/fleet/fleet_vars_test.go @@ -16,7 +16,7 @@ echo words${FLEET_SECRET_BAR}words $FLEET_SECRET_BAZ ${FLEET_SECRET_QUX} ` - secrets := ContainsPrefixVars(script, FLEET_SECRET_PREFIX) + secrets := ContainsPrefixVars(script, ServerSecretPrefix) require.Contains(t, secrets, "FOO") require.Contains(t, secrets, "BAR") require.Contains(t, secrets, "BAZ") @@ -39,8 +39,8 @@ We want to remember BREAD and alsoSHORTCAKEare important. } mapper := func(s string) (string, bool) { - if strings.HasPrefix(s, FLEET_SECRET_PREFIX) { - return mapping[strings.TrimPrefix(s, FLEET_SECRET_PREFIX)], true + if strings.HasPrefix(s, ServerSecretPrefix) { + return mapping[strings.TrimPrefix(s, ServerSecretPrefix)], true } return "", false } diff --git a/server/fleet/secrets.go b/server/fleet/secrets.go index 8f5b5d5c0bd8..0688a825d0f1 100644 --- a/server/fleet/secrets.go +++ b/server/fleet/secrets.go @@ -1,3 +1,24 @@ package fleet -const FLEET_SECRET_PREFIX = "FLEET_SECRET_" +import ( + "fmt" + "strings" +) + +const ServerSecretPrefix = "FLEET_SECRET_" + +type MissingSecretsError struct { + MissingSecrets []string +} + +func (e MissingSecretsError) Error() string { + secretVars := make([]string, 0, len(e.MissingSecrets)) + for _, secret := range e.MissingSecrets { + secretVars = append(secretVars, fmt.Sprintf("\"$%s%s\"", ServerSecretPrefix, secret)) + } + plural := "" + if len(secretVars) > 1 { + plural = "s" + } + return fmt.Sprintf("Couldn't add. Variable%s %s missing", plural, strings.Join(secretVars, ", ")) +} diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 2bbfba33b72a..89eeb21e3882 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1177,6 +1177,10 @@ type UpsertSecretVariablesFunc func(ctx context.Context, secretVariables []fleet type GetSecretVariablesFunc func(ctx context.Context, names []string) ([]fleet.SecretVariable, error) +type ValidateEmbeddedSecretsFunc func(ctx context.Context, documents []string) error + +type ExpandEmbeddedSecretsFunc func(ctx context.Context, document string) (string, error) + type DataStore struct { HealthCheckFunc HealthCheckFunc HealthCheckFuncInvoked bool @@ -2912,6 +2916,12 @@ type DataStore struct { GetSecretVariablesFunc GetSecretVariablesFunc GetSecretVariablesFuncInvoked bool + ValidateEmbeddedSecretsFunc ValidateEmbeddedSecretsFunc + ValidateEmbeddedSecretsFuncInvoked bool + + ExpandEmbeddedSecretsFunc ExpandEmbeddedSecretsFunc + ExpandEmbeddedSecretsFuncInvoked bool + mu sync.Mutex } @@ -6960,3 +6970,17 @@ func (s *DataStore) GetSecretVariables(ctx context.Context, names []string) ([]f s.mu.Unlock() return s.GetSecretVariablesFunc(ctx, names) } + +func (s *DataStore) ValidateEmbeddedSecrets(ctx context.Context, documents []string) error { + s.mu.Lock() + s.ValidateEmbeddedSecretsFuncInvoked = true + s.mu.Unlock() + return s.ValidateEmbeddedSecretsFunc(ctx, documents) +} + +func (s *DataStore) ExpandEmbeddedSecrets(ctx context.Context, document string) (string, error) { + s.mu.Lock() + s.ExpandEmbeddedSecretsFuncInvoked = true + s.mu.Unlock() + return s.ExpandEmbeddedSecretsFunc(ctx, document) +} diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 56dc433539d8..60095583958f 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -380,12 +380,24 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, r }) } - cp, err := fleet.NewMDMAppleConfigProfile(b, &teamID) + if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{string(b)}); err != nil { + return nil, fleet.NewInvalidArgumentError("profile", err.Error()) + } + + // Expand secrets in profile for validation + expanded, err := svc.ds.ExpandEmbeddedSecrets(ctx, string(b)) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "expanding secrets in profile for parsing") + } + + cp, err := fleet.NewMDMAppleConfigProfile([]byte(expanded), &teamID) if err != nil { return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{ Message: fmt.Sprintf("failed to parse config profile: %s", err.Error()), }) } + // Save the original unexpanded profile + cp.Mobileconfig = b if err := cp.ValidateUserProvided(); err != nil { return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()}) @@ -405,6 +417,7 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, r default: // TODO what happens if mode is not set?s } + err = validateConfigProfileFleetVariables(string(cp.Mobileconfig)) if err != nil { return nil, ctxerr.Wrap(ctx, err, "validating fleet variables") @@ -503,6 +516,10 @@ func (svc *Service) NewMDMAppleDeclaration(ctx context.Context, teamID uint, r i return nil, err } + if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{string(data)}); err != nil { + return nil, fleet.NewInvalidArgumentError("profile", err.Error()) + } + if err := validateDeclarationFleetVariables(string(data)); err != nil { return nil, err } @@ -1968,12 +1985,21 @@ 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"), ) } - mdmProf, err := fleet.NewMDMAppleConfigProfile(prof, tmID) + // Expand profile for validation + expanded, err := svc.ds.ExpandEmbeddedSecrets(ctx, string(prof)) + if err != nil { + return ctxerr.Wrap(ctx, + fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), err.Error()), + "missing fleet secrets") + } + mdmProf, err := fleet.NewMDMAppleConfigProfile([]byte(expanded), tmID) if err != nil { return ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), err.Error()), "invalid mobileconfig profile") } + // Store original unexpanded profile + mdmProf.Mobileconfig = prof if err := mdmProf.ValidateUserProvided(); err != nil { return ctxerr.Wrap(ctx, @@ -1997,6 +2023,15 @@ func (svc *Service) BatchSetMDMAppleProfiles(ctx context.Context, tmID *uint, tm profs = append(profs, mdmProf) } + profStrings := make([]string, 0, len(profs)) + for _, prof := range profs { + profStrings = append(profStrings, string(prof.Mobileconfig)) + } + + if err := svc.ds.ValidateEmbeddedSecrets(ctx, profStrings); err != nil { + return fleet.NewInvalidArgumentError("profiles", err.Error()) + } + if !skipBulkPending { // check for duplicates with existing profiles, skipBulkPending signals that the caller // is responsible for ensuring that the profiles names are unique (e.g., MDMAppleMatchPreassignment) diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 940c32d1fee6..b60c72d8517b 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -209,6 +209,12 @@ func setupAppleMDMService(t *testing.T, license *fleet.LicenseInfo) (fleet.Servi ds.MDMDeleteEULAFunc = func(ctx context.Context, token string) error { return nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) { + return document, nil + } apnsCert, apnsKey, err := mysql.GenerateTestCertBytes() require.NoError(t, err) crt, key, err := apple_mdm.NewSCEPCACertKey() diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 9b7e5b693c4a..dd050dad14e9 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -6192,6 +6192,11 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() { err := s.ds.MarkHostsSeen(ctx, []uint{host.ID}, time.Now()) require.NoError(t, err) + // make sure invalid secrets aren't allowed + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo $FLEET_SECRET_INVALID"}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `$FLEET_SECRET_INVALID`) + // create a valid script execution request s.DoJSON("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusAccepted, &runResp) require.Equal(t, host.ID, runResp.HostID) @@ -7038,6 +7043,13 @@ func (s *integrationEnterpriseTestSuite) TestSavedScripts() { errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, "no file headers for script") + // contains invalid fleet secret + body, headers = generateNewScriptMultipartRequest(t, + "secrets.sh", []byte(`echo "$FLEET_SECRET_INVALID"`), s.token, nil) + res = s.DoRawWithHeaders("POST", "/api/latest/fleet/scripts", body.Bytes(), http.StatusUnprocessableEntity, headers) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, "$FLEET_SECRET_INVALID") + // file name is not .sh body, headers = generateNewScriptMultipartRequest(t, "not_sh.txt", []byte(`echo "hello"`), s.token, nil) @@ -7952,6 +7964,11 @@ func (s *integrationEnterpriseTestSuite) TestBatchApplyScriptsEndpoints() { {Name: "", ScriptContents: []byte("foo")}, }}, http.StatusUnprocessableEntity, "team_id", fmt.Sprint(tm.ID)) + // invalid secret + s.Do("POST", "/api/v1/fleet/scripts/batch", batchSetScriptsRequest{Scripts: []fleet.ScriptPayload{ + {Name: "N2.sh", ScriptContents: []byte("echo $FLEET_SECRET_INVALID")}, + }}, http.StatusUnprocessableEntity, "team_id", fmt.Sprint(tm.ID)) + // successfully apply a scripts for the team saveAndCheckScripts(tm, []fleet.ScriptPayload{ {Name: "N1.sh", ScriptContents: []byte("foo")}, diff --git a/server/service/integration_mdm_dep_test.go b/server/service/integration_mdm_dep_test.go index c63a6a2916c3..d0616ec3419d 100644 --- a/server/service/integration_mdm_dep_test.go +++ b/server/service/integration_mdm_dep_test.go @@ -1846,6 +1846,11 @@ func (s *integrationMDMTestSuite) TestSetupExperienceScript() { err = json.NewDecoder(res.Body).Decode(&newScriptResp) require.NoError(t, err) + // test script secret validation + body, headers = generateNewScriptMultipartRequest(t, + "script.sh", []byte(`echo "$FLEET_SECRET_INVALID"`), s.token, map[string][]string{}) + s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusUnprocessableEntity, headers) + // get team script metadata var getScriptResp getSetupExperienceScriptResponse s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/setup_experience/script?team_id=%d", tm.ID), nil, http.StatusOK, &getScriptResp) diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index f9e73e1eee5f..bdb95c14c073 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -80,6 +80,32 @@ func (s *integrationMDMTestSuite) TestAppleProfileManagement() { // add global profiles s.Do("POST", "/api/v1/fleet/mdm/apple/profiles/batch", batchSetMDMAppleProfilesRequest{Profiles: globalProfiles}, http.StatusNoContent) + // invalid secrets + var invalidSecretsProfile = []byte(` + + + + + PayloadContent + + PayloadDisplayName + $FLEET_SECRET_INVALID + PayloadIdentifier + N3 + PayloadType + Configuration + PayloadUUID + 601E0B42-0989-4FAD-A61B-18656BA3670E + PayloadVersion + 1 + + +`) + + res := s.Do("POST", "/api/v1/fleet/mdm/apple/profiles/batch", batchSetMDMAppleProfilesRequest{Profiles: [][]byte{invalidSecretsProfile}}, http.StatusUnprocessableEntity) + errMsg := extractServerErrorText(res.Body) + require.Contains(t, errMsg, "$FLEET_SECRET_INVALID") + // create a new team tm, err := s.ds.NewTeam(ctx, &fleet.Team{Name: "batch_set_mdm_profiles"}) require.NoError(t, err) @@ -224,8 +250,8 @@ func (s *integrationMDMTestSuite) TestAppleProfileManagement() { s.checkMDMProfilesSummaries(t, &tm.ID, fleet.MDMProfilesSummary{Verifying: 1}, nil) // can't resend profile while verifying - res := s.DoRaw("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/configuration_profiles/%s/resend", host.ID, mcUUID), nil, http.StatusConflict) - errMsg := extractServerErrorText(res.Body) + res = s.DoRaw("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/configuration_profiles/%s/resend", host.ID, mcUUID), nil, http.StatusConflict) + errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, "Couldn’t resend. Configuration profiles with “pending” or “verifying” status can’t be resent.") // set the profile to pending, can't resend @@ -4628,6 +4654,36 @@ func (s *integrationMDMTestSuite) TestMDMAppleConfigProfileCRUD() { getPath = fmt.Sprintf("/api/latest/fleet/mdm/apple/profiles/%d", deletedCP.ProfileID) _ = s.DoRawWithHeaders("GET", getPath, nil, http.StatusNotFound, map[string]string{"Authorization": fmt.Sprintf("Bearer %s", s.token)}) + // fail to create new profile (no team), invalid fleet secret + testProfiles["badSecrets"] = fleet.MDMAppleConfigProfile{ + Name: "badSecrets", + Identifier: "badSecrets.One", + Mobileconfig: mobileconfig.Mobileconfig(` + + + + PayloadContent + + PayloadDisplayName + badSecrets + PayloadIdentifier + badSecrets.One + PayloadType + Configuration + PayloadUUID + $FLEET_SECRET_INVALID.35E2029E-A0C2-4754-B709-4CAAB1B8D3CB + PayloadVersion + 1 + + +`), + } + + body, headers = generateNewReq("badSecrets", nil) + newResp = s.DoRawWithHeaders("POST", "/api/latest/fleet/mdm/apple/profiles", body.Bytes(), http.StatusUnprocessableEntity, headers) + errMsg := extractServerErrorText(newResp.Body) + require.Contains(t, errMsg, "$FLEET_SECRET_INVALID") + // trying to add/delete profiles with identifiers managed by Fleet fails for p := range mobileconfig.FleetPayloadIdentifiers() { generateTestProfile("TestNoTeam", p) diff --git a/server/service/mdm.go b/server/service/mdm.go index 44759b8dc657..ffc9c3d7e626 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -1425,6 +1425,10 @@ func (svc *Service) NewMDMWindowsConfigProfile(ctx context.Context, teamID uint, cp.LabelsIncludeAll = labelMap } + if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{string(cp.SyncML)}); err != nil { + return nil, ctxerr.Wrap(ctx, err, "validating fleet secrets") + } + err = validateWindowsProfileFleetVariables(string(cp.SyncML)) if err != nil { return nil, ctxerr.Wrap(ctx, err, "validating Windows profile") @@ -1632,13 +1636,18 @@ func (svc *Service) BatchSetMDMProfiles( return ctxerr.Wrap(ctx, err, "validating cross-platform profile names") } - err = validateFleetVariables(ctx, appleProfiles, windowsProfiles, appleDecls) + if dryRun { + return nil + } + + err = svc.validateFleetSecrets(ctx, appleProfiles, windowsProfiles, appleDecls) if err != nil { return err } - if dryRun { - return nil + err = validateFleetVariables(ctx, appleProfiles, windowsProfiles, appleDecls) + if err != nil { + return err } var profUpdates fleet.MDMProfilesUpdates @@ -1706,6 +1715,7 @@ func validateFleetVariables(ctx context.Context, appleProfiles []*fleet.MDMApple windowsProfiles []*fleet.MDMWindowsConfigProfile, appleDecls []*fleet.MDMAppleDeclaration, ) error { var err error + for _, p := range appleProfiles { err = validateConfigProfileFleetVariables(string(p.Mobileconfig)) if err != nil { @@ -1727,6 +1737,23 @@ func validateFleetVariables(ctx context.Context, appleProfiles []*fleet.MDMApple return nil } +func (svc *Service) validateFleetSecrets(ctx context.Context, appleProfiles []*fleet.MDMAppleConfigProfile, windowsProfiles []*fleet.MDMWindowsConfigProfile, appleDecls []*fleet.MDMAppleDeclaration) error { + allProfiles := make([]string, 0, len(appleProfiles)+len(appleDecls)+len(windowsProfiles)) + for _, p := range appleProfiles { + allProfiles = append(allProfiles, string(p.Mobileconfig)) + } + for _, p := range appleDecls { + allProfiles = append(allProfiles, string(p.RawJSON)) + } + for _, p := range windowsProfiles { + allProfiles = append(allProfiles, string(p.SyncML)) + } + if err := svc.ds.ValidateEmbeddedSecrets(ctx, allProfiles); err != nil { + return ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("profiles", err.Error())) + } + return nil +} + func (svc *Service) validateCrossPlatformProfileNames(ctx context.Context, appleProfiles []*fleet.MDMAppleConfigProfile, windowsProfiles []*fleet.MDMWindowsConfigProfile, appleDecls []*fleet.MDMAppleDeclaration) error { // map all profile names to check for duplicates, regardless of platform; key is name, value is one of // ".mobileconfig" or ".json" or ".xml" diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index ef0821cecf59..f89451e210e4 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -1110,6 +1110,9 @@ func TestMDMWindowsConfigProfileAuthz(t *testing.T) { ) (updates fleet.MDMProfilesUpdates, err error) { return fleet.MDMProfilesUpdates{}, nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } checkShouldFail := func(t *testing.T, err error, shouldFail bool) { if !shouldFail { @@ -1186,6 +1189,12 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) { ) (updates fleet.MDMProfilesUpdates, err error) { return fleet.MDMProfilesUpdates{}, nil } + ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) { + return document, nil + } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } cases := []struct { desc string @@ -1282,6 +1291,12 @@ func TestMDMBatchSetProfiles(t *testing.T) { ) (updates fleet.MDMProfilesUpdates, err error) { return fleet.MDMProfilesUpdates{}, nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) { + return document, nil + } testCases := []struct { name string @@ -2090,6 +2105,12 @@ func TestBatchSetMDMProfilesLabels(t *testing.T) { } return m, nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) { + return document, nil + } profiles := []fleet.MDMProfileBatchPayload{ // macOS diff --git a/server/service/scripts.go b/server/service/scripts.go index 71ed203d325c..c55d18be1238 100644 --- a/server/service/scripts.go +++ b/server/service/scripts.go @@ -175,6 +175,13 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript } } + if request.ScriptContents != "" { + if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{request.ScriptContents}); err != nil { + svc.authz.SkipAuthorization(ctx) + return nil, fleet.NewInvalidArgumentError("script", err.Error()) + } + } + if request.ScriptName != "" { scriptID, err := svc.GetScriptIDByName(ctx, request.ScriptName, &request.TeamID) if err != nil { @@ -510,6 +517,11 @@ func (svc *Service) NewScript(ctx context.Context, teamID *uint, name string, r Name: name, ScriptContents: file.Dos2UnixNewlines(string(b)), } + + if err := svc.ds.ValidateEmbeddedSecrets(ctx, []string{script.ScriptContents}); err != nil { + return nil, fleet.NewInvalidArgumentError("script", err.Error()) + } + if err := script.ValidateNewScript(); err != nil { return nil, fleet.NewInvalidArgumentError("script", err.Error()) } @@ -849,6 +861,7 @@ func (svc *Service) BatchSetScripts(ctx context.Context, maybeTmID *uint, maybeT // any duplicate name in the provided set results in an error scripts := make([]*fleet.Script, 0, len(payloads)) byName := make(map[string]bool, len(payloads)) + scriptContents := []string{} for i, p := range payloads { script := &fleet.Script{ ScriptContents: string(p.ScriptContents), @@ -867,6 +880,7 @@ func (svc *Service) BatchSetScripts(ctx context.Context, maybeTmID *uint, maybeT "duplicate script by name") } byName[script.Name] = true + scriptContents = append(scriptContents, script.ScriptContents) scripts = append(scripts, script) } @@ -874,6 +888,10 @@ func (svc *Service) BatchSetScripts(ctx context.Context, maybeTmID *uint, maybeT return nil, nil } + if err := svc.ds.ValidateEmbeddedSecrets(ctx, scriptContents); err != nil { + return nil, fleet.NewInvalidArgumentError("script", err.Error()) + } + scriptResponses, err := svc.ds.BatchSetScripts(ctx, teamID, scripts) if err != nil { return nil, ctxerr.Wrap(ctx, err, "batch saving scripts") diff --git a/server/service/scripts_test.go b/server/service/scripts_test.go index d1957ea0e797..4f3169fd8ba1 100644 --- a/server/service/scripts_test.go +++ b/server/service/scripts_test.go @@ -60,6 +60,9 @@ func TestHostRunScript(t *testing.T) { return []byte("echo"), nil } ds.IsExecutionPendingForHostFunc = func(ctx context.Context, hostID, scriptID uint) (bool, error) { return false, nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } t.Run("authorization checks", func(t *testing.T) { testCases := []struct { @@ -539,6 +542,12 @@ func TestSavedScripts(t *testing.T) { ds.TeamFunc = func(ctx context.Context, id uint) (*fleet.Team, error) { return &fleet.Team{ID: 0}, nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) { + return document, nil + } testCases := []struct { name string diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index af1ca985ae2a..7696775c155b 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -57,6 +57,12 @@ func TestSetupExperienceAuth(t *testing.T) { ds.TeamFunc = func(ctx context.Context, id uint) (*fleet.Team, error) { return &fleet.Team{ID: id}, nil } + ds.ValidateEmbeddedSecretsFunc = func(ctx context.Context, documents []string) error { + return nil + } + ds.ExpandEmbeddedSecretsFunc = func(ctx context.Context, document string) (string, error) { + return document, nil + } testCases := []struct { name string