diff --git a/.github/workflows/test-go.yaml b/.github/workflows/test-go.yaml index e3dd676ac5..8cf30cf10b 100644 --- a/.github/workflows/test-go.yaml +++ b/.github/workflows/test-go.yaml @@ -91,6 +91,7 @@ jobs: if [[ "${{ matrix.suite }}" == "main" ]]; then echo "CI_TEST_PKG=main" >> $GITHUB_ENV echo "NEED_DOCKER=1" >> $GITHUB_ENV + echo "DOCKER_COMMAND=${{ env.DOCKER_COMMAND }} localstack" >> $GITHUB_ENV elif [[ "${{ matrix.suite }}" == "fast" ]]; then # DO NOT add any dependencies in this test suite. echo "CI_TEST_PKG=${{ matrix.suite }}" >> $GITHUB_ENV @@ -258,6 +259,8 @@ jobs: MINIO_STORAGE_TEST=1 \ SAML_IDP_TEST=1 \ MAIL_TEST=1 \ + AWS_ENDPOINT_URL="http://localhost:4566" \ + AWS_REGION=us-east-1 \ NETWORK_TEST_GITHUB_TOKEN=${{ secrets.FLEET_RELEASE_GITHUB_PAT }} \ CI_TEST_PKG="${{ env.CI_TEST_PKG }}" \ make test-go 2>&1 | tee /tmp/gotest.log diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index b1039813cc..ed1246351e 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -203,6 +203,7 @@ the way that the Fleet server works. privateKey, err := configpkg.RetrieveSecretsManagerSecret( context.Background(), config.Server.PrivateKeySecretArn, + config.Server.PrivateKeySecretRegion, config.Server.PrivateKeySecretSTSAssumeRoleArn, config.Server.PrivateKeySecretSTSExternalID, ) diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go index 813937c7b8..9d6b69def6 100644 --- a/pkg/spec/spec_test.go +++ b/pkg/spec/spec_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/fleetdm/fleet/v4/pkg/testutils" "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -161,8 +162,11 @@ func TestExpandEnv(t *testing.T) { {map[string]string{"foo": "", "bar": "", "zoo": ""}, `$foo${bar}$zoo`, ``, nil}, {map[string]string{}, `$foo`, ``, checkMultiErrors(t, "environment variable \"foo\" not set")}, {map[string]string{"foo": "1"}, `$foo$bar`, ``, checkMultiErrors(t, "environment variable \"bar\" not set")}, - {map[string]string{"bar": "1"}, `$foo $bar $zoo`, ``, - checkMultiErrors(t, "environment variable \"foo\" not set", "environment variable \"zoo\" not set")}, + { + map[string]string{"bar": "1"}, + `$foo $bar $zoo`, ``, + checkMultiErrors(t, "environment variable \"foo\" not set", "environment variable \"zoo\" not set"), + }, {map[string]string{"foo": "4", "bar": "2"}, `$foo$bar`, `42`, nil}, {map[string]string{"foo": "42", "bar": ""}, `$foo$bar`, `42`, nil}, {map[string]string{}, `$$`, ``, checkMultiErrors(t, "environment variable \"$\" not set")}, @@ -180,9 +184,14 @@ func TestExpandEnv(t *testing.T) { {map[string]string{"foo": "", "$": "2"}, `${$}${foo}var`, `2var`, nil}, {map[string]string{}, `${foo}var`, ``, checkMultiErrors(t, "environment variable \"foo\" not set")}, {map[string]string{}, `foo PREVENT_ESCAPING_bar $ FLEET_VAR_`, `foo PREVENT_ESCAPING_bar $ FLEET_VAR_`, nil}, // nothing to replace - {map[string]string{"foo": "BAR"}, `\$FLEET_VAR_$foo \${FLEET_VAR_$foo} \${FLEET_VAR_${foo}2}`, - `$FLEET_VAR_BAR ${FLEET_VAR_BAR} ${FLEET_VAR_BAR2}`, nil}, // nested variables + { + map[string]string{"foo": "BAR"}, + `\$FLEET_VAR_$foo \${FLEET_VAR_$foo} \${FLEET_VAR_${foo}2}`, + `$FLEET_VAR_BAR ${FLEET_VAR_BAR} ${FLEET_VAR_BAR2}`, nil, + }, // nested variables } { + // save the current env before clearing it. + testutils.SaveEnv(t) os.Clearenv() for k, v := range tc.environment { _ = os.Setenv(k, v) @@ -206,9 +215,15 @@ func TestLookupEnvSecrets(t *testing.T) { }{ {map[string]string{"foo": "1"}, `$foo`, map[string]string{}, nil}, {map[string]string{"FLEET_SECRET_foo": "1"}, `$FLEET_SECRET_foo`, map[string]string{"FLEET_SECRET_foo": "1"}, nil}, - {map[string]string{"foo": "1"}, `$FLEET_SECRET_foo`, map[string]string{}, - checkMultiErrors(t, "environment variable \"FLEET_SECRET_foo\" not set")}, + { + map[string]string{"foo": "1"}, + `$FLEET_SECRET_foo`, + map[string]string{}, + checkMultiErrors(t, "environment variable \"FLEET_SECRET_foo\" not set"), + }, } { + // save the current env before clearing it. + testutils.SaveEnv(t) os.Clearenv() for k, v := range tc.environment { _ = os.Setenv(k, v) @@ -262,26 +277,26 @@ func TestGetExclusionZones(t *testing.T) { { []string{"testdata", "policies", "policies.yml"}, map[[2]int]string{ - [2]int{46, 106}: " description: This policy should always fail.\n resolution:", - [2]int{93, 155}: " resolution: There is no resolution for this policy.\n query:", - [2]int{268, 328}: " description: This policy should always pass.\n resolution:", - [2]int{315, 678}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", + {46, 106}: " description: This policy should always fail.\n resolution:", + {93, 155}: " resolution: There is no resolution for this policy.\n query:", + {268, 328}: " description: This policy should always pass.\n resolution:", + {315, 678}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", }, }, { []string{"testdata", "global_config_no_paths.yml"}, map[[2]int]string{ - [2]int{866, 949}: " description: Collect osquery performance stats directly from osquery\n query:", // - [2]int{1754, 1818}: " description: This policy should always fail.\n resolution:", // - [2]int{1803, 1869}: " resolution: There is no resolution for this policy.\n query:", // - [2]int{1986, 2050}: " description: This policy should always pass.\n resolution:", // - [2]int{2035, 2101}: " resolution: There is no resolution for this policy.\n query:", // - [2]int{2394, 2458}: " description: This policy should always fail.\n resolution:", // - [2]int{2443, 2509}: " resolution: There is no resolution for this policy.\n query:", // - [2]int{2613, 2677}: " description: This policy should always fail.\n resolution:", // - [2]int{2662, 3035}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", - [2]int{6102, 6149}: " description: A cool global label\n query:", // - [2]int{6246, 6292}: " description: A fly global label\n hosts:", // + {866, 949}: " description: Collect osquery performance stats directly from osquery\n query:", // + {1754, 1818}: " description: This policy should always fail.\n resolution:", // + {1803, 1869}: " resolution: There is no resolution for this policy.\n query:", // + {1986, 2050}: " description: This policy should always pass.\n resolution:", // + {2035, 2101}: " resolution: There is no resolution for this policy.\n query:", // + {2394, 2458}: " description: This policy should always fail.\n resolution:", // + {2443, 2509}: " resolution: There is no resolution for this policy.\n query:", // + {2613, 2677}: " description: This policy should always fail.\n resolution:", // + {2662, 3035}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", + {6102, 6149}: " description: A cool global label\n query:", // + {6246, 6292}: " description: A fly global label\n hosts:", // }, }, } diff --git a/pkg/testutils/testutils.go b/pkg/testutils/testutils.go new file mode 100644 index 0000000000..3cf8c10740 --- /dev/null +++ b/pkg/testutils/testutils.go @@ -0,0 +1,30 @@ +package testutils + +import ( + "os" + "strings" + "testing" +) + +// SaveEnv snapshots the current environment and restores it when the test +// ends. +// +// Do _not_ use this in parallel tests, as it clears the entire environment. +func SaveEnv(t *testing.T) { + saved := os.Environ() + t.Cleanup(func() { + os.Clearenv() + for _, kv := range saved { + parts := strings.SplitN(kv, "=", 2) + key := parts[0] + val := "" + if len(parts) == 2 { + val = parts[1] + } + err := os.Setenv(key, val) + if err != nil { + t.Logf("error restoring env var %s: %v", key, err) + } + } + }) +} diff --git a/server/config/config.go b/server/config/config.go index 26ea64f6e2..f166895056 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -107,9 +107,10 @@ type ServerConfig struct { FrequentCleanupsEnabled bool `yaml:"frequent_cleanups_enabled"` ForceH2C bool `yaml:"force_h2c"` PrivateKey string `yaml:"private_key"` - PrivateKeySecretArn string `yaml:"private_key_secret_arn"` - PrivateKeySecretSTSAssumeRoleArn string `yaml:"private_key_secret_sts_assume_role_arn"` - PrivateKeySecretSTSExternalID string `yaml:"private_key_secret_sts_external_id"` + PrivateKeySecretArn string `yaml:"private_key_arn"` + PrivateKeySecretRegion string `yaml:"private_key_region"` + PrivateKeySecretSTSAssumeRoleArn string `yaml:"private_key_sts_assume_role_arn"` + PrivateKeySecretSTSExternalID string `yaml:"private_key_sts_external_id"` VPPVerifyTimeout time.Duration `yaml:"vpp_verify_timeout"` VPPVerifyRequestDelay time.Duration `yaml:"vpp_verify_request_delay"` } @@ -1124,9 +1125,10 @@ func (man Manager) addConfigs() { man.addConfigBool("server.frequent_cleanups_enabled", false, "Enable frequent cleanups of expired data (15 minute interval)") man.addConfigBool("server.force_h2c", false, "Force the fleet server to use HTTP2 cleartext aka h2c (ignored if using TLS)") man.addConfigString("server.private_key", "", "Used for encrypting sensitive data, such as MDM certificates.") - man.addConfigString("server.private_key_secret_arn", "", "ARN of AWS Secrets Manager secret containing server private key") - man.addConfigString("server.private_key_secret_sts_assume_role_arn", "", "ARN of role to assume for accessing private key secret") - man.addConfigString("server.private_key_secret_sts_external_id", "", "External ID for STS role assumption when accessing private key secret") + man.addConfigString("server.private_key_region", "", "AWS region of the Secrets Manager secret containing server private key") + man.addConfigString("server.private_key_arn", "", "ARN of AWS Secrets Manager secret containing server private key") + man.addConfigString("server.private_key_sts_assume_role_arn", "", "ARN of role to assume for accessing private key secret") + man.addConfigString("server.private_key_sts_external_id", "", "External ID for STS role assumption when accessing private key secret") man.addConfigDuration("server.vpp_verify_timeout", 10*time.Minute, "Maximum amout of time to wait for VPP app install verification") man.addConfigDuration("server.vpp_verify_request_delay", 5*time.Second, "Delay in between requests to verify VPP app installs") @@ -1566,9 +1568,10 @@ func (man Manager) LoadConfig() FleetConfig { FrequentCleanupsEnabled: man.getConfigBool("server.frequent_cleanups_enabled"), ForceH2C: man.getConfigBool("server.force_h2c"), PrivateKey: man.getConfigString("server.private_key"), - PrivateKeySecretArn: man.getConfigString("server.private_key_secret_arn"), - PrivateKeySecretSTSAssumeRoleArn: man.getConfigString("server.private_key_secret_sts_assume_role_arn"), - PrivateKeySecretSTSExternalID: man.getConfigString("server.private_key_secret_sts_external_id"), + PrivateKeySecretArn: man.getConfigString("server.private_key_arn"), + PrivateKeySecretRegion: man.getConfigString("server.private_key_region"), + PrivateKeySecretSTSAssumeRoleArn: man.getConfigString("server.private_key_sts_assume_role_arn"), + PrivateKeySecretSTSExternalID: man.getConfigString("server.private_key_sts_external_id"), VPPVerifyTimeout: man.getConfigDuration("server.vpp_verify_timeout"), VPPVerifyRequestDelay: man.getConfigDuration("server.vpp_verify_request_delay"), }, diff --git a/server/config/config_test.go b/server/config/config_test.go index af2386f0a5..aa80377dcb 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "github.com/fleetdm/fleet/v4/pkg/testutils" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,6 +31,9 @@ func TestConfigRoundtrip(t *testing.T) { // viper tries to load config from the environment too, clear it in case // any config values are set in the environment. + + // save the current env before clearing it. + testutils.SaveEnv(t) os.Clearenv() cmd := &cobra.Command{} @@ -302,7 +306,9 @@ osquery: // test-case values, but that didn't seem to work, not sure how it can // be done in our particular setup. - // set the environment variables + // save the current env before clearing it. + testutils.SaveEnv(t) + os.Clearenv() for _, env := range c.envVars { kv := strings.SplitN(env, "=", 2) diff --git a/server/config/secrets_manager.go b/server/config/secrets_manager.go index 7bac38852f..f4507cb952 100644 --- a/server/config/secrets_manager.go +++ b/server/config/secrets_manager.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "math/rand" - "strings" "time" aws_config "github.com/aws/aws-sdk-go-v2/config" @@ -20,22 +19,6 @@ type SecretsManagerClient interface { optFns ...func(*secretsmanager.Options)) (*secretsmanager.GetSecretValueOutput, error) } -// parseRegionFromSecretARN extracts the AWS region from a Secrets Manager ARN -func parseRegionFromSecretARN(arn string) (string, error) { - // ARN format: arn:aws:secretsmanager:region:account:secret:name - parts := strings.Split(arn, ":") - if len(parts) < 6 || parts[0] != "arn" || parts[1] != "aws" || parts[2] != "secretsmanager" { - return "", fmt.Errorf("invalid Secrets Manager ARN format: %s", arn) - } - - region := parts[3] - if region == "" { - return "", fmt.Errorf("region not found in ARN: %s", arn) - } - - return region, nil -} - // retrieveSecretWithRetry retrieves the secret from AWS with retry logic func retrieveSecretWithRetry(ctx context.Context, client SecretsManagerClient, secretArn string) (string, error) { const maxRetries = 3 @@ -43,8 +26,8 @@ func retrieveSecretWithRetry(ctx context.Context, client SecretsManagerClient, s for attempt := 0; attempt < maxRetries; attempt++ { if attempt > 0 { - // Exponential backoff with jitter: base 100ms with ±50% randomization - baseBackoff := time.Duration(100*(1<