From 4e9e9b7eda270ea451742b4d118d6ab699f3f2f6 Mon Sep 17 00:00:00 2001 From: akshya96 <87045294+akshya96@users.noreply.github.com> Date: Tue, 17 May 2022 11:34:31 -0700 Subject: [PATCH] Vault-6037 making filesystem permissions check opt-in (#15452) * adding env var changes * adding changelog * adding strcov.ParseBool --- changelog/15452.txt | 3 +++ command/operator_diagnose_test.go | 2 -- command/server/config.go | 22 ++++++++++++++++++++-- command/server_test.go | 2 -- http/plugin_test.go | 1 - sdk/helper/consts/consts.go | 2 +- vault/core.go | 12 +++++++++++- vault/logical_system_integ_test.go | 3 --- 8 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 changelog/15452.txt diff --git a/changelog/15452.txt b/changelog/15452.txt new file mode 100644 index 000000000..301416807 --- /dev/null +++ b/changelog/15452.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: renaming the environment variable VAULT_DISABLE_FILE_PERMISSIONS_CHECK to VAULT_ENABLE_FILE_PERMISSIONS_CHECK and adjusting the logic +``` \ No newline at end of file diff --git a/command/operator_diagnose_test.go b/command/operator_diagnose_test.go index bacbbeb2a..99834b9d4 100644 --- a/command/operator_diagnose_test.go +++ b/command/operator_diagnose_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/vault/diagnose" "github.com/mitchellh/cli" ) @@ -479,7 +478,6 @@ func TestOperatorDiagnoseCommand_Run(t *testing.T) { t.Parallel() client, closer := testVaultServer(t) defer closer() - os.Setenv(consts.VaultDisableFilePermissionsCheckEnv, "true") cmd := testOperatorDiagnoseCommand(t) cmd.client = client diff --git a/command/server/config.go b/command/server/config.go index 145672ea8..a3526e16e 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -369,7 +369,16 @@ func LoadConfig(path string) (*Config, error) { if fi.IsDir() { // check permissions on the config directory - if os.Getenv(consts.VaultDisableFilePermissionsCheckEnv) != "true" { + var enableFilePermissionsCheck bool + if enableFilePermissionsCheckEnv := os.Getenv(consts.VaultEnableFilePermissionsCheckEnv); enableFilePermissionsCheckEnv != "" { + var err error + enableFilePermissionsCheck, err = strconv.ParseBool(enableFilePermissionsCheckEnv) + if err != nil { + return nil, errors.New("Error parsing the environment variable VAULT_ENABLE_FILE_PERMISSIONS_CHECK") + } + } + + if enableFilePermissionsCheck { err = osutil.OwnerPermissionsMatch(path, 0, 0) if err != nil { return nil, err @@ -410,7 +419,16 @@ func LoadConfigFile(path string) (*Config, error) { return nil, err } - if os.Getenv(consts.VaultDisableFilePermissionsCheckEnv) != "true" { + var enableFilePermissionsCheck bool + if enableFilePermissionsCheckEnv := os.Getenv(consts.VaultEnableFilePermissionsCheckEnv); enableFilePermissionsCheckEnv != "" { + var err error + enableFilePermissionsCheck, err = strconv.ParseBool(enableFilePermissionsCheckEnv) + if err != nil { + return nil, errors.New("Error parsing the environment variable VAULT_ENABLE_FILE_PERMISSIONS_CHECK") + } + } + + if enableFilePermissionsCheck { // check permissions of the config file err = osutil.OwnerPermissionsMatch(path, 0, 0) if err != nil { diff --git a/command/server_test.go b/command/server_test.go index 3f23b1ebd..d836b1104 100644 --- a/command/server_test.go +++ b/command/server_test.go @@ -18,7 +18,6 @@ import ( "testing" "time" - "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/physical" physInmem "github.com/hashicorp/vault/sdk/physical/inmem" "github.com/mitchellh/cli" @@ -116,7 +115,6 @@ func TestServer_ReloadListener(t *testing.T) { defer os.RemoveAll(td) wg := &sync.WaitGroup{} - os.Setenv(consts.VaultDisableFilePermissionsCheckEnv, "true") // Setup initial certs inBytes, _ := ioutil.ReadFile(wd + "reload_foo.pem") ioutil.WriteFile(td+"/reload_cert.pem", inBytes, 0o777) diff --git a/http/plugin_test.go b/http/plugin_test.go index 9aa838b48..38b8669eb 100644 --- a/http/plugin_test.go +++ b/http/plugin_test.go @@ -31,7 +31,6 @@ func getPluginClusterAndCore(t testing.TB, logger log.Logger) (*vault.TestCluste if err != nil { t.Fatal(err) } - os.Setenv(consts.VaultDisableFilePermissionsCheckEnv, "true") coreConfig := &vault.CoreConfig{ Physical: inm, diff --git a/sdk/helper/consts/consts.go b/sdk/helper/consts/consts.go index 86d4f0696..c431e2e59 100644 --- a/sdk/helper/consts/consts.go +++ b/sdk/helper/consts/consts.go @@ -33,5 +33,5 @@ const ( // resolving replicaiton addresses ReplicationResolverALPN = "replication_resolver_v1" - VaultDisableFilePermissionsCheckEnv = "VAULT_DISABLE_FILE_PERMISSIONS_CHECK" + VaultEnableFilePermissionsCheckEnv = "VAULT_ENABLE_FILE_PERMISSIONS_CHECK" ) diff --git a/vault/core.go b/vault/core.go index 7a0e3fc69..5c7d93708 100644 --- a/vault/core.go +++ b/vault/core.go @@ -16,6 +16,7 @@ import ( "net/url" "os" "path/filepath" + "strconv" "strings" "sync" "sync/atomic" @@ -3270,7 +3271,16 @@ func (c *Core) GetHAPeerNodesCached() []PeerNode { } func (c *Core) CheckPluginPerms(pluginName string) (err error) { - if c.pluginDirectory != "" && os.Getenv(consts.VaultDisableFilePermissionsCheckEnv) != "true" { + var enableFilePermissionsCheck bool + if enableFilePermissionsCheckEnv := os.Getenv(consts.VaultEnableFilePermissionsCheckEnv); enableFilePermissionsCheckEnv != "" { + var err error + enableFilePermissionsCheck, err = strconv.ParseBool(enableFilePermissionsCheckEnv) + if err != nil { + return errors.New("Error parsing the environment variable VAULT_ENABLE_FILE_PERMISSIONS_CHECK") + } + } + + if c.pluginDirectory != "" && enableFilePermissionsCheck { err = osutil.OwnerPermissionsMatch(c.pluginDirectory, c.pluginFileUid, c.pluginFilePermissions) if err != nil { return err diff --git a/vault/logical_system_integ_test.go b/vault/logical_system_integ_test.go index aaf101b86..a32351a3a 100644 --- a/vault/logical_system_integ_test.go +++ b/vault/logical_system_integ_test.go @@ -529,8 +529,6 @@ func testSystemBackendMock(t *testing.T, numCores, numMounts int, backendType lo }, } - os.Setenv(consts.VaultDisableFilePermissionsCheckEnv, "true") - // Create a tempdir, cluster.Cleanup will clean up this directory tempDir, err := ioutil.TempDir("", "vault-test-cluster") if err != nil { @@ -603,7 +601,6 @@ func testSystemBackend_SingleCluster_Env(t *testing.T, env []string) *vault.Test "test": plugin.Factory, }, } - os.Setenv(consts.VaultDisableFilePermissionsCheckEnv, "true") // Create a tempdir, cluster.Cleanup will clean up this directory tempDir, err := ioutil.TempDir("", "vault-test-cluster") if err != nil {