diff --git a/acl/policy.go b/acl/policy.go index 3743a74c8..bb22b45b8 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -45,8 +45,9 @@ const ( // 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" + HostVolumeCapabilityDeny = "deny" + HostVolumeCapabilityMountReadOnly = "mount-readonly" + HostVolumeCapabilityMountReadWrite = "mount-readwrite" ) var ( @@ -160,7 +161,7 @@ func expandNamespacePolicy(policy string) []string { func isHostVolumeCapabilityValid(cap string) bool { switch cap { - case HostVolumeCapabilityDeny, HostVolumeCapabilityMount: + case HostVolumeCapabilityDeny, HostVolumeCapabilityMountReadOnly, HostVolumeCapabilityMountReadWrite: return true default: return false @@ -172,9 +173,9 @@ func expandHostVolumePolicy(policy string) []string { case PolicyDeny: return []string{HostVolumeCapabilityDeny} case PolicyRead: - return []string{HostVolumeCapabilityDeny} + return []string{HostVolumeCapabilityMountReadOnly} case PolicyWrite: - return []string{HostVolumeCapabilityMount} + return []string{HostVolumeCapabilityMountReadOnly, HostVolumeCapabilityMountReadWrite} default: return nil } diff --git a/acl/policy_test.go b/acl/policy_test.go index d5ef42e4b..831e80076 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -202,7 +202,7 @@ func TestParse(t *testing.T) { { ` host_volume "production-tls-*" { - capabilities = ["mount"] + capabilities = ["mount-readonly"] } `, "", @@ -212,7 +212,26 @@ func TestParse(t *testing.T) { Name: "production-tls-*", Policy: "", Capabilities: []string{ - HostVolumeCapabilityMount, + HostVolumeCapabilityMountReadOnly, + }, + }, + }, + }, + }, + { + ` + host_volume "production-tls-*" { + capabilities = ["mount-readwrite"] + } + `, + "", + &Policy{ + HostVolumes: []*HostVolumePolicy{ + { + Name: "production-tls-*", + Policy: "", + Capabilities: []string{ + HostVolumeCapabilityMountReadWrite, }, }, }, @@ -221,7 +240,7 @@ func TestParse(t *testing.T) { { ` host_volume "volume has a space" { - capabilities = ["mount"] + capabilities = ["mount-readwrite"] } `, "Invalid host volume name", diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 50259b343..573bbb453 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -117,8 +117,18 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis return structs.ErrPermissionDenied } - if !aclObj.AllowHostVolumeOperation(cfg.Source, acl.HostVolumeCapabilityMount) { - return structs.ErrPermissionDenied + // If a volume is readonly, then we allow access if the user has ReadOnly + // or ReadWrite access to the volume. Otherwise we only allow access if + // they have ReadWrite access. + if vol.ReadOnly { + if !aclObj.AllowHostVolumeOperation(cfg.Source, acl.HostVolumeCapabilityMountReadOnly) && + !aclObj.AllowHostVolumeOperation(cfg.Source, acl.HostVolumeCapabilityMountReadWrite) { + return structs.ErrPermissionDenied + } + } else { + if !aclObj.AllowHostVolumeOperation(cfg.Source, acl.HostVolumeCapabilityMountReadWrite) { + return structs.ErrPermissionDenied + } } } } diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 627914bd7..26a65d633 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -185,7 +185,7 @@ func TestJobEndpoint_Register_ACL(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - newVolumeJob := func() *structs.Job { + newVolumeJob := func(readonlyVolume bool) *structs.Job { j := mock.Job() tg := j.TaskGroups[0] tg.Volumes = map[string]*structs.VolumeRequest{ @@ -194,6 +194,7 @@ func TestJobEndpoint_Register_ACL(t *testing.T) { Config: map[string]interface{}{ "source": "prod-ca-certs", }, + ReadOnly: readonlyVolume, }, } @@ -201,7 +202,8 @@ func TestJobEndpoint_Register_ACL(t *testing.T) { { Volume: "ca-certs", Destination: "/etc/ca-certificates", - ReadOnly: true, + // Task readonly does not effect acls + ReadOnly: true, }, } @@ -212,9 +214,13 @@ func TestJobEndpoint_Register_ACL(t *testing.T) { submitJobToken := mock.CreatePolicyAndToken(t, s1.State(), 1001, "test-submit-job", submitJobPolicy) - volumesPolicy := mock.HostVolumePolicy("prod-*", "", []string{acl.HostVolumeCapabilityMount}) + volumesPolicyReadWrite := mock.HostVolumePolicy("prod-*", "", []string{acl.HostVolumeCapabilityMountReadWrite}) - submitJobWithVolumesToken := mock.CreatePolicyAndToken(t, s1.State(), 1002, "test-submit-volumes", submitJobPolicy+"\n"+volumesPolicy) + submitJobWithVolumesReadWriteToken := mock.CreatePolicyAndToken(t, s1.State(), 1002, "test-submit-volumes", submitJobPolicy+"\n"+volumesPolicyReadWrite) + + volumesPolicyReadOnly := mock.HostVolumePolicy("prod-*", "", []string{acl.HostVolumeCapabilityMountReadOnly}) + + submitJobWithVolumesReadOnlyToken := mock.CreatePolicyAndToken(t, s1.State(), 1003, "test-submit-volumes-readonly", submitJobPolicy+"\n"+volumesPolicyReadOnly) cases := []struct { Name string @@ -235,15 +241,27 @@ func TestJobEndpoint_Register_ACL(t *testing.T) { ErrExpected: false, }, { - Name: "with a token that can submit a job, but not use a required volumes", - Job: newVolumeJob(), + Name: "with a token that can submit a job, but not use a required volume", + Job: newVolumeJob(false), Token: submitJobToken.SecretID, ErrExpected: true, }, { Name: "with a token that can submit a job, and use all required volumes", - Job: newVolumeJob(), - Token: submitJobWithVolumesToken.SecretID, + Job: newVolumeJob(false), + Token: submitJobWithVolumesReadWriteToken.SecretID, + ErrExpected: false, + }, + { + Name: "with a token that can submit a job, but only has readonly access", + Job: newVolumeJob(false), + Token: submitJobWithVolumesReadOnlyToken.SecretID, + ErrExpected: true, + }, + { + Name: "with a token that can submit a job, and readonly volume access is enough", + Job: newVolumeJob(true), + Token: submitJobWithVolumesReadOnlyToken.SecretID, ErrExpected: false, }, }