Merge pull request #1850 from hashicorp/f-fs-secret

Disallow fs to read secret directory

client/allocdir/alloc_dir.go

@@ -467,6 +467,15 @@ func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) {
 	}
 
 	p := filepath.Join(d.AllocDir, path)
+
+	// Check if it is trying to read into a secret directory
+	for _, dir := range d.TaskDirs {
+		sdir := filepath.Join(dir, TaskSecrets)
+		if filepath.HasPrefix(p, sdir) {
+			return nil, fmt.Errorf("Reading secret file prohibited: %s", path)
+		}
+	}
+
 	f, err := os.Open(p)
 	if err != nil {
 		return nil, err

client/allocdir/alloc_dir_test.go

@@ -390,3 +390,24 @@ func TestAllocDir_EscapeChecking(t *testing.T) {
 		t.Fatalf("ChangeEvents of escaping path didn't error: %v", err)
 	}
 }
+
+func TestAllocDir_ReadAt_SecretDir(t *testing.T) {
+	tmp, err := ioutil.TempDir("", "AllocDir")
+	if err != nil {
+		t.Fatalf("Couldn't create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmp)
+
+	d := NewAllocDir(tmp)
+	defer d.Destroy()
+	tasks := []*structs.Task{t1, t2}
+	if err := d.Build(tasks); err != nil {
+		t.Fatalf("Build(%v) failed: %v", tasks, err)
+	}
+
+	// ReadAt of secret dir should fail
+	secret := filepath.Join(t1.Name, TaskSecrets, "test_file")
+	if _, err := d.ReadAt(secret, 0); err == nil || !strings.Contains(err.Error(), "secret file prohibited") {
+		t.Fatalf("ReadAt of secret file didn't error: %v", err)
+	}
+}

command/fs.go

@@ -298,7 +298,9 @@ func (f *FSCommand) Run(args []string) int {
 		}
 	}
 
+	if r != nil {
 		defer r.Close()
+	}
 	if readErr != nil {
 		f.Ui.Error(readErr.Error())
 		return 1