Skip to content

Commit

Permalink
Add file_perms parameter to job's vault stanza
Browse files Browse the repository at this point in the history
At the moment, `/secrets/vault_token` is written using the nomad
process' umask, which - by default - is `0022`, which results in
writing the vault_token world-readable (`0644` / `rw-r--r--`).

This patch aims to address this by allowing to change the
permissions used from the default `666` (before umask).

The name `file_perms` was chosen to distinguish this from the
parameter `perms` in the `template` stanza (as templates always
operate on files, while Vault tokens are about permissions as
well, which would've been confusing otherwise).

Example of use:

    vault {
        policies = ["nomad-tls-policy"]
        change_mode = "noop"
        file_perms = "600"
    }

I tried to extend unit tests to include basic checks for the
feature, not sure if this is sufficient, though.

Resolves #11900
  • Loading branch information
grembo committed Jan 22, 2022
1 parent 1471de4 commit da3d2f3
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 5 deletions.
4 changes: 4 additions & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
FilePerms *string `mapstructure:"file_perms" hcl:"file_perms,optional"`
}

func (v *Vault) Canonicalize() {
Expand All @@ -871,6 +872,9 @@ func (v *Vault) Canonicalize() {
if v.ChangeSignal == nil {
v.ChangeSignal = stringToPtr("SIGHUP")
}
if v.FilePerms == nil {
v.FilePerms = stringToPtr("0666")
}
}

// NewTask creates and initializes a new Task.
Expand Down
17 changes: 16 additions & 1 deletion client/allocrunner/taskrunner/vault_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import (
"context"
"fmt"
"io/ioutil"
"io/fs"
"os"
"path/filepath"
"strconv"
"sync"
"time"

Expand Down Expand Up @@ -81,6 +83,9 @@ type vaultHook struct {
// tokenPath is the path in which to read and write the token
tokenPath string

// tokenFilePerms are the file permissions applied to tokenPath
tokenPerms fs.FileMode

// alloc is the allocation
alloc *structs.Allocation

Expand All @@ -103,6 +108,7 @@ func newVaultHook(config *vaultHookConfig) *vaultHook {
lifecycle: config.lifecycle,
updater: config.updater,
alloc: config.alloc,
tokenPerms: 0666,
taskName: config.task,
firstRun: true,
ctx: ctx,
Expand All @@ -126,6 +132,15 @@ func (h *vaultHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRe
return nil
}

// Set vaultTokenPath file permissions
if h.vaultStanza.FilePerms != "" {
v, err := strconv.ParseUint(h.vaultStanza.FilePerms, 8, 12)
if err != nil {
return fmt.Errorf("Failed to parse %q as octal: %v", h.vaultStanza.FilePerms, err)
}
h.tokenPerms = fs.FileMode(v)
}

// Try to recover a token if it was previously written in the secrets
// directory
recoveredToken := ""
Expand Down Expand Up @@ -343,7 +358,7 @@ 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), 0666); err != nil {
if err := ioutil.WriteFile(h.tokenPath, []byte(token), h.tokenPerms); err != nil {
return fmt.Errorf("failed to write vault token: %v", err)
}

Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
Env: *apiTask.Vault.Env,
ChangeMode: *apiTask.Vault.ChangeMode,
ChangeSignal: *apiTask.Vault.ChangeSignal,
FilePerms: *apiTask.Vault.FilePerms,
}
}

Expand Down
1 change: 1 addition & 0 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ func parseVault(result *api.Vault, list *ast.ObjectList) error {
"env",
"change_mode",
"change_signal",
"file_perms",
}
if err := checkHCLKeys(listVal, valid); err != nil {
return multierror.Prefix(err, "vault ->")
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ func TestParse(t *testing.T) {
Env: boolToPtr(false),
ChangeMode: stringToPtr(vaultChangeModeSignal),
ChangeSignal: stringToPtr("SIGUSR1"),
FilePerms: stringToPtr("0666"),
},
},
},
Expand Down
1 change: 1 addition & 0 deletions jobspec/test-fixtures/basic.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ job "binstore-storagelocker" {
env = false
change_mode = "signal"
change_signal = "SIGUSR1"
file_perms = "644"
}
}

