VAULT-12299 Use file.Stat when checking file permissions (hashicorp#1…

…9311)

* use file.Stat for config files

* cleanup and add path

* include directory path

* revert changes to LoadConfigDir

* remove path, add additional test:

* add changelog

changelog/19311.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+server/config:  Use file.Stat when checking file permissions when VAULT_ENABLE_FILE_PERMISSIONS_CHECK is enabled
+```

command/server/config.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"math"
 	"os"
 	"path/filepath"
@@ -465,9 +464,14 @@ func LoadConfig(path string) (*Config, error) {
 				return nil, errors.New("Error parsing the environment variable VAULT_ENABLE_FILE_PERMISSIONS_CHECK")
 			}
 		}
+		f, err := os.Open(path)
+		if err != nil {
+			return nil, err
+		}
+		defer f.Close()
 
 		if enableFilePermissionsCheck {
-			err = osutil.OwnerPermissionsMatch(path, 0, 0)
+			err = osutil.OwnerPermissionsMatchFile(f, 0, 0)
 			if err != nil {
 				return nil, err
 			}
@@ -496,8 +500,14 @@ func CheckConfig(c *Config, e error) (*Config, error) {
 
 // LoadConfigFile loads the configuration from the given file.
 func LoadConfigFile(path string) (*Config, error) {
+	// Open the file
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
 	// Read the file
-	d, err := ioutil.ReadFile(path)
+	d, err := io.ReadAll(f)
 	if err != nil {
 		return nil, err
 	}
@@ -518,7 +528,7 @@ func LoadConfigFile(path string) (*Config, error) {
 
 	if enableFilePermissionsCheck {
 		// check permissions of the config file
-		err = osutil.OwnerPermissionsMatch(path, 0, 0)
+		err = osutil.OwnerPermissionsMatchFile(f, 0, 0)
 		if err != nil {
 			return nil, err
 		}

helper/osutil/fileinfo.go

@@ -64,3 +64,17 @@ func OwnerPermissionsMatch(path string, uid int, permissions int) error {
 
 	return nil
 }
+
+// OwnerPermissionsMatchFile checks if vault user is the owner and permissions are secure for the input file
+func OwnerPermissionsMatchFile(file *os.File, uid int, permissions int) error {
+	info, err := file.Stat()
+	if err != nil {
+		return fmt.Errorf("file stat error on path %q: %w", file.Name(), err)
+	}
+	err = checkPathInfo(info, file.Name(), uid, permissions)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}

helper/osutil/fileinfo_test.go

@@ -4,6 +4,7 @@ import (
 	"io/fs"
 	"os"
 	"os/user"
+	"path/filepath"
 	"runtime"
 	"strconv"
 	"testing"
@@ -82,3 +83,98 @@ func TestCheckPathInfo(t *testing.T) {
 		}
 	}
 }
+
+// TestOwnerPermissionsMatchFile creates a file and verifies that the current user of the process is the owner of the
+// file
+func TestOwnerPermissionsMatchFile(t *testing.T) {
+	currentUser, err := user.Current()
+	if err != nil {
+		t.Fatal("failed to get current user", err)
+	}
+	uid, err := strconv.ParseInt(currentUser.Uid, 0, 64)
+	if err != nil {
+		t.Fatal("failed to convert uid", err)
+	}
+	dir := t.TempDir()
+	path := filepath.Join(dir, "foo")
+	f, err := os.Create(path)
+	if err != nil {
+		t.Fatal("failed to create test file", err)
+	}
+	defer f.Close()
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatal("failed to stat test file", err)
+	}
+
+	if err := OwnerPermissionsMatchFile(f, int(uid), int(info.Mode())); err != nil {
+		t.Fatalf("expected no error but got %v", err)
+	}
+}
+
+// TestOwnerPermissionsMatchFile_OtherUser creates a file using the user that started the current process and verifies
+// that a different user is not the owner of the file
+func TestOwnerPermissionsMatchFile_OtherUser(t *testing.T) {
+	currentUser, err := user.Current()
+	if err != nil {
+		t.Fatal("failed to get current user", err)
+	}
+	uid, err := strconv.ParseInt(currentUser.Uid, 0, 64)
+	if err != nil {
+		t.Fatal("failed to convert uid", err)
+	}
+	dir := t.TempDir()
+	path := filepath.Join(dir, "foo")
+	f, err := os.Create(path)
+	if err != nil {
+		t.Fatal("failed to create test file", err)
+	}
+	defer f.Close()
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatal("failed to stat test file", err)
+	}
+
+	if err := OwnerPermissionsMatchFile(f, int(uid)+1, int(info.Mode())); err == nil {
+		t.Fatalf("expected error but none")
+	}
+}
+
+// TestOwnerPermissionsMatchFile_Symlink creates a file and a symlink to that file. The test verifies that the current
+// user of the process is the owner of the file
+func TestOwnerPermissionsMatchFile_Symlink(t *testing.T) {
+	currentUser, err := user.Current()
+	if err != nil {
+		t.Fatal("failed to get current user", err)
+	}
+	uid, err := strconv.ParseInt(currentUser.Uid, 0, 64)
+	if err != nil {
+		t.Fatal("failed to convert uid", err)
+	}
+	dir := t.TempDir()
+	path := filepath.Join(dir, "foo")
+	f, err := os.Create(path)
+	if err != nil {
+		t.Fatal("failed to create test file", err)
+	}
+	defer f.Close()
+
+	symlink := filepath.Join(dir, "symlink")
+	err = os.Symlink(path, symlink)
+	if err != nil {
+		t.Fatal("failed to symlink file", err)
+	}
+	symlinkedFile, err := os.Open(symlink)
+	if err != nil {
+		t.Fatal("failed to open file", err)
+	}
+	info, err := os.Stat(symlink)
+	if err != nil {
+		t.Fatal("failed to stat test file", err)
+	}
+	if err := OwnerPermissionsMatchFile(symlinkedFile, int(uid), int(info.Mode())); err != nil {
+		t.Fatalf("expected no error but got %v", err)
+	}
+}