Add new file parameter to vault stanza

This parameter controls whether the Vault token is actually
written to `secrets/vault_token`.

The Vault token is always written to `<task_dir>/private/vault_token`,
`private` being a new directory to hold private secrets data not to be
shared with the chroot and also not to be shared with Nomad UI.

If `file` is set to `true` (the default), it is also written to
`<task_dir>/secrets/vault_token`, using the permissions configured
in `file_perms`.

There are two rationales for this design:

1. Have one authoritative place where `vault_token` exists.
2. Don't read `vault_token` from a place that operates on a lower
   security level on Nomad restart. Ultimately, `secrets/vault_token`
   could have been altered in unfortunate or malicious ways from within
   the chroot. This was the primary reason to add an additional write
   operation instead of just modifying `tokenPath` depending on the
   value of `file`.

Open questions:
- Is creating a separate `private` directory worth the overhead?

api/tasks.go

@@ -856,6 +856,7 @@ type Vault struct {
 	Env          *bool    `hcl:"env,optional"`
 	ChangeMode   *string  `mapstructure:"change_mode" hcl:"change_mode,optional"`
 	ChangeSignal *string  `mapstructure:"change_signal" hcl:"change_signal,optional"`
+	File         *bool    `mapstructure:"file" hcl:"file,optional"`
 	FilePerms    *string  `mapstructure:"file_perms" hcl:"file_perms,optional"`
 }
 
@@ -872,6 +873,9 @@ func (v *Vault) Canonicalize() {
 	if v.ChangeSignal == nil {
 		v.ChangeSignal = stringToPtr("SIGHUP")
 	}
+	if v.File == nil {
+		v.File = boolToPtr(true)
+	}
 	if v.FilePerms == nil {
 		v.FilePerms = stringToPtr("0666")
 	}

client/allocdir/alloc_dir.go

@@ -58,6 +58,10 @@ var (
 	// directory
 	TaskSecrets = "secrets"
 
+	// TaskPrivate is the name of the pruvate directory inside each task
+	// directory
+	TaskPrivate = "private"
+
 	// TaskDirs is the set of directories created in each tasks directory.
 	TaskDirs = map[string]os.FileMode{TmpDirName: os.ModeSticky | 0777}
 
@@ -304,6 +308,13 @@ func (d *AllocDir) UnmountAll() error {
 			}
 		}
 
+		if pathExists(dir.PrivateDir) {
+			if err := removeSecretDir(dir.PrivateDir); err != nil {
+				mErr.Errors = append(mErr.Errors,
+					fmt.Errorf("failed to remove the private dir %q: %v", dir.PrivateDir, err))
+			}
+		}
+
 		// Unmount dev/ and proc/ have been mounted.
 		if err := dir.unmountSpecialDirs(); err != nil {
 			mErr.Errors = append(mErr.Errors, err)
@@ -441,6 +452,10 @@ func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) {
 			d.mu.RUnlock()
 			return nil, fmt.Errorf("Reading secret file prohibited: %s", path)
 		}
+		if filepath.HasPrefix(p, dir.PrivateDir) {
+			d.mu.RUnlock()
+			return nil, fmt.Errorf("Reading private file prohibited: %s", path)
+		}
 	}
 	d.mu.RUnlock()
 

client/allocdir/task_dir.go

@@ -39,6 +39,10 @@ type TaskDir struct {
 	// <task_dir>/secrets/
 	SecretsDir string
 
+	// PrivateDir is the path to private/ directory on the host
+	// <task_dir>/private/
+	PrivateDir string
+
 	// skip embedding these paths in chroots. Used for avoiding embedding
 	// client.alloc_dir recursively.
 	skip map[string]struct{}
@@ -66,6 +70,7 @@ func newTaskDir(logger hclog.Logger, clientAllocDir, allocDir, taskName string)
 		SharedTaskDir:  filepath.Join(taskDir, SharedAllocName),
 		LocalDir:       filepath.Join(taskDir, TaskLocal),
 		SecretsDir:     filepath.Join(taskDir, TaskSecrets),
+		PrivateDir:     filepath.Join(taskDir, TaskPrivate),
 		skip:           skip,
 		logger:         logger,
 	}
@@ -128,6 +133,15 @@ func (t *TaskDir) Build(createChroot bool, chroot map[string]string) error {
 		return err
 	}
 
