Skip to content

Commit

Permalink
Add file parameter to job's vault stanza
Browse files Browse the repository at this point in the history
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
  • Loading branch information
grembo committed Jun 12, 2022
1 parent dd71afb commit 9556e5b
Show file tree
Hide file tree
Showing 21 changed files with 148 additions and 10 deletions.
4 changes: 4 additions & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,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() {
Expand All @@ -872,6 +873,9 @@ func (v *Vault) Canonicalize() {
if v.ChangeSignal == nil {
v.ChangeSignal = stringToPtr("SIGHUP")
}
if v.File == nil {
v.File = boolToPtr(true)
}
}

// NewTask creates and initializes a new Task.
Expand Down
1 change: 1 addition & 0 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ func TestTask_Canonicalize_Vault(t *testing.T) {
Namespace: stringToPtr(""),
ChangeMode: stringToPtr("restart"),
ChangeSignal: stringToPtr("SIGHUP"),
File: boolToPtr(true),
},
},
}
Expand Down
15 changes: 15 additions & 0 deletions client/allocdir/alloc_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()

Expand Down
14 changes: 14 additions & 0 deletions client/allocdir/task_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions client/allocrunner/taskrunner/task_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1610,7 +1610,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))
Expand Down Expand Up @@ -1675,7 +1675,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))
Expand Down
14 changes: 12 additions & 2 deletions client/allocrunner/taskrunner/vault_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,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

Expand Down Expand Up @@ -129,7 +133,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 := ioutil.ReadFile(h.tokenPath)
if err != nil {
if !os.IsNotExist(err) {
Expand All @@ -141,6 +145,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)
Expand Down Expand Up @@ -343,9 +348,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), 0666); 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), 0666); err != nil {
return fmt.Errorf("failed to write vault token to secrets dir: %v", err)
}
}

return nil
}
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 @@ -1203,6 +1203,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,
}
}

Expand Down
2 changes: 2 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2723,6 +2723,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Env: helper.BoolToPtr(true),
ChangeMode: helper.StringToPtr("c"),
ChangeSignal: helper.StringToPtr("sighup"),
File: helper.BoolToPtr(true),
},
Templates: []*api.Template{
{
Expand Down Expand Up @@ -3128,6 +3129,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Env: true,
ChangeMode: "c",
ChangeSignal: "sighup",
File: true,
},
Templates: []*structs.Template{
{
Expand Down
1 change: 1 addition & 0 deletions drivers/shared/executor/executor_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ etc/
lib/
lib64/
local/
private/
proc/
secrets/
sys/
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",
}
if err := checkHCLKeys(listVal, valid); err != nil {
return multierror.Prefix(err, "vault ->")
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,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 {
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,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 {
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,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 {
Expand Down
5 changes: 5 additions & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ func TestParse(t *testing.T) {
Policies: []string{"foo", "bar"},
Env: boolToPtr(true),
ChangeMode: stringToPtr(vaultChangeModeRestart),
File: boolToPtr(true),
},
Templates: []*api.Template{
{
Expand Down Expand Up @@ -406,6 +407,7 @@ func TestParse(t *testing.T) {
Env: boolToPtr(false),
ChangeMode: stringToPtr(vaultChangeModeSignal),
ChangeSignal: stringToPtr("SIGUSR1"),
File: boolToPtr(false),
},
},
},
Expand Down Expand Up @@ -765,6 +767,7 @@ func TestParse(t *testing.T) {
Policies: []string{"group"},
Env: boolToPtr(true),
ChangeMode: stringToPtr(vaultChangeModeRestart),
File: boolToPtr(true),
},
},
{
Expand All @@ -773,6 +776,7 @@ func TestParse(t *testing.T) {
Policies: []string{"task"},
Env: boolToPtr(false),
ChangeMode: stringToPtr(vaultChangeModeRestart),
File: boolToPtr(false),
},
},
},
Expand All @@ -786,6 +790,7 @@ func TestParse(t *testing.T) {
Policies: []string{"job"},
Env: boolToPtr(true),
ChangeMode: stringToPtr(vaultChangeModeRestart),
File: boolToPtr(true),
},
},
},
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 @@ -350,6 +350,7 @@ job "binstore-storagelocker" {
env = false
change_mode = "signal"
change_signal = "SIGUSR1"
file = false
}
}

Expand Down
1 change: 1 addition & 0 deletions jobspec/test-fixtures/vault_inheritance.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ job "example" {
vault {
policies = ["task"]
env = false
file = false
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions jobspec2/parse_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ func normalizeVault(v *api.Vault) {
if v.ChangeMode == nil {
v.ChangeMode = stringToPtr("restart")
}
if v.File == nil {
v.File = boolToPtr(true)
}
}

func normalizeNetworkPorts(networks []*api.NetworkResource) {
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 @@ -6760,6 +6760,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
File: true,
},
},
Expected: &TaskDiff{
Expand Down Expand Up @@ -6787,6 +6788,12 @@ func TestTaskDiff(t *testing.T) {
Old: "",
New: "true",
},
{
Type: DiffTypeAdded,
Name: "File",
Old: "",
New: "true",
},
},
Objects: []*ObjectDiff{
{
Expand Down Expand Up @@ -6820,6 +6827,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
File: true,
},
},
New: &Task{},
Expand Down Expand Up @@ -6848,6 +6856,12 @@ func TestTaskDiff(t *testing.T) {
Old: "true",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "File",
Old: "true",
New: "",
},
},
Objects: []*ObjectDiff{
{
Expand Down Expand Up @@ -6882,6 +6896,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
File: true,
},
},
New: &Task{
Expand All @@ -6891,6 +6906,7 @@ func TestTaskDiff(t *testing.T) {
Env: false,
ChangeMode: "restart",
ChangeSignal: "foo",
File: false,
},
},
Expected: &TaskDiff{
Expand Down Expand Up @@ -6918,6 +6934,12 @@ func TestTaskDiff(t *testing.T) {
Old: "true",
New: "false",
},
{
Type: DiffTypeEdited,
Name: "File",
Old: "true",
New: "false",
},
{
Type: DiffTypeEdited,
Name: "Namespace",
Expand Down Expand Up @@ -6959,6 +6981,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
File: true,
},
},
New: &Task{
Expand All @@ -6968,6 +6991,7 @@ func TestTaskDiff(t *testing.T) {
Env: true,
ChangeMode: "signal",
ChangeSignal: "SIGUSR1",
File: true,
},
},
Expected: &TaskDiff{
Expand Down Expand Up @@ -6995,6 +7019,12 @@ func TestTaskDiff(t *testing.T) {
Old: "true",
New: "true",
},
{
Type: DiffTypeNone,
Name: "File",
Old: "true",
New: "true",
},
{
Type: DiffTypeNone,
Name: "Namespace",
Expand Down
Loading

0 comments on commit 9556e5b

Please sign in to comment.