From 3e61cc42b0ffe65dbd5d8b4b92371ee05545e9fe Mon Sep 17 00:00:00 2001 From: Michael Gmelin Date: Sat, 22 Jan 2022 14:57:07 +0100 Subject: [PATCH 01/11] Add `file` parameter to job's `vault` stanza This complements the `env` parameter, so that the operator can author tasks that don't share their Vault token with the payload. As a result, more powerful tokens can be used in a job definition, allowing it to use template stanzas to issue all kinds of secrets (database secrets, Vault tokens with very specific policies, etc.), without sharing that issuing power with the task itself as long as a driver with `image` isolation is used. This is accomplished by creating a directory called `private` within the task's working directory, which shares many properties of the `secrets` directory (tmpfs where possible, not accessible by `nomad alloc fs` or Nomad's web UI), but isn't mounted into/bound to the container. If the `file` parameter is set to `true` (its default), the Vault token is also written to the NOMAD_SECRETS_DIR, so the default behavior is backwards compatible. Even if the operator never changes the default, they will still benefit from the improved behavior of Nomad never reading the token back in from that - potentially altered - location. See #11900 --- api/tasks.go | 4 +++ api/tasks_test.go | 1 + client/allocdir/alloc_dir.go | 15 ++++++++ client/allocdir/task_dir.go | 14 ++++++++ .../taskrunner/task_runner_test.go | 4 +-- client/allocrunner/taskrunner/vault_hook.go | 14 ++++++-- command/agent/job_endpoint.go | 1 + command/agent/job_endpoint_test.go | 2 ++ .../shared/executor/executor_linux_test.go | 1 + jobspec/parse.go | 1 + jobspec/parse_group.go | 1 + jobspec/parse_job.go | 1 + jobspec/parse_task.go | 1 + jobspec/parse_test.go | 5 +++ jobspec/test-fixtures/basic.hcl | 1 + jobspec/test-fixtures/vault_inheritance.hcl | 1 + jobspec2/parse_job.go | 3 ++ nomad/structs/diff_test.go | 30 ++++++++++++++++ nomad/structs/structs.go | 9 ++--- website/content/docs/concepts/filesystem.mdx | 15 ++++++++ .../content/docs/job-specification/vault.mdx | 36 +++++++++++++++++-- 21 files changed, 148 insertions(+), 12 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 65344290510..4ff4b1cf98e 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -921,6 +921,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"` } func (v *Vault) Canonicalize() { @@ -936,6 +937,9 @@ func (v *Vault) Canonicalize() { if v.ChangeSignal == nil { v.ChangeSignal = pointerOf("SIGHUP") } + if v.File == nil { + v.File = pointerOf(true) + } } // NewTask creates and initializes a new Task. diff --git a/api/tasks_test.go b/api/tasks_test.go index 10b917b43fe..358f186ffc6 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -463,6 +463,7 @@ func TestTask_Canonicalize_Vault(t *testing.T) { Namespace: pointerOf(""), ChangeMode: pointerOf("restart"), ChangeSignal: pointerOf("SIGHUP"), + File: pointerOf(true), }, }, } diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 63752476178..50be1167dcc 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -60,6 +60,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} @@ -306,6 +310,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) @@ -447,6 +458,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() diff --git a/client/allocdir/task_dir.go b/client/allocdir/task_dir.go index 120212bf233..eb6afc6227f 100644 --- a/client/allocdir/task_dir.go +++ b/client/allocdir/task_dir.go @@ -41,6 +41,10 @@ type TaskDir struct { // /secrets/ SecretsDir string + // PrivateDir is the path to private/ directory on the host + // /private/ + PrivateDir string + // skip embedding these paths in chroots. Used for avoiding embedding // client.alloc_dir recursively. skip map[string]struct{} @@ -68,6 +72,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, } @@ -130,6 +135,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 { diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index b25a261f3f9..5ad0ebb4946 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -1644,7 +1644,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 := os.ReadFile(tokenPath) require.NoError(t, err) require.Equal(t, token, string(data)) @@ -1721,7 +1721,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 := os.ReadFile(tokenPath) require.NoError(t, err) require.Equal(t, token, string(data)) diff --git a/client/allocrunner/taskrunner/vault_hook.go b/client/allocrunner/taskrunner/vault_hook.go index 2a2ce87af9c..5ff44cfc019 100644 --- a/client/allocrunner/taskrunner/vault_hook.go +++ b/client/allocrunner/taskrunner/vault_hook.go @@ -83,6 +83,10 @@ 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 + // alloc is the allocation alloc *structs.Allocation @@ -131,7 +135,7 @@ func (h *vaultHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRe // 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 := os.ReadFile(h.tokenPath) if err != nil { if !os.IsNotExist(err) { @@ -143,6 +147,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) @@ -345,9 +350,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 := os.WriteFile(h.tokenPath, []byte(token), 0666); err != nil { + if err := os.WriteFile(h.tokenPath, []byte(token), 0600); err != nil { return fmt.Errorf("failed to write vault token: %v", err) } + if h.vaultBlock.File { + if err := os.WriteFile(h.sharedTokenPath, []byte(token), 0666); err != nil { + return fmt.Errorf("failed to write vault token to secrets dir: %v", err) + } + } return nil } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index eeb114a4041..56cc2cfda60 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1267,6 +1267,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, } } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 056f4a68677..6d9d2be85f3 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2787,6 +2787,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Env: pointer.Of(true), ChangeMode: pointer.Of("c"), ChangeSignal: pointer.Of("sighup"), + File: pointer.Of(true), }, Templates: []*api.Template{ { @@ -3206,6 +3207,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Env: true, ChangeMode: "c", ChangeSignal: "sighup", + File: true, }, Templates: []*structs.Template{ { diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 7172e1136dc..edd4e4b4cd7 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -244,6 +244,7 @@ etc/ lib/ lib64/ local/ +private/ proc/ secrets/ sys/ diff --git a/jobspec/parse.go b/jobspec/parse.go index b840a247f7f..5c7007d0f6b 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -513,6 +513,7 @@ func parseVault(result *api.Vault, list *ast.ObjectList) error { "env", "change_mode", "change_signal", + "file", } if err := checkHCLKeys(listVal, valid); err != nil { return multierror.Prefix(err, "vault ->") diff --git a/jobspec/parse_group.go b/jobspec/parse_group.go index 7a4fdf74650..15857a3610e 100644 --- a/jobspec/parse_group.go +++ b/jobspec/parse_group.go @@ -215,6 +215,7 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { tgVault := &api.Vault{ Env: boolToPtr(true), ChangeMode: stringToPtr("restart"), + File: boolToPtr(true), } if err := parseVault(tgVault, o); err != nil { diff --git a/jobspec/parse_job.go b/jobspec/parse_job.go index b51b256669b..d2ec45d862b 100644 --- a/jobspec/parse_job.go +++ b/jobspec/parse_job.go @@ -196,6 +196,7 @@ func parseJob(result *api.Job, list *ast.ObjectList) error { jobVault := &api.Vault{ Env: boolToPtr(true), ChangeMode: stringToPtr("restart"), + File: boolToPtr(true), } if err := parseVault(jobVault, o); err != nil { diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index a7a091c3227..00941f73cfc 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -316,6 +316,7 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { v := &api.Vault{ Env: boolToPtr(true), ChangeMode: stringToPtr("restart"), + File: boolToPtr(true), } if err := parseVault(v, o); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 6ccc7225497..a7c5dec75c6 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -373,6 +373,7 @@ func TestParse(t *testing.T) { Policies: []string{"foo", "bar"}, Env: boolToPtr(true), ChangeMode: stringToPtr(vaultChangeModeRestart), + File: boolToPtr(true), }, Templates: []*api.Template{ { @@ -437,6 +438,7 @@ func TestParse(t *testing.T) { Env: boolToPtr(false), ChangeMode: stringToPtr(vaultChangeModeSignal), ChangeSignal: stringToPtr("SIGUSR1"), + File: boolToPtr(false), }, }, }, @@ -804,6 +806,7 @@ func TestParse(t *testing.T) { Policies: []string{"group"}, Env: boolToPtr(true), ChangeMode: stringToPtr(vaultChangeModeRestart), + File: boolToPtr(true), }, }, { @@ -812,6 +815,7 @@ func TestParse(t *testing.T) { Policies: []string{"task"}, Env: boolToPtr(false), ChangeMode: stringToPtr(vaultChangeModeRestart), + File: boolToPtr(false), }, }, }, @@ -825,6 +829,7 @@ func TestParse(t *testing.T) { Policies: []string{"job"}, Env: boolToPtr(true), ChangeMode: stringToPtr(vaultChangeModeRestart), + File: boolToPtr(true), }, }, }, diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 17bd0a0fff5..6aaa96da1cc 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -364,6 +364,7 @@ job "binstore-storagelocker" { env = false change_mode = "signal" change_signal = "SIGUSR1" + file = false } } diff --git a/jobspec/test-fixtures/vault_inheritance.hcl b/jobspec/test-fixtures/vault_inheritance.hcl index c844f8d1b7a..37bd1638d40 100644 --- a/jobspec/test-fixtures/vault_inheritance.hcl +++ b/jobspec/test-fixtures/vault_inheritance.hcl @@ -17,6 +17,7 @@ job "example" { vault { policies = ["task"] env = false + file = false } } } diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index e1cd81cb400..9fc258dfec9 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -68,6 +68,9 @@ func normalizeVault(v *api.Vault) { if v.ChangeMode == nil { v.ChangeMode = pointer.Of("restart") } + if v.File == nil { + v.File = pointer.Of(true) + } } func normalizeNetworkPorts(networks []*api.NetworkResource) { diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index a93fa98a521..00645bdccda 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -6889,6 +6889,7 @@ func TestTaskDiff(t *testing.T) { Env: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", + File: true, }, }, Expected: &TaskDiff{ @@ -6916,6 +6917,12 @@ func TestTaskDiff(t *testing.T) { Old: "", New: "true", }, + { + Type: DiffTypeAdded, + Name: "File", + Old: "", + New: "true", + }, }, Objects: []*ObjectDiff{ { @@ -6949,6 +6956,7 @@ func TestTaskDiff(t *testing.T) { Env: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", + File: true, }, }, New: &Task{}, @@ -6977,6 +6985,12 @@ func TestTaskDiff(t *testing.T) { Old: "true", New: "", }, + { + Type: DiffTypeDeleted, + Name: "File", + Old: "true", + New: "", + }, }, Objects: []*ObjectDiff{ { @@ -7011,6 +7025,7 @@ func TestTaskDiff(t *testing.T) { Env: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", + File: true, }, }, New: &Task{ @@ -7020,6 +7035,7 @@ func TestTaskDiff(t *testing.T) { Env: false, ChangeMode: "restart", ChangeSignal: "foo", + File: false, }, }, Expected: &TaskDiff{ @@ -7047,6 +7063,12 @@ func TestTaskDiff(t *testing.T) { Old: "true", New: "false", }, + { + Type: DiffTypeEdited, + Name: "File", + Old: "true", + New: "false", + }, { Type: DiffTypeEdited, Name: "Namespace", @@ -7088,6 +7110,7 @@ func TestTaskDiff(t *testing.T) { Env: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", + File: true, }, }, New: &Task{ @@ -7097,6 +7120,7 @@ func TestTaskDiff(t *testing.T) { Env: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", + File: true, }, }, Expected: &TaskDiff{ @@ -7124,6 +7148,12 @@ func TestTaskDiff(t *testing.T) { Old: "true", New: "true", }, + { + Type: DiffTypeNone, + Name: "File", + Old: "true", + New: "true", + }, { Type: DiffTypeNone, Name: "Namespace", diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 83ef6ce8861..e2308fc03a8 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -9767,13 +9767,10 @@ type Vault struct { // ChangeSignal is the signal sent to the task when a new token is // retrieved. This is only valid when using the signal change mode. ChangeSignal string -} -func DefaultVaultBlock() *Vault { - return &Vault{ - Env: true, - ChangeMode: VaultChangeModeRestart, - } + // File marks whether the Vault Token should be exposed in the file + // vault_token in the task's secrets directory. + File bool } func (v *Vault) Equal(o *Vault) bool { diff --git a/website/content/docs/concepts/filesystem.mdx b/website/content/docs/concepts/filesystem.mdx index c0514750e9a..e5370eb5c4e 100644 --- a/website/content/docs/concepts/filesystem.mdx +++ b/website/content/docs/concepts/filesystem.mdx @@ -29,10 +29,12 @@ allocation directory like the one below. │ └── tmp ├── task1 │ ├── local +│ ├── private │ ├── secrets │ └── tmp └── task2 ├── local + ├── private ├── secrets └── tmp ``` @@ -68,6 +70,11 @@ allocation directory like the one below. `NOMAD_TASK_DIR`. Note this is not the same as the "task working directory". This directory is private to the task. + - **«taskname»/private/**: This directory is used by nomad to store private files + related to the allocation, e.g., vault tokens, that are not always shared with tasks + when using `image` isolation. The contents of files in this directory cannot be read + by the `nomad alloc fs` command. + - **«taskname»/secrets/**: This directory is the location provided to the task as `NOMAD_SECRETS_DIR`. The contents of files in this directory cannot be read by the `nomad alloc fs` command. It can be used to store secret data that @@ -97,6 +104,7 @@ drwxrwxrwx 4.0 KiB 2020-10-27T18:00:32Z tmp/ $ nomad alloc fs c0b2245f task1/ Mode Size Modified Time Name drwxrwxrwx 4.0 KiB 2020-10-27T18:00:33Z local/ +drwxrwxrwx 60 B 2020-10-27T18:00:32Z private/ drwxrwxrwx 60 B 2020-10-27T18:00:32Z secrets/ dtrwxrwxrwx 4.0 KiB 2020-10-27T18:00:32Z tmp/ ``` @@ -150,6 +158,7 @@ minimal filesystem tree: │ └── tmp └── task1 ├── local + ├── private ├── secrets └── tmp ``` @@ -165,6 +174,7 @@ drwxrwxrwx 4.0 KiB 2020-10-27T18:51:54Z task1/ $ nomad alloc fs b0686b27 task1 Mode Size Modified Time Name drwxrwxrwx 4.0 KiB 2020-10-27T18:51:54Z local/ +drwxrwxrwx 60 B 2020-10-27T18:51:54Z private/ drwxrwxrwx 60 B 2020-10-27T18:51:54Z secrets/ dtrwxrwxrwx 4.0 KiB 2020-10-27T18:51:54Z tmp/ @@ -287,6 +297,7 @@ contents], in addition to the `NOMAD_ALLOC_DIR`, `NOMAD_TASK_DIR`, and ├── lib32 ├── lib64 ├── local + ├── private ├── proc ├── run ├── sbin @@ -315,6 +326,7 @@ drwxr-xr-x 4.0 KiB 2020-10-27T19:05:22Z lib/ drwxr-xr-x 4.0 KiB 2020-10-27T19:05:22Z lib32/ drwxr-xr-x 4.0 KiB 2020-10-27T19:05:22Z lib64/ drwxrwxrwx 4.0 KiB 2020-10-27T19:05:22Z local/ +drwxrwxrwx 60 B 2020-10-27T19:05:22Z private/ drwxr-xr-x 4.0 KiB 2020-10-27T19:05:24Z proc/ drwxr-xr-x 4.0 KiB 2020-10-27T19:05:22Z run/ drwxr-xr-x 12 KiB 2020-10-27T19:05:22Z sbin/ @@ -334,6 +346,7 @@ $ nomad alloc exec eebd13a7 /bin/sh $ mount ... /dev/mapper/root on /alloc type ext4 (rw,relatime,errors=remount-ro,data=ordered) +tmpfs on /private type tmpfs (rw,noexec,relatime,size=1024k) tmpfs on /secrets type tmpfs (rw,noexec,relatime,size=1024k) ... ``` @@ -377,6 +390,7 @@ minimal filesystem tree: └── task3 ├── executor.out ├── local + ├── private ├── secrets └── tmp ``` @@ -388,6 +402,7 @@ $ nomad alloc fs 87ec7d12 task3 Mode Size Modified Time Name -rw-r--r-- 140 B 2020-10-27T19:15:33Z executor.out drwxrwxrwx 4.0 KiB 2020-10-27T19:15:33Z local/ +drwxrwxrwx 60 B 2020-10-27T19:15:33Z private/ drwxrwxrwx 60 B 2020-10-27T19:15:33Z secrets/ dtrwxrwxrwx 4.0 KiB 2020-10-27T19:15:33Z tmp/ ``` diff --git a/website/content/docs/job-specification/vault.mdx b/website/content/docs/job-specification/vault.mdx index 20fe3ab27cf..d028b606c36 100644 --- a/website/content/docs/job-specification/vault.mdx +++ b/website/content/docs/job-specification/vault.mdx @@ -45,6 +45,7 @@ to the secret directory at `secrets/vault_token` and by injecting a `VAULT_TOKEN environment variable. If the Nomad cluster is [configured](/nomad/docs/configuration/vault#namespace) to use [Vault Namespaces](/vault/docs/enterprise/namespaces), a `VAULT_NAMESPACE` environment variable will be injected whenever `VAULT_TOKEN` is set. +This behavior can be altered using the `env` and `file` parameters. If Nomad is unable to renew the Vault token (perhaps due to a Vault outage or network error), the client will attempt to retrieve a new Vault token. If successful, the @@ -78,6 +79,9 @@ with Vault as well. the task requires. The Nomad client will retrieve a Vault token that is limited to those policies. +- `file` `(bool: true)` - Specifies if the Vault token should be written to + `secrets/vault_token`. + ## `vault` Examples The following examples only show the `vault` blocks. Remember that the @@ -109,6 +113,35 @@ vault { } ``` +### Private Token and Noop + +This example retrieves a Vault token that is not shared with the task when using +a driver that provides `image` isolation like [Docker][docker]. + +This allows to get a powerful Vault token that interacts with the task's +[`template`][template] stanzas to issue all kinds of secrets (e.g., database +secrets, other vault tokens, etc.), without sharing that issuing power with +the task itself: + +```hcl +vault { + policies = ["tls-policy", "nomad-job-policy"] + change_mode = "noop" + env = false + file = false +} + +template { + data = <<-EOH + {{with secret "auth/token/create/nomad-job" "policies=examplepolicy"}}{{.Auth.ClientToken}}{{ end }} + EOH + destination = "${NOMAD_SECRETS_DIR}/examplepolicy.token" + change_mode = "noop" + perms = "600" +} +``` + + ### Vault Namespace This example shows specifying a particular Vault namespace for a given task. @@ -125,8 +158,7 @@ vault { } ``` +[docker]: /nomad/docs/drivers/docker "Docker Driver" [restart]: /nomad/docs/job-specification/restart "Nomad restart Job Specification" - [template]: /nomad/docs/job-specification/template "Nomad template Job Specification" - [vault]: https://www.vaultproject.io/ "Vault by HashiCorp" From 8b8c226e2686db253ce0c286853d91633a92174a Mon Sep 17 00:00:00 2001 From: Michael Gmelin Date: Thu, 21 Jul 2022 15:38:54 +0200 Subject: [PATCH 02/11] Improve example of using private token and noop Show multiple templates using different change modes and explain what they mean. --- .../content/docs/job-specification/vault.mdx | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/website/content/docs/job-specification/vault.mdx b/website/content/docs/job-specification/vault.mdx index d028b606c36..47b086a8a22 100644 --- a/website/content/docs/job-specification/vault.mdx +++ b/website/content/docs/job-specification/vault.mdx @@ -139,8 +139,29 @@ template { change_mode = "noop" perms = "600" } + +template { + data = <<-EOH + {{ with secret "pki_int/issue/nomad-task" + "common_name=example.service.consul" "ttl=72h" + "alt_names=localhost" "ip_sans=127.0.0.1"}} + {{ .Data.certificate }} + {{ .Data.private_key }} + {{ end }} + EOH + destination = "${NOMAD_SECRETS_DIR}/client.crt" + change_mode = "restart" + perms = "600" +} ``` +The example above uses `change_mode = "noop"` in the `template` stanza for +`examplepolicy.token`, which means that the payload is responsible for +detecting and handling changes to that file. In contrast, the `template` stanza +for `client.crt` is configured so that Nomad will restart the task whenever +the certificate is reissued, as indicated by `change_mode = "restart"` +(which is the default value for `change_mode`). + ### Vault Namespace From 537407d40e842d9a4c225e46420e9dc09295621e Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 22 Jun 2023 23:24:26 -0400 Subject: [PATCH 03/11] vault: bring Env and File attributes next to each other --- api/tasks.go | 8 ++++---- api/tasks_test.go | 2 +- command/agent/job_endpoint.go | 2 +- command/agent/job_endpoint_test.go | 4 ++-- jobspec/parse.go | 2 +- jobspec/parse_group.go | 2 +- jobspec/parse_job.go | 2 +- jobspec/parse_task.go | 2 +- jobspec/parse_test.go | 10 +++++----- jobspec/test-fixtures/basic.hcl | 2 +- jobspec2/parse_job.go | 6 +++--- nomad/structs/diff_test.go | 12 ++++++------ nomad/structs/structs.go | 8 ++++---- website/content/docs/job-specification/vault.mdx | 6 +++--- 14 files changed, 34 insertions(+), 34 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 4ff4b1cf98e..b667556693b 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -919,15 +919,18 @@ type Vault struct { Policies []string `hcl:"policies,optional"` Namespace *string `mapstructure:"namespace" hcl:"namespace,optional"` Env *bool `hcl:"env,optional"` + File *bool `mapstructure:"file" hcl:"file,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"` } func (v *Vault) Canonicalize() { if v.Env == nil { v.Env = pointerOf(true) } + if v.File == nil { + v.File = pointerOf(true) + } if v.Namespace == nil { v.Namespace = pointerOf("") } @@ -937,9 +940,6 @@ func (v *Vault) Canonicalize() { if v.ChangeSignal == nil { v.ChangeSignal = pointerOf("SIGHUP") } - if v.File == nil { - v.File = pointerOf(true) - } } // NewTask creates and initializes a new Task. diff --git a/api/tasks_test.go b/api/tasks_test.go index 358f186ffc6..1a8159bb026 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -460,10 +460,10 @@ func TestTask_Canonicalize_Vault(t *testing.T) { input: &Vault{}, expected: &Vault{ Env: pointerOf(true), + File: pointerOf(true), Namespace: pointerOf(""), ChangeMode: pointerOf("restart"), ChangeSignal: pointerOf("SIGHUP"), - File: pointerOf(true), }, }, } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 56cc2cfda60..072accc895d 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1265,9 +1265,9 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, Policies: apiTask.Vault.Policies, Namespace: *apiTask.Vault.Namespace, Env: *apiTask.Vault.Env, + File: *apiTask.Vault.File, ChangeMode: *apiTask.Vault.ChangeMode, ChangeSignal: *apiTask.Vault.ChangeSignal, - File: *apiTask.Vault.File, } } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 6d9d2be85f3..ad700460102 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2785,9 +2785,9 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Namespace: pointer.Of("ns1"), Policies: []string{"a", "b", "c"}, Env: pointer.Of(true), + File: pointer.Of(true), ChangeMode: pointer.Of("c"), ChangeSignal: pointer.Of("sighup"), - File: pointer.Of(true), }, Templates: []*api.Template{ { @@ -3205,9 +3205,9 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Namespace: "ns1", Policies: []string{"a", "b", "c"}, Env: true, + File: true, ChangeMode: "c", ChangeSignal: "sighup", - File: true, }, Templates: []*structs.Template{ { diff --git a/jobspec/parse.go b/jobspec/parse.go index 5c7007d0f6b..df2e3bb003d 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -511,9 +511,9 @@ func parseVault(result *api.Vault, list *ast.ObjectList) error { "namespace", "policies", "env", + "file", "change_mode", "change_signal", - "file", } if err := checkHCLKeys(listVal, valid); err != nil { return multierror.Prefix(err, "vault ->") diff --git a/jobspec/parse_group.go b/jobspec/parse_group.go index 15857a3610e..37a9bb7a7fa 100644 --- a/jobspec/parse_group.go +++ b/jobspec/parse_group.go @@ -214,8 +214,8 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { if o := listVal.Filter("vault"); len(o.Items) > 0 { tgVault := &api.Vault{ Env: boolToPtr(true), - ChangeMode: stringToPtr("restart"), File: boolToPtr(true), + ChangeMode: stringToPtr("restart"), } if err := parseVault(tgVault, o); err != nil { diff --git a/jobspec/parse_job.go b/jobspec/parse_job.go index d2ec45d862b..b52b0e48497 100644 --- a/jobspec/parse_job.go +++ b/jobspec/parse_job.go @@ -195,8 +195,8 @@ func parseJob(result *api.Job, list *ast.ObjectList) error { if o := listVal.Filter("vault"); len(o.Items) > 0 { jobVault := &api.Vault{ Env: boolToPtr(true), - ChangeMode: stringToPtr("restart"), File: boolToPtr(true), + ChangeMode: stringToPtr("restart"), } if err := parseVault(jobVault, o); err != nil { diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index 00941f73cfc..e6b23ba9f7b 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -315,8 +315,8 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { if o := listVal.Filter("vault"); len(o.Items) > 0 { v := &api.Vault{ Env: boolToPtr(true), - ChangeMode: stringToPtr("restart"), File: boolToPtr(true), + ChangeMode: stringToPtr("restart"), } if err := parseVault(v, o); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index a7c5dec75c6..b10556a6cb7 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -372,8 +372,8 @@ func TestParse(t *testing.T) { Namespace: stringToPtr("ns1"), Policies: []string{"foo", "bar"}, Env: boolToPtr(true), - ChangeMode: stringToPtr(vaultChangeModeRestart), File: boolToPtr(true), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, Templates: []*api.Template{ { @@ -436,9 +436,9 @@ func TestParse(t *testing.T) { Vault: &api.Vault{ Policies: []string{"foo", "bar"}, Env: boolToPtr(false), + File: boolToPtr(false), ChangeMode: stringToPtr(vaultChangeModeSignal), ChangeSignal: stringToPtr("SIGUSR1"), - File: boolToPtr(false), }, }, }, @@ -805,8 +805,8 @@ func TestParse(t *testing.T) { Vault: &api.Vault{ Policies: []string{"group"}, Env: boolToPtr(true), - ChangeMode: stringToPtr(vaultChangeModeRestart), File: boolToPtr(true), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, }, { @@ -814,8 +814,8 @@ func TestParse(t *testing.T) { Vault: &api.Vault{ Policies: []string{"task"}, Env: boolToPtr(false), - ChangeMode: stringToPtr(vaultChangeModeRestart), File: boolToPtr(false), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, }, }, @@ -828,8 +828,8 @@ func TestParse(t *testing.T) { Vault: &api.Vault{ Policies: []string{"job"}, Env: boolToPtr(true), - ChangeMode: stringToPtr(vaultChangeModeRestart), File: boolToPtr(true), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, }, }, diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 6aaa96da1cc..388c26487ba 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -362,9 +362,9 @@ job "binstore-storagelocker" { vault { policies = ["foo", "bar"] env = false + file = false change_mode = "signal" change_signal = "SIGUSR1" - file = false } } diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index 9fc258dfec9..3fa065f3ee9 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -65,12 +65,12 @@ func normalizeVault(v *api.Vault) { if v.Env == nil { v.Env = pointer.Of(true) } - if v.ChangeMode == nil { - v.ChangeMode = pointer.Of("restart") - } if v.File == nil { v.File = pointer.Of(true) } + if v.ChangeMode == nil { + v.ChangeMode = pointer.Of("restart") + } } func normalizeNetworkPorts(networks []*api.NetworkResource) { diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 00645bdccda..17bf454fb13 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -6887,9 +6887,9 @@ func TestTaskDiff(t *testing.T) { Vault: &Vault{ Policies: []string{"foo", "bar"}, Env: true, + File: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", - File: true, }, }, Expected: &TaskDiff{ @@ -6954,9 +6954,9 @@ func TestTaskDiff(t *testing.T) { Vault: &Vault{ Policies: []string{"foo", "bar"}, Env: true, + File: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", - File: true, }, }, New: &Task{}, @@ -7023,9 +7023,9 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns1", Policies: []string{"foo", "bar"}, Env: true, + File: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", - File: true, }, }, New: &Task{ @@ -7033,9 +7033,9 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns2", Policies: []string{"bar", "baz"}, Env: false, + File: false, ChangeMode: "restart", ChangeSignal: "foo", - File: false, }, }, Expected: &TaskDiff{ @@ -7108,9 +7108,9 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns1", Policies: []string{"foo", "bar"}, Env: true, + File: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", - File: true, }, }, New: &Task{ @@ -7118,9 +7118,9 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns1", Policies: []string{"bar", "baz"}, Env: true, + File: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", - File: true, }, }, Expected: &TaskDiff{ diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index e2308fc03a8..afcb8fcb602 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -9760,6 +9760,10 @@ type Vault struct { // variable Env bool + // File marks whether the Vault Token should be exposed in the file + // vault_token in the task's secrets directory. + File bool + // ChangeMode is used to configure the task's behavior when the Vault // token changes because the original token could not be renewed in time. ChangeMode string @@ -9767,10 +9771,6 @@ type Vault struct { // ChangeSignal is the signal sent to the task when a new token is // retrieved. This is only valid when using the signal change mode. ChangeSignal string - - // File marks whether the Vault Token should be exposed in the file - // vault_token in the task's secrets directory. - File bool } func (v *Vault) Equal(o *Vault) bool { diff --git a/website/content/docs/job-specification/vault.mdx b/website/content/docs/job-specification/vault.mdx index 47b086a8a22..924e210837c 100644 --- a/website/content/docs/job-specification/vault.mdx +++ b/website/content/docs/job-specification/vault.mdx @@ -71,6 +71,9 @@ with Vault as well. - `env` `(bool: true)` - Specifies if the `VAULT_TOKEN` and `VAULT_NAMESPACE` environment variables should be set when starting the task. +- `file` `(bool: true)` - Specifies if the Vault token should be written to + `secrets/vault_token`. + - `namespace` `(string: "")` - Specifies the Vault Namespace to use for the task. The Nomad client will retrieve a Vault token that is scoped to this particular namespace. @@ -79,9 +82,6 @@ with Vault as well. the task requires. The Nomad client will retrieve a Vault token that is limited to those policies. -- `file` `(bool: true)` - Specifies if the Vault token should be written to - `secrets/vault_token`. - ## `vault` Examples The following examples only show the `vault` blocks. Remember that the From 1150f4bd3292db23ec925a95afb3b5065fea5353 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 22 Jun 2023 23:31:20 -0400 Subject: [PATCH 04/11] fix typo --- client/allocdir/alloc_dir.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 50be1167dcc..8ef9c6c658e 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -60,7 +60,7 @@ var ( // directory TaskSecrets = "secrets" - // TaskPrivate is the name of the pruvate directory inside each task + // TaskPrivate is the name of the private directory inside each task // directory TaskPrivate = "private" From aea7ee180944bdf433f559242f9bec3e3dd625fe Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Thu, 22 Jun 2023 23:33:33 -0400 Subject: [PATCH 05/11] docs: minor touch-ups on Vault jobspec --- website/content/docs/concepts/filesystem.mdx | 4 +-- .../content/docs/job-specification/vault.mdx | 32 ++++++++++--------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/website/content/docs/concepts/filesystem.mdx b/website/content/docs/concepts/filesystem.mdx index e5370eb5c4e..0289c6d5dcf 100644 --- a/website/content/docs/concepts/filesystem.mdx +++ b/website/content/docs/concepts/filesystem.mdx @@ -70,8 +70,8 @@ allocation directory like the one below. `NOMAD_TASK_DIR`. Note this is not the same as the "task working directory". This directory is private to the task. - - **«taskname»/private/**: This directory is used by nomad to store private files - related to the allocation, e.g., vault tokens, that are not always shared with tasks + - **«taskname»/private/**: This directory is used by Nomad to store private files + related to the allocation, such as Vault tokens, that are not always shared with tasks when using `image` isolation. The contents of files in this directory cannot be read by the `nomad alloc fs` command. diff --git a/website/content/docs/job-specification/vault.mdx b/website/content/docs/job-specification/vault.mdx index 924e210837c..8a9f76669a9 100644 --- a/website/content/docs/job-specification/vault.mdx +++ b/website/content/docs/job-specification/vault.mdx @@ -113,7 +113,7 @@ vault { } ``` -### Private Token and Noop +### Private Token and Change Modes This example retrieves a Vault token that is not shared with the task when using a driver that provides `image` isolation like [Docker][docker]. @@ -125,33 +125,35 @@ the task itself: ```hcl vault { - policies = ["tls-policy", "nomad-job-policy"] + policies = ["tls-policy", "nomad-job-policy"] change_mode = "noop" - env = false - file = false + env = false + file = false } template { data = <<-EOH - {{with secret "auth/token/create/nomad-job" "policies=examplepolicy"}}{{.Auth.ClientToken}}{{ end }} - EOH +{{with secret "auth/token/create/nomad-job" "policies=examplepolicy"}}{{.Auth.ClientToken}}{{ end }} +EOH + destination = "${NOMAD_SECRETS_DIR}/examplepolicy.token" change_mode = "noop" - perms = "600" + perms = "600" } template { data = <<-EOH - {{ with secret "pki_int/issue/nomad-task" - "common_name=example.service.consul" "ttl=72h" - "alt_names=localhost" "ip_sans=127.0.0.1"}} - {{ .Data.certificate }} - {{ .Data.private_key }} - {{ end }} - EOH +{{ with secret "pki_int/issue/nomad-task" + "common_name=example.service.consul" "ttl=72h" + "alt_names=localhost" "ip_sans=127.0.0.1"}} +{{ .Data.certificate }} +{{ .Data.private_key }} +{{ end }} +EOH + destination = "${NOMAD_SECRETS_DIR}/client.crt" change_mode = "restart" - perms = "600" + perms = "600" } ``` From 026fbedd8455a8be809e2319f5a7c87f13baa449 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 23 Jun 2023 03:02:54 -0400 Subject: [PATCH 06/11] vault: change task config `file` to `disable_file` In order to maintan backwards compatibility we are not able to create a new field where the expected default is different from its zero value because during an upgrade the Raft log entries will not have this field and therefore existing tasks will be set to non-backwards compatible zero value. --- api/tasks.go | 6 ++-- api/tasks_test.go | 2 +- client/allocrunner/taskrunner/vault_hook.go | 2 +- command/agent/job_endpoint.go | 2 +- command/agent/job_endpoint_test.go | 4 +-- jobspec/parse.go | 2 +- jobspec/parse_group.go | 6 ++-- jobspec/parse_job.go | 6 ++-- jobspec/parse_task.go | 6 ++-- jobspec/parse_test.go | 36 +++++++++---------- jobspec/test-fixtures/basic.hcl | 2 +- jobspec/test-fixtures/vault_inheritance.hcl | 6 ++-- jobspec2/parse_job.go | 4 +-- nomad/structs/diff_test.go | 28 +++++++-------- nomad/structs/structs.go | 4 +-- .../content/docs/job-specification/vault.mdx | 4 +-- 16 files changed, 60 insertions(+), 60 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index b667556693b..188fa8649bb 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -919,7 +919,7 @@ type Vault struct { Policies []string `hcl:"policies,optional"` Namespace *string `mapstructure:"namespace" hcl:"namespace,optional"` Env *bool `hcl:"env,optional"` - File *bool `mapstructure:"file" hcl:"file,optional"` + DisableFile *bool `mapstructure:"disable_file" hcl:"disable_file,optional"` ChangeMode *string `mapstructure:"change_mode" hcl:"change_mode,optional"` ChangeSignal *string `mapstructure:"change_signal" hcl:"change_signal,optional"` } @@ -928,8 +928,8 @@ func (v *Vault) Canonicalize() { if v.Env == nil { v.Env = pointerOf(true) } - if v.File == nil { - v.File = pointerOf(true) + if v.DisableFile == nil { + v.DisableFile = pointerOf(false) } if v.Namespace == nil { v.Namespace = pointerOf("") diff --git a/api/tasks_test.go b/api/tasks_test.go index 1a8159bb026..231993906fd 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -460,7 +460,7 @@ func TestTask_Canonicalize_Vault(t *testing.T) { input: &Vault{}, expected: &Vault{ Env: pointerOf(true), - File: pointerOf(true), + DisableFile: pointerOf(false), Namespace: pointerOf(""), ChangeMode: pointerOf("restart"), ChangeSignal: pointerOf("SIGHUP"), diff --git a/client/allocrunner/taskrunner/vault_hook.go b/client/allocrunner/taskrunner/vault_hook.go index 5ff44cfc019..7dc10bed40b 100644 --- a/client/allocrunner/taskrunner/vault_hook.go +++ b/client/allocrunner/taskrunner/vault_hook.go @@ -353,7 +353,7 @@ func (h *vaultHook) writeToken(token string) error { if err := os.WriteFile(h.tokenPath, []byte(token), 0600); err != nil { return fmt.Errorf("failed to write vault token: %v", err) } - if h.vaultBlock.File { + if !h.vaultBlock.DisableFile { if err := os.WriteFile(h.sharedTokenPath, []byte(token), 0666); err != nil { return fmt.Errorf("failed to write vault token to secrets dir: %v", err) } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 072accc895d..b12599141b6 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1265,7 +1265,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, Policies: apiTask.Vault.Policies, Namespace: *apiTask.Vault.Namespace, Env: *apiTask.Vault.Env, - File: *apiTask.Vault.File, + DisableFile: *apiTask.Vault.DisableFile, ChangeMode: *apiTask.Vault.ChangeMode, ChangeSignal: *apiTask.Vault.ChangeSignal, } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index ad700460102..9b6d9b0a831 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2785,7 +2785,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Namespace: pointer.Of("ns1"), Policies: []string{"a", "b", "c"}, Env: pointer.Of(true), - File: pointer.Of(true), + DisableFile: pointer.Of(false), ChangeMode: pointer.Of("c"), ChangeSignal: pointer.Of("sighup"), }, @@ -3205,7 +3205,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Namespace: "ns1", Policies: []string{"a", "b", "c"}, Env: true, - File: true, + DisableFile: false, ChangeMode: "c", ChangeSignal: "sighup", }, diff --git a/jobspec/parse.go b/jobspec/parse.go index df2e3bb003d..7286b6e33a8 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -511,7 +511,7 @@ func parseVault(result *api.Vault, list *ast.ObjectList) error { "namespace", "policies", "env", - "file", + "disable_file", "change_mode", "change_signal", } diff --git a/jobspec/parse_group.go b/jobspec/parse_group.go index 37a9bb7a7fa..213e286a53f 100644 --- a/jobspec/parse_group.go +++ b/jobspec/parse_group.go @@ -213,9 +213,9 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { // If we have a vault block, then parse that if o := listVal.Filter("vault"); len(o.Items) > 0 { tgVault := &api.Vault{ - Env: boolToPtr(true), - File: boolToPtr(true), - ChangeMode: stringToPtr("restart"), + Env: boolToPtr(true), + DisableFile: boolToPtr(false), + ChangeMode: stringToPtr("restart"), } if err := parseVault(tgVault, o); err != nil { diff --git a/jobspec/parse_job.go b/jobspec/parse_job.go index b52b0e48497..46c3dd7573f 100644 --- a/jobspec/parse_job.go +++ b/jobspec/parse_job.go @@ -194,9 +194,9 @@ func parseJob(result *api.Job, list *ast.ObjectList) error { // If we have a vault block, then parse that if o := listVal.Filter("vault"); len(o.Items) > 0 { jobVault := &api.Vault{ - Env: boolToPtr(true), - File: boolToPtr(true), - ChangeMode: stringToPtr("restart"), + Env: boolToPtr(true), + DisableFile: boolToPtr(false), + ChangeMode: stringToPtr("restart"), } if err := parseVault(jobVault, o); err != nil { diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index e6b23ba9f7b..5d596738d94 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -314,9 +314,9 @@ func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { // If we have a vault block, then parse that if o := listVal.Filter("vault"); len(o.Items) > 0 { v := &api.Vault{ - Env: boolToPtr(true), - File: boolToPtr(true), - ChangeMode: stringToPtr("restart"), + Env: boolToPtr(true), + DisableFile: boolToPtr(false), + ChangeMode: stringToPtr("restart"), } if err := parseVault(v, o); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index b10556a6cb7..3fe453c21a9 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -369,11 +369,11 @@ func TestParse(t *testing.T) { }, }, Vault: &api.Vault{ - Namespace: stringToPtr("ns1"), - Policies: []string{"foo", "bar"}, - Env: boolToPtr(true), - File: boolToPtr(true), - ChangeMode: stringToPtr(vaultChangeModeRestart), + Namespace: stringToPtr("ns1"), + Policies: []string{"foo", "bar"}, + Env: boolToPtr(true), + DisableFile: boolToPtr(false), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, Templates: []*api.Template{ { @@ -436,7 +436,7 @@ func TestParse(t *testing.T) { Vault: &api.Vault{ Policies: []string{"foo", "bar"}, Env: boolToPtr(false), - File: boolToPtr(false), + DisableFile: boolToPtr(false), ChangeMode: stringToPtr(vaultChangeModeSignal), ChangeSignal: stringToPtr("SIGUSR1"), }, @@ -803,19 +803,19 @@ func TestParse(t *testing.T) { { Name: "redis", Vault: &api.Vault{ - Policies: []string{"group"}, - Env: boolToPtr(true), - File: boolToPtr(true), - ChangeMode: stringToPtr(vaultChangeModeRestart), + Policies: []string{"group"}, + Env: boolToPtr(true), + DisableFile: boolToPtr(false), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, }, { Name: "redis2", Vault: &api.Vault{ - Policies: []string{"task"}, - Env: boolToPtr(false), - File: boolToPtr(false), - ChangeMode: stringToPtr(vaultChangeModeRestart), + Policies: []string{"task"}, + Env: boolToPtr(false), + DisableFile: boolToPtr(true), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, }, }, @@ -826,10 +826,10 @@ func TestParse(t *testing.T) { { Name: "redis", Vault: &api.Vault{ - Policies: []string{"job"}, - Env: boolToPtr(true), - File: boolToPtr(true), - ChangeMode: stringToPtr(vaultChangeModeRestart), + Policies: []string{"job"}, + Env: boolToPtr(true), + DisableFile: boolToPtr(false), + ChangeMode: stringToPtr(vaultChangeModeRestart), }, }, }, diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 388c26487ba..66695c1c326 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -362,7 +362,7 @@ job "binstore-storagelocker" { vault { policies = ["foo", "bar"] env = false - file = false + disable_file = false change_mode = "signal" change_signal = "SIGUSR1" } diff --git a/jobspec/test-fixtures/vault_inheritance.hcl b/jobspec/test-fixtures/vault_inheritance.hcl index 37bd1638d40..4ee5921874f 100644 --- a/jobspec/test-fixtures/vault_inheritance.hcl +++ b/jobspec/test-fixtures/vault_inheritance.hcl @@ -15,9 +15,9 @@ job "example" { task "redis2" { vault { - policies = ["task"] - env = false - file = false + policies = ["task"] + env = false + disable_file = true } } } diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index 3fa065f3ee9..001d23b0c2b 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -65,8 +65,8 @@ func normalizeVault(v *api.Vault) { if v.Env == nil { v.Env = pointer.Of(true) } - if v.File == nil { - v.File = pointer.Of(true) + if v.DisableFile == nil { + v.DisableFile = pointer.Of(false) } if v.ChangeMode == nil { v.ChangeMode = pointer.Of("restart") diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 17bf454fb13..7d03c1fd48a 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -6887,7 +6887,7 @@ func TestTaskDiff(t *testing.T) { Vault: &Vault{ Policies: []string{"foo", "bar"}, Env: true, - File: true, + DisableFile: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", }, @@ -6913,13 +6913,13 @@ func TestTaskDiff(t *testing.T) { }, { Type: DiffTypeAdded, - Name: "Env", + Name: "DisableFile", Old: "", New: "true", }, { Type: DiffTypeAdded, - Name: "File", + Name: "Env", Old: "", New: "true", }, @@ -6954,7 +6954,7 @@ func TestTaskDiff(t *testing.T) { Vault: &Vault{ Policies: []string{"foo", "bar"}, Env: true, - File: true, + DisableFile: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", }, @@ -6981,13 +6981,13 @@ func TestTaskDiff(t *testing.T) { }, { Type: DiffTypeDeleted, - Name: "Env", + Name: "DisableFile", Old: "true", New: "", }, { Type: DiffTypeDeleted, - Name: "File", + Name: "Env", Old: "true", New: "", }, @@ -7023,7 +7023,7 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns1", Policies: []string{"foo", "bar"}, Env: true, - File: true, + DisableFile: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", }, @@ -7033,7 +7033,7 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns2", Policies: []string{"bar", "baz"}, Env: false, - File: false, + DisableFile: false, ChangeMode: "restart", ChangeSignal: "foo", }, @@ -7059,13 +7059,13 @@ func TestTaskDiff(t *testing.T) { }, { Type: DiffTypeEdited, - Name: "Env", + Name: "DisableFile", Old: "true", New: "false", }, { Type: DiffTypeEdited, - Name: "File", + Name: "Env", Old: "true", New: "false", }, @@ -7108,7 +7108,7 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns1", Policies: []string{"foo", "bar"}, Env: true, - File: true, + DisableFile: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", }, @@ -7118,7 +7118,7 @@ func TestTaskDiff(t *testing.T) { Namespace: "ns1", Policies: []string{"bar", "baz"}, Env: true, - File: true, + DisableFile: true, ChangeMode: "signal", ChangeSignal: "SIGUSR1", }, @@ -7144,13 +7144,13 @@ func TestTaskDiff(t *testing.T) { }, { Type: DiffTypeNone, - Name: "Env", + Name: "DisableFile", Old: "true", New: "true", }, { Type: DiffTypeNone, - Name: "File", + Name: "Env", Old: "true", New: "true", }, diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index afcb8fcb602..1a645d58b4a 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -9760,9 +9760,9 @@ type Vault struct { // variable Env bool - // File marks whether the Vault Token should be exposed in the file + // DisableFile marks whether the Vault Token should be exposed in the file // vault_token in the task's secrets directory. - File bool + DisableFile bool // ChangeMode is used to configure the task's behavior when the Vault // token changes because the original token could not be renewed in time. diff --git a/website/content/docs/job-specification/vault.mdx b/website/content/docs/job-specification/vault.mdx index 8a9f76669a9..cfeb6dcf5d6 100644 --- a/website/content/docs/job-specification/vault.mdx +++ b/website/content/docs/job-specification/vault.mdx @@ -71,8 +71,8 @@ with Vault as well. - `env` `(bool: true)` - Specifies if the `VAULT_TOKEN` and `VAULT_NAMESPACE` environment variables should be set when starting the task. -- `file` `(bool: true)` - Specifies if the Vault token should be written to - `secrets/vault_token`. +- `disable_file` `(bool: false)` - Specifies if the Vault token should be + written to `secrets/vault_token`. - `namespace` `(string: "")` - Specifies the Vault Namespace to use for the task. The Nomad client will retrieve a Vault token that is scoped to From 2f881b7664fcc0e4b8cbf9e20be6cde6b430aaef Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 23 Jun 2023 03:19:17 -0400 Subject: [PATCH 07/11] vault: handle upgrade path for private directory Tasks that were created before a Nomad upgrade will not have the private directory in their filesystem, so the task restore process would fail. This commit adds upgrade path logic to handle cases where the private directory doesn't exist. --- .../taskrunner/task_runner_test.go | 118 ++++++++++++++++++ client/allocrunner/taskrunner/vault_hook.go | 55 +++++--- 2 files changed, 155 insertions(+), 18 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 5ad0ebb4946..d4b22c5814e 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -1649,6 +1649,11 @@ func TestTaskRunner_BlockForVaultToken(t *testing.T) { require.NoError(t, err) require.Equal(t, token, string(data)) + tokenPath = filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) + data, err = os.ReadFile(tokenPath) + require.NoError(t, err) + require.Equal(t, token, string(data)) + // Kill task runner to trigger stop hooks tr.Kill(context.Background(), structs.NewTaskEvent("kill")) select { @@ -1672,6 +1677,119 @@ func TestTaskRunner_BlockForVaultToken(t *testing.T) { }) } +func TestTaskRunner_DisableFileForVaultToken(t *testing.T) { + ci.Parallel(t) + + // Create test allocation with a Vault block disabling the token file in + // the secrets dir. + alloc := mock.BatchAlloc() + task := alloc.Job.TaskGroups[0].Tasks[0] + task.Config = map[string]any{ + "run_for": "0s", + } + task.Vault = &structs.Vault{ + Policies: []string{"default"}, + DisableFile: true, + } + + conf, cleanup := testTaskRunnerConfig(t, alloc, task.Name) + defer cleanup() + + // Setup a test Vault client + token := "1234" + handler := func(*structs.Allocation, []string) (map[string]string, error) { + return map[string]string{task.Name: token}, nil + } + vaultClient := conf.Vault.(*vaultclient.MockVaultClient) + vaultClient.DeriveTokenFn = handler + + // Start task runner and wait for it to complete. + tr, err := NewTaskRunner(conf) + must.NoError(t, err) + defer tr.Kill(context.Background(), structs.NewTaskEvent("cleanup")) + go tr.Run() + + testWaitForTaskToDie(t, tr) + + // Verify task exited successfully. + finalState := tr.TaskState() + must.Eq(t, structs.TaskStateDead, finalState.State) + must.False(t, finalState.Failed) + + // Verfiry token is in the private dir. + tokenPath := filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile) + data, err := os.ReadFile(tokenPath) + must.NoError(t, err) + must.Eq(t, token, string(data)) + + // Verify token is not in secrets dir. + tokenPath = filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) + _, err = os.Stat(tokenPath) + must.ErrorIs(t, err, os.ErrNotExist) +} + +func TestTaskRunner_DisableFileForVaultToken_UpgradePath(t *testing.T) { + ci.Parallel(t) + + // Create test allocation with a Vault block. + alloc := mock.BatchAlloc() + task := alloc.Job.TaskGroups[0].Tasks[0] + task.Config = map[string]any{ + "run_for": "0s", + } + task.Vault = &structs.Vault{ + Policies: []string{"default"}, + } + + conf, cleanup := testTaskRunnerConfig(t, alloc, task.Name) + defer cleanup() + + // Remove private dir and write the Vault token to the secrets dir to + // simulate an old task. + err := conf.TaskDir.Build(false, nil) + must.NoError(t, err) + err = os.Remove(conf.TaskDir.PrivateDir) + must.NoError(t, err) + + token := "1234" + tokenPath := filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) + err = os.WriteFile(tokenPath, []byte(token), 0666) + must.NoError(t, err) + + // Setup a test Vault client. + handler := func(*structs.Allocation, []string) (map[string]string, error) { + return map[string]string{task.Name: token}, nil + } + vaultClient := conf.Vault.(*vaultclient.MockVaultClient) + vaultClient.DeriveTokenFn = handler + + // Start task runner and wait for task to finish. + tr, err := NewTaskRunner(conf) + must.NoError(t, err) + defer tr.Kill(context.Background(), structs.NewTaskEvent("cleanup")) + go tr.Run() + time.Sleep(500 * time.Millisecond) + + testWaitForTaskToDie(t, tr) + + // Verify task exited successfully. + finalState := tr.TaskState() + must.Eq(t, structs.TaskStateDead, finalState.State) + must.False(t, finalState.Failed) + + // Verfiry token is in secrets dir. + tokenPath = filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) + data, err := os.ReadFile(tokenPath) + must.NoError(t, err) + must.Eq(t, token, string(data)) + + // Varify token is not in private dir since the allocation doesn't have + // this path. + tokenPath = filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile) + _, err = os.Stat(tokenPath) + must.ErrorIs(t, err, os.ErrNotExist) +} + // TestTaskRunner_DeriveToken_Retry asserts that if a recoverable error is // returned when deriving a vault token a task will continue to block while // it's retried. diff --git a/client/allocrunner/taskrunner/vault_hook.go b/client/allocrunner/taskrunner/vault_hook.go index 7dc10bed40b..b979e055c0c 100644 --- a/client/allocrunner/taskrunner/vault_hook.go +++ b/client/allocrunner/taskrunner/vault_hook.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "os" + "path" "path/filepath" "sync" "time" @@ -80,12 +81,13 @@ type vaultHook struct { ctx context.Context cancel context.CancelFunc - // tokenPath is the path in which to read and write the token - tokenPath string + // privateDirTokenPath is the path inside the task's private directory where + // the Vault token is read and written. + privateDirTokenPath string - // sharedTokenPath is the path in which to only write, but never - // read the token from - sharedTokenPath string + // secretsDirTokenPath is the path inside the task's secret directory where the + // Vault token is written unless disabled by the task. + secretsDirTokenPath string // alloc is the allocation alloc *structs.Allocation @@ -135,19 +137,25 @@ func (h *vaultHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRe // Try to recover a token if it was previously written in the secrets // directory recoveredToken := "" - h.tokenPath = filepath.Join(req.TaskDir.PrivateDir, vaultTokenFile) - data, err := os.ReadFile(h.tokenPath) - if err != nil { - if !os.IsNotExist(err) { - return fmt.Errorf("failed to recover vault token: %v", err) - } + h.privateDirTokenPath = filepath.Join(req.TaskDir.PrivateDir, vaultTokenFile) + h.secretsDirTokenPath = filepath.Join(req.TaskDir.SecretsDir, vaultTokenFile) - // Token file doesn't exist - } else { - // Store the recovered token - recoveredToken = string(data) + // Handle upgrade path by searching for the previous token in all possible + // paths where the token may be. + for _, path := range []string{h.privateDirTokenPath, h.secretsDirTokenPath} { + data, err := os.ReadFile(path) + if err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("failed to recover vault token from %s: %v", path, err) + } + + // Token file doesn't exist in this path. + } else { + // Store the recovered token + recoveredToken = string(data) + break + } } - h.sharedTokenPath = filepath.Join(req.TaskDir.SecretsDir, vaultTokenFile) // Launch the token manager go h.run(recoveredToken) @@ -350,11 +358,22 @@ func (h *vaultHook) deriveVaultToken() (token string, exit bool) { // writeToken writes the given token to disk func (h *vaultHook) writeToken(token string) error { - if err := os.WriteFile(h.tokenPath, []byte(token), 0600); err != nil { + // Handle upgrade path by first checking if the tasks private directory + // exists. If it doesn't, this allocation probably existed before the + // private directory was introduced, so keep using the secret directory to + // prevent unnecessary errors during task recovery. + if _, err := os.Stat(path.Dir(h.privateDirTokenPath)); os.IsNotExist(err) { + if err := os.WriteFile(h.secretsDirTokenPath, []byte(token), 0666); err != nil { + return fmt.Errorf("failed to write vault token to secrets dir: %v", err) + } + return nil + } + + if err := os.WriteFile(h.privateDirTokenPath, []byte(token), 0600); err != nil { return fmt.Errorf("failed to write vault token: %v", err) } if !h.vaultBlock.DisableFile { - if err := os.WriteFile(h.sharedTokenPath, []byte(token), 0666); err != nil { + if err := os.WriteFile(h.secretsDirTokenPath, []byte(token), 0666); err != nil { return fmt.Errorf("failed to write vault token to secrets dir: %v", err) } } From 83eeb46eaf5ef53744465a6d3afe53a8bd13818b Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 23 Jun 2023 10:18:18 -0400 Subject: [PATCH 08/11] vault: recreate group when disable_file changes If the task's Vault `disable_file` config changes the task group must be recreated to make sure the allocation file system is properly setup. For example, in an upgrade path the private directory will not exist for previous allocations. --- nomad/structs/structs.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 1a645d58b4a..af9ea2a3b9a 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -9784,6 +9784,8 @@ func (v *Vault) Equal(o *Vault) bool { return false case v.Env != o.Env: return false + case v.DisableFile != o.DisableFile: + return false case v.ChangeMode != o.ChangeMode: return false case v.ChangeSignal != o.ChangeSignal: From 371105a15c0b7046aaf7138b40793ad3c4314a82 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 23 Jun 2023 12:51:54 -0400 Subject: [PATCH 09/11] vault: fix vault token upgrade path test This test requires a task path to be deleted, but on Linux systems it is mounted in a tmpfs, resulting in an error if the path is deleted before unmounting. Since our CI runs on Linux machines, restrict the test to Linux environments and unmount path before deleting. --- .../taskrunner/task_runner_linux_test.go | 85 +++++++++++++++++++ .../taskrunner/task_runner_test.go | 62 -------------- 2 files changed, 85 insertions(+), 62 deletions(-) create mode 100644 client/allocrunner/taskrunner/task_runner_linux_test.go diff --git a/client/allocrunner/taskrunner/task_runner_linux_test.go b/client/allocrunner/taskrunner/task_runner_linux_test.go new file mode 100644 index 00000000000..05de054a6b9 --- /dev/null +++ b/client/allocrunner/taskrunner/task_runner_linux_test.go @@ -0,0 +1,85 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package taskrunner + +import ( + "context" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/client/vaultclient" + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test/must" +) + +func TestTaskRunner_DisableFileForVaultToken_UpgradePath(t *testing.T) { + ci.Parallel(t) + ci.SkipTestWithoutRootAccess(t) + + // Create test allocation with a Vault block. + alloc := mock.BatchAlloc() + task := alloc.Job.TaskGroups[0].Tasks[0] + task.Config = map[string]any{ + "run_for": "0s", + } + task.Vault = &structs.Vault{ + Policies: []string{"default"}, + } + + conf, cleanup := testTaskRunnerConfig(t, alloc, task.Name) + defer cleanup() + + // Remove private dir and write the Vault token to the secrets dir to + // simulate an old task. + err := conf.TaskDir.Build(false, nil) + must.NoError(t, err) + + err = syscall.Unmount(conf.TaskDir.PrivateDir, 0) + must.NoError(t, err) + err = os.Remove(conf.TaskDir.PrivateDir) + must.NoError(t, err) + + token := "1234" + tokenPath := filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) + err = os.WriteFile(tokenPath, []byte(token), 0666) + must.NoError(t, err) + + // Setup a test Vault client. + handler := func(*structs.Allocation, []string) (map[string]string, error) { + return map[string]string{task.Name: token}, nil + } + vaultClient := conf.Vault.(*vaultclient.MockVaultClient) + vaultClient.DeriveTokenFn = handler + + // Start task runner and wait for task to finish. + tr, err := NewTaskRunner(conf) + must.NoError(t, err) + defer tr.Kill(context.Background(), structs.NewTaskEvent("cleanup")) + go tr.Run() + time.Sleep(500 * time.Millisecond) + + testWaitForTaskToDie(t, tr) + + // Verify task exited successfully. + finalState := tr.TaskState() + must.Eq(t, structs.TaskStateDead, finalState.State) + must.False(t, finalState.Failed) + + // Verfiry token is in secrets dir. + tokenPath = filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) + data, err := os.ReadFile(tokenPath) + must.NoError(t, err) + must.Eq(t, token, string(data)) + + // Varify token is not in private dir since the allocation doesn't have + // this path. + tokenPath = filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile) + _, err = os.Stat(tokenPath) + must.ErrorIs(t, err, os.ErrNotExist) +} diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index d4b22c5814e..ae8d1f3fc84 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -1728,68 +1728,6 @@ func TestTaskRunner_DisableFileForVaultToken(t *testing.T) { must.ErrorIs(t, err, os.ErrNotExist) } -func TestTaskRunner_DisableFileForVaultToken_UpgradePath(t *testing.T) { - ci.Parallel(t) - - // Create test allocation with a Vault block. - alloc := mock.BatchAlloc() - task := alloc.Job.TaskGroups[0].Tasks[0] - task.Config = map[string]any{ - "run_for": "0s", - } - task.Vault = &structs.Vault{ - Policies: []string{"default"}, - } - - conf, cleanup := testTaskRunnerConfig(t, alloc, task.Name) - defer cleanup() - - // Remove private dir and write the Vault token to the secrets dir to - // simulate an old task. - err := conf.TaskDir.Build(false, nil) - must.NoError(t, err) - err = os.Remove(conf.TaskDir.PrivateDir) - must.NoError(t, err) - - token := "1234" - tokenPath := filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) - err = os.WriteFile(tokenPath, []byte(token), 0666) - must.NoError(t, err) - - // Setup a test Vault client. - handler := func(*structs.Allocation, []string) (map[string]string, error) { - return map[string]string{task.Name: token}, nil - } - vaultClient := conf.Vault.(*vaultclient.MockVaultClient) - vaultClient.DeriveTokenFn = handler - - // Start task runner and wait for task to finish. - tr, err := NewTaskRunner(conf) - must.NoError(t, err) - defer tr.Kill(context.Background(), structs.NewTaskEvent("cleanup")) - go tr.Run() - time.Sleep(500 * time.Millisecond) - - testWaitForTaskToDie(t, tr) - - // Verify task exited successfully. - finalState := tr.TaskState() - must.Eq(t, structs.TaskStateDead, finalState.State) - must.False(t, finalState.Failed) - - // Verfiry token is in secrets dir. - tokenPath = filepath.Join(conf.TaskDir.SecretsDir, vaultTokenFile) - data, err := os.ReadFile(tokenPath) - must.NoError(t, err) - must.Eq(t, token, string(data)) - - // Varify token is not in private dir since the allocation doesn't have - // this path. - tokenPath = filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile) - _, err = os.Stat(tokenPath) - must.ErrorIs(t, err, os.ErrNotExist) -} - // TestTaskRunner_DeriveToken_Retry asserts that if a recoverable error is // returned when deriving a vault token a task will continue to block while // it's retried. From 6d7dd030bbffe2e70d74dd7b87991d9b48a62d45 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 23 Jun 2023 14:40:48 -0400 Subject: [PATCH 10/11] apply code review --- client/allocrunner/taskrunner/task_runner_test.go | 2 +- website/content/docs/concepts/filesystem.mdx | 12 +++++++++--- website/content/docs/job-specification/vault.mdx | 14 ++++++++++++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index ae8d1f3fc84..24757b6ce92 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -1716,7 +1716,7 @@ func TestTaskRunner_DisableFileForVaultToken(t *testing.T) { must.Eq(t, structs.TaskStateDead, finalState.State) must.False(t, finalState.Failed) - // Verfiry token is in the private dir. + // Verify token is in the private dir. tokenPath := filepath.Join(conf.TaskDir.PrivateDir, vaultTokenFile) data, err := os.ReadFile(tokenPath) must.NoError(t, err) diff --git a/website/content/docs/concepts/filesystem.mdx b/website/content/docs/concepts/filesystem.mdx index 0289c6d5dcf..74dff6d50a0 100644 --- a/website/content/docs/concepts/filesystem.mdx +++ b/website/content/docs/concepts/filesystem.mdx @@ -71,9 +71,15 @@ allocation directory like the one below. directory". This directory is private to the task. - **«taskname»/private/**: This directory is used by Nomad to store private files - related to the allocation, such as Vault tokens, that are not always shared with tasks - when using `image` isolation. The contents of files in this directory cannot be read - by the `nomad alloc fs` command. + related to the allocation, such as Vault tokens, that are not shared with tasks + when using [`image` isolation](#image-isolation). The contents of files in this + directory cannot be read by the `nomad alloc fs` command or the via Nomad's + API. + + While not shared with tasks that use image isolation, this + path is still accessible by tasks using + chroot or none isolation + - **«taskname»/secrets/**: This directory is the location provided to the task as `NOMAD_SECRETS_DIR`. The contents of files in this directory cannot be read diff --git a/website/content/docs/job-specification/vault.mdx b/website/content/docs/job-specification/vault.mdx index cfeb6dcf5d6..cfe5a1893c6 100644 --- a/website/content/docs/job-specification/vault.mdx +++ b/website/content/docs/job-specification/vault.mdx @@ -73,6 +73,16 @@ with Vault as well. - `disable_file` `(bool: false)` - Specifies if the Vault token should be written to `secrets/vault_token`. + + While the secrets path is not shared with tasks that + use + image + filesystem isolation, it is still accessible by tasks using + chroot + or none + isolation. + + - `namespace` `(string: "")` - Specifies the Vault Namespace to use for the task. The Nomad client will retrieve a Vault token that is scoped to @@ -118,7 +128,7 @@ vault { This example retrieves a Vault token that is not shared with the task when using a driver that provides `image` isolation like [Docker][docker]. -This allows to get a powerful Vault token that interacts with the task's +This allows Nomad to use a powerful Vault token that interacts with the task's [`template`][template] stanzas to issue all kinds of secrets (e.g., database secrets, other vault tokens, etc.), without sharing that issuing power with the task itself: @@ -158,7 +168,7 @@ EOH ``` The example above uses `change_mode = "noop"` in the `template` stanza for -`examplepolicy.token`, which means that the payload is responsible for +`examplepolicy.token`, which means that the task's workload is responsible for detecting and handling changes to that file. In contrast, the `template` stanza for `client.crt` is configured so that Nomad will restart the task whenever the certificate is reissued, as indicated by `change_mode = "restart"` From 984c22ada49abf2dbd2b54c9c540fc00746b30bb Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 23 Jun 2023 14:47:26 -0400 Subject: [PATCH 11/11] changelog: add entry for #13343 --- .changelog/13343.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/13343.txt diff --git a/.changelog/13343.txt b/.changelog/13343.txt new file mode 100644 index 00000000000..0de61d8b840 --- /dev/null +++ b/.changelog/13343.txt @@ -0,0 +1,3 @@ +```release-note:improvement +vault: Add new configuration `disable_file` to prevent access to the Vault token by tasks that use `image` filesystem isolation +```