From 4605fca1365f16339d093664e80ef2ad5a41204c Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 20 Apr 2021 14:23:30 -0600 Subject: [PATCH] e2e: add e2e tests for consul namespaces on ent with acls This PR adds e2e tests for Consul Namespaces for Nomad Enterprise with Consul ACLs enabled. Needed to add support for Consul ACL tokens with `namespace` and `namespace_prefix` blocks, which Nomad parses and validates before tossing the token. These bits will need to be picked back to OSS. --- command/agent/consul/acl_testing.go | 93 +++- e2e/connect/acls.go | 24 +- e2e/connect/connect.go | 4 +- e2e/consul/namespaces.go | 40 +- e2e/consulacls/nomad-client-policy.hcl | 23 +- e2e/consulacls/nomad-server-policy.hcl | 14 +- e2e/e2eutil/consul.go | 94 +++- nomad/consul.go | 24 +- nomad/consul_policy.go | 210 ++++++--- nomad/consul_policy_test.go | 597 +++++++++++++++++++++---- nomad/consul_test.go | 23 +- 11 files changed, 934 insertions(+), 212 deletions(-) diff --git a/command/agent/consul/acl_testing.go b/command/agent/consul/acl_testing.go index 8bd0821d6..498cfe038 100644 --- a/command/agent/consul/acl_testing.go +++ b/command/agent/consul/acl_testing.go @@ -119,8 +119,8 @@ func (m *MockACLsAPI) RoleRead(roleID string, _ *api.QueryOptions) (*api.ACLRole } } -// Example Consul "operator" tokens for use in tests. - +// Example Consul ACL tokens for use in tests. These tokens belong to the +// default Consul namespace. const ( ExampleOperatorTokenID0 = "de591604-86eb-1e6f-8b44-d4db752921ae" ExampleOperatorTokenID1 = "59c219c2-47e4-43f3-bb45-258fd13f59d5" @@ -130,7 +130,20 @@ const ( ExampleOperatorTokenID5 = "097cbb45-506b-c79c-ec38-82eb0dc0794a" ) +// Example Consul ACL tokens for use in tests that match the policies as the +// tokens above, but these belong to the "banana' Consul namespace. +const ( + ExampleOperatorTokenID10 = "ddfe688f-655f-e8dd-1db5-5650eed00aeb" + ExampleOperatorTokenID11 = "46d09394-598c-1e55-b7fd-64cd2f409707" + ExampleOperatorTokenID12 = "a041cb88-0f4b-0314-89f6-10e1e093d2e5" + ExampleOperatorTokenID13 = "cc22a583-243f-3258-14ad-db0e56749657" + ExampleOperatorTokenID14 = "5b6d0508-13a6-4bc3-33a1-ba1941e1175b" + ExampleOperatorTokenID15 = "e9db1754-c075-d0fc-0a7e-de1e9e7bff98" +) + var ( + // In Consul namespace "default" + ExampleOperatorToken0 = &api.ACLToken{ SecretID: ExampleOperatorTokenID0, AccessorID: "228865c6-3bf6-6683-df03-06dea2779088 ", @@ -189,6 +202,67 @@ var ( }}, Namespace: "default", } + + // In Consul namespace "banana" + + ExampleOperatorToken10 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID0, + AccessorID: "76a2c3b5-5d64-9089-f701-660eec2d3554", + Description: "Operator Token 0", + Namespace: "banana", + } + + ExampleOperatorToken11 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID1, + AccessorID: "40f2a36a-0a65-1972-106c-b2e5dd46d6e8", + Description: "Operator Token 1", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID1, + }}, + Namespace: "banana", + } + + ExampleOperatorToken12 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID2, + AccessorID: "894f2c5c-b285-71bf-4acb-6344cecf71f3", + Description: "Operator Token 2", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID2, + }}, + Namespace: "banana", + } + + ExampleOperatorToken13 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID3, + AccessorID: "2a81ec0b-692e-845e-f5b8-c33c05e5af22", + Description: "Operator Token 3", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID3, + }}, + Namespace: "banana", + } + + ExampleOperatorToken14 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID4, + AccessorID: "4273f1cc-5626-7a77-dc65-1f24af035ed5d", + Description: "Operator Token 4", + Policies: nil, // no direct policy, only roles + Roles: []*api.ACLTokenRoleLink{{ + ID: ExampleRoleID1, + Name: "example-role-1", + }}, + Namespace: "banana", + } + + ExampleOperatorToken15 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID5, + AccessorID: "5b78e186-87d8-c1ad-966f-f5fa87b05c9a", + Description: "Operator Token 5", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID4, + }}, + Namespace: "banana", + } ) func (m *MockACLsAPI) TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.QueryMeta, error) { @@ -209,6 +283,21 @@ func (m *MockACLsAPI) TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.Qu case ExampleOperatorTokenID5: return ExampleOperatorToken5, nil, nil + case ExampleOperatorTokenID11: + return ExampleOperatorToken11, nil, nil + + case ExampleOperatorTokenID12: + return ExampleOperatorToken12, nil, nil + + case ExampleOperatorTokenID13: + return ExampleOperatorToken13, nil, nil + + case ExampleOperatorTokenID14: + return ExampleOperatorToken14, nil, nil + + case ExampleOperatorTokenID15: + return ExampleOperatorToken15, nil, nil + default: return nil, nil, errors.New("no such token") } diff --git a/e2e/connect/acls.go b/e2e/connect/acls.go index 51c4dbfd0..2a455c937 100644 --- a/e2e/connect/acls.go +++ b/e2e/connect/acls.go @@ -22,9 +22,10 @@ type ConnectACLsE2ETest struct { // manageConsulACLs is used to 'enable' and 'disable' Consul ACLs in the // Consul Cluster that has been setup for e2e testing. manageConsulACLs consulacls.Manager - // consulMasterToken is set to the generated Consul ACL token after using + + // consulManagementToken is set to the generated Consul ACL token after using // the consul-acls-manage.sh script to enable ACLs. - consulMasterToken string + consulManagementToken string // things to cleanup after each test case jobIDs []string @@ -46,7 +47,7 @@ func (tc *ConnectACLsE2ETest) BeforeAll(f *framework.F) { // Validate the consul master token exists, otherwise tests are just // going to be a train wreck. - tokenLength := len(tc.consulMasterToken) + tokenLength := len(tc.consulManagementToken) require.Equal(f.T(), 36, tokenLength, "consul master token wrong length") // Validate the CONSUL_HTTP_TOKEN is NOT set, because that will cause @@ -63,7 +64,7 @@ func (tc *ConnectACLsE2ETest) BeforeAll(f *framework.F) { // enableConsulACLs effectively executes `consul-acls-manage.sh enable`, which // will activate Consul ACLs, going through the bootstrap process if necessary. func (tc *ConnectACLsE2ETest) enableConsulACLs(f *framework.F) { - tc.consulMasterToken = tc.manageConsulACLs.Enable(f.T()) + tc.consulManagementToken = tc.manageConsulACLs.Enable(f.T()) } // AfterAll runs after all tests are complete. @@ -100,14 +101,14 @@ func (tc *ConnectACLsE2ETest) AfterEach(f *framework.F) { // cleanup consul tokens for _, id := range tc.consulTokenIDs { t.Log("cleanup: delete consul token id:", id) - _, err := tc.Consul().ACL().TokenDelete(id, &consulapi.WriteOptions{Token: tc.consulMasterToken}) + _, err := tc.Consul().ACL().TokenDelete(id, &consulapi.WriteOptions{Token: tc.consulManagementToken}) f.NoError(err) } // cleanup consul policies for _, id := range tc.consulPolicyIDs { t.Log("cleanup: delete consul policy id:", id) - _, err := tc.Consul().ACL().PolicyDelete(id, &consulapi.WriteOptions{Token: tc.consulMasterToken}) + _, err := tc.Consul().ACL().PolicyDelete(id, &consulapi.WriteOptions{Token: tc.consulManagementToken}) f.NoError(err) } @@ -128,27 +129,30 @@ func (tc *ConnectACLsE2ETest) AfterEach(f *framework.F) { tc.consulPolicyIDs = []string{} } +// todo(shoenig): follow up refactor with e2eutil.ConsulPolicy type consulPolicy struct { Name string // e.g. nomad-operator Rules string // e.g. service "" { policy="write" } } +// todo(shoenig): follow up refactor with e2eutil.ConsulPolicy func (tc *ConnectACLsE2ETest) createConsulPolicy(p consulPolicy, f *framework.F) string { result, _, err := tc.Consul().ACL().PolicyCreate(&consulapi.ACLPolicy{ Name: p.Name, Description: "test policy " + p.Name, Rules: p.Rules, - }, &consulapi.WriteOptions{Token: tc.consulMasterToken}) + }, &consulapi.WriteOptions{Token: tc.consulManagementToken}) f.NoError(err, "failed to create consul policy") tc.consulPolicyIDs = append(tc.consulPolicyIDs, result.ID) return result.ID } +// todo(shoenig): follow up refactor with e2eutil.ConsulPolicy func (tc *ConnectACLsE2ETest) createOperatorToken(policyID string, f *framework.F) string { token, _, err := tc.Consul().ACL().TokenCreate(&consulapi.ACLToken{ Description: "operator token", Policies: []*consulapi.ACLTokenPolicyLink{{ID: policyID}}, - }, &consulapi.WriteOptions{Token: tc.consulMasterToken}) + }, &consulapi.WriteOptions{Token: tc.consulManagementToken}) f.NoError(err, "failed to create operator token") tc.consulTokenIDs = append(tc.consulTokenIDs, token.AccessorID) return token.SecretID @@ -170,7 +174,7 @@ func (tc *ConnectACLsE2ETest) TestConnectACLsRegisterMasterToken(f *framework.F) // Set the job file to use the consul master token. // One should never do this in practice, but, it should work. // https://www.consul.io/docs/acl/acl-system.html#builtin-tokens - job.ConsulToken = &tc.consulMasterToken + job.ConsulToken = &tc.consulManagementToken // Avoid using Register here, because that would actually create and run the // Job which runs the task, creates the SI token, which all needs to be @@ -368,7 +372,7 @@ func (tc *ConnectACLsE2ETest) serviceofSIToken(description string) string { func (tc *ConnectACLsE2ETest) countSITokens(t *testing.T) map[string]int { aclAPI := tc.Consul().ACL() tokens, _, err := aclAPI.TokenList(&consulapi.QueryOptions{ - Token: tc.consulMasterToken, + Token: tc.consulManagementToken, }) require.NoError(t, err) diff --git a/e2e/connect/connect.go b/e2e/connect/connect.go index e6e578ceb..d224b0161 100644 --- a/e2e/connect/connect.go +++ b/e2e/connect/connect.go @@ -50,11 +50,11 @@ func init() { }) // Connect tests with Consul ACLs enabled. These are now gated behind the - // NOMAD_TEST_CONNECT_ACLS environment variable, because they cause lots of + // NOMAD_TEST_CONSUL_ACLS environment variable, because they cause lots of // problems for e2e test flakiness (due to restarting consul, nomad, etc.). // // Run these tests locally when working on Connect. - if os.Getenv("NOMAD_TEST_CONNECT_ACLS") == "1" { + if os.Getenv("NOMAD_TEST_CONSUL_ACLS") == "1" { framework.AddSuites(&framework.TestSuite{ Component: "ConnectACLs", CanRunLocal: false, diff --git a/e2e/consul/namespaces.go b/e2e/consul/namespaces.go index 74f89adb2..407e6dfb0 100644 --- a/e2e/consul/namespaces.go +++ b/e2e/consul/namespaces.go @@ -39,7 +39,12 @@ var ( type ConsulNamespacesE2ETest struct { framework.TC + jobIDs []string + + // cToken contains the Consul global-management token during ACL enabled + // tests (i.e. ConsulNamespacesE2ETestACLs which embeds ConsulNamespacesE2ETest). + cToken string } func (tc *ConsulNamespacesE2ETest) BeforeAll(f *framework.F) { @@ -56,6 +61,9 @@ func (tc *ConsulNamespacesE2ETest) BeforeAll(f *framework.F) { value := fmt.Sprintf("ns_%s", namespace) e2eutil.PutConsulKey(f.T(), tc.Consul(), namespace, "ns-kv-example", value) } + + // make the unused variable linter happy in oss + f.T().Log("Consul global-management token:", tc.cToken) } func (tc *ConsulNamespacesE2ETest) AfterAll(f *framework.F) { @@ -68,13 +76,13 @@ func (tc *ConsulNamespacesE2ETest) TestNamespacesExist(f *framework.F) { require.True(f.T(), helper.CompareSliceSetString(namespaces, append(consulNamespaces, "default"))) } -func (tc *ConsulNamespacesE2ETest) testConsulRegisterGroupServices(f *framework.F, nsA, nsB, nsC, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulRegisterGroupServices(f *framework.F, token, nsA, nsB, nsC, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-group-services" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobGroupServices, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobGroupServices, jobID, token) require.Len(f.T(), allocations, 3) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) @@ -112,13 +120,13 @@ func (tc *ConsulNamespacesE2ETest) testConsulRegisterGroupServices(f *framework. e2eutil.RequireConsulDeregistered(r, c, nsZ, "z2") } -func (tc *ConsulNamespacesE2ETest) testConsulRegisterTaskServices(f *framework.F, nsA, nsB, nsC, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulRegisterTaskServices(f *framework.F, token, nsA, nsB, nsC, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-task-services" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobTaskServices, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobTaskServices, jobID, token) require.Len(f.T(), allocations, 3) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) @@ -154,14 +162,14 @@ func (tc *ConsulNamespacesE2ETest) testConsulRegisterTaskServices(f *framework.F e2eutil.RequireConsulDeregistered(r, c, nsZ, "z2") } -func (tc *ConsulNamespacesE2ETest) testConsulTemplateKV(f *framework.F, expB, expZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulTemplateKV(f *framework.F, token, expB, expZ string) { t := f.T() nomadClient := tc.Nomad() jobID := "cns-template-kv" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs to complete - allocations := e2eutil.RegisterAndWaitForAllocs(t, nomadClient, cnsJobTemplateKV, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(t, nomadClient, cnsJobTemplateKV, jobID, token) require.Len(t, allocations, 2) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsStopped(f.T(), tc.Nomad(), allocIDs) @@ -183,13 +191,13 @@ func (tc *ConsulNamespacesE2ETest) testConsulTemplateKV(f *framework.F, expB, ex e2eutil.WaitForJobStopped(t, nomadClient, jobID) } -func (tc *ConsulNamespacesE2ETest) testConsulConnectSidecars(f *framework.F, nsA, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulConnectSidecars(f *framework.F, token, nsA, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-connect-sidecars" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobConnectSidecars, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobConnectSidecars, jobID, token) require.Len(f.T(), allocations, 4) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) @@ -223,13 +231,13 @@ func (tc *ConsulNamespacesE2ETest) testConsulConnectSidecars(f *framework.F, nsA e2eutil.RequireConsulDeregistered(r, c, nsZ, "count-dashboard-z-sidecar-proxy") } -func (tc *ConsulNamespacesE2ETest) testConsulConnectIngressGateway(f *framework.F, nsA, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulConnectIngressGateway(f *framework.F, token, nsA, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-connect-ingress" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobConnectIngress, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobConnectIngress, jobID, token) require.Len(f.T(), allocations, 4) // 2 x (1 service + 1 gateway) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) @@ -261,13 +269,13 @@ func (tc *ConsulNamespacesE2ETest) testConsulConnectIngressGateway(f *framework. e2eutil.DeleteConsulConfigEntry(f.T(), c, nsZ, "ingress-gateway", "my-ingress-service-z") } -func (tc *ConsulNamespacesE2ETest) testConsulConnectTerminatingGateway(f *framework.F, nsA, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulConnectTerminatingGateway(f *framework.F, token, nsA, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-connect-terminating" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobConnectTerminating, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobConnectTerminating, jobID, token) require.Len(f.T(), allocations, 6) // 2 x (2 services + 1 gateway) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) @@ -301,13 +309,13 @@ func (tc *ConsulNamespacesE2ETest) testConsulConnectTerminatingGateway(f *framew e2eutil.DeleteConsulConfigEntry(f.T(), c, nsZ, "terminating-gateway", "api-gateway-z") } -func (tc *ConsulNamespacesE2ETest) testConsulScriptChecksTask(f *framework.F, nsA, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulScriptChecksTask(f *framework.F, token, nsA, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-script-checks-task" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobScriptChecksTask, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobScriptChecksTask, jobID, token) require.Len(f.T(), allocations, 2) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) @@ -351,13 +359,13 @@ func (tc *ConsulNamespacesE2ETest) testConsulScriptChecksTask(f *framework.F, ns e2eutil.WaitForJobStopped(f.T(), nomadClient, jobID) } -func (tc *ConsulNamespacesE2ETest) testConsulScriptChecksGroup(f *framework.F, nsA, nsZ string) { +func (tc *ConsulNamespacesE2ETest) testConsulScriptChecksGroup(f *framework.F, token, nsA, nsZ string) { nomadClient := tc.Nomad() jobID := "cns-script-checks-group" tc.jobIDs = append(tc.jobIDs, jobID) // Run job and wait for allocs - allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobScriptChecksGroup, jobID, "") + allocations := e2eutil.RegisterAndWaitForAllocs(f.T(), nomadClient, cnsJobScriptChecksGroup, jobID, token) require.Len(f.T(), allocations, 2) allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocations) e2eutil.WaitForAllocsRunning(f.T(), tc.Nomad(), allocIDs) diff --git a/e2e/consulacls/nomad-client-policy.hcl b/e2e/consulacls/nomad-client-policy.hcl index 3cd908769..cd3c3b1c8 100644 --- a/e2e/consulacls/nomad-client-policy.hcl +++ b/e2e/consulacls/nomad-client-policy.hcl @@ -1,13 +1,22 @@ // The Nomad Client will be registering things into its buddy Consul Client. - -service_prefix "" { - policy = "write" -} +// Note: because we also test the use of Consul namespaces, this token must be +// able to register services, read the keystore, and read node data for any +// namespace. agent_prefix "" { policy = "read" } -node_prefix "" { - policy = "read" -} +namespace_prefix "" { + key_prefix "" { + policy = "read" + } + + node_prefix "" { + policy = "read" + } + + service_prefix "" { + policy = "write" + } +} \ No newline at end of file diff --git a/e2e/consulacls/nomad-server-policy.hcl b/e2e/consulacls/nomad-server-policy.hcl index 4753838a2..6dd5a64c3 100644 --- a/e2e/consulacls/nomad-server-policy.hcl +++ b/e2e/consulacls/nomad-server-policy.hcl @@ -1,11 +1,15 @@ -// The acl=write permission is required for generating Consul Service Identity -// tokens for consul connect services. -acl = "write" - // The operator=write permission is required for creating config entries for -// connect ingress gateways. +// connect ingress gateways. operator ACLs are not namespaced, though the +// config entries they can generate are. operator = "write" +namespace_prefix "" { + // The acl=write permission is required for generating Consul Service Identity + // tokens for consul connect services. Those services could be configured for + // any Consul namespace the job-submitter has access to. + acl = "write" +} + service_prefix "" { policy = "write" } diff --git a/e2e/e2eutil/consul.go b/e2e/e2eutil/consul.go index d804feee6..27f8cf5ab 100644 --- a/e2e/e2eutil/consul.go +++ b/e2e/e2eutil/consul.go @@ -52,8 +52,8 @@ func RequireConsulDeregistered(require *require.Assertions, client *capi.Client, // RequireConsulRegistered assert that the service is registered in Consul. func RequireConsulRegistered(require *require.Assertions, client *capi.Client, namespace, service string, count int) { - testutil.WaitForResultRetries(5, func() (bool, error) { - defer time.Sleep(time.Second) + testutil.WaitForResultRetries(10, func() (bool, error) { + defer time.Sleep(2 * time.Second) services, _, err := client.Catalog().Service(service, "", &capi.QueryOptions{Namespace: namespace}) require.NoError(err) @@ -72,6 +72,7 @@ func RequireConsulRegistered(require *require.Assertions, client *capi.Client, n // Requires Consul Enterprise. func CreateConsulNamespaces(t *testing.T, client *capi.Client, namespaces []string) { nsClient := client.Namespaces() + for _, namespace := range namespaces { _, _, err := nsClient.Create(&capi.Namespace{ Name: namespace, @@ -86,6 +87,7 @@ func CreateConsulNamespaces(t *testing.T, client *capi.Client, namespaces []stri // Requires Consul Enterprise. func DeleteConsulNamespaces(t *testing.T, client *capi.Client, namespaces []string) { nsClient := client.Namespaces() + for _, namespace := range namespaces { _, err := nsClient.Delete(namespace, nil) assert.NoError(t, err) // be lenient; used in cleanup @@ -97,8 +99,10 @@ func DeleteConsulNamespaces(t *testing.T, client *capi.Client, namespaces []stri // Requires Consul Enterprise. func ListConsulNamespaces(t *testing.T, client *capi.Client) []string { nsClient := client.Namespaces() + namespaces, _, err := nsClient.List(nil) require.NoError(t, err) + result := make([]string, 0, len(namespaces)) for _, namespace := range namespaces { result = append(result, namespace.Name) @@ -111,7 +115,9 @@ func ListConsulNamespaces(t *testing.T, client *capi.Client) []string { // Requires Consul Enterprise. func PutConsulKey(t *testing.T, client *capi.Client, namespace, key, value string) { kvClient := client.KV() - _, err := kvClient.Put(&capi.KVPair{Key: key, Value: []byte(value)}, &capi.WriteOptions{Namespace: namespace}) + opts := &capi.WriteOptions{Namespace: namespace} + + _, err := kvClient.Put(&capi.KVPair{Key: key, Value: []byte(value)}, opts) require.NoError(t, err) } @@ -120,7 +126,9 @@ func PutConsulKey(t *testing.T, client *capi.Client, namespace, key, value strin // Requires Consul Enterprise. func DeleteConsulKey(t *testing.T, client *capi.Client, namespace, key string) { kvClient := client.KV() - _, err := kvClient.Delete(key, &capi.WriteOptions{Namespace: namespace}) + opts := &capi.WriteOptions{Namespace: namespace} + + _, err := kvClient.Delete(key, opts) require.NoError(t, err) } @@ -130,7 +138,9 @@ func DeleteConsulKey(t *testing.T, client *capi.Client, namespace, key string) { // Requires Consul Enterprise. func ReadConsulConfigEntry(t *testing.T, client *capi.Client, namespace, kind, name string) capi.ConfigEntry { ceClient := client.ConfigEntries() - ce, _, err := ceClient.Get(kind, name, &capi.QueryOptions{Namespace: namespace}) + opts := &capi.QueryOptions{Namespace: namespace} + + ce, _, err := ceClient.Get(kind, name, opts) require.NoError(t, err) return ce } @@ -141,6 +151,78 @@ func ReadConsulConfigEntry(t *testing.T, client *capi.Client, namespace, kind, n // Requires Consul Enterprise. func DeleteConsulConfigEntry(t *testing.T, client *capi.Client, namespace, kind, name string) { ceClient := client.ConfigEntries() - _, err := ceClient.Delete(kind, name, &capi.WriteOptions{Namespace: namespace}) + opts := &capi.WriteOptions{Namespace: namespace} + + _, err := ceClient.Delete(kind, name, opts) require.NoError(t, err) } + +// ConsulPolicy is used for create Consul ACL policies that Consul ACL tokens +// can make use of. +type ConsulPolicy struct { + Name string // e.g. nomad-operator + Rules string // e.g. service "" { policy="write" } +} + +// CreateConsulPolicy is used to create a Consul ACL policy backed by the given +// ConsulPolicy in the specified namespace. +// +// Requires Consul Enterprise. +func CreateConsulPolicy(t *testing.T, client *capi.Client, namespace string, policy ConsulPolicy) string { + aclClient := client.ACL() + opts := &capi.WriteOptions{Namespace: namespace} + + result, _, err := aclClient.PolicyCreate(&capi.ACLPolicy{ + Name: policy.Name, + Rules: policy.Rules, + Description: fmt.Sprintf("An e2e test policy %q", policy.Name), + }, opts) + require.NoError(t, err, "failed to create consul acl policy") + return result.ID +} + +// DeleteConsulPolicies is used to delete a set Consul ACL policies from Consul. +// +// Requires Consul Enterprise. +func DeleteConsulPolicies(t *testing.T, client *capi.Client, policies map[string][]string) { + aclClient := client.ACL() + + for namespace, policyIDs := range policies { + opts := &capi.WriteOptions{Namespace: namespace} + for _, policyID := range policyIDs { + _, err := aclClient.PolicyDelete(policyID, opts) + assert.NoError(t, err) + } + } +} + +// CreateConsulToken is used to create a Consul ACL token backed by the policy of +// the given policyID in the specified namespace. +// +// Requires Consul Enterprise. +func CreateConsulToken(t *testing.T, client *capi.Client, namespace, policyID string) string { + aclClient := client.ACL() + opts := &capi.WriteOptions{Namespace: namespace} + + token, _, err := aclClient.TokenCreate(&capi.ACLToken{ + Policies: []*capi.ACLTokenPolicyLink{{ID: policyID}}, + Description: "An e2e test token", + }, opts) + require.NoError(t, err, "failed to create consul acl token") + return token.SecretID +} + +// DeleteConsulTokens is used to delete a set of tokens from Consul. +// +// Requires Consul Enterprise. +func DeleteConsulTokens(t *testing.T, client *capi.Client, tokens map[string][]string) { + aclClient := client.ACL() + + for namespace, tokenIDs := range tokens { + opts := &capi.WriteOptions{Namespace: namespace} + for _, tokenID := range tokenIDs { + _, err := aclClient.TokenDelete(tokenID, opts) + assert.NoError(t, err) + } + } +} diff --git a/nomad/consul.go b/nomad/consul.go index 3a8fe56ae..35a89277e 100644 --- a/nomad/consul.go +++ b/nomad/consul.go @@ -223,19 +223,25 @@ func (c *consulACLsAPI) CheckPermissions(ctx context.Context, namespace string, } // lookup the token from consul - token, err := c.readToken(ctx, secretID) - if err != nil { - return err + token, readErr := c.readToken(ctx, secretID) + if readErr != nil { + return readErr } - // verify the token namespace matches namespace in job - if token.Namespace != namespace { - return errors.Errorf("consul ACL token cannot use namespace %q", namespace) + // if the token is a global-management token, it has unrestricted privileges + if c.isManagementToken(token) { + return nil + } + + // if the token cannot possibly be used to act on objects in the desired + // namespace, reject it immediately + if err := namespaceCheck(namespace, token); err != nil { + return err } // verify token has keystore read permission, if using template if usage.KV { - allowable, err := c.canReadKeystore(token) + allowable, err := c.canReadKeystore(namespace, token) if err != nil { return err } else if !allowable { @@ -245,7 +251,7 @@ func (c *consulACLsAPI) CheckPermissions(ctx context.Context, namespace string, // verify token has service write permission for group+task services for _, service := range usage.Services { - allowable, err := c.canWriteService(service, token) + allowable, err := c.canWriteService(namespace, service, token) if err != nil { return err } else if !allowable { @@ -256,7 +262,7 @@ func (c *consulACLsAPI) CheckPermissions(ctx context.Context, namespace string, // verify token has service identity permission for connect services for _, kind := range usage.Kinds { service := kind.Value() - allowable, err := c.canWriteService(service, token) + allowable, err := c.canWriteService(namespace, service, token) if err != nil { return err } else if !allowable { diff --git a/nomad/consul_policy.go b/nomad/consul_policy.go index b7ba5fecb..77bfb47e8 100644 --- a/nomad/consul_policy.go +++ b/nomad/consul_policy.go @@ -8,6 +8,14 @@ import ( "github.com/pkg/errors" ) +const ( + // consulGlobalManagementPolicyID is the built-in policy ID used by Consul + // to denote global-management tokens. + // + // https://www.consul.io/docs/security/acl/acl-system#builtin-policies + consulGlobalManagementPolicyID = "00000000-0000-0000-0000-000000000001" +) + // ConsulServiceRule represents a policy for a service. type ConsulServiceRule struct { Name string `hcl:",key"` @@ -23,41 +31,67 @@ type ConsulKeyRule struct { // ConsulPolicy represents the parts of a ConsulServiceRule Policy that are // relevant to Service Identity authorizations. type ConsulPolicy struct { - Services []*ConsulServiceRule `hcl:"service,expand"` - ServicePrefixes []*ConsulServiceRule `hcl:"service_prefix,expand"` - KeyPrefixes []*ConsulKeyRule `hcl:"key_prefix,expand"` + Services []*ConsulServiceRule `hcl:"service,expand"` + ServicePrefixes []*ConsulServiceRule `hcl:"service_prefix,expand"` + KeyPrefixes []*ConsulKeyRule `hcl:"key_prefix,expand"` + Namespaces map[string]*ConsulPolicy `hcl:"namespace,expand"` + NamespacePrefixes map[string]*ConsulPolicy `hcl:"namespace_prefix,expand"` } -// IsEmpty returns true if there are no Services, ServicePrefixes, or KeyPrefixes -// defined for the ConsulPolicy. -func (cp *ConsulPolicy) IsEmpty() bool { - if cp == nil { - return true - } - - policies := len(cp.Services) + len(cp.ServicePrefixes) + len(cp.KeyPrefixes) - return policies == 0 -} - -// ParseConsulPolicy parses raw string s into a ConsulPolicy. An error is +// parseConsulPolicy parses raw string s into a ConsulPolicy. An error is // returned if decoding the policy fails, or if the decoded policy has no // Services or ServicePrefixes defined. -func ParseConsulPolicy(s string) (*ConsulPolicy, error) { +func parseConsulPolicy(s string) (*ConsulPolicy, error) { cp := new(ConsulPolicy) if err := hcl.Decode(cp, s); err != nil { return nil, errors.Wrap(err, "failed to parse ACL policy") } - if cp.IsEmpty() { - // the only use case for now, may as well validate asap - return nil, errors.New("consul policy contains no service rules") - } return cp, nil } -func (c *consulACLsAPI) canReadKeystore(token *api.ACLToken) (bool, error) { +// isManagementToken returns true if the Consul token is backed by the +// built-in global-management policy. Such a token has complete, unrestricted +// access to all of Consul. +// +// https://www.consul.io/docs/security/acl/acl-system#builtin-policies +func (c *consulACLsAPI) isManagementToken(token *api.ACLToken) bool { + if token == nil { + return false + } + + for _, policy := range token.Policies { + if policy.ID == consulGlobalManagementPolicyID { + return true + } + } + return false +} + +// namespaceCheck is used to verify the namespace of the object matches the +// namespace of the ACL token provided. +// +// exception: iff token is in the default namespace, it may contain policies +// that extend into other namespaces using namespace_prefix, which must bypass +// this early check and validate in the service/keystore helpers +func namespaceCheck(namespace string, token *api.ACLToken) error { + if token.Namespace != "default" && token.Namespace != namespace { + return errors.Errorf("consul ACL token cannot use namespace %q", namespace) + } + return nil +} + +func (c *consulACLsAPI) canReadKeystore(namespace string, token *api.ACLToken) (bool, error) { + // early check the token is compatible with desired namespace + if err := namespaceCheck(namespace, token); err != nil { + return false, nil + } + + // determines whether a top-level ACL policy will be applicable + matches := namespace == token.Namespace + // check each policy directly attached to the token for _, policyRef := range token.Policies { - if allowable, err := c.policyAllowsKeystoreRead(policyRef.ID); err != nil { + if allowable, err := c.policyAllowsKeystoreRead(matches, namespace, policyRef.ID); err != nil { return false, err } else if allowable { return true, nil @@ -74,7 +108,7 @@ func (c *consulACLsAPI) canReadKeystore(token *api.ACLToken) (bool, error) { } for _, policyLink := range role.Policies { - allowable, err := c.policyAllowsKeystoreRead(policyLink.ID) + allowable, err := c.policyAllowsKeystoreRead(matches, namespace, policyLink.ID) if err != nil { return false, err } else if allowable { @@ -86,10 +120,18 @@ func (c *consulACLsAPI) canReadKeystore(token *api.ACLToken) (bool, error) { return false, nil } -func (c *consulACLsAPI) canWriteService(service string, token *api.ACLToken) (bool, error) { +func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.ACLToken) (bool, error) { + // early check the token is compatible with desired namespace + if err := namespaceCheck(namespace, token); err != nil { + return false, nil + } + + // determines whether a top-level ACL policy will be applicable + matches := namespace == token.Namespace + // check each policy directly attached to the token for _, policyRef := range token.Policies { - if allowable, err := c.policyAllowsServiceWrite(service, policyRef.ID); err != nil { + if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID); err != nil { return false, err } else if allowable { return true, nil @@ -106,9 +148,9 @@ func (c *consulACLsAPI) canWriteService(service string, token *api.ACLToken) (bo } for _, policyLink := range role.Policies { - allowable, err := c.policyAllowsServiceWrite(service, policyLink.ID) - if err != nil { - return false, err + allowable, wErr := c.policyAllowsServiceWrite(matches, namespace, service, policyLink.ID) + if wErr != nil { + return false, wErr } else if allowable { return true, nil } @@ -118,7 +160,7 @@ func (c *consulACLsAPI) canWriteService(service string, token *api.ACLToken) (bo return false, nil } -func (c *consulACLsAPI) policyAllowsServiceWrite(service string, policyID string) (bool, error) { +func (c *consulACLsAPI) policyAllowsServiceWrite(matches bool, namespace, service string, policyID string) (bool, error) { policy, _, err := c.aclClient.PolicyRead(policyID, &api.QueryOptions{ AllowStale: false, }) @@ -129,15 +171,14 @@ func (c *consulACLsAPI) policyAllowsServiceWrite(service string, policyID string // compare policy to the necessary permission for service write // e.g. service "db" { policy = "write" } // e.g. service_prefix "" { policy == "write" } - cp, err := ParseConsulPolicy(policy.Rules) + cp, err := parseConsulPolicy(policy.Rules) if err != nil { return false, err } - if cp.allowsServiceWrite(service) { + if cp.allowsServiceWrite(matches, namespace, service) { return true, nil } - return false, nil } @@ -145,30 +186,65 @@ const ( serviceNameWildcard = "*" ) -func (cp *ConsulPolicy) allowsServiceWrite(task string) bool { - for _, service := range cp.Services { - name := strings.ToLower(service.Name) - policy := strings.ToLower(service.Policy) - if policy == ConsulPolicyWrite { - if name == task || name == serviceNameWildcard { +func (cp *ConsulPolicy) allowsServiceWrite(matches bool, namespace, task string) bool { + canWriteService := func(services []*ConsulServiceRule) bool { + for _, service := range services { + name := strings.ToLower(service.Name) + policy := strings.ToLower(service.Policy) + if policy == ConsulPolicyWrite { + if name == task || name == serviceNameWildcard { + return true + } + } + } + return false + } + + canWriteServicePrefix := func(services []*ConsulServiceRule) bool { + for _, servicePrefix := range services { + prefix := strings.ToLower(servicePrefix.Name) + policy := strings.ToLower(servicePrefix.Policy) + if policy == ConsulPolicyWrite { + if strings.HasPrefix(task, prefix) { + return true + } + } + } + return false + } + + if matches { + // check the top-level service/service_prefix rules + if canWriteService(cp.Services) || canWriteServicePrefix(cp.ServicePrefixes) { + return true + } + } + + // for each namespace rule, if that namespace and the desired namespace + // are a match, we can then check the service/service_prefix policy rules + for ns, policy := range cp.Namespaces { + if ns == namespace { + if canWriteService(policy.Services) || canWriteServicePrefix(policy.ServicePrefixes) { return true } } } - for _, servicePrefix := range cp.ServicePrefixes { - prefix := strings.ToLower(servicePrefix.Name) - policy := strings.ToLower(servicePrefix.Policy) - if policy == ConsulPolicyWrite { - if strings.HasPrefix(task, prefix) { + // for each namespace_prefix rule, see if that namespace_prefix applies + // to this namespace, and if yes, also check those service/service_prefix + // policy rules + for prefix, policy := range cp.NamespacePrefixes { + if strings.HasPrefix(namespace, prefix) { + if canWriteService(policy.Services) || canWriteServicePrefix(policy.ServicePrefixes) { return true } } } + return false } -func (c *consulACLsAPI) policyAllowsKeystoreRead(policyID string) (bool, error) { +func (c *consulACLsAPI) policyAllowsKeystoreRead(matches bool, namespace, policyID string) (bool, error) { policy, _, err := c.aclClient.PolicyRead(policyID, &api.QueryOptions{ AllowStale: false, }) @@ -176,27 +252,57 @@ func (c *consulACLsAPI) policyAllowsKeystoreRead(policyID string) (bool, error) return false, err } - cp, err := ParseConsulPolicy(policy.Rules) + cp, err := parseConsulPolicy(policy.Rules) if err != nil { return false, err } - if cp.allowsKeystoreRead() { + if cp.allowsKeystoreRead(matches, namespace) { return true, nil } return false, nil } -func (cp *ConsulPolicy) allowsKeystoreRead() bool { - for _, keyPrefix := range cp.KeyPrefixes { - name := strings.ToLower(keyPrefix.Name) - policy := strings.ToLower(keyPrefix.Policy) - if name == "" { - if policy == ConsulPolicyWrite || policy == ConsulPolicyRead { +func (cp *ConsulPolicy) allowsKeystoreRead(matches bool, namespace string) bool { + canReadKeystore := func(prefixes []*ConsulKeyRule) bool { + for _, keyPrefix := range prefixes { + name := strings.ToLower(keyPrefix.Name) + policy := strings.ToLower(keyPrefix.Policy) + if name == "" { + if policy == ConsulPolicyWrite || policy == ConsulPolicyRead { + return true + } + } + } + return false + } + + // check the top-level key_prefix rules, but only if the desired namespace + // matches the namespace of the consul acl token + if matches && canReadKeystore(cp.KeyPrefixes) { + return true + } + + // for each namespace rule, if that namespace matches the desired namespace + // we chan then check the keystore policy + for ns, policy := range cp.Namespaces { + if ns == namespace { + if canReadKeystore(policy.KeyPrefixes) { return true } } } + + // for each namespace_prefix rule, see if that namespace_prefix applies to + // this namespace, and if yes, also check those key_prefix policy rules + for prefix, policy := range cp.NamespacePrefixes { + if strings.HasPrefix(namespace, prefix) { + if canReadKeystore(policy.KeyPrefixes) { + return true + } + } + } + return false } diff --git a/nomad/consul_policy_test.go b/nomad/consul_policy_test.go index a38d57e60..2764b4967 100644 --- a/nomad/consul_policy_test.go +++ b/nomad/consul_policy_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/helper/uuid" "github.com/stretchr/testify/require" ) @@ -13,10 +14,9 @@ func TestConsulPolicy_ParseConsulPolicy(t *testing.T) { t.Parallel() try := func(t *testing.T, text string, expPolicy *ConsulPolicy, expErr string) { - policy, err := ParseConsulPolicy(text) + policy, err := parseConsulPolicy(text) if expErr != "" { require.EqualError(t, err, expErr) - require.True(t, policy.IsEmpty()) } else { require.NoError(t, err) require.Equal(t, expPolicy, policy) @@ -26,8 +26,7 @@ func TestConsulPolicy_ParseConsulPolicy(t *testing.T) { t.Run("service", func(t *testing.T) { text := `service "web" { policy = "read" }` exp := &ConsulPolicy{ - Services: []*ConsulServiceRule{{Name: "web", Policy: "read"}}, - ServicePrefixes: []*ConsulServiceRule(nil), + Services: []*ConsulServiceRule{{Name: "web", Policy: "read"}}, } try(t, text, exp, "") }) @@ -35,16 +34,17 @@ func TestConsulPolicy_ParseConsulPolicy(t *testing.T) { t.Run("service_prefix", func(t *testing.T) { text := `service_prefix "data" { policy = "write" }` exp := &ConsulPolicy{ - Services: []*ConsulServiceRule(nil), ServicePrefixes: []*ConsulServiceRule{{Name: "data", Policy: "write"}}, } try(t, text, exp, "") }) - t.Run("empty", func(t *testing.T) { - text := `` - expErr := "consul policy contains no service rules" - try(t, text, nil, expErr) + t.Run("key_prefix", func(t *testing.T) { + text := `key_prefix "keys" { policy = "read" }` + exp := &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{Name: "keys", Policy: "read"}}, + } + try(t, text, exp, "") }) t.Run("malformed", func(t *testing.T) { @@ -52,53 +52,75 @@ func TestConsulPolicy_ParseConsulPolicy(t *testing.T) { expErr := "failed to parse ACL policy: At 1:22: illegal char" try(t, text, nil, expErr) }) + + t.Run("multi-namespace", func(t *testing.T) { + text := ` +service_prefix "z" { policy = "write" } + +namespace_prefix "b" { + service_prefix "b" { policy = "write" } + key_prefix "" { policy = "read" } } -func TestConsulPolicy_IsEmpty(t *testing.T) { - t.Parallel() +namespace_prefix "c" { + service_prefix "c" { policy = "read" } + key_prefix "" { policy = "read" } +} - try := func(t *testing.T, cp *ConsulPolicy, exp bool) { - result := cp.IsEmpty() - require.Equal(t, exp, result) - } +namespace_prefix "" { + key_prefix "shared/" { policy = "read" } +} - t.Run("nil", func(t *testing.T) { - cp := (*ConsulPolicy)(nil) - try(t, cp, true) - }) - - t.Run("empty slices", func(t *testing.T) { - cp := &ConsulPolicy{ - Services: []*ConsulServiceRule(nil), - ServicePrefixes: []*ConsulServiceRule(nil), +namespace "foo" { + service "bar" { policy = "read" } + service_prefix "foo-" { policy = "write" } + key_prefix "" { policy = "read" } +} +` + exp := &ConsulPolicy{ + ServicePrefixes: []*ConsulServiceRule{{Name: "z", Policy: "write"}}, + NamespacePrefixes: map[string]*ConsulPolicy{ + "b": { + ServicePrefixes: []*ConsulServiceRule{{Name: "b", Policy: "write"}}, + KeyPrefixes: []*ConsulKeyRule{{Name: "", Policy: "read"}}, + }, + "c": { + ServicePrefixes: []*ConsulServiceRule{{Name: "c", Policy: "read"}}, + KeyPrefixes: []*ConsulKeyRule{{Name: "", Policy: "read"}}, + }, + "": { + KeyPrefixes: []*ConsulKeyRule{{Name: "shared/", Policy: "read"}}, + }, + }, + Namespaces: map[string]*ConsulPolicy{ + "foo": { + Services: []*ConsulServiceRule{{Name: "bar", Policy: "read"}}, + ServicePrefixes: []*ConsulServiceRule{{Name: "foo-", Policy: "write"}}, + KeyPrefixes: []*ConsulKeyRule{{Name: "", Policy: "read"}}, + }, + }, } - try(t, cp, true) - }) - - t.Run("services nonempty", func(t *testing.T) { - cp := &ConsulPolicy{ - Services: []*ConsulServiceRule{{Name: "example", Policy: "write"}}, - } - try(t, cp, false) - }) - - t.Run("service_prefixes nonempty", func(t *testing.T) { - cp := &ConsulPolicy{ - ServicePrefixes: []*ConsulServiceRule{{Name: "pre", Policy: "read"}}, - } - try(t, cp, false) + try(t, text, exp, "") }) } func TestConsulACLsAPI_allowsServiceWrite(t *testing.T) { t.Parallel() - try := func(t *testing.T, task string, cp *ConsulPolicy, exp bool) { - result := cp.allowsServiceWrite(task) + try := func(t *testing.T, matches bool, namespace, task string, cp *ConsulPolicy, exp bool) { + // If matches is false, the implication is that the consul acl token is in + // the default namespace, otherwise prior validation would stop the request + // before getting to policy checks. Only consul acl tokens in the default + // namespace are allowed to have namespace_prefix blocks. + result := cp.allowsServiceWrite(matches, namespace, task) require.Equal(t, exp, result) } - makeCP := func(services [][2]string, prefixes [][2]string) *ConsulPolicy { + // create a consul policy backed by service and/or service_prefix rules + // + // if namespace == "_", use the top level service/service_prefix rules, otherwise + // set the rules as a namespace_prefix ruleset + makeCP := func(namespace string, services [][2]string, prefixes [][2]string) *ConsulPolicy { serviceRules := make([]*ConsulServiceRule, 0, len(services)) for _, service := range services { serviceRules = append(serviceRules, &ConsulServiceRule{Name: service[0], Policy: service[1]}) @@ -107,147 +129,526 @@ func TestConsulACLsAPI_allowsServiceWrite(t *testing.T) { for _, prefix := range prefixes { prefixRules = append(prefixRules, &ConsulServiceRule{Name: prefix[0], Policy: prefix[1]}) } - return &ConsulPolicy{Services: serviceRules, ServicePrefixes: prefixRules} + + if namespace == "_" { + return &ConsulPolicy{Services: serviceRules, ServicePrefixes: prefixRules} + } + + return &ConsulPolicy{ + Namespaces: map[string]*ConsulPolicy{ + namespace: { + Services: serviceRules, + ServicePrefixes: prefixRules, + }, + }, + NamespacePrefixes: map[string]*ConsulPolicy{ + namespace: { + Services: serviceRules, + ServicePrefixes: prefixRules, + }, + }} } t.Run("matching service policy write", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"task1", "write"}}, - nil, - ), true) + rule := [][2]string{{"task1", "write"}} + const task = "task1" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", rule, nil), true) + try(t, matches, "default", task, makeCP("default", rule, nil), true) + try(t, matches, "apple", task, makeCP("_", rule, nil), true) + try(t, matches, "apple", task, makeCP("apple", rule, nil), true) + try(t, matches, "apple", task, makeCP("app", rule, nil), true) + try(t, matches, "other", task, makeCP("", rule, nil), true) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("apple", rule, nil), true) + try(t, matches, "apple", task, makeCP("app", rule, nil), true) + try(t, matches, "other", task, makeCP("", rule, nil), true) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) }) t.Run("matching service policy read", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"task1", "read"}}, - nil, - ), false) + rule := [][2]string{{"task1", "read"}} + const task = "task1" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", rule, nil), false) + try(t, matches, "default", task, makeCP("default", rule, nil), false) + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("apple", rule, nil), false) + try(t, matches, "apple", task, makeCP("app", rule, nil), false) + try(t, matches, "other", task, makeCP("", rule, nil), false) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("apple", rule, nil), false) + try(t, matches, "apple", task, makeCP("app", rule, nil), false) + try(t, matches, "other", task, makeCP("", rule, nil), false) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) }) t.Run("wildcard service policy write", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"*", "write"}}, - nil, - ), true) + rule := [][2]string{{"*", "write"}} + const task = "task1" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", rule, nil), true) + try(t, matches, "default", task, makeCP("default", rule, nil), true) + try(t, matches, "apple", task, makeCP("_", rule, nil), true) + try(t, matches, "apple", task, makeCP("app", rule, nil), true) + try(t, matches, "other", task, makeCP("", rule, nil), true) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("app", rule, nil), true) + try(t, matches, "other", task, makeCP("", rule, nil), true) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) }) t.Run("wrong service policy write", func(t *testing.T) { - try(t, "other1", makeCP( - [][2]string{{"task1", "write"}}, - nil, - ), false) + rule := [][2]string{{"task1", "write"}} + const task = "other1" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", rule, nil), false) + try(t, matches, "default", task, makeCP("default", rule, nil), false) + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("app", rule, nil), false) + try(t, matches, "other", task, makeCP("", rule, nil), false) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = true + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("app", rule, nil), false) + try(t, matches, "other", task, makeCP("", rule, nil), false) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) }) t.Run("matching prefix policy write", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"task-", "write"}}, - ), true) + rule := [][2]string{{"task-", "write"}} + const task = "task-one" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", nil, rule), true) + try(t, matches, "default", task, makeCP("default", nil, rule), true) + try(t, matches, "apple", task, makeCP("_", nil, rule), true) + try(t, matches, "apple", task, makeCP("app", nil, rule), true) + try(t, matches, "other", task, makeCP("", nil, rule), true) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", nil, rule), false) + try(t, matches, "apple", task, makeCP("app", nil, rule), true) + try(t, matches, "other", task, makeCP("", nil, rule), true) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) }) t.Run("matching prefix policy read", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"task-", "read"}}, - ), false) + rule := [][2]string{{"task-", "read"}} + const task = "task-one" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", nil, rule), false) + try(t, matches, "default", task, makeCP("default", nil, rule), false) + try(t, matches, "apple", task, makeCP("_", nil, rule), false) + try(t, matches, "apple", task, makeCP("app", nil, rule), false) + try(t, matches, "other", task, makeCP("", nil, rule), false) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", nil, rule), false) + try(t, matches, "apple", task, makeCP("app", nil, rule), false) + try(t, matches, "other", task, makeCP("", nil, rule), false) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) }) t.Run("empty prefix policy write", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"", "write"}}, - ), true) + rule := [][2]string{{"", "write"}} + const task = "task-one" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", nil, rule), true) + try(t, matches, "default", task, makeCP("default", nil, rule), true) + try(t, matches, "apple", task, makeCP("_", nil, rule), true) + try(t, matches, "apple", task, makeCP("app", nil, rule), true) + try(t, matches, "other", task, makeCP("", nil, rule), true) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", nil, rule), false) + try(t, matches, "apple", task, makeCP("app", nil, rule), true) + try(t, matches, "other", task, makeCP("", nil, rule), true) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) }) t.Run("late matching service", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"task0", "write"}, {"task1", "write"}}, - nil, - ), true) + rule := [][2]string{{"task0", "write"}, {"task1", "write"}} + const task = "task1" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", rule, nil), true) + try(t, matches, "default", task, makeCP("default", rule, nil), true) + try(t, matches, "apple", task, makeCP("_", rule, nil), true) + try(t, matches, "apple", task, makeCP("app", rule, nil), true) + try(t, matches, "other", task, makeCP("", rule, nil), true) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", rule, nil), false) + try(t, matches, "apple", task, makeCP("app", rule, nil), true) + try(t, matches, "other", task, makeCP("", rule, nil), true) + try(t, matches, "other", task, makeCP("apple", rule, nil), false) + }) }) t.Run("late matching prefix", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"foo-", "write"}, {"task-", "write"}}, - ), true) + rule := [][2]string{{"foo-", "write"}, {"task-", "write"}} + const task = "task-one" + t.Run("namespaces match", func(t *testing.T) { + const matches = true + try(t, matches, "default", task, makeCP("_", nil, rule), true) + try(t, matches, "default", task, makeCP("default", nil, rule), true) + try(t, matches, "apple", task, makeCP("_", nil, rule), true) + try(t, matches, "apple", task, makeCP("app", nil, rule), true) + try(t, matches, "other", task, makeCP("", nil, rule), true) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) + t.Run("namespaces do not match", func(t *testing.T) { + const matches = false + try(t, matches, "apple", task, makeCP("_", nil, rule), false) + try(t, matches, "apple", task, makeCP("app", nil, rule), true) + try(t, matches, "other", task, makeCP("", nil, rule), true) + try(t, matches, "other", task, makeCP("apple", nil, rule), false) + }) }) } func TestConsulACLsAPI_hasSufficientPolicy(t *testing.T) { t.Parallel() - try := func(t *testing.T, task string, token *api.ACLToken, exp bool) { + try := func(t *testing.T, namespace, task string, token *api.ACLToken, exp bool) { logger := testlog.HCLogger(t) cAPI := &consulACLsAPI{ aclClient: consul.NewMockACLsAPI(logger), logger: logger, } - result, err := cAPI.canWriteService(task, token) + result, err := cAPI.canWriteService(namespace, task, token) require.NoError(t, err) require.Equal(t, exp, result) } - t.Run("no useful policy or role", func(t *testing.T) { - try(t, "service1", consul.ExampleOperatorToken0, false) + t.Run("default namespace with default token", func(t *testing.T) { + t.Run("no useful policy or role", func(t *testing.T) { + try(t, "default", "service1", consul.ExampleOperatorToken0, false) + }) + + t.Run("working policy only", func(t *testing.T) { + try(t, "default", "service1", consul.ExampleOperatorToken1, true) + }) + + t.Run("working role only", func(t *testing.T) { + try(t, "default", "service1", consul.ExampleOperatorToken4, true) + }) }) - t.Run("working policy only", func(t *testing.T) { - try(t, "service1", consul.ExampleOperatorToken1, true) + t.Run("other namespace with default token", func(t *testing.T) { + t.Run("no useful policy or role", func(t *testing.T) { + try(t, "other", "service1", consul.ExampleOperatorToken0, false) + }) + + t.Run("working policy only", func(t *testing.T) { + try(t, "other", "service1", consul.ExampleOperatorToken1, false) + }) + + t.Run("working role only", func(t *testing.T) { + try(t, "other", "service1", consul.ExampleOperatorToken4, false) + }) }) - t.Run("working role only", func(t *testing.T) { - try(t, "service1", consul.ExampleOperatorToken4, true) + t.Run("default namespace with banana token", func(t *testing.T) { + t.Run("no useful policy or role", func(t *testing.T) { + try(t, "default", "service1", consul.ExampleOperatorToken10, false) + }) + + t.Run("working policy only", func(t *testing.T) { + try(t, "default", "service1", consul.ExampleOperatorToken11, false) + }) + + t.Run("working role only", func(t *testing.T) { + try(t, "default", "service1", consul.ExampleOperatorToken14, false) + }) + }) + + t.Run("banana namespace with banana token", func(t *testing.T) { + t.Run("no useful policy or role", func(t *testing.T) { + try(t, "banana", "service1", consul.ExampleOperatorToken10, false) + }) + + t.Run("working policy only", func(t *testing.T) { + try(t, "banana", "service1", consul.ExampleOperatorToken11, true) + }) + + t.Run("working role only", func(t *testing.T) { + try(t, "banana", "service1", consul.ExampleOperatorToken14, true) + }) }) } func TestConsulPolicy_allowKeystoreRead(t *testing.T) { t.Run("empty", func(t *testing.T) { - require.False(t, new(ConsulPolicy).allowsKeystoreRead()) + require.False(t, new(ConsulPolicy).allowsKeystoreRead(true, "default")) }) t.Run("services only", func(t *testing.T) { - require.False(t, (&ConsulPolicy{ + policy := &ConsulPolicy{ Services: []*ConsulServiceRule{{ Name: "service1", Policy: "write", }}, - }).allowsKeystoreRead()) + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(false, "apple")) }) + // using top-level key_prefix block + t.Run("kv any read", func(t *testing.T) { - require.True(t, (&ConsulPolicy{ + policy := &ConsulPolicy{ KeyPrefixes: []*ConsulKeyRule{{ Name: "", Policy: "read", }}, - }).allowsKeystoreRead()) + } + require.True(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(false, "apple")) }) t.Run("kv any write", func(t *testing.T) { - require.True(t, (&ConsulPolicy{ + policy := &ConsulPolicy{ KeyPrefixes: []*ConsulKeyRule{{ Name: "", Policy: "write", }}, - }).allowsKeystoreRead()) + } + require.True(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(false, "apple")) }) t.Run("kv limited read", func(t *testing.T) { - require.False(t, (&ConsulPolicy{ + policy := &ConsulPolicy{ KeyPrefixes: []*ConsulKeyRule{{ Name: "foo/bar", Policy: "read", }}, - }).allowsKeystoreRead()) + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(false, "apple")) }) t.Run("kv limited write", func(t *testing.T) { - require.False(t, (&ConsulPolicy{ + policy := &ConsulPolicy{ KeyPrefixes: []*ConsulKeyRule{{ Name: "foo/bar", Policy: "write", }}, - }).allowsKeystoreRead()) + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(false, "apple")) + }) + + // using namespace_prefix block + + t.Run("kv wild namespace prefix any read", func(t *testing.T) { + policy := &ConsulPolicy{ + NamespacePrefixes: map[string]*ConsulPolicy{ + "": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.True(t, policy.allowsKeystoreRead(true, "default")) + require.True(t, policy.allowsKeystoreRead(false, "apple")) + }) + + t.Run("kv apple namespace prefix any read", func(t *testing.T) { + policy := &ConsulPolicy{ + NamespacePrefixes: map[string]*ConsulPolicy{ + "apple": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.True(t, policy.allowsKeystoreRead(false, "apple")) + }) + + t.Run("kv matching namespace prefix any read", func(t *testing.T) { + policy := &ConsulPolicy{ + NamespacePrefixes: map[string]*ConsulPolicy{ + "app": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.True(t, policy.allowsKeystoreRead(false, "apple")) + }) + + t.Run("kv other namespace prefix any read", func(t *testing.T) { + policy := &ConsulPolicy{ + NamespacePrefixes: map[string]*ConsulPolicy{ + "other": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(false, "apple")) + }) + + // using namespace block + + t.Run("kv match namespace any read", func(t *testing.T) { + policy := &ConsulPolicy{ + Namespaces: map[string]*ConsulPolicy{ + "apple": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.True(t, policy.allowsKeystoreRead(true, "apple")) + }) + + t.Run("kv mismatch namespace any read", func(t *testing.T) { + policy := &ConsulPolicy{ + Namespaces: map[string]*ConsulPolicy{ + "other": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(true, "apple")) + }) + + t.Run("kv matching namespace prefix any read", func(t *testing.T) { + policy := &ConsulPolicy{ + Namespaces: map[string]*ConsulPolicy{ + "apple": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(false, "default")) + require.True(t, policy.allowsKeystoreRead(false, "apple")) + }) + + t.Run("kv mismatch namespace prefix any read", func(t *testing.T) { + policy := &ConsulPolicy{ + Namespaces: map[string]*ConsulPolicy{ + "other": &ConsulPolicy{ + KeyPrefixes: []*ConsulKeyRule{{ + Name: "", + Policy: "read", + }}, + }, + }, + } + require.False(t, policy.allowsKeystoreRead(true, "default")) + require.False(t, policy.allowsKeystoreRead(true, "apple")) + }) +} + +func TestConsulPolicy_isManagementToken(t *testing.T) { + aclsAPI := new(consulACLsAPI) + + t.Run("nil", func(t *testing.T) { + token := (*api.ACLToken)(nil) + result := aclsAPI.isManagementToken(token) + require.False(t, result) + }) + + t.Run("no policies", func(t *testing.T) { + token := &api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{}, + } + result := aclsAPI.isManagementToken(token) + require.False(t, result) + }) + + t.Run("management policy", func(t *testing.T) { + token := &api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{{ + ID: consulGlobalManagementPolicyID, + }}, + } + result := aclsAPI.isManagementToken(token) + require.True(t, result) + }) + + t.Run("other policy", func(t *testing.T) { + token := &api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{{ + ID: uuid.Generate(), + }}, + } + result := aclsAPI.isManagementToken(token) + require.False(t, result) + }) + + t.Run("mixed policies", func(t *testing.T) { + token := &api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{{ + ID: uuid.Generate(), + }, { + ID: consulGlobalManagementPolicyID, + }, { + ID: uuid.Generate(), + }}, + } + result := aclsAPI.isManagementToken(token) + require.True(t, result) }) } diff --git a/nomad/consul_test.go b/nomad/consul_test.go index 79a20a4be..864d8ba8f 100644 --- a/nomad/consul_test.go +++ b/nomad/consul_test.go @@ -386,9 +386,14 @@ func TestConsulACLsAPI_CheckPermissions(t *testing.T) { try(t, "default", u, "", nil) }) - t.Run("uses kv wrong namespace", func(t *testing.T) { + t.Run("uses kv default token missing permissions", func(t *testing.T) { u := &structs.ConsulUsage{KV: true} - try(t, "other", u, consul.ExampleOperatorTokenID5, errors.New(`consul ACL token cannot use namespace "other"`)) + try(t, "other", u, consul.ExampleOperatorTokenID5, errors.New(`insufficient Consul ACL permissions to use template`)) + }) + + t.Run("uses kv token in wrong namespace", func(t *testing.T) { + u := &structs.ConsulUsage{KV: true} + try(t, "other", u, consul.ExampleOperatorTokenID15, errors.New(`consul ACL token cannot use namespace "other"`)) }) }) @@ -399,8 +404,12 @@ func TestConsulACLsAPI_CheckPermissions(t *testing.T) { try(t, "default", usage, consul.ExampleOperatorTokenID1, nil) }) - t.Run("operator has service wrote wrong ns", func(t *testing.T) { - try(t, "other", usage, consul.ExampleOperatorTokenID1, errors.New(`consul ACL token cannot use namespace "other"`)) + t.Run("operator has service write but no policy", func(t *testing.T) { + try(t, "other", usage, consul.ExampleOperatorTokenID1, errors.New(`insufficient Consul ACL permissions to write service "service1"`)) + }) + + t.Run("operator has token in wrong namespace", func(t *testing.T) { + try(t, "other", usage, consul.ExampleOperatorTokenID11, errors.New(`consul ACL token cannot use namespace "other"`)) }) t.Run("operator has service_prefix write", func(t *testing.T) { @@ -434,7 +443,11 @@ func TestConsulACLsAPI_CheckPermissions(t *testing.T) { }) t.Run("operator has service write wrong ns", func(t *testing.T) { - try(t, "other", usage, consul.ExampleOperatorTokenID1, errors.New(`consul ACL token cannot use namespace "other"`)) + try(t, "other", usage, consul.ExampleOperatorTokenID1, errors.New(`insufficient Consul ACL permissions to write Connect service "service1"`)) + }) + + t.Run("operator has token in wrong namespace", func(t *testing.T) { + try(t, "other", usage, consul.ExampleOperatorTokenID11, errors.New(`consul ACL token cannot use namespace "other"`)) }) t.Run("operator has service_prefix write", func(t *testing.T) {