Merge pull request #13000 from hashicorp/secure-variables

Secure Variables (feature branch)

.semgrep/rpc_endpoint.yml

@@ -36,6 +36,15 @@ rules:
           if done, err := $A.$B.forward($METHOD, ...); done {
             return err
           }
+      # Pattern used by endpoints that support both normal ACLs and
+      # workload identity
+      - pattern-not-inside: |
+          if done, err := $A.$B.forward($METHOD, ...); done {
+            return err
+          }
+          ...
+          ... := $T.handleMixedAuthEndpoint(...)
+          ...
       # Pattern used by some Node endpoints.
       - pattern-not-inside: |
           if done, err := $A.$B.forward($METHOD, ...); done {

acl/acl.go

@@ -61,6 +61,9 @@ type ACL struct {
 	// We use an iradix for the purposes of ordered iteration.
 	wildcardHostVolumes *iradix.Tree
 
+	secureVariables         *iradix.Tree
+	wildcardSecureVariables *iradix.Tree
+
 	agent    string
 	node     string
 	operator string
@@ -98,6 +101,8 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) {
 	wnsTxn := iradix.New().Txn()
 	hvTxn := iradix.New().Txn()
 	whvTxn := iradix.New().Txn()
+	svTxn := iradix.New().Txn()
+	wsvTxn := iradix.New().Txn()
 
 	for _, policy := range policies {
 	NAMESPACES:
@@ -126,6 +131,33 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) {
 				}
 			}
 
+			if ns.SecureVariables != nil {
+				for _, pathPolicy := range ns.SecureVariables.Paths {
+					key := []byte(ns.Name + "\x00" + pathPolicy.PathSpec)
+					var svCapabilities capabilitySet
+					if globDefinition || strings.Contains(pathPolicy.PathSpec, "*") {
+						raw, ok := wsvTxn.Get(key)
+						if ok {
+							svCapabilities = raw.(capabilitySet)
+						} else {
+							svCapabilities = make(capabilitySet)
+						}
+						wsvTxn.Insert(key, svCapabilities)
+					} else {
+						raw, ok := svTxn.Get(key)
+						if ok {
+							svCapabilities = raw.(capabilitySet)
+						} else {
+							svCapabilities = make(capabilitySet)
+						}
+						svTxn.Insert(key, svCapabilities)
+					}
+					for _, cap := range pathPolicy.Capabilities {
+						svCapabilities.Set(cap)
+					}
+				}
+			}
+
 			// Deny always takes precedence
 			if capabilities.Check(NamespaceCapabilityDeny) {
 				continue NAMESPACES
@@ -209,6 +241,8 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) {
 	acl.wildcardNamespaces = wnsTxn.Commit()
 	acl.hostVolumes = hvTxn.Commit()
 	acl.wildcardHostVolumes = whvTxn.Commit()
+	acl.secureVariables = svTxn.Commit()
+	acl.wildcardSecureVariables = wsvTxn.Commit()
 
 	return acl, nil
 }
@@ -324,6 +358,21 @@ func (a *ACL) AllowHostVolume(ns string) bool {
 	return !capabilities.Check(PolicyDeny)
 }
 
+func (a *ACL) AllowSecureVariableOperation(ns, path, op string) bool {
+	if a.management {
+		return true
+	}
+
+	// Check for a matching capability set
+	capabilities, ok := a.matchingSecureVariablesCapabilitySet(ns, path)
+	if !ok {
+		return false
+	}
+
+	return capabilities.Check(op)
+
+}
+
 // matchingNamespaceCapabilitySet looks for a capabilitySet that matches the namespace,
 // if no concrete definitions are found, then we return the closest matching
 // glob.
@@ -392,6 +441,22 @@ func (a *ACL) matchingHostVolumeCapabilitySet(name string) (capabilitySet, bool)
 	return a.findClosestMatchingGlob(a.wildcardHostVolumes, name)
 }
 
+// matchingSecureVariablesCapabilitySet looks for a capabilitySet that matches the namespace and path,
+// if no concrete definitions are found, then we return the closest matching
+// glob.
+// The closest matching glob is the one that has the smallest character
+// difference between the namespace and the glob.
+func (a *ACL) matchingSecureVariablesCapabilitySet(ns, path string) (capabilitySet, bool) {
+	// Check for a concrete matching capability set
+	raw, ok := a.secureVariables.Get([]byte(ns + "\x00" + path))
+	if ok {
+		return raw.(capabilitySet), true
+	}
+
+	// We didn't find a concrete match, so lets try and evaluate globs.
+	return a.findClosestMatchingGlob(a.wildcardSecureVariables, ns+"\x00"+path)
+}
+
 type matchingGlob struct {
 	name          string
 	difference    int

acl/acl_test.go

@@ -447,6 +447,161 @@ func TestWildcardHostVolumeMatching(t *testing.T) {
 		})
 	}
 }
+
+func TestSecureVariablesMatching(t *testing.T) {
+	ci.Parallel(t)
+
+	tests := []struct {
+		name   string
+		policy string
+		ns     string
+		path   string
+		op     string
+		allow  bool
+	}{
+		{
+			name: "concrete namespace with concrete path matches",
+			policy: `namespace "ns" {
+					secure_variables { path "foo/bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: true,
+		},
+		{
+			name: "concrete namespace with concrete path matches for expanded caps",
+			policy: `namespace "ns" {
+					secure_variables { path "foo/bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "list",
+			allow: true,
+		},
+		{
+			name: "concrete namespace with wildcard path matches",
+			policy: `namespace "ns" {
+					secure_variables { path "foo/*" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: true,
+		},
+		{
+			name: "concrete namespace with non-prefix wildcard path matches",
+			policy: `namespace "ns" {
+					secure_variables { path "*/bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: true,
+		},
+		{
+			name: "concrete namespace with overlapping wildcard path prefix over suffix matches",
+			policy: `namespace "ns" {
+					secure_variables {
+						path "*/bar" { capabilities = ["list"] }
+						path "foo/*" { capabilities = ["write"] }
+					}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "write",
+			allow: true,
+		},
+		{
+			name: "concrete namespace with overlapping wildcard path prefix over suffix denied",
+			policy: `namespace "ns" {
+					secure_variables {
+						path "*/bar" { capabilities = ["list"] }
+						path "foo/*" { capabilities = ["write"] }
+					}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "list",
+			allow: false,
+		},
+		{
+			name: "concrete namespace with wildcard path matches most specific only",
+			policy: `namespace "ns" {
+					secure_variables {
+						path "*" { capabilities = ["read"] }
+						path "foo/*" { capabilities = ["read"] }
+						path "foo/bar" { capabilities = ["list"] }
+					}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: false,
+		},
+		{
+			name: "concrete namespace with invalid concrete path fails",
+			policy: `namespace "ns" {
+					secure_variables { path "bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: false,
+		},
+		{
+			name: "concrete namespace with invalid wildcard path fails",
+			policy: `namespace "ns" {
+					secure_variables { path "*/foo" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: false,
+		},
+		{
+			name: "wildcard namespace with concrete path matches",
+			policy: `namespace "*" {
+					secure_variables { path "foo/bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: true,
+		},
+		{
+			name: "wildcard namespace with invalid concrete path fails",
+			policy: `namespace "*" {
+					secure_variables { path "bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "foo/bar",
+			op:    "read",
+			allow: false,
+		},
+		{
+			name: "wildcard in user provided path fails",
+			policy: `namespace "ns" {
+					secure_variables { path "foo/bar" { capabilities = ["read"] }}}`,
+			ns:    "ns",
+			path:  "*",
+			op:    "read",
+			allow: false,
+		},
+		{
+			name: "wildcard attempt to bypass delimiter null byte fails",
+			policy: `namespace "ns" {
+					secure_variables { path "foo/bar" { capabilities = ["read"] }}}`,
+			ns:    "ns*",
+			path:  "bar",
+			op:    "read",
+			allow: false,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			policy, err := Parse(tc.policy)
+			require.NoError(t, err)
+			require.NotNil(t, policy.Namespaces[0].SecureVariables)
+
+			acl, err := NewACL(false, []*Policy{policy})
+			require.NoError(t, err)
+			require.Equal(t, tc.allow, acl.AllowSecureVariableOperation(tc.ns, tc.path, tc.op))
+		})
+	}
+}
+
 func TestACL_matchingCapabilitySet_returnsAllMatches(t *testing.T) {
 	ci.Parallel(t)
 

acl/policy.go

@@ -67,6 +67,16 @@ var (
 	validVolume = regexp.MustCompile("^[a-zA-Z0-9-*]{1,128}$")
 )
 
+const (
+	// The following are the fine-grained capabilities that can be
+	// granted for a secure variables path. When capabilities are
+	// combined we take the union of all capabilities.
+	SecureVariablesCapabilityList    = "list"
+	SecureVariablesCapabilityRead    = "read"
+	SecureVariablesCapabilityWrite   = "write"
+	SecureVariablesCapabilityDestroy = "destroy"
+)
+
 // Policy represents a parsed HCL or JSON policy.
 type Policy struct {
 	Namespaces  []*NamespacePolicy  `hcl:"namespace,expand"`
@@ -93,8 +103,18 @@ func (p *Policy) IsEmpty() bool {
 
 // NamespacePolicy is the policy for a specific namespace
 type NamespacePolicy struct {
-	Name         string `hcl:",key"`
-	Policy       string
+	Name            string `hcl:",key"`
+	Policy          string
+	Capabilities    []string
+	SecureVariables *SecureVariablesPolicy `hcl:"secure_variables"`
+}
+
+type SecureVariablesPolicy struct {
+	Paths []*SecureVariablesPathPolicy `hcl:"path"`
+}
+
+type SecureVariablesPathPolicy struct {
+	PathSpec     string `hcl:",key"`
 	Capabilities []string
 }
 
@@ -162,6 +182,18 @@ func isNamespaceCapabilityValid(cap string) bool {
 	}
 }
 
+// isPathCapabilityValid ensures the given capability is valid for a
+// secure variables path policy
+func isPathCapabilityValid(cap string) bool {
+	switch cap {
+	case SecureVariablesCapabilityWrite, SecureVariablesCapabilityRead,
+		SecureVariablesCapabilityList, SecureVariablesCapabilityDestroy:
+		return true
+	default:
+		return false
+	}
+}
+
 // expandNamespacePolicy provides the equivalent set of capabilities for
 // a namespace policy
 func expandNamespacePolicy(policy string) []string {
@@ -233,6 +265,22 @@ func expandHostVolumePolicy(policy string) []string {
 	}
 }
 
+func expandSecureVariablesCapabilities(caps []string) []string {
+	var foundRead, foundList bool
+	for _, cap := range caps {
+		switch cap {
+		case SecureVariablesCapabilityRead:
+			foundRead = true
+		case SecureVariablesCapabilityList:
+			foundList = true
+		}
+	}
+	if foundRead && !foundList {
+		caps = append(caps, PolicyList)
+	}
+	return caps
+}
+
 // Parse is used to parse the specified ACL rules into an
 // intermediary set of policies, before being compiled into
 // the ACL
@@ -275,6 +323,24 @@ func Parse(rules string) (*Policy, error) {
 			extraCap := expandNamespacePolicy(ns.Policy)
 			ns.Capabilities = append(ns.Capabilities, extraCap...)
 		}
+
+		if ns.SecureVariables != nil {
+			for _, pathPolicy := range ns.SecureVariables.Paths {
+				if pathPolicy.PathSpec == "" {
+					return nil, fmt.Errorf("Invalid missing secure variable path in namespace %#v", ns)
+				}
+				for _, cap := range pathPolicy.Capabilities {
+					if !isPathCapabilityValid(cap) {
+						return nil, fmt.Errorf(
+							"Invalid secure variable capability '%s' in namespace %#v", cap, ns)
+					}
+				}
+				pathPolicy.Capabilities = expandSecureVariablesCapabilities(pathPolicy.Capabilities)
+
+			}
+
+		}
+
 	}
 
 	for _, hv := range p.HostVolumes {