agent: allow changing file ownership in file sink (#27123)

* agent: allow changing file ownership in file sink

Allow changing the ownership of the token file in file sink.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

* Consistency: id -> ID

* Add changelog

* Remove empty line in changelog

* agent: add godoc for TestFileSinkMode_Ownership

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

---------

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>

changelog/27123.txt

@@ -0,0 +1,6 @@
+```release-note:improvement
+agent/sink: Allow configuration of the user and group ID of the file sink.
+```
+```release-note:improvement
+proxy/sink: Allow configuration of the user and group ID of the file sink.
+```

command/agentproxyshared/sink/file/file_sink.go

@@ -19,6 +19,8 @@ import (
 type fileSink struct {
 	path   string
 	mode   os.FileMode
+	owner  int
+	group  int
 	logger hclog.Logger
 }
 
@@ -33,6 +35,8 @@ func NewFileSink(conf *sink.SinkConfig) (sink.Sink, error) {
 	f := &fileSink{
 		logger: conf.Logger,
 		mode:   0o640,
+		owner:  os.Getuid(),
+		group:  os.Getgid(),
 	}
 
 	pathRaw, ok := conf.Config["path"]
@@ -61,11 +65,31 @@ func NewFileSink(conf *sink.SinkConfig) (sink.Sink, error) {
 		f.mode = os.FileMode(mode)
 	}
 
+	if modeRaw, ok := conf.Config["owner"]; ok {
+		owner, typeOK := modeRaw.(int)
+		if !typeOK {
+			return nil, errors.New("could not parse 'owner' as integer")
+		}
+
+		f.logger.Debug("overriding default file sink", "owner", owner)
+		f.owner = owner
+	}
+
+	if modeRaw, ok := conf.Config["group"]; ok {
+		group, typeOK := modeRaw.(int)
+		if !typeOK {
+			return nil, errors.New("could not parse 'group' as integer")
+		}
+
+		f.logger.Debug("overriding default file sink", "group", group)
+		f.group = group
+	}
+
 	if err := f.WriteToken(""); err != nil {
 		return nil, fmt.Errorf("error during write check: %w", err)
 	}
 
-	f.logger.Info("file sink configured", "path", f.path, "mode", f.mode)
+	f.logger.Info("file sink configured", "path", f.path, "mode", f.mode, "owner", f.owner, "group", f.group)
 
 	return f, nil
 }
@@ -93,6 +117,10 @@ func (f *fileSink) WriteToken(token string) error {
 		return fmt.Errorf("error opening temp file in dir %s for writing: %w", targetDir, err)
 	}
 
+	if err := tmpFile.Chown(f.owner, f.group); err != nil {
+		return fmt.Errorf("error changing ownership of %s: %w", tmpFile.Name(), err)
+	}
+
 	valToWrite := token
 	if token == "" {
 		valToWrite = u

command/agentproxyshared/sink/file/file_sink_test.go

@@ -4,10 +4,9 @@
 package file
 
 import (
-	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
 
 	hclog "github.com/hashicorp/go-hclog"
@@ -16,15 +15,8 @@ import (
 	"github.com/hashicorp/vault/sdk/helper/logging"
 )
 
-const (
-	fileServerTestDir = "vault-agent-file-test"
-)
-
 func testFileSink(t *testing.T, log hclog.Logger) (*sink.SinkConfig, string) {
-	tmpDir, err := ioutil.TempDir("", fmt.Sprintf("%s.", fileServerTestDir))
-	if err != nil {
-		t.Fatal(err)
-	}
+	tmpDir := t.TempDir()
 
 	path := filepath.Join(tmpDir, "token")
 
@@ -74,7 +66,7 @@ func TestFileSink(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	fileBytes, err := ioutil.ReadFile(path)
+	fileBytes, err := os.ReadFile(path)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -84,19 +76,17 @@ func TestFileSink(t *testing.T) {
 	}
 }
 
-func testFileSinkMode(t *testing.T, log hclog.Logger) (*sink.SinkConfig, string) {
-	tmpDir, err := ioutil.TempDir("", fmt.Sprintf("%s.", fileServerTestDir))
-	if err != nil {
-		t.Fatal(err)
-	}
+func testFileSinkMode(t *testing.T, log hclog.Logger, gid int) (*sink.SinkConfig, string) {
+	tmpDir := t.TempDir()
 
 	path := filepath.Join(tmpDir, "token")
 
 	config := &sink.SinkConfig{
 		Logger: log.Named("sink.file"),
 		Config: map[string]interface{}{
-			"path": path,
-			"mode": 0o644,
+			"path":  path,
+			"mode":  0o644,
+			"group": gid,
 		},
 	}
 
@@ -112,7 +102,7 @@ func testFileSinkMode(t *testing.T, log hclog.Logger) (*sink.SinkConfig, string)
 func TestFileSinkMode(t *testing.T) {
 	log := logging.NewVaultLogger(hclog.Trace)
 
-	fs, tmpDir := testFileSinkMode(t, log)
+	fs, tmpDir := testFileSinkMode(t, log, os.Getegid())
 	defer os.RemoveAll(tmpDir)
 
 	path := filepath.Join(tmpDir, "token")
@@ -136,7 +126,69 @@ func TestFileSinkMode(t *testing.T) {
 		t.Fatalf("wrong file mode was detected at %s", path)
 	}
 
-	fileBytes, err := ioutil.ReadFile(path)
+	fileBytes, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if string(fileBytes) != uuidStr {
+		t.Fatalf("expected %s, got %s", uuidStr, string(fileBytes))
+	}
+}
+
+// TestFileSinkMode_Ownership tests that the file is owned by the group specified
+// in the configuration. This test requires the current user to be in at least two
+// groups. If the user is not in two groups, the test will be skipped.
+func TestFileSinkMode_Ownership(t *testing.T) {
+	groups, err := os.Getgroups()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(groups) < 2 {
+		t.Skip("not enough groups to test file ownership")
+	}
+
+	// find a group that is not the current group
+	var gid int
+	for _, g := range groups {
+		if g != os.Getegid() {
+			gid = g
+			break
+		}
+	}
+
+	log := logging.NewVaultLogger(hclog.Trace)
+
+	fs, tmpDir := testFileSinkMode(t, log, gid)
+	defer os.RemoveAll(tmpDir)
+
+	path := filepath.Join(tmpDir, "token")
+
+	uuidStr, _ := uuid.GenerateUUID()
+	if err := fs.WriteToken(uuidStr); err != nil {
+		t.Fatal(err)
+	}
+
+	file, err := os.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer file.Close()
+
+	fi, err := file.Stat()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fi.Mode() != os.FileMode(0o644) {
+		t.Fatalf("wrong file mode was detected at %s", path)
+	}
+	// check if file is owned by the group
+	if fi.Sys().(*syscall.Stat_t).Gid != uint32(gid) {
+		t.Fatalf("file is not owned by the group %d", gid)
+	}
+
+	fileBytes, err := os.ReadFile(path)
 	if err != nil {
 		t.Fatal(err)
 	}

website/content/docs/agent-and-proxy/autoauth/sinks/file.mdx

@@ -21,7 +21,9 @@ written with `0640` permissions as default, but can be overridden with the optio
 ## Configuration
 
 - `path` `(string: required)` - The path to use to write the token file
-- `mode` `(int: optional)` - A string containing an octal number representing the bit pattern for the file mode, similar to chmod. Set to `0000` to prevent Vault from modifying the file mode. Note: This configuration option is only available in Vault 1.3.0 and above.
+- `mode` `(int: optional)` - Octal number string representing the bit pattern for the file mode, similar to `chmod`.
+- `owner` `(int: optional)` - The UID to use for the token file. Defaults to the current user ID.
+- `group` `(int: optional)` - The GID to use for token file. Defaults to the current group ID.
 
 ~> Note: Configuration options for response-wrapping and encryption for the sink
 file are located within the [options common to all sinks](/vault/docs/agent-and-proxy/autoauth#configuration-sinks) documentation.