Expand Down
30 changes: 30 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6624,6 +6624,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
FilePerms: "0644",
},
},
Expected: &TaskDiff{
Expand Down Expand Up @@ -6651,6 +6652,12 @@ func TestTaskDiff(t *testing.T) {
Old: "",
New: "true",
},
{
Type: DiffTypeAdded,
Name: "FilePerms",
Old: "",
New: "0644",
},
},
Objects: []*ObjectDiff{
{
Expand Down Expand Up @@ -6684,6 +6691,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
FilePerms: "0644",
},
},
New: &Task{},
Expand Down Expand Up @@ -6712,6 +6720,12 @@ func TestTaskDiff(t *testing.T) {
Old: "true",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "FilePerms",
Old: "0644",
New: "",
},
},
Objects: []*ObjectDiff{
{
Expand Down Expand Up @@ -6746,6 +6760,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
FilePerms: "0666",
},
},
New: &Task{
Expand All @@ -6755,6 +6770,7 @@ func TestTaskDiff(t *testing.T) {
Env: false,
ChangeMode: "restart",
ChangeSignal: "foo",
FilePerms: "0644",
},
},
Expected: &TaskDiff{
Expand Down Expand Up @@ -6782,6 +6798,12 @@ func TestTaskDiff(t *testing.T) {
Old: "true",
New: "false",
},
{
Type: DiffTypeEdited,
Name: "FilePerms",
Old: "0666",
New: "0644",
},
{
Type: DiffTypeEdited,
Name: "Namespace",
Expand Down Expand Up @@ -6823,6 +6845,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
FilePerms: "",
},
},
New: &Task{
Expand All @@ -6832,6 +6855,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
FilePerms: "",
},
},
Expected: &TaskDiff{
Expand Down Expand Up @@ -6859,6 +6883,12 @@ func TestTaskDiff(t *testing.T) {
Old: "true",
New: "true",
},
{
Type: DiffTypeNone,
Name: "FilePerms",
Old: "",
New: "",
},
{
Type: DiffTypeNone,
Name: "Namespace",
Expand Down
11 changes: 11 additions & 0 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -8851,12 +8851,16 @@ 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

// FilePerms is the file permissions vault_token should be written out with.
FilePerms string
}

func DefaultVaultBlock() *Vault {
return &Vault{
Env: true,
ChangeMode: VaultChangeModeRestart,
FilePerms: "0666",
}
}

Expand Down Expand Up @@ -8904,6 +8908,13 @@ func (v *Vault) Validate() error {
_ = multierror.Append(&mErr, fmt.Errorf("Unknown change mode %q", v.ChangeMode))
}

// Verify vault_token file permissions
if v.FilePerms != "" {
if _, err := strconv.ParseUint(v.FilePerms, 8, 12); err != nil {
_ = multierror.Append(&mErr, fmt.Errorf("Failed to parse %q as octal: %v", v.FilePerms, err))
}
}

return mErr.ErrorOrNil()
}

Expand Down
17 changes: 13 additions & 4 deletions website/content/docs/job-specification/vault.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,20 @@ job "docs" {
change_mode = "signal"
change_signal = "SIGUSR1"
file_perms = "600"
}
}
}
}
```

The Nomad client will make the Vault token available to the task by writing it
to the secret directory at `secrets/vault_token` and by injecting a `VAULT_TOKEN`
environment variable. If the Nomad cluster is [configured](/docs/configuration/vault#namespace)
to use [Vault Namespaces](https://www.vaultproject.io/docs/enterprise/namespaces),
a `VAULT_NAMESPACE` environment variable will be injected whenever `VAULT_TOKEN` is set.
to the secret directory at `secrets/vault_token` with access permissions as
specified in `file_perms` and by injecting a `VAULT_TOKEN` environment variable. If the
Nomad cluster is [configured](/docs/configuration/vault#namespace) to use
[Vault Namespaces](https://www.vaultproject.io/docs/enterprise/namespaces),
a `VAULT_NAMESPACE` environment variable will be injected whenever `VAULT_TOKEN`
is set.

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
Expand Down Expand Up @@ -78,6 +81,12 @@ with Vault as well.
the task requires. The Nomad client will retrieve a Vault token that is
limited to those policies.

- `file_perms` `(string: "666")` - Specifies file permissions when creating
`secrets/vault_token`, the file containing the Vault token retrieved by
Nomad. File permissions are given in Unix octal notation _before applying
the Nomad process' umask_. Given the common _umask_ of `0022`, default
`file_perms` result in effective file permissions of `644` (`rw-r--r--`).

## `vault` Examples

The following examples only show the `vault` stanzas. Remember that the
Expand Down

0 comments on commit da3d2f3

Please sign in to comment.