diff --git a/changes/23238-use-secrets-in-scripts-profiles b/changes/23238-use-secrets-in-scripts-profiles index 1eb445de15..f42a1c6a88 100644 --- a/changes/23238-use-secrets-in-scripts-profiles +++ b/changes/23238-use-secrets-in-scripts-profiles @@ -1,2 +1,3 @@ -Added ability to use secrets ($FLEET_SECRET_NAME) in scripts and profiles. +Added ability to use secrets ($FLEET_SECRET_YOURNAME) in scripts and profiles. - Added `/fleet/spec/secret_variables` API endpoint. +- fleetctl gitops identifies secrets in scripts and profiles and saves them on the Fleet server. diff --git a/cmd/fleetctl/gitops_integration_test.go b/cmd/fleetctl/gitops_integration_test.go index f83fad380b..610f1401ea 100644 --- a/cmd/fleetctl/gitops_integration_test.go +++ b/cmd/fleetctl/gitops_integration_test.go @@ -39,6 +39,7 @@ func (s *integrationGitopsTestSuite) SetupSuite() { require.NoError(s.T(), err) appConf.MDM.EnabledAndConfigured = true appConf.MDM.AppleBMEnabledAndConfigured = true + appConf.MDM.WindowsEnabledAndConfigured = true err = s.ds.SaveAppConfig(context.Background(), appConf) require.NoError(s.T(), err) @@ -152,29 +153,44 @@ contexts: func (s *integrationGitopsTestSuite) TestFleetGitopsWithFleetSecrets() { t := s.T() - const secretName = "NAME" + const ( + secretName1 = "NAME" + secretName2 = "length" + ) ctx := context.Background() fleetctlConfig := s.createFleetctlConfig() // Set the required environment variables t.Setenv("FLEET_URL", s.server.URL) t.Setenv("FLEET_GLOBAL_ENROLL_SECRET", "global_enroll_secret") - t.Setenv("FLEET_SECRET_"+secretName, "secret_value") + t.Setenv("FLEET_SECRET_"+secretName1, "secret_value") + t.Setenv("FLEET_SECRET_"+secretName2, "2") globalFile := path.Join("testdata", "gitops", "global_integration.yml") // Dry run _ = runAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", globalFile, "--dry-run"}) - secrets, err := s.ds.GetSecretVariables(ctx, []string{secretName}) + secrets, err := s.ds.GetSecretVariables(ctx, []string{secretName1}) require.NoError(t, err) require.Empty(t, secrets) // Real run _ = runAppForTest(t, []string{"gitops", "--config", fleetctlConfig.Name(), "-f", globalFile}) - secrets, err = s.ds.GetSecretVariables(ctx, []string{secretName}) + // Check secrets + secrets, err = s.ds.GetSecretVariables(ctx, []string{secretName1, secretName2}) require.NoError(t, err) - require.Len(t, secrets, 1) - assert.Equal(t, secretName, secrets[0].Name) - assert.Equal(t, "secret_value", secrets[0].Value) + require.Len(t, secrets, 2) + for _, secret := range secrets { + switch secret.Name { + case secretName1: + assert.Equal(t, "secret_value", secret.Value) + case secretName2: + assert.Equal(t, "2", secret.Value) + default: + t.Fatalf("unexpected secret %s", secret.Name) + } + } + + // Check script(s) scriptID, err := s.ds.GetScriptIDByName(ctx, "fleet-secret.sh", nil) require.NoError(t, err) expected, err := os.ReadFile("testdata/gitops/lib/fleet-secret.sh") @@ -183,4 +199,24 @@ func (s *integrationGitopsTestSuite) TestFleetGitopsWithFleetSecrets() { require.NoError(t, err) assert.Equal(t, expected, script) + // Check Apple profiles + profiles, err := s.ds.ListMDMAppleConfigProfiles(ctx, nil) + require.NoError(t, err) + require.Len(t, profiles, 1) + assert.Contains(t, string(profiles[0].Mobileconfig), "$FLEET_SECRET_"+secretName1) + // Check Windows profiles + allProfiles, _, err := s.ds.ListMDMConfigProfiles(ctx, nil, fleet.ListOptions{}) + require.NoError(t, err) + require.Len(t, allProfiles, 2) + var windowsProfileUUID string + for _, profile := range allProfiles { + if profile.Platform == "windows" { + windowsProfileUUID = profile.ProfileUUID + } + } + require.NotEmpty(t, windowsProfileUUID) + winProfile, err := s.ds.GetMDMWindowsConfigProfile(ctx, windowsProfileUUID) + require.NoError(t, err) + assert.Contains(t, string(winProfile.SyncML), "${FLEET_SECRET_"+secretName2+"}") + } diff --git a/cmd/fleetctl/testdata/gitops/global_integration.yml b/cmd/fleetctl/testdata/gitops/global_integration.yml index f277b4e58c..85bf631675 100644 --- a/cmd/fleetctl/testdata/gitops/global_integration.yml +++ b/cmd/fleetctl/testdata/gitops/global_integration.yml @@ -3,6 +3,13 @@ queries: agent_options: path: ./lib/agent-options.yml controls: + macos_settings: + custom_settings: + - path: ./lib/macos-password-secret.mobileconfig + windows_enabled_and_configured: true + windows_settings: + custom_settings: + - path: ./lib/windows-screenlock-secret.xml scripts: - path: ./lib/fleet-secret.sh org_settings: diff --git a/cmd/fleetctl/testdata/gitops/lib/macos-password-secret.mobileconfig b/cmd/fleetctl/testdata/gitops/lib/macos-password-secret.mobileconfig new file mode 100644 index 0000000000..03b34b6728 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/lib/macos-password-secret.mobileconfig @@ -0,0 +1,55 @@ + + + + + PayloadContent + + + PayloadDescription + Configures Passcode settings + PayloadDisplayName + $FLEET_SECRET_NAME + PayloadIdentifier + com.github.erikberglund.ProfileCreator.F7CF282E-D91B-44E9-922F-A719634F9C8E.com.apple.mobiledevice.passwordpolicy.231DFC90-D5A7-41B8-9246-564056048AC5 + PayloadOrganization + + PayloadType + com.apple.mobiledevice.passwordpolicy + PayloadUUID + 231DFC90-D5A7-41B8-9246-564056048AC5 + PayloadVersion + 1 + allowSimple + + forcePIN + + maxFailedAttempts + 11 + maxGracePeriod + 1 + maxInactivity + 15 + minLength + 10 + requireAlphanumeric + + + + PayloadDescription + Configures our Macs to require passwords that are 10 character long + PayloadDisplayName + Password policy - require 10 characters + PayloadIdentifier + com.github.erikberglund.ProfileCreator.F7CF282E-D91B-44E9-922F-A719634F9C8E + PayloadOrganization + FleetDM + PayloadScope + System + PayloadType + Configuration + PayloadUUID + F7CF282E-D91B-44E9-922F-A719634F9C8E + PayloadVersion + 1 + + diff --git a/cmd/fleetctl/testdata/gitops/lib/windows-screenlock-secret.xml b/cmd/fleetctl/testdata/gitops/lib/windows-screenlock-secret.xml new file mode 100644 index 0000000000..aa6c628d81 --- /dev/null +++ b/cmd/fleetctl/testdata/gitops/lib/windows-screenlock-secret.xml @@ -0,0 +1,48 @@ + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/DevicePasswordEnabled + + 0 + + + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/MaxInactivityTimeDeviceLock + + 15 + + + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/MinDevicePasswordLength + + ${FLEET_SECRET_length} + + + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/MinDevicePasswordComplexCharacters + + 2 + + diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index 219035c6a2..e5f4549182 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -434,6 +434,7 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin return multierror.Append(multiError, fmt.Errorf("failed to unmarshal controls: %v", err)) } controlsTop.Defined = true + controlsDir := baseDir if controlsTop.Path == nil { controlsTop.Scripts, err = resolveScriptPaths(controlsTop.Scripts, baseDir) if err != nil { @@ -470,6 +471,7 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin } result.Controls = pathControls } + controlsDir = filepath.Dir(controlsFilePath) } // Find Fleet secrets in scripts. @@ -488,6 +490,50 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin } } + // Find Fleet secrets in profiles + var profiles []fleet.MDMProfileSpec + if result.Controls.MacOSSettings != nil { + // We are marshalling/unmarshalling to get the data into the fleet.MacOSSettings struct. + // This is inefficient, but it is more robust and less error-prone. + var macOSSettings fleet.MacOSSettings + data, err := json.Marshal(result.Controls.MacOSSettings) + if err != nil { + return multierror.Append(multiError, fmt.Errorf("failed to process controls.macos_settings: %v", err)) + } + err = json.Unmarshal(data, &macOSSettings) + if err != nil { + return multierror.Append(multiError, fmt.Errorf("failed to process controls.macos_settings: %v", err)) + } + profiles = append(profiles, macOSSettings.CustomSettings...) + } + if result.Controls.WindowsSettings != nil { + // We are marshalling/unmarshalling to get the data into the fleet.WindowsSettings struct. + // This is inefficient, but it is more robust and less error-prone. + var windowsSettings fleet.WindowsSettings + data, err := json.Marshal(result.Controls.WindowsSettings) + if err != nil { + return multierror.Append(multiError, fmt.Errorf("failed to process controls.windows_settings: %v", err)) + } + err = json.Unmarshal(data, &windowsSettings) + if err != nil { + return multierror.Append(multiError, fmt.Errorf("failed to process controls.windows_settings: %v", err)) + } + if windowsSettings.CustomSettings.Valid { + profiles = append(profiles, windowsSettings.CustomSettings.Value...) + } + } + for _, profile := range profiles { + resolvedPath := resolveApplyRelativePath(controlsDir, profile.Path) + fileBytes, err := os.ReadFile(resolvedPath) + if err != nil { + return multierror.Append(multiError, fmt.Errorf("failed to read profile file %s: %v", resolvedPath, err)) + } + err = LookupEnvSecrets(string(fileBytes), result.FleetSecrets) + if err != nil { + return multierror.Append(multiError, err) + } + } + return multiError } diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index a861ab85c6..d32868bbf7 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -81,6 +81,8 @@ func TestValidGitOpsYaml(t *testing.T) { "global_config_no_paths": { environment: map[string]string{ "FLEET_SECRET_FLEET_SECRET_": "fleet_secret", + "FLEET_SECRET_NAME": "secret_name", + "FLEET_SECRET_length": "10", }, filePath: "testdata/global_config_no_paths.yml", }, @@ -90,12 +92,16 @@ func TestValidGitOpsYaml(t *testing.T) { "DISTRIBUTED_DENYLIST_DURATION": "0", "ORG_NAME": "Fleet Device Management", "FLEET_SECRET_FLEET_SECRET_": "fleet_secret", + "FLEET_SECRET_NAME": "secret_name", + "FLEET_SECRET_length": "10", }, filePath: "testdata/global_config.yml", }, "team_config_no_paths": { environment: map[string]string{ "FLEET_SECRET_FLEET_SECRET_": "fleet_secret", + "FLEET_SECRET_NAME": "secret_name", + "FLEET_SECRET_length": "10", }, filePath: "testdata/team_config_no_paths.yml", isTeam: true, @@ -107,6 +113,8 @@ func TestValidGitOpsYaml(t *testing.T) { "DISTRIBUTED_DENYLIST_DURATION": "0", "ENABLE_FAILING_POLICIES_WEBHOOK": "true", "FLEET_SECRET_FLEET_SECRET_": "fleet_secret", + "FLEET_SECRET_NAME": "secret_name", + "FLEET_SECRET_length": "10", }, filePath: "testdata/team_config.yml", isTeam: true, @@ -228,8 +236,10 @@ func TestValidGitOpsYaml(t *testing.T) { assert.True(t, ok, "windows_migration_enabled not found") _, ok = gitops.Controls.WindowsUpdates.(map[string]interface{}) assert.True(t, ok, "windows_updates not found") - require.Len(t, gitops.FleetSecrets, 1) + require.Len(t, gitops.FleetSecrets, 3) assert.Equal(t, "fleet_secret", gitops.FleetSecrets["FLEET_SECRET_FLEET_SECRET_"]) + assert.Equal(t, "secret_name", gitops.FleetSecrets["FLEET_SECRET_NAME"]) + assert.Equal(t, "10", gitops.FleetSecrets["FLEET_SECRET_length"]) // Check agent options assert.NotNil(t, gitops.AgentOptions) diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 7eb0e0ea14..08eed07042 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -157,7 +157,7 @@ func generateRandomString(sizeBytes int) string { } func ExpandEnv(s string) (string, error) { - out, err := expandEnv(s, nil) + out, err := expandEnv(s, true) return out, err } @@ -167,7 +167,7 @@ func ExpandEnv(s string) (string, error) { // $FLEET_VAR_XXX will not be expanded. These variables are expanded on the server. // If secretsMap is not nil, $FLEET_SECRET_XXX will be evaluated and put in the map // If secretsMap is nil, $FLEET_SECRET_XXX will cause an error. -func expandEnv(s string, secretsMap map[string]string) (string, error) { +func expandEnv(s string, failOnSecret bool) (string, error) { // Generate a random escaping prefix that doesn't exist in s. var preventEscapingPrefix string for { @@ -187,20 +187,11 @@ func expandEnv(s string, secretsMap map[string]string) (string, error) { // Don't expand fleet vars -- they will be expanded on the server return "", false case strings.HasPrefix(env, fleet.FLEET_SECRET_PREFIX): - if secretsMap != nil { - // lookup the secret and save it, but don't replace - v, ok := os.LookupEnv(env) - if !ok { - err = multierror.Append(err, fmt.Errorf("environment variable %q not set", env)) - return "", false - } - secretsMap[env] = v - return "", false - } else { + 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)) - return "", false } + return "", false } v, ok := os.LookupEnv(env) if !ok { @@ -223,6 +214,14 @@ func ExpandEnvBytes(b []byte) ([]byte, error) { return []byte(s), nil } +func ExpandEnvBytesIgnoreSecrets(b []byte) ([]byte, error) { + s, err := expandEnv(string(b), false) + if err != nil { + return nil, err + } + return []byte(s), nil +} + // LookupEnvSecrets only looks up FLEET_SECRET_XXX environment variables. Escaping is not supported. // This is used for finding secrets in scripts only. The original string is not modified. // A map of secret names to values is updated. @@ -239,9 +238,6 @@ func LookupEnvSecrets(s string, secretsMap map[string]string) error { err = multierror.Append(err, fmt.Errorf("environment variable %q not set", env)) return "", false } - if secretsMap == nil { - secretsMap = make(map[string]string, 1) - } secretsMap[env] = v } return "", false diff --git a/pkg/spec/testdata/lib/macos-password.mobileconfig b/pkg/spec/testdata/lib/macos-password.mobileconfig new file mode 100644 index 0000000000..69b9108418 --- /dev/null +++ b/pkg/spec/testdata/lib/macos-password.mobileconfig @@ -0,0 +1,55 @@ + + + + + PayloadContent + + + PayloadDescription + Configures Passcode settings + PayloadDisplayName + $FLEET_SECRET_NAME + PayloadIdentifier + com.github.erikberglund.ProfileCreator.F7CF282E-D91B-44E9-922F-A719634F9C8E.com.apple.mobiledevice.passwordpolicy.231DFC90-D5A7-41B8-9246-564056048AC5 + PayloadOrganization + + PayloadType + com.apple.mobiledevice.passwordpolicy + PayloadUUID + 231DFC90-D5A7-41B8-9246-564056048AC5 + PayloadVersion + 1 + allowSimple + + forcePIN + + maxFailedAttempts + 11 + maxGracePeriod + 1 + maxInactivity + 15 + minLength + 10 + requireAlphanumeric + + + + PayloadDescription + Configures our Macs to require passwords that are 10 character long + PayloadDisplayName + Password policy - require 10 characters + PayloadIdentifier + com.github.erikberglund.ProfileCreator.F7CF282E-D91B-44E9-922F-A719634F9C8E + PayloadOrganization + FleetDM + PayloadScope + System + PayloadType + Configuration + PayloadUUID + F7CF282E-D91B-44E9-922F-A719634F9C8E + PayloadVersion + 1 + + diff --git a/pkg/spec/testdata/lib/windows-screenlock.xml b/pkg/spec/testdata/lib/windows-screenlock.xml new file mode 100644 index 0000000000..9c498423af --- /dev/null +++ b/pkg/spec/testdata/lib/windows-screenlock.xml @@ -0,0 +1,48 @@ + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/DevicePasswordEnabled + + 0 + + + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/MaxInactivityTimeDeviceLock + + 15 + + + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/MinDevicePasswordLength + + ${FLEET_SECRET_length} + + + + + + + int + + + ./Device/Vendor/MSFT/Policy/Config/DeviceLock/MinDevicePasswordComplexCharacters + + 2 + + diff --git a/server/service/client.go b/server/service/client.go index 2bde1672bb..ee65d4a8d6 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -323,7 +323,8 @@ func getProfilesContents(baseDir string, macProfiles []fleet.MDMProfileSpec, win } if expandEnv { - fileContents, err = spec.ExpandEnvBytes(fileContents) + // Secrets are handled earlier in the flow when config files are initially read + fileContents, err = spec.ExpandEnvBytesIgnoreSecrets(fileContents) if err != nil { return nil, fmt.Errorf("expanding environment on file %q: %w", profile.Path, err) }