secure vars: rename automatically accessible vars path for jobs (#13848)

Tasks are automatically granted access to variables on a path that matches their
workload identity, with a well-known prefix. Change the prefix to `nomad/jobs`
to allow for future prefixes like `nomad/volumes` or `nomad/plugins`. Reserve
the prefix by emitting errors during validation.

nomad/secure_variables_endpoint.go

@@ -515,7 +515,7 @@ func (sv *SecureVariables) authValidatePrefix(claims *structs.IdentityClaims, ns
 	}
 
 	parts := strings.Split(pathOrPrefix, "/")
-	expect := []string{"jobs", alloc.Job.ID, alloc.TaskGroup, claims.TaskName}
+	expect := []string{"nomad", "jobs", alloc.Job.ID, alloc.TaskGroup, claims.TaskName}
 	if len(parts) > len(expect) {
 		return structs.ErrPermissionDenied
 	}

nomad/secure_variables_endpoint_test.go

@@ -66,7 +66,7 @@ func TestSecureVariablesEndpoint_auth(t *testing.T) {
 	policy.Name = fmt.Sprintf("_:%s/%s/%s", ns, jobID, alloc1.TaskGroup)
 	policy.Rules = `namespace "nondefault-namespace" {
 		secure_variables {
-		    path "jobs/*" { capabilities = ["read"] }
+		    path "nomad/jobs/*" { capabilities = ["read"] }
 		    path "other/path" { capabilities = ["read"] }
 		}}`
 	policy.SetHash()
@@ -81,7 +81,7 @@ func TestSecureVariablesEndpoint_auth(t *testing.T) {
 	t.Run("terminal alloc should be denied", func(t *testing.T) {
 		err = srv.staticEndpoints.SecureVariables.handleMixedAuthEndpoint(
 			structs.QueryOptions{AuthToken: idToken, Namespace: ns}, "n/a",
-			fmt.Sprintf("jobs/%s/web/web", jobID))
+			fmt.Sprintf("nomad/jobs/%s/web/web", jobID))
 		require.EqualError(t, err, structs.ErrPermissionDenied.Error())
 	})
 
@@ -93,7 +93,7 @@ func TestSecureVariablesEndpoint_auth(t *testing.T) {
 	t.Run("wrong namespace should be denied", func(t *testing.T) {
 		err = srv.staticEndpoints.SecureVariables.handleMixedAuthEndpoint(
 			structs.QueryOptions{AuthToken: idToken, Namespace: structs.DefaultNamespace}, "n/a",
-			fmt.Sprintf("jobs/%s/web/web", jobID))
+			fmt.Sprintf("nomad/jobs/%s/web/web", jobID))
 		require.EqualError(t, err, structs.ErrPermissionDenied.Error())
 	})
 
@@ -108,28 +108,28 @@ func TestSecureVariablesEndpoint_auth(t *testing.T) {
 			name:        "valid claim for path with task secret",
 			token:       idToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/web/web", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web/web", jobID),
 			expectedErr: nil,
 		},
 		{
 			name:        "valid claim for path with group secret",
 			token:       idToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/web", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web", jobID),
 			expectedErr: nil,
 		},
 		{
 			name:        "valid claim for path with job secret",
 			token:       idToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s", jobID),
 			expectedErr: nil,
 		},
 		{
 			name:        "valid claim for path with namespace secret",
 			token:       idToken,
 			cap:         "n/a",
-			path:        "jobs",
+			path:        "nomad/jobs",
 			expectedErr: nil,
 		},
 		{
@@ -157,62 +157,62 @@ func TestSecureVariablesEndpoint_auth(t *testing.T) {
 			name:        "valid claim with no permissions denied by path",
 			token:       noPermissionsToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/w", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/w", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 		{
 			name:        "valid claim with no permissions allowed by namespace",
 			token:       noPermissionsToken,
 			cap:         "n/a",
-			path:        "jobs",
+			path:        "nomad/jobs",
 			expectedErr: nil,
 		},
 		{
 			name:        "valid claim with no permissions denied by capability",
 			token:       noPermissionsToken,
 			cap:         acl.PolicyRead,
-			path:        fmt.Sprintf("jobs/%s/w", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/w", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 		{
 			name:        "extra trailing slash is denied",
 			token:       idToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/web/", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web/", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 		{
 			name:        "invalid prefix is denied",
 			token:       idToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/w", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/w", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 		{
 			name:        "missing auth token is denied",
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/web/web", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web/web", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 		{
 			name:        "invalid signature is denied",
 			token:       invalidIDToken,
 			cap:         "n/a",
-			path:        fmt.Sprintf("jobs/%s/web/web", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web/web", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 		{
 			name:        "acl token read policy is allowed to list",
 			token:       aclToken.SecretID,
 			cap:         acl.PolicyList,
-			path:        fmt.Sprintf("jobs/%s/web/web", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web/web", jobID),
 			expectedErr: nil,
 		},
 		{
 			name:        "acl token read policy is not allowed to write",
 			token:       aclToken.SecretID,
 			cap:         acl.PolicyWrite,
-			path:        fmt.Sprintf("jobs/%s/web/web", jobID),
+			path:        fmt.Sprintf("nomad/jobs/%s/web/web", jobID),
 			expectedErr: structs.ErrPermissionDenied,
 		},
 	}

nomad/structs/secure_variables.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"strings"
 	"time"
 
 	// note: this is aliased so that it's more noticeable if someone
@@ -148,6 +149,18 @@ func (sv SecureVariableData) Copy() SecureVariableData {
 }
 
 func (sv SecureVariableDecrypted) Validate() error {
+
+	if len(sv.Path) == 0 {
+		return fmt.Errorf("variable requires path")
+	}
+	parts := strings.Split(sv.Path, "/")
+	switch {
+	case len(parts) == 1 && parts[0] == "nomad":
+		return fmt.Errorf("\"nomad\" is a reserved top-level directory path, but you may write variables to \"nomad/jobs\" or below")
+	case len(parts) >= 2 && parts[0] == "nomad" && parts[1] != "jobs":
+		return fmt.Errorf("only paths at \"nomad/jobs\" or below are valid paths under the top-level \"nomad\" directory")
+	}
+
 	if len(sv.Items) == 0 {
 		return errors.New("empty variables are invalid")
 	}

nomad/structs/secure_variables_test.go

@@ -31,3 +31,35 @@ func TestStructs_SecureVariableDecrypted_Copy(t *testing.T) {
 	sv2.Items["new"] = "new"
 	require.False(t, sv.Equals(sv2), "sv and sv2 should not be equal")
 }
+
+func TestStructs_SecureVariableDecrypted_Validate(t *testing.T) {
+	ci.Parallel(t)
+
+	sv := SecureVariableDecrypted{
+		SecureVariableMetadata: SecureVariableMetadata{Namespace: "a"},
+		Items:                  SecureVariableItems{"foo": "bar"},
+	}
+
+	testCases := []struct {
+		path string
+		ok   bool
+	}{
+		{path: ""},
+		{path: "nomad"},
+		{path: "nomad/other"},
+		{path: "a/b/c", ok: true},
+		{path: "nomad/jobs", ok: true},
+		{path: "nomadjobs", ok: true},
+		{path: "nomad/jobs/whatever", ok: true},
+	}
+	for _, tc := range testCases {
+		tc := tc
+		sv.Path = tc.path
+		err := sv.Validate()
+		if tc.ok {
+			require.NoError(t, err, "should not get error for: %s", tc.path)
+		} else {
+			require.Error(t, err, "should get error for: %s", tc.path)
+		}
+	}
+}