+	// Create the private directory
+	if err := createSecretDir(t.PrivateDir); err != nil {
+		return err
+	}
+
+	if err := dropDirPermissions(t.PrivateDir, os.ModePerm); err != nil {
+		return err
+	}
+
 	// Build chroot if chroot filesystem isolation is going to be used
 	if createChroot {
 		if err := t.buildChroot(chroot); err != nil {

client/allocrunner/taskrunner/task_runner_test.go

@@ -1557,7 +1557,7 @@ func TestTaskRunner_BlockForVaultToken(t *testing.T) {
 	require.False(t, finalState.Failed)
 
 	// Check that the token is on disk
-	tokenPath := filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile)
+	tokenPath := filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile)
 	data, err := ioutil.ReadFile(tokenPath)
 	require.NoError(t, err)
 	require.Equal(t, token, string(data))
@@ -1622,7 +1622,7 @@ func TestTaskRunner_DeriveToken_Retry(t *testing.T) {
 	require.Equal(t, 1, count)
 
 	// Check that the token is on disk
-	tokenPath := filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile)
+	tokenPath := filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile)
 	data, err := ioutil.ReadFile(tokenPath)
 	require.NoError(t, err)
 	require.Equal(t, token, string(data))

client/allocrunner/taskrunner/vault_hook.go

@@ -83,8 +83,12 @@ type vaultHook struct {
 	// tokenPath is the path in which to read and write the token
 	tokenPath string
 
+	// sharedTokenPath is the path in which to only write, but never
+	// read the token from
+	sharedTokenPath string
+
 	// tokenFilePerms are the file permissions applied to tokenPath
-	tokenPerms fs.FileMode
+	sharedTokenPerms fs.FileMode
 
 	// alloc is the allocation
 	alloc *structs.Allocation
@@ -102,18 +106,18 @@ type vaultHook struct {
 func newVaultHook(config *vaultHookConfig) *vaultHook {
 	ctx, cancel := context.WithCancel(context.Background())
 	h := &vaultHook{
-		vaultStanza:  config.vaultStanza,
-		client:       config.client,
-		eventEmitter: config.events,
-		lifecycle:    config.lifecycle,
-		updater:      config.updater,
-		alloc:        config.alloc,
-		tokenPerms:   0666,
-		taskName:     config.task,
-		firstRun:     true,
-		ctx:          ctx,
-		cancel:       cancel,
-		future:       newTokenFuture(),
+		vaultStanza:      config.vaultStanza,
+		client:           config.client,
+		eventEmitter:     config.events,
+		lifecycle:        config.lifecycle,
+		updater:          config.updater,
+		alloc:            config.alloc,
+		sharedTokenPerms: 0666,
+		taskName:         config.task,
+		firstRun:         true,
+		ctx:              ctx,
+		cancel:           cancel,
+		future:           newTokenFuture(),
 	}
 	h.logger = config.logger.Named(h.Name())
 	return h
@@ -138,13 +142,13 @@ func (h *vaultHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRe
 		if err != nil {
 			return fmt.Errorf("Failed to parse %q as octal: %v", h.vaultStanza.FilePerms, err)
 		}
-		h.tokenPerms = fs.FileMode(v)
+		h.sharedTokenPerms = fs.FileMode(v)
 	}
 
 	// Try to recover a token if it was previously written in the secrets
 	// directory
 	recoveredToken := ""
-	h.tokenPath = filepath.Join(req.TaskDir.SecretsDir, vaultTokenFile)
+	h.tokenPath = filepath.Join(req.TaskDir.PrivateDir, vaultTokenFile)
 	data, err := ioutil.ReadFile(h.tokenPath)
 	if err != nil {
 		if !os.IsNotExist(err) {
@@ -156,6 +160,7 @@ func (h *vaultHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRe
 		// Store the recovered token
 		recoveredToken = string(data)
 	}
+	h.sharedTokenPath = filepath.Join(req.TaskDir.SecretsDir, vaultTokenFile)
 
 	// Launch the token manager
 	go h.run(recoveredToken)
@@ -358,9 +363,14 @@ func (h *vaultHook) deriveVaultToken() (token string, exit bool) {
 
 // writeToken writes the given token to disk
 func (h *vaultHook) writeToken(token string) error {
-	if err := ioutil.WriteFile(h.tokenPath, []byte(token), h.tokenPerms); err != nil {
+	if err := ioutil.WriteFile(h.tokenPath, []byte(token), 0600); err != nil {
 		return fmt.Errorf("failed to write vault token: %v", err)
 	}
+	if h.vaultStanza.File {
+		if err := ioutil.WriteFile(h.sharedTokenPath, []byte(token), h.sharedTokenPerms); err != nil {
+			return fmt.Errorf("failed to write vault token to secrets dir: %v", err)
+		}
+	}
 
 	return nil
 }

command/agent/job_endpoint.go

@@ -1140,6 +1140,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
 			Env:          *apiTask.Vault.Env,
 			ChangeMode:   *apiTask.Vault.ChangeMode,
 			ChangeSignal: *apiTask.Vault.ChangeSignal,
+			File:         *apiTask.Vault.File,
 			FilePerms:    *apiTask.Vault.FilePerms,
 		}
 	}

command/agent/job_endpoint_test.go

@@ -2485,6 +2485,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 							Env:          helper.BoolToPtr(true),
 							ChangeMode:   helper.StringToPtr("c"),
 							ChangeSignal: helper.StringToPtr("sighup"),
+							File:         helper.BoolToPtr(true),
 							FilePerms:    helper.StringToPtr("0666"),
 						},
 						Templates: []*api.Template{
@@ -2883,6 +2884,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 							Env:          true,
 							ChangeMode:   "c",
 							ChangeSignal: "sighup",
+							File:         true,
 							FilePerms:    "0666",
 						},
 						Templates: []*structs.Template{

drivers/shared/executor/executor_linux_test.go

@@ -229,6 +229,7 @@ etc/
 lib/
 lib64/
 local/
+private/
 proc/
 secrets/
 sys/

jobspec/parse.go

@@ -510,6 +510,7 @@ func parseVault(result *api.Vault, list *ast.ObjectList) error {
 		"env",
 		"change_mode",
 		"change_signal",
+		"file",
 		"file_perms",
 	}
 	if err := checkHCLKeys(listVal, valid); err != nil {

jobspec/parse_group.go

@@ -211,6 +211,8 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error {
 			tgVault := &api.Vault{
 				Env:        boolToPtr(true),
 				ChangeMode: stringToPtr("restart"),
+				File:       boolToPtr(true),
+				FilePerms:  stringToPtr("0666"),
 			}
 
 			if err := parseVault(tgVault, o); err != nil {

jobspec/parse_job.go

@@ -193,6 +193,8 @@ func parseJob(result *api.Job, list *ast.ObjectList) error {
 		jobVault := &api.Vault{
 			Env:        boolToPtr(true),
 			ChangeMode: stringToPtr("restart"),
+			File:       boolToPtr(true),
+			FilePerms:  stringToPtr("0666"),
 		}
 
 		if err := parseVault(jobVault, o); err != nil {

jobspec/parse_task.go

@@ -291,6 +291,8 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) {
 		v := &api.Vault{
 			Env:        boolToPtr(true),
 			ChangeMode: stringToPtr("restart"),
+			File:       boolToPtr(true),
+			FilePerms:  stringToPtr("0666"),
 		}
 
 		if err := parseVault(v, o); err != nil {

jobspec/parse_test.go

@@ -350,6 +350,8 @@ func TestParse(t *testing.T) {
 									Policies:   []string{"foo", "bar"},
 									Env:        boolToPtr(true),
 									ChangeMode: stringToPtr(vaultChangeModeRestart),
+									File:       boolToPtr(true),
+									FilePerms:  stringToPtr("0666"),
 								},
 								Templates: []*api.Template{
 									{
@@ -402,6 +404,7 @@ func TestParse(t *testing.T) {
 									Env:          boolToPtr(false),
 									ChangeMode:   stringToPtr(vaultChangeModeSignal),
 									ChangeSignal: stringToPtr("SIGUSR1"),
+									File:         boolToPtr(false),
 									FilePerms:    stringToPtr("644"),
 								},
 							},
@@ -762,6 +765,8 @@ func TestParse(t *testing.T) {
 									Policies:   []string{"group"},
 									Env:        boolToPtr(true),
 									ChangeMode: stringToPtr(vaultChangeModeRestart),
+									File:       boolToPtr(true),
+									FilePerms:  stringToPtr("0666"),
 								},
 							},
 							{
@@ -770,6 +775,8 @@ func TestParse(t *testing.T) {
 									Policies:   []string{"task"},
 									Env:        boolToPtr(false),
 									ChangeMode: stringToPtr(vaultChangeModeRestart),
+									File:       boolToPtr(false),
+									FilePerms:  stringToPtr("0666"),
 								},
 							},
 						},
@@ -783,6 +790,8 @@ func TestParse(t *testing.T) {
 									Policies:   []string{"job"},
 									Env:        boolToPtr(true),
 									ChangeMode: stringToPtr(vaultChangeModeRestart),
+									File:       boolToPtr(true),
+									FilePerms:  stringToPtr("0666"),
 								},
 							},
 						},

jobspec/test-fixtures/basic.hcl

@@ -349,6 +349,7 @@ job "binstore-storagelocker" {
         env           = false
         change_mode   = "signal"
         change_signal = "SIGUSR1"
+        file          = false
         file_perms    = "644"
       }
     }

jobspec/test-fixtures/vault_inheritance.hcl

@@ -14,6 +14,7 @@ job "example" {
       vault {
         policies = ["task"]
         env      = false
+        file     = false
       }
     }
   }

jobspec2/parse_job.go

@@ -64,6 +64,12 @@ func normalizeVault(v *api.Vault) {
 	if v.ChangeMode == nil {
 		v.ChangeMode = stringToPtr("restart")
 	}
+	if v.File == nil {
+		v.File = boolToPtr(true)
+	}
+	if v.FilePerms == nil {
+		v.FilePerms = stringToPtr("0666")
+	}
 }
 
 func normalizeNetworkPorts(networks []*api.NetworkResource) {