diff --git a/command/agent/consul/acl_testing.go b/command/agent/consul/acl_testing.go index 498cfe038fd9..e570fb0370b2 100644 --- a/command/agent/consul/acl_testing.go +++ b/command/agent/consul/acl_testing.go @@ -131,7 +131,7 @@ const ( ) // Example Consul ACL tokens for use in tests that match the policies as the -// tokens above, but these belong to the "banana' Consul namespace. +// tokens above, but these belong to the "banana" Consul namespace. const ( ExampleOperatorTokenID10 = "ddfe688f-655f-e8dd-1db5-5650eed00aeb" ExampleOperatorTokenID11 = "46d09394-598c-1e55-b7fd-64cd2f409707" @@ -141,14 +141,21 @@ const ( ExampleOperatorTokenID15 = "e9db1754-c075-d0fc-0a7e-de1e9e7bff98" ) +// Example Consul ACL tokens for use in tests that match the policies as the +// tokens above, but these belong to the "default" Consul namespace. +const ( + ExampleOperatorTokenID20 = "937b3287-557c-5af8-beb0-d62191988719" + ExampleOperatorTokenID21 = "067fd927-abfb-d98f-b693-bb05dccea565" +) + var ( - // In Consul namespace "default" + // In no Consul namespace (OSS, ENT w/o Namespaces) ExampleOperatorToken0 = &api.ACLToken{ SecretID: ExampleOperatorTokenID0, AccessorID: "228865c6-3bf6-6683-df03-06dea2779088 ", Description: "Operator Token 0", - Namespace: "default", + Namespace: "", } ExampleOperatorToken1 = &api.ACLToken{ @@ -158,7 +165,7 @@ var ( Policies: []*api.ACLTokenPolicyLink{{ ID: ExamplePolicyID1, }}, - Namespace: "default", + Namespace: "", } ExampleOperatorToken2 = &api.ACLToken{ @@ -168,7 +175,7 @@ var ( Policies: []*api.ACLTokenPolicyLink{{ ID: ExamplePolicyID2, }}, - Namespace: "default", + Namespace: "", } ExampleOperatorToken3 = &api.ACLToken{ @@ -178,7 +185,7 @@ var ( Policies: []*api.ACLTokenPolicyLink{{ ID: ExamplePolicyID3, }}, - Namespace: "default", + Namespace: "", } ExampleOperatorToken4 = &api.ACLToken{ @@ -190,7 +197,7 @@ var ( ID: ExampleRoleID1, Name: "example-role-1", }}, - Namespace: "default", + Namespace: "", } ExampleOperatorToken5 = &api.ACLToken{ @@ -200,20 +207,20 @@ var ( Policies: []*api.ACLTokenPolicyLink{{ ID: ExamplePolicyID4, }}, - Namespace: "default", + Namespace: "", } // In Consul namespace "banana" ExampleOperatorToken10 = &api.ACLToken{ - SecretID: ExampleOperatorTokenID0, + SecretID: ExampleOperatorTokenID10, AccessorID: "76a2c3b5-5d64-9089-f701-660eec2d3554", Description: "Operator Token 0", Namespace: "banana", } ExampleOperatorToken11 = &api.ACLToken{ - SecretID: ExampleOperatorTokenID1, + SecretID: ExampleOperatorTokenID11, AccessorID: "40f2a36a-0a65-1972-106c-b2e5dd46d6e8", Description: "Operator Token 1", Policies: []*api.ACLTokenPolicyLink{{ @@ -223,7 +230,7 @@ var ( } ExampleOperatorToken12 = &api.ACLToken{ - SecretID: ExampleOperatorTokenID2, + SecretID: ExampleOperatorTokenID12, AccessorID: "894f2c5c-b285-71bf-4acb-6344cecf71f3", Description: "Operator Token 2", Policies: []*api.ACLTokenPolicyLink{{ @@ -233,7 +240,7 @@ var ( } ExampleOperatorToken13 = &api.ACLToken{ - SecretID: ExampleOperatorTokenID3, + SecretID: ExampleOperatorTokenID13, AccessorID: "2a81ec0b-692e-845e-f5b8-c33c05e5af22", Description: "Operator Token 3", Policies: []*api.ACLTokenPolicyLink{{ @@ -243,7 +250,7 @@ var ( } ExampleOperatorToken14 = &api.ACLToken{ - SecretID: ExampleOperatorTokenID4, + SecretID: ExampleOperatorTokenID14, AccessorID: "4273f1cc-5626-7a77-dc65-1f24af035ed5d", Description: "Operator Token 4", Policies: nil, // no direct policy, only roles @@ -255,7 +262,7 @@ var ( } ExampleOperatorToken15 = &api.ACLToken{ - SecretID: ExampleOperatorTokenID5, + SecretID: ExampleOperatorTokenID15, AccessorID: "5b78e186-87d8-c1ad-966f-f5fa87b05c9a", Description: "Operator Token 5", Policies: []*api.ACLTokenPolicyLink{{ @@ -263,6 +270,27 @@ var ( }}, Namespace: "banana", } + + // In Consul namespace "default" + + ExampleOperatorToken20 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID20, + AccessorID: "228865c6-3bf6-6683-df03-06dea2779088", + Description: "Operator Token 0", + // Should still be able to register jobs where no namespace was set + Namespace: "default", + } + + ExampleOperatorToken21 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID21, + AccessorID: "54d01af9-5036-31d3-296b-b15b941d7aa2", + Description: "Operator Token 1", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID1, + }}, + // Should still be able to register jobs where no namespace was set + Namespace: "default", + } ) func (m *MockACLsAPI) TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.QueryMeta, error) { @@ -283,6 +311,9 @@ func (m *MockACLsAPI) TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.Qu case ExampleOperatorTokenID5: return ExampleOperatorToken5, nil, nil + case ExampleOperatorTokenID10: + return ExampleOperatorToken10, nil, nil + case ExampleOperatorTokenID11: return ExampleOperatorToken11, nil, nil @@ -298,6 +329,12 @@ func (m *MockACLsAPI) TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.Qu case ExampleOperatorTokenID15: return ExampleOperatorToken15, nil, nil + case ExampleOperatorTokenID20: + return ExampleOperatorToken20, nil, nil + + case ExampleOperatorTokenID21: + return ExampleOperatorToken21, nil, nil + default: return nil, nil, errors.New("no such token") } diff --git a/nomad/consul.go b/nomad/consul.go index 35a89277e8be..df36a7545aa3 100644 --- a/nomad/consul.go +++ b/nomad/consul.go @@ -215,13 +215,6 @@ func (c *consulACLsAPI) CheckPermissions(ctx context.Context, namespace string, return nil } - // If namespace is not declared on nomad jobs, assume default consul namespace - // when comparing with the consul ACL token. This maintains backwards compatibility - // with existing connect jobs, which may already be authorized with Consul tokens. - if namespace == "" { - namespace = "default" - } - // lookup the token from consul token, readErr := c.readToken(ctx, secretID) if readErr != nil { diff --git a/nomad/consul_oss_test.go b/nomad/consul_oss_test.go new file mode 100644 index 000000000000..6fbd6e36909f --- /dev/null +++ b/nomad/consul_oss_test.go @@ -0,0 +1,123 @@ +//+build !ent + +package nomad + +import ( + "context" + "errors" + "testing" + + "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/require" +) + +func TestConsulACLsAPI_CheckPermissions_oss(t *testing.T) { + + // In Nomad OSS, CheckPermissions will only receive "" as input for the + // namespace parameter - as the ConsulUsage map from namespace to usages will + // always contain one key - the empty string. + + t.Parallel() + + try := func(t *testing.T, namespace string, usage *structs.ConsulUsage, secretID string, exp error) { + logger := testlog.HCLogger(t) + aclAPI := consul.NewMockACLsAPI(logger) + cAPI := NewConsulACLsAPI(aclAPI, logger, nil) + + err := cAPI.CheckPermissions(context.Background(), namespace, usage, secretID) + if exp == nil { + require.NoError(t, err) + } else { + require.Equal(t, exp.Error(), err.Error()) + } + } + + t.Run("check-permissions kv read", func(t *testing.T) { + t.Run("uses kv has permission", func(t *testing.T) { + u := &structs.ConsulUsage{KV: true} + try(t, "", u, consul.ExampleOperatorTokenID5, nil) + }) + + t.Run("uses kv without permission", func(t *testing.T) { + u := &structs.ConsulUsage{KV: true} + try(t, "", u, consul.ExampleOperatorTokenID1, errors.New("insufficient Consul ACL permissions to use template")) + }) + + t.Run("uses kv no token", func(t *testing.T) { + u := &structs.ConsulUsage{KV: true} + try(t, "", u, "", errors.New("missing consul token")) + }) + + t.Run("uses kv nonsense token", func(t *testing.T) { + u := &structs.ConsulUsage{KV: true} + try(t, "", u, "47d33e22-720a-7fe6-7d7f-418bf844a0be", errors.New("unable to read consul token: no such token")) + }) + + t.Run("no kv no token", func(t *testing.T) { + u := &structs.ConsulUsage{KV: false} + try(t, "", u, "", nil) + }) + }) + + t.Run("check-permissions service write", func(t *testing.T) { + usage := &structs.ConsulUsage{Services: []string{"service1"}} + + t.Run("operator has service write", func(t *testing.T) { + try(t, "", usage, consul.ExampleOperatorTokenID1, nil) + }) + + t.Run("operator has service_prefix write", func(t *testing.T) { + u := &structs.ConsulUsage{Services: []string{"foo-service1"}} + try(t, "", u, consul.ExampleOperatorTokenID2, nil) + }) + + t.Run("operator has service_prefix write wrong prefix", func(t *testing.T) { + u := &structs.ConsulUsage{Services: []string{"bar-service1"}} + try(t, "", u, consul.ExampleOperatorTokenID2, errors.New(`insufficient Consul ACL permissions to write service "bar-service1"`)) + }) + + t.Run("operator permissions insufficient", func(t *testing.T) { + try(t, "", usage, consul.ExampleOperatorTokenID3, errors.New(`insufficient Consul ACL permissions to write service "service1"`)) + }) + + t.Run("operator provided no token", func(t *testing.T) { + try(t, "", usage, "", errors.New("missing consul token")) + }) + + t.Run("operator provided nonsense token", func(t *testing.T) { + try(t, "", usage, "f1682bde-1e71-90b1-9204-85d35467ba61", errors.New("unable to read consul token: no such token")) + }) + }) + + t.Run("check-permissions connect service identity write", func(t *testing.T) { + usage := &structs.ConsulUsage{Kinds: []structs.TaskKind{structs.NewTaskKind(structs.ConnectProxyPrefix, "service1")}} + + t.Run("operator has service write", func(t *testing.T) { + try(t, "", usage, consul.ExampleOperatorTokenID1, nil) + }) + + t.Run("operator has service_prefix write", func(t *testing.T) { + u := &structs.ConsulUsage{Kinds: []structs.TaskKind{structs.NewTaskKind(structs.ConnectProxyPrefix, "foo-service1")}} + try(t, "", u, consul.ExampleOperatorTokenID2, nil) + }) + + t.Run("operator has service_prefix write wrong prefix", func(t *testing.T) { + u := &structs.ConsulUsage{Kinds: []structs.TaskKind{structs.NewTaskKind(structs.ConnectProxyPrefix, "bar-service1")}} + try(t, "", u, consul.ExampleOperatorTokenID2, errors.New(`insufficient Consul ACL permissions to write Connect service "bar-service1"`)) + }) + + t.Run("operator permissions insufficient", func(t *testing.T) { + try(t, "", usage, consul.ExampleOperatorTokenID3, errors.New(`insufficient Consul ACL permissions to write Connect service "service1"`)) + }) + + t.Run("operator provided no token", func(t *testing.T) { + try(t, "", usage, "", errors.New("missing consul token")) + }) + + t.Run("operator provided nonsense token", func(t *testing.T) { + try(t, "", usage, "f1682bde-1e71-90b1-9204-85d35467ba61", errors.New("unable to read consul token: no such token")) + }) + }) +} diff --git a/nomad/consul_policy.go b/nomad/consul_policy.go index 77bfb47e8dd1..f88ff7649a63 100644 --- a/nomad/consul_policy.go +++ b/nomad/consul_policy.go @@ -67,17 +67,42 @@ func (c *consulACLsAPI) isManagementToken(token *api.ACLToken) bool { return false } -// namespaceCheck is used to verify the namespace of the object matches the -// namespace of the ACL token provided. +// namespaceCheck is used to fail the request if the namespace of the object does +// not match the namespace of the ACL token provided. // -// exception: iff token is in the default namespace, it may contain policies +// *exception*: if 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 +// +// *exception*: if token is not in a namespace, consul namespaces are not enabled +// and there is nothing to validate +// +// If the namespaces match, whether the token is allowed to perform an operation +// is checked later. func namespaceCheck(namespace string, token *api.ACLToken) error { - if token.Namespace != "default" && token.Namespace != namespace { + + switch { + case namespace == token.Namespace: + // ACLs enabled, namespaces are the same + return nil + + case token.Namespace == "default": + // ACLs enabled, must defer to per-object checking, since the token could + // have namespace or namespace_prefix blocks with extended policies that + // allow an operation. Using namespace or namespace_prefix blocks is only + // applicable to tokens in the "default" namespace. + // + // https://www.consul.io/docs/security/acl/acl-rules#namespace-rules + return nil + + case namespace == "" && token.Namespace != "default": + // ACLs enabled with non-default token, but namespace on job not set, so + // provide a more informative error message. + return errors.Errorf("consul ACL token requires using namespace %q", token.Namespace) + + default: return errors.Errorf("consul ACL token cannot use namespace %q", namespace) } - return nil } func (c *consulACLsAPI) canReadKeystore(namespace string, token *api.ACLToken) (bool, error) { @@ -87,7 +112,10 @@ func (c *consulACLsAPI) canReadKeystore(namespace string, token *api.ACLToken) ( } // determines whether a top-level ACL policy will be applicable - matches := namespace == token.Namespace + // + // if the namespace is not set in the job and the token is in the default namespace, + // treat that like an exact match to preserve backwards compatibility + matches := (namespace == token.Namespace) || (namespace == "" && token.Namespace == "default") // check each policy directly attached to the token for _, policyRef := range token.Policies { @@ -127,7 +155,10 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC } // determines whether a top-level ACL policy will be applicable - matches := namespace == token.Namespace + // + // if the namespace is not set in the job and the token is in the default namespace, + // treat that like an exact match to preserve backwards compatibility + matches := (namespace == token.Namespace) || (namespace == "" && token.Namespace == "default") // check each policy directly attached to the token for _, policyRef := range token.Policies { @@ -179,6 +210,7 @@ func (c *consulACLsAPI) policyAllowsServiceWrite(matches bool, namespace, servic if cp.allowsServiceWrite(matches, namespace, service) { return true, nil } + return false, nil } diff --git a/nomad/consul_policy_oss_test.go b/nomad/consul_policy_oss_test.go new file mode 100644 index 000000000000..d7b9125f119f --- /dev/null +++ b/nomad/consul_policy_oss_test.go @@ -0,0 +1,43 @@ +//+build !ent + +package nomad + +import ( + "testing" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/stretchr/testify/require" +) + +func TestConsulACLsAPI_hasSufficientPolicy(t *testing.T) { + t.Parallel() + + 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(namespace, task, token) + require.NoError(t, err) + require.Equal(t, exp, result) + } + + // In Nomad OSS, group consul namespace will always be empty string. + + t.Run("no namespace with default token", func(t *testing.T) { + t.Run("no useful policy or role", func(t *testing.T) { + try(t, "", "service1", consul.ExampleOperatorToken0, false) + }) + + t.Run("working policy only", func(t *testing.T) { + try(t, "", "service1", consul.ExampleOperatorToken1, true) + }) + + t.Run("working role only", func(t *testing.T) { + try(t, "", "service1", consul.ExampleOperatorToken4, true) + }) + }) +} diff --git a/nomad/consul_policy_test.go b/nomad/consul_policy_test.go index 2764b49679a9..4e5f9cea065c 100644 --- a/nomad/consul_policy_test.go +++ b/nomad/consul_policy_test.go @@ -4,8 +4,6 @@ import ( "testing" "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" ) @@ -343,74 +341,116 @@ func TestConsulACLsAPI_allowsServiceWrite(t *testing.T) { }) } -func TestConsulACLsAPI_hasSufficientPolicy(t *testing.T) { - t.Parallel() +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) + }) - 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, + t.Run("no policies", func(t *testing.T) { + token := &api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{}, } - result, err := cAPI.canWriteService(namespace, task, token) - require.NoError(t, err) - require.Equal(t, exp, result) - } + result := aclsAPI.isManagementToken(token) + require.False(t, result) + }) - 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("management policy", func(t *testing.T) { + token := &api.ACLToken{ + Policies: []*api.ACLTokenPolicyLink{{ + ID: consulGlobalManagementPolicyID, + }}, + } + result := aclsAPI.isManagementToken(token) + require.True(t, result) + }) - t.Run("working policy only", func(t *testing.T) { - try(t, "default", "service1", consul.ExampleOperatorToken1, true) - }) + 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("working role only", func(t *testing.T) { - try(t, "default", "service1", consul.ExampleOperatorToken4, true) - }) + 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) }) +} - 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) - }) +func TestConsulPolicy_namespaceCheck(t *testing.T) { + withoutNS := &api.ACLToken{Namespace: ""} + withDefault := &api.ACLToken{Namespace: "default"} + withOther := &api.ACLToken{Namespace: "other"} - t.Run("working policy only", func(t *testing.T) { - try(t, "other", "service1", consul.ExampleOperatorToken1, false) - }) + // ACLs not enabled - t.Run("working role only", func(t *testing.T) { - try(t, "other", "service1", consul.ExampleOperatorToken4, false) - }) + t.Run("acl:disable ns:unset", func(t *testing.T) { + err := namespaceCheck("", withoutNS) + require.NoError(t, err) }) - 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("acl:disable ns:default", func(t *testing.T) { + err := namespaceCheck("default", withoutNS) + require.EqualError(t, err, `consul ACL token cannot use namespace "default"`) + }) - t.Run("working policy only", func(t *testing.T) { - try(t, "default", "service1", consul.ExampleOperatorToken11, false) - }) + t.Run("acl:disable ns:other", func(t *testing.T) { + err := namespaceCheck("other", withoutNS) + require.EqualError(t, err, `consul ACL token cannot use namespace "other"`) + }) - t.Run("working role only", func(t *testing.T) { - try(t, "default", "service1", consul.ExampleOperatorToken14, false) - }) + // ACLs with "default" token + + t.Run("acl:enable token:default ns:unset", func(t *testing.T) { + // the bypass case where a legacy job (with no namespace set) should work + // with the a token in the "default" consul namespace + err := namespaceCheck("", withDefault) + require.NoError(t, err) }) - 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("acl:enable token:default ns:default", func(t *testing.T) { + err := namespaceCheck("default", withDefault) + require.NoError(t, err) + }) - t.Run("working policy only", func(t *testing.T) { - try(t, "banana", "service1", consul.ExampleOperatorToken11, true) - }) + t.Run("acl:enable token:default ns:other", func(t *testing.T) { + // the bypass case where a default token could have namespace_prefix + // blocks + err := namespaceCheck("other", withDefault) + require.NoError(t, err) + }) - t.Run("working role only", func(t *testing.T) { - try(t, "banana", "service1", consul.ExampleOperatorToken14, true) - }) + // ACLs with non-"default" token + + t.Run("acl:enable token:other ns:unset", func(t *testing.T) { + err := namespaceCheck("", withOther) + require.EqualError(t, err, `consul ACL token requires using namespace "other"`) + }) + + t.Run("acl:enable token:other ns:default", func(t *testing.T) { + err := namespaceCheck("default", withOther) + require.EqualError(t, err, `consul ACL token cannot use namespace "default"`) + }) + + t.Run("acl:enable token:other ns:other", func(t *testing.T) { + err := namespaceCheck("other", withOther) + require.NoError(t, err) }) } @@ -600,55 +640,3 @@ func TestConsulPolicy_allowKeystoreRead(t *testing.T) { 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 864d8ba8f76f..f1561d651a66 100644 --- a/nomad/consul_test.go +++ b/nomad/consul_test.go @@ -343,133 +343,3 @@ func TestConsulACLsAPI_Stop(t *testing.T) { }) require.Error(t, err) } - -func TestConsulACLsAPI_CheckPermissions(t *testing.T) { - t.Parallel() - - try := func(t *testing.T, namespace string, usage *structs.ConsulUsage, secretID string, exp error) { - logger := testlog.HCLogger(t) - aclAPI := consul.NewMockACLsAPI(logger) - cAPI := NewConsulACLsAPI(aclAPI, logger, nil) - - err := cAPI.CheckPermissions(context.Background(), namespace, usage, secretID) - if exp == nil { - require.NoError(t, err) - } else { - require.Equal(t, exp.Error(), err.Error()) - } - } - - t.Run("check-permissions kv read", func(t *testing.T) { - t.Run("uses kv has permission", func(t *testing.T) { - u := &structs.ConsulUsage{KV: true} - try(t, "default", u, consul.ExampleOperatorTokenID5, nil) - }) - - t.Run("uses kv without permission", func(t *testing.T) { - u := &structs.ConsulUsage{KV: true} - try(t, "default", u, consul.ExampleOperatorTokenID1, errors.New("insufficient Consul ACL permissions to use template")) - }) - - t.Run("uses kv no token", func(t *testing.T) { - u := &structs.ConsulUsage{KV: true} - try(t, "default", u, "", errors.New("missing consul token")) - }) - - t.Run("uses kv nonsense token", func(t *testing.T) { - u := &structs.ConsulUsage{KV: true} - try(t, "default", u, "47d33e22-720a-7fe6-7d7f-418bf844a0be", errors.New("unable to read consul token: no such token")) - }) - - t.Run("no kv no token", func(t *testing.T) { - u := &structs.ConsulUsage{KV: false} - try(t, "default", u, "", nil) - }) - - 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(`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"`)) - }) - }) - - t.Run("check-permissions service write", func(t *testing.T) { - usage := &structs.ConsulUsage{Services: []string{"service1"}} - - t.Run("operator has service write", func(t *testing.T) { - try(t, "default", usage, consul.ExampleOperatorTokenID1, nil) - }) - - 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) { - u := &structs.ConsulUsage{Services: []string{"foo-service1"}} - try(t, "default", u, consul.ExampleOperatorTokenID2, nil) - }) - - t.Run("operator has service_prefix write wrong prefix", func(t *testing.T) { - u := &structs.ConsulUsage{Services: []string{"bar-service1"}} - try(t, "default", u, consul.ExampleOperatorTokenID2, errors.New(`insufficient Consul ACL permissions to write service "bar-service1"`)) - }) - - t.Run("operator permissions insufficient", func(t *testing.T) { - try(t, "default", usage, consul.ExampleOperatorTokenID3, errors.New(`insufficient Consul ACL permissions to write service "service1"`)) - }) - - t.Run("operator provided no token", func(t *testing.T) { - try(t, "default", usage, "", errors.New("missing consul token")) - }) - - t.Run("operator provided nonsense token", func(t *testing.T) { - try(t, "default", usage, "f1682bde-1e71-90b1-9204-85d35467ba61", errors.New("unable to read consul token: no such token")) - }) - }) - - t.Run("check-permissions connect service identity write", func(t *testing.T) { - usage := &structs.ConsulUsage{Kinds: []structs.TaskKind{structs.NewTaskKind(structs.ConnectProxyPrefix, "service1")}} - - t.Run("operator has service write", func(t *testing.T) { - try(t, "default", usage, consul.ExampleOperatorTokenID1, nil) - }) - - t.Run("operator has service write wrong ns", func(t *testing.T) { - 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) { - u := &structs.ConsulUsage{Kinds: []structs.TaskKind{structs.NewTaskKind(structs.ConnectProxyPrefix, "foo-service1")}} - try(t, "default", u, consul.ExampleOperatorTokenID2, nil) - }) - - t.Run("operator has service_prefix write wrong prefix", func(t *testing.T) { - u := &structs.ConsulUsage{Kinds: []structs.TaskKind{structs.NewTaskKind(structs.ConnectProxyPrefix, "bar-service1")}} - try(t, "default", u, consul.ExampleOperatorTokenID2, errors.New(`insufficient Consul ACL permissions to write Connect service "bar-service1"`)) - }) - - t.Run("operator permissions insufficient", func(t *testing.T) { - try(t, "default", usage, consul.ExampleOperatorTokenID3, errors.New(`insufficient Consul ACL permissions to write Connect service "service1"`)) - }) - - t.Run("operator provided no token", func(t *testing.T) { - try(t, "default", usage, "", errors.New("missing consul token")) - }) - - t.Run("operator provided nonsense token", func(t *testing.T) { - try(t, "default", usage, "f1682bde-1e71-90b1-9204-85d35467ba61", errors.New("unable to read consul token: no such token")) - }) - }) -} diff --git a/nomad/job_endpoint_oss_test.go b/nomad/job_endpoint_oss_test.go new file mode 100644 index 000000000000..24ac312aaa14 --- /dev/null +++ b/nomad/job_endpoint_oss_test.go @@ -0,0 +1,332 @@ +// +build !ent + +package nomad + +import ( + "testing" + + "github.com/hashicorp/go-memdb" + msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper" + "github.com/hashicorp/nomad/helper/uuid" + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/require" +) + +// TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse asserts that a job +// submission fails allow_unauthenticated is false, and either an invalid or no +// operator Consul token is provided. +func TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse_oss(t *testing.T) { + t.Parallel() + + s1, cleanupS1 := TestServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + c.ConsulConfig.AllowUnauthenticated = helper.BoolToPtr(false) + }) + defer cleanupS1() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + newJob := func(namespace string) *structs.Job { + // Create the register request + job := mock.Job() + job.TaskGroups[0].Networks[0].Mode = "bridge" + job.TaskGroups[0].Services = []*structs.Service{ + { + Name: "service1", // matches consul.ExamplePolicyID1 + PortLabel: "8080", + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{}, + }, + }, + } + // For this test we only care about authorizing the connect service + job.TaskGroups[0].Tasks[0].Services = nil + + // If testing with a Consul namespace, set it on the group + if namespace != "" { + job.TaskGroups[0].Consul = &structs.Consul{ + Namespace: namespace, + } + } + return job + } + + newRequest := func(job *structs.Job) *structs.JobRegisterRequest { + return &structs.JobRegisterRequest{ + Job: job, + WriteRequest: structs.WriteRequest{ + Region: "global", + Namespace: job.Namespace, + }, + } + } + + noTokenOnJob := func(t *testing.T, job *structs.Job) { + fsmState := s1.State() + ws := memdb.NewWatchSet() + storedJob, err := fsmState.JobByID(ws, job.Namespace, job.ID) + require.NoError(t, err) + require.NotNil(t, storedJob) + require.Empty(t, storedJob.ConsulToken) + } + + // Non-sense Consul ACL tokens that should be rejected + missingToken := "" + fakeToken := uuid.Generate() + + // Consul ACL tokens in no Consul namespace + ossTokenNoPolicyNoNS := consul.ExampleOperatorTokenID3 + ossTokenNoNS := consul.ExampleOperatorTokenID1 + + // Consul ACL tokens in "default" Consul namespace + entTokenNoPolicyDefaultNS := consul.ExampleOperatorTokenID20 + entTokenDefaultNS := consul.ExampleOperatorTokenID21 + + // Consul ACL tokens in "banana" Consul namespace + entTokenNoPolicyBananaNS := consul.ExampleOperatorTokenID10 + entTokenBananaNS := consul.ExampleOperatorTokenID11 + + t.Run("group consul namespace unset", func(t *testing.T) { + // When the group namespace is unset (which is always the case with + // Nomad OSS), Consul tokens with no namespace or are in the "default" + // namespace should be accepted (assuming a sufficient service policy). + namespace := "" + + t.Run("no token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = missingToken + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, "job-submitter consul token denied: missing consul token") + }) + + t.Run("unknown token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = fakeToken + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, "job-submitter consul token denied: unable to read consul token: no such token") + }) + + t.Run("unauthorized oss token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = ossTokenNoPolicyNoNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) + }) + + t.Run("authorized oss token provided", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = ossTokenNoNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.NoError(t, err) + noTokenOnJob(t, job) + }) + + t.Run("unauthorized token in default namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenNoPolicyDefaultNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) + }) + + t.Run("authorized token in default namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenDefaultNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.NoError(t, err) + noTokenOnJob(t, job) + }) + + t.Run("unauthorized token in banana namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenNoPolicyBananaNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: consul ACL token requires using namespace "banana"`) + }) + + t.Run("authorized token in banana namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenBananaNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: consul ACL token requires using namespace "banana"`) + }) + }) + + t.Run("group consul namespace banana", func(t *testing.T) { + // Nomad OSS does not respect setting the consul namespace field on the group, + // and for backwards compatibility accepts tokens in the "default" namespace + // for groups with no namespace set. The net result is setting the group namespace + // to something like "banana" and using a token in "default" namespace will + // be accepted in Nomad OSS (assuming sufficient service write policy). + // + // Using a Consul token in the non-"default" namespace will always fail in + // Nomad OSS, again because the group namespace is ignored. + namespace := "banana" + + t.Run("no token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = missingToken + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, "job-submitter consul token denied: missing consul token") + }) + + t.Run("unknown token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = fakeToken + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, "job-submitter consul token denied: unable to read consul token: no such token") + }) + + t.Run("unauthorized oss token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = ossTokenNoPolicyNoNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) + }) + + t.Run("authorized oss token provided", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = ossTokenNoNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.NoError(t, err) + noTokenOnJob(t, job) + }) + + t.Run("unauthorized token in default namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenNoPolicyDefaultNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) + }) + + t.Run("authorized token in default namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenDefaultNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.NoError(t, err) + noTokenOnJob(t, job) + }) + + // Consul token in custom namespace will always fail in nomad oss + + t.Run("unauthorized token in banana namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenNoPolicyBananaNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: consul ACL token requires using namespace "banana"`) + }) + + t.Run("authorized token in banana namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenBananaNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: consul ACL token requires using namespace "banana"`) + }) + }) + + t.Run("group consul namespace default", func(t *testing.T) { + // Nomad OSS ignores the group consul namespace, and setting it as default + // should effectively be the same as leaving it unset. + namespace := "default" + + t.Run("no token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = missingToken + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, "job-submitter consul token denied: missing consul token") + }) + + t.Run("unknown token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = fakeToken + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, "job-submitter consul token denied: unable to read consul token: no such token") + }) + + t.Run("unauthorized oss token provided", func(t *testing.T) { + request := newRequest(newJob(namespace)) + request.Job.ConsulToken = ossTokenNoPolicyNoNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) + }) + + t.Run("authorized oss token provided", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = ossTokenNoNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.NoError(t, err) + noTokenOnJob(t, job) + }) + + t.Run("unauthorized token in default namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenNoPolicyDefaultNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) + }) + + t.Run("authorized token in default namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenDefaultNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.NoError(t, err) + noTokenOnJob(t, job) + }) + + t.Run("unauthorized token in banana namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenNoPolicyBananaNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: consul ACL token requires using namespace "banana"`) + }) + + t.Run("authorized token in banana namespace", func(t *testing.T) { + job := newJob(namespace) + request := newRequest(job) + request.Job.ConsulToken = entTokenBananaNS + var response structs.JobRegisterResponse + err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) + require.EqualError(t, err, `job-submitter consul token denied: consul ACL token requires using namespace "banana"`) + }) + }) +} diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 010b05784258..889dacba1212 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/require" "github.com/hashicorp/nomad/acl" - "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" @@ -703,95 +702,6 @@ func TestJobEndpoint_Register_Connect_ValidatesWithoutSidecarTask(t *testing.T) require.Contains(t, err.Error(), "exposed_no_sidecar requires use of sidecar_proxy") } -// TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse asserts that a job -// submission fails allow_unauthenticated is false, and either an invalid or no -// operator Consul token is provided. -func TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse(t *testing.T) { - t.Parallel() - - s1, cleanupS1 := TestServer(t, func(c *Config) { - c.NumSchedulers = 0 // Prevent automatic dequeue - c.ConsulConfig.AllowUnauthenticated = helper.BoolToPtr(false) - }) - defer cleanupS1() - codec := rpcClient(t, s1) - testutil.WaitForLeader(t, s1.RPC) - - // Create the register request - job := mock.Job() - job.TaskGroups[0].Networks[0].Mode = "bridge" - job.TaskGroups[0].Services = []*structs.Service{ - { - Name: "service1", // matches consul.ExamplePolicyID1 - PortLabel: "8080", - Connect: &structs.ConsulConnect{ - SidecarService: &structs.ConsulSidecarService{}, - }, - }, - } - - // For this test we only care about authorizing the connect service - job.TaskGroups[0].Tasks[0].Services = nil - - newRequest := func(job *structs.Job) *structs.JobRegisterRequest { - return &structs.JobRegisterRequest{ - Job: job, - WriteRequest: structs.WriteRequest{ - Region: "global", - Namespace: job.Namespace, - }, - } - } - - noTokenOnJob := func(t *testing.T) { - fsmState := s1.State() - ws := memdb.NewWatchSet() - storedJob, err := fsmState.JobByID(ws, job.Namespace, job.ID) - require.NoError(t, err) - require.NotNil(t, storedJob) - require.Empty(t, storedJob.ConsulToken) - } - - // Each variation of the provided Consul operator token - noOpToken := "" - unrecognizedOpToken := uuid.Generate() - unauthorizedOpToken := consul.ExampleOperatorTokenID3 - authorizedOpToken := consul.ExampleOperatorTokenID1 - - t.Run("no token provided", func(t *testing.T) { - request := newRequest(job) - request.Job.ConsulToken = noOpToken - var response structs.JobRegisterResponse - err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) - require.EqualError(t, err, "job-submitter consul token denied: missing consul token") - }) - - t.Run("unknown token provided", func(t *testing.T) { - request := newRequest(job) - request.Job.ConsulToken = unrecognizedOpToken - var response structs.JobRegisterResponse - err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) - require.EqualError(t, err, "job-submitter consul token denied: unable to read consul token: no such token") - }) - - t.Run("unauthorized token provided", func(t *testing.T) { - request := newRequest(job) - request.Job.ConsulToken = unauthorizedOpToken - var response structs.JobRegisterResponse - err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) - require.EqualError(t, err, `job-submitter consul token denied: insufficient Consul ACL permissions to write service "service1"`) - }) - - t.Run("authorized token provided", func(t *testing.T) { - request := newRequest(job) - request.Job.ConsulToken = authorizedOpToken - var response structs.JobRegisterResponse - err := msgpackrpc.CallWithCodec(codec, "Job.Register", request, &response) - require.NoError(t, err) - noTokenOnJob(t) - }) -} - func TestJobEndpoint_Register_ACL(t *testing.T) { t.Parallel()