From 2677f05b5352997f58c3368c8e0ab212e30bef47 Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Wed, 8 May 2019 13:14:24 +0200 Subject: [PATCH 1/2] acl: Add HostVolume ACLs This adds an initial implementation of ACLs for HostVolumes. Because HostVolumes are a cluster-wide resource, they cannot be tied to a namespace, thus here we allow similar wildcard definitions based on their names, tied to a set of capabilities. Initially, the only available capabilities are deny, or mount. These may be extended in the future to allow read-fs, mount-readonly and similar capabilities. --- acl/acl.go | 135 ++++++++++++++++++++++++++++++++++++++++----- acl/acl_test.go | 56 ++++++++++++++++++- acl/policy.go | 79 ++++++++++++++++++++++++-- acl/policy_test.go | 28 ++++++++++ 4 files changed, 276 insertions(+), 22 deletions(-) diff --git a/acl/acl.go b/acl/acl.go index f5a76ea03c4..11aee174c13 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -51,6 +51,13 @@ type ACL struct { // We use an iradix for the purposes of ordered iteration. wildcardNamespaces *iradix.Tree + // hostVolumes maps a named host volume to a capabilitySet + hostVolumes *iradix.Tree + + // wildcardHostVolumes maps a glob pattern of host volume names to a capabilitySet + // We use an iradix for the purposes of ordered iteration. + wildcardHostVolumes *iradix.Tree + agent string node string operator string @@ -83,6 +90,8 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) { acl := &ACL{} nsTxn := iradix.New().Txn() wnsTxn := iradix.New().Txn() + hvTxn := iradix.New().Txn() + whvTxn := iradix.New().Txn() for _, policy := range policies { NAMESPACES: @@ -128,6 +137,49 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) { } } + HOSTVOLUMES: + for _, hv := range policy.HostVolumes { + // Should the volume be matched using a glob? + globDefinition := strings.Contains(hv.Name, "*") + + // Check for existing capabilities + var capabilities capabilitySet + + if globDefinition { + raw, ok := whvTxn.Get([]byte(hv.Name)) + if ok { + capabilities = raw.(capabilitySet) + } else { + capabilities = make(capabilitySet) + whvTxn.Insert([]byte(hv.Name), capabilities) + } + } else { + raw, ok := hvTxn.Get([]byte(hv.Name)) + if ok { + capabilities = raw.(capabilitySet) + } else { + capabilities = make(capabilitySet) + hvTxn.Insert([]byte(hv.Name), capabilities) + } + } + + // Deny always takes precedence + if capabilities.Check(HostVolumeCapabilityDeny) { + continue + } + + // Add in all the capabilities + for _, cap := range hv.Capabilities { + if cap == HostVolumeCapabilityDeny { + // Overwrite any existing capabilities + capabilities.Clear() + capabilities.Set(HostVolumeCapabilityDeny) + continue HOSTVOLUMES + } + capabilities.Set(cap) + } + } + // Take the maximum privilege for agent, node, and operator if policy.Agent != nil { acl.agent = maxPrivilege(acl.agent, policy.Agent.Policy) @@ -146,6 +198,9 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) { // Finalize the namespaces acl.namespaces = nsTxn.Commit() acl.wildcardNamespaces = wnsTxn.Commit() + acl.hostVolumes = hvTxn.Commit() + acl.wildcardHostVolumes = whvTxn.Commit() + return acl, nil } @@ -162,7 +217,7 @@ func (a *ACL) AllowNamespaceOperation(ns string, op string) bool { } // Check for a matching capability set - capabilities, ok := a.matchingCapabilitySet(ns) + capabilities, ok := a.matchingNamespaceCapabilitySet(ns) if !ok { return false } @@ -179,7 +234,45 @@ func (a *ACL) AllowNamespace(ns string) bool { } // Check for a matching capability set - capabilities, ok := a.matchingCapabilitySet(ns) + capabilities, ok := a.matchingNamespaceCapabilitySet(ns) + if !ok { + return false + } + + // Check if the capability has been granted + if len(capabilities) == 0 { + return false + } + + return !capabilities.Check(PolicyDeny) +} + +// AllowHostVolumeOperation checks if a given operation is allowed for a host volume +func (a *ACL) AllowHostVolumeOperation(hv string, op string) bool { + // Hot path management tokens + if a.management { + return true + } + + // Check for a matching capability set + capabilities, ok := a.matchingHostVolumeCapabilitySet(hv) + if !ok { + return false + } + + // Check if the capability has been granted + return capabilities.Check(op) +} + +// AllowHostVolume checks if any operations are allowed for a HostVolume +func (a *ACL) AllowHostVolume(ns string) bool { + // Hot path management tokens + if a.management { + return true + } + + // Check for a matching capability set + capabilities, ok := a.matchingHostVolumeCapabilitySet(ns) if !ok { return false } @@ -192,12 +285,12 @@ func (a *ACL) AllowNamespace(ns string) bool { return !capabilities.Check(PolicyDeny) } -// matchingCapabilitySet looks for a capabilitySet that matches the namespace, +// matchingNamespaceCapabilitySet looks for a capabilitySet that matches the namespace, // 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) matchingCapabilitySet(ns string) (capabilitySet, bool) { +func (a *ACL) matchingNamespaceCapabilitySet(ns string) (capabilitySet, bool) { // Check for a concrete matching capability set raw, ok := a.namespaces.Get([]byte(ns)) if ok { @@ -205,18 +298,34 @@ func (a *ACL) matchingCapabilitySet(ns string) (capabilitySet, bool) { } // We didn't find a concrete match, so lets try and evaluate globs. - return a.findClosestMatchingGlob(ns) + return a.findClosestMatchingGlob(a.wildcardNamespaces, ns) +} + +// matchingHostVolumeCapabilitySet looks for a capabilitySet that matches the host volume name, +// 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 volume name and the glob. +func (a *ACL) matchingHostVolumeCapabilitySet(name string) (capabilitySet, bool) { + // Check for a concrete matching capability set + raw, ok := a.hostVolumes.Get([]byte(name)) + if ok { + return raw.(capabilitySet), true + } + + // We didn't find a concrete match, so lets try and evaluate globs. + return a.findClosestMatchingGlob(a.wildcardHostVolumes, name) } type matchingGlob struct { - ns string + name string difference int capabilitySet capabilitySet } -func (a *ACL) findClosestMatchingGlob(ns string) (capabilitySet, bool) { +func (a *ACL) findClosestMatchingGlob(radix *iradix.Tree, ns string) (capabilitySet, bool) { // First, find all globs that match. - matchingGlobs := a.findAllMatchingWildcards(ns) + matchingGlobs := findAllMatchingWildcards(radix, ns) // If none match, let's return. if len(matchingGlobs) == 0 { @@ -238,19 +347,19 @@ func (a *ACL) findClosestMatchingGlob(ns string) (capabilitySet, bool) { return matchingGlobs[0].capabilitySet, true } -func (a *ACL) findAllMatchingWildcards(ns string) []matchingGlob { +func findAllMatchingWildcards(radix *iradix.Tree, name string) []matchingGlob { var matches []matchingGlob - nsLen := len(ns) + nsLen := len(name) - a.wildcardNamespaces.Root().Walk(func(bk []byte, iv interface{}) bool { + radix.Root().Walk(func(bk []byte, iv interface{}) bool { k := string(bk) v := iv.(capabilitySet) - isMatch := glob.Glob(k, ns) + isMatch := glob.Glob(k, name) if isMatch { pair := matchingGlob{ - ns: k, + name: k, difference: nsLen - len(k) + strings.Count(k, glob.GLOB), capabilitySet: v, } diff --git a/acl/acl_test.go b/acl/acl_test.go index 52320823397..b819bc8afce 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -314,6 +314,56 @@ func TestWildcardNamespaceMatching(t *testing.T) { } } +func TestWildcardHostVolumeMatching(t *testing.T) { + tests := []struct { + Policy string + Allow bool + }{ + { // Wildcard matches + Policy: `host_volume "prod-api-*" { policy = "write" }`, + Allow: true, + }, + { // Non globbed volumes are not wildcards + Policy: `host_volume "prod-api" { policy = "write" }`, + Allow: false, + }, + { // Concrete matches take precedence + Policy: `host_volume "prod-api-services" { policy = "deny" } + host_volume "prod-api-*" { policy = "write" }`, + Allow: false, + }, + { + Policy: `host_volume "prod-api-*" { policy = "deny" } + host_volume "prod-api-services" { policy = "write" }`, + Allow: true, + }, + { // The closest character match wins + Policy: `host_volume "*-api-services" { policy = "deny" } + host_volume "prod-api-*" { policy = "write" }`, // 4 vs 8 chars + Allow: false, + }, + { + Policy: `host_volume "prod-api-*" { policy = "write" } + host_volume "*-api-services" { policy = "deny" }`, // 4 vs 8 chars + Allow: false, + }, + } + + for _, tc := range tests { + t.Run(tc.Policy, func(t *testing.T) { + assert := assert.New(t) + + policy, err := Parse(tc.Policy) + assert.NoError(err) + assert.NotNil(policy.HostVolumes) + + acl, err := NewACL(false, []*Policy{policy}) + assert.Nil(err) + + assert.Equal(tc.Allow, acl.AllowHostVolume("prod-api-services")) + }) + } +} func TestACL_matchingCapabilitySet_returnsAllMatches(t *testing.T) { tests := []struct { Policy string @@ -351,8 +401,8 @@ func TestACL_matchingCapabilitySet_returnsAllMatches(t *testing.T) { assert.Nil(err) var namespaces []string - for _, cs := range acl.findAllMatchingWildcards(tc.NS) { - namespaces = append(namespaces, cs.ns) + for _, cs := range findAllMatchingWildcards(acl.wildcardNamespaces, tc.NS) { + namespaces = append(namespaces, cs.name) } assert.Equal(tc.MatchingGlobs, namespaces) @@ -404,7 +454,7 @@ func TestACL_matchingCapabilitySet_difference(t *testing.T) { acl, err := NewACL(false, []*Policy{policy}) assert.Nil(err) - matches := acl.findAllMatchingWildcards(tc.NS) + matches := findAllMatchingWildcards(acl.wildcardNamespaces, tc.NS) assert.Equal(tc.Difference, matches[0].difference) }) } diff --git a/acl/policy.go b/acl/policy.go index b6efaa642ca..3743a74c8e0 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -21,6 +21,7 @@ const ( // The Policy stanza is a short hand for granting several of these. When capabilities are // combined we take the union of all capabilities. If the deny capability is present, it // takes precedence and overwrites all other capabilities. + NamespaceCapabilityDeny = "deny" NamespaceCapabilityListJobs = "list-jobs" NamespaceCapabilityReadJob = "read-job" @@ -38,20 +39,36 @@ var ( validNamespace = regexp.MustCompile("^[a-zA-Z0-9-*]{1,128}$") ) +const ( + // The following are the fine-grained capabilities that can be granted for a volume set. + // The Policy stanza is a short hand for granting several of these. When capabilities are + // combined we take the union of all capabilities. If the deny capability is present, it + // takes precedence and overwrites all other capabilities. + + HostVolumeCapabilityDeny = "deny" + HostVolumeCapabilityMount = "mount" +) + +var ( + validVolume = regexp.MustCompile("^[a-zA-Z0-9-*]{1,128}$") +) + // Policy represents a parsed HCL or JSON policy. type Policy struct { - Namespaces []*NamespacePolicy `hcl:"namespace,expand"` - Agent *AgentPolicy `hcl:"agent"` - Node *NodePolicy `hcl:"node"` - Operator *OperatorPolicy `hcl:"operator"` - Quota *QuotaPolicy `hcl:"quota"` - Raw string `hcl:"-"` + Namespaces []*NamespacePolicy `hcl:"namespace,expand"` + HostVolumes []*HostVolumePolicy `hcl:"host_volume,expand"` + Agent *AgentPolicy `hcl:"agent"` + Node *NodePolicy `hcl:"node"` + Operator *OperatorPolicy `hcl:"operator"` + Quota *QuotaPolicy `hcl:"quota"` + Raw string `hcl:"-"` } // IsEmpty checks to make sure that at least one policy has been set and is not // comprised of only a raw policy. func (p *Policy) IsEmpty() bool { return len(p.Namespaces) == 0 && + len(p.HostVolumes) == 0 && p.Agent == nil && p.Node == nil && p.Operator == nil && @@ -65,6 +82,13 @@ type NamespacePolicy struct { Capabilities []string } +// HostVolumePolicy is the policy for a specific named host volume +type HostVolumePolicy struct { + Name string `hcl:",key"` + Policy string + Capabilities []string +} + type AgentPolicy struct { Policy string } @@ -134,6 +158,28 @@ func expandNamespacePolicy(policy string) []string { } } +func isHostVolumeCapabilityValid(cap string) bool { + switch cap { + case HostVolumeCapabilityDeny, HostVolumeCapabilityMount: + return true + default: + return false + } +} + +func expandHostVolumePolicy(policy string) []string { + switch policy { + case PolicyDeny: + return []string{HostVolumeCapabilityDeny} + case PolicyRead: + return []string{HostVolumeCapabilityDeny} + case PolicyWrite: + return []string{HostVolumeCapabilityMount} + default: + return nil + } +} + // Parse is used to parse the specified ACL rules into an // intermediary set of policies, before being compiled into // the ACL @@ -178,6 +224,27 @@ func Parse(rules string) (*Policy, error) { } } + for _, hv := range p.HostVolumes { + if !validVolume.MatchString(hv.Name) { + return nil, fmt.Errorf("Invalid host volume name: %#v", hv) + } + if hv.Policy != "" && !isPolicyValid(hv.Policy) { + return nil, fmt.Errorf("Invalid host volume policy: %#v", hv) + } + for _, cap := range hv.Capabilities { + if !isHostVolumeCapabilityValid(cap) { + return nil, fmt.Errorf("Invalid host volume capability '%s': %#v", cap, hv) + } + } + + // Expand the short hand policy to the capabilities and + // add to any existing capabilities + if hv.Policy != "" { + extraCap := expandHostVolumePolicy(hv.Policy) + hv.Capabilities = append(hv.Capabilities, extraCap...) + } + } + if p.Agent != nil && !isPolicyValid(p.Agent.Policy) { return nil, fmt.Errorf("Invalid agent policy: %#v", p.Agent) } diff --git a/acl/policy_test.go b/acl/policy_test.go index 4665d0d4585..d5ef42e4be6 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -199,6 +199,34 @@ func TestParse(t *testing.T) { }, }, }, + { + ` + host_volume "production-tls-*" { + capabilities = ["mount"] + } + `, + "", + &Policy{ + HostVolumes: []*HostVolumePolicy{ + { + Name: "production-tls-*", + Policy: "", + Capabilities: []string{ + HostVolumeCapabilityMount, + }, + }, + }, + }, + }, + { + ` + host_volume "volume has a space" { + capabilities = ["mount"] + } + `, + "Invalid host volume name", + nil, + }, } for idx, tc := range tcases { From fac0813840eba82e1823aee334c397eb86fe8bab Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Thu, 25 Jul 2019 16:32:19 +0200 Subject: [PATCH 2/2] job_endpoint: Validate volume permissions --- nomad/job_endpoint.go | 18 ++++++ nomad/job_endpoint_test.go | 118 ++++++++++++++++++++++++++++--------- nomad/mock/acl.go | 20 +++++++ 3 files changed, 129 insertions(+), 27 deletions(-) diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 88bbd478013..f01231adbd4 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -89,6 +89,24 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis if !aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilitySubmitJob) { return structs.ErrPermissionDenied } + // Validate Volume Permsissions + for _, tg := range args.Job.TaskGroups { + for _, vol := range tg.Volumes { + if vol.Type != structs.VolumeTypeHost { + return structs.ErrPermissionDenied + } + + cfg, err := structs.ParseHostVolumeConfig(vol.Config) + if err != nil { + return structs.ErrPermissionDenied + } + + if !aclObj.AllowHostVolumeOperation(cfg.Source, acl.HostVolumeCapabilityMount) { + return structs.ErrPermissionDenied + } + } + } + // Check if override is set and we do not have permissions if args.PolicyOverride { if !aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilitySentinelOverride) { diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index c98084587cd..a9bfd9ae26b 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -106,44 +106,108 @@ func TestJobEndpoint_Register(t *testing.T) { func TestJobEndpoint_Register_ACL(t *testing.T) { t.Parallel() + s1, root := TestACLServer(t, func(c *Config) { c.NumSchedulers = 0 // Prevent automatic dequeue }) defer s1.Shutdown() - codec := rpcClient(t, s1) testutil.WaitForLeader(t, s1.RPC) - // Create the register request - job := mock.Job() - req := &structs.JobRegisterRequest{ - Job: job, - WriteRequest: structs.WriteRequest{Region: "global"}, - } + newVolumeJob := func() *structs.Job { + j := mock.Job() + tg := j.TaskGroups[0] + tg.Volumes = map[string]*structs.VolumeRequest{ + "ca-certs": { + Type: structs.VolumeTypeHost, + Config: map[string]interface{}{ + "source": "prod-ca-certs", + }, + }, + } - // Try without a token, expect failure - var resp structs.JobRegisterResponse - if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err == nil { - t.Fatalf("expected error") - } + tg.Tasks[0].VolumeMounts = []*structs.VolumeMount{ + { + Volume: "ca-certs", + Destination: "/etc/ca-certificates", + ReadOnly: true, + }, + } - // Try with a token - req.AuthToken = root.SecretID - if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err != nil { - t.Fatalf("err: %v", err) - } - if resp.Index == 0 { - t.Fatalf("bad index: %d", resp.Index) + return j } - // Check for the node in the FSM - state := s1.fsm.State() - ws := memdb.NewWatchSet() - out, err := state.JobByID(ws, job.Namespace, job.ID) - if err != nil { - t.Fatalf("err: %v", err) + submitJobPolicy := mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob, acl.NamespaceCapabilitySubmitJob}) + + submitJobToken := mock.CreatePolicyAndToken(t, s1.State(), 1001, "test-submit-job", submitJobPolicy) + + volumesPolicy := mock.HostVolumePolicy("prod-*", "", []string{acl.HostVolumeCapabilityMount}) + + submitJobWithVolumesToken := mock.CreatePolicyAndToken(t, s1.State(), 1002, "test-submit-volumes", submitJobPolicy+"\n"+volumesPolicy) + + cases := []struct { + Name string + Job *structs.Job + Token string + ErrExpected bool + }{ + { + Name: "without a token", + Job: mock.Job(), + Token: "", + ErrExpected: true, + }, + { + Name: "with a token", + Job: mock.Job(), + Token: root.SecretID, + ErrExpected: false, + }, + { + Name: "with a token that can submit a job, but not use a required volumes", + Job: newVolumeJob(), + Token: submitJobToken.SecretID, + ErrExpected: true, + }, + { + Name: "with a token that can submit a job, and use all required volumes", + Job: newVolumeJob(), + Token: submitJobWithVolumesToken.SecretID, + ErrExpected: false, + }, } - if out == nil { - t.Fatalf("expected job") + + for _, tt := range cases { + t.Run(tt.Name, func(t *testing.T) { + codec := rpcClient(t, s1) + req := &structs.JobRegisterRequest{ + Job: tt.Job, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + req.AuthToken = tt.Token + + // Try without a token, expect failure + var resp structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp) + + // If we expected an error, then the job should _not_ be registered. + if tt.ErrExpected { + require.Error(t, err, "expected error") + return + } + + if !tt.ErrExpected { + require.NoError(t, err, "unexpected error") + } + + require.NotEqual(t, 0, resp.Index) + + state := s1.fsm.State() + ws := memdb.NewWatchSet() + out, err := state.JobByID(ws, tt.Job.Namespace, tt.Job.ID) + require.NoError(t, err) + require.NotNil(t, out) + require.Equal(t, tt.Job.TaskGroups, out.TaskGroups) + }) } } diff --git a/nomad/mock/acl.go b/nomad/mock/acl.go index 3166db8cbb1..599bed4b548 100644 --- a/nomad/mock/acl.go +++ b/nomad/mock/acl.go @@ -38,6 +38,26 @@ func NamespacePolicy(namespace string, policy string, capabilities []string) str return policyHCL } +// HostVolumePolicy is a helper for generating the policy hcl for a given +// host-volume. Either policy or capabilities may be nil but not both. +func HostVolumePolicy(vol string, policy string, capabilities []string) string { + policyHCL := fmt.Sprintf("host_volume %q {", vol) + if policy != "" { + policyHCL += fmt.Sprintf("\n\tpolicy = %q", policy) + } + if len(capabilities) != 0 { + for i, s := range capabilities { + if !strings.HasPrefix(s, "\"") { + capabilities[i] = strconv.Quote(s) + } + } + + policyHCL += fmt.Sprintf("\n\tcapabilities = [%v]", strings.Join(capabilities, ",")) + } + policyHCL += "\n}" + return policyHCL +} + // AgentPolicy is a helper for generating the hcl for a given agent policy. func AgentPolicy(policy string) string { return fmt.Sprintf("agent {\n\tpolicy = %q\n}\n", policy)