Skip to content

Commit

Permalink
Fixes Consul token checking when policies exist within namespaces (#1…
Browse files Browse the repository at this point in the history 
…8516)

* e2e/connect: adds test for namespace policies

* consul: use token namespace when fetching policies

* changelog

* fixup! e2e/connect: adds test for namespace policies
  • Loading branch information
@t-davies
t-davies authored Dec 11, 2023
1 parent 268e92e commit c983a8f
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 10 deletions.
3 changes: 3 additions & 0 deletions .changelog/18516.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
consul: uses token namespace to fetch policies for verification
```
81 changes: 77 additions & 4 deletions e2e/connect/acls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type ConnectACLsE2ETest struct {
jobIDs []string
consulPolicyIDs []string
consulTokenIDs []string
consulNamespace string
}

func (tc *ConnectACLsE2ETest) BeforeAll(f *framework.F) {
Expand Down Expand Up @@ -72,14 +73,28 @@ 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.consulManagementToken})
_, err := tc.Consul().ACL().TokenDelete(id, &consulapi.WriteOptions{
Token: tc.consulManagementToken,
Namespace: tc.consulNamespace,
})
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.consulManagementToken})
_, err := tc.Consul().ACL().PolicyDelete(id, &consulapi.WriteOptions{
Token: tc.consulManagementToken,
Namespace: tc.consulNamespace,
})
f.NoError(err)
}

if tc.consulNamespace != "" {
t.Log("cleanup: delete consul namespace:", tc.consulNamespace)
_, err := tc.Consul().Namespaces().Delete(tc.consulNamespace, &consulapi.WriteOptions{
Token: tc.consulManagementToken,
})
f.NoError(err)
}

Expand All @@ -98,12 +113,14 @@ func (tc *ConnectACLsE2ETest) AfterEach(f *framework.F) {
tc.jobIDs = []string{}
tc.consulTokenIDs = []string{}
tc.consulPolicyIDs = []string{}
tc.consulNamespace = ""
}

// todo(shoenig): follow up refactor with e2eutil.ConsulPolicy
type consulPolicy struct {
Name string // e.g. nomad-operator
Rules string // e.g. service "" { policy="write" }
Name string // e.g. nomad-operator
Rules string // e.g. service "" { policy="write" }
Namespace string // e.g. default
}

// todo(shoenig): follow up refactor with e2eutil.ConsulPolicy
Expand All @@ -112,17 +129,31 @@ func (tc *ConnectACLsE2ETest) createConsulPolicy(p consulPolicy, f *framework.F)
Name: p.Name,
Description: "test policy " + p.Name,
Rules: p.Rules,
Namespace: p.Namespace,
}, &consulapi.WriteOptions{Token: tc.consulManagementToken})
f.NoError(err, "failed to create consul policy")
tc.consulPolicyIDs = append(tc.consulPolicyIDs, result.ID)
return result.ID
}

func (tc *ConnectACLsE2ETest) createConsulNamespace(namespace string, f *framework.F) string {
result, _, err := tc.Consul().Namespaces().Create(&consulapi.Namespace{
Name: namespace,
}, &consulapi.WriteOptions{Token: tc.consulManagementToken})
f.NoError(err, "failed to create consul namespace")
return result.Name
}

// todo(shoenig): follow up refactor with e2eutil.ConsulPolicy
func (tc *ConnectACLsE2ETest) createOperatorToken(policyID string, f *framework.F) string {
return tc.createOperatorTokenNamespaced(policyID, "default", f)
}

func (tc *ConnectACLsE2ETest) createOperatorTokenNamespaced(policyID string, namespace string, f *framework.F) string {
token, _, err := tc.Consul().ACL().TokenCreate(&consulapi.ACLToken{
Description: "operator token",
Policies: []*consulapi.ACLTokenPolicyLink{{ID: policyID}},
Namespace: namespace,
}, &consulapi.WriteOptions{Token: tc.consulManagementToken})
f.NoError(err, "failed to create operator token")
tc.consulTokenIDs = append(tc.consulTokenIDs, token.AccessorID)
Expand Down Expand Up @@ -250,6 +281,48 @@ func (tc *ConnectACLsE2ETest) TestConnectACLsConnectDemo(f *framework.F) {
t.Log("connect legacy job with ACLs enable finished")
}

func (tc *ConnectACLsE2ETest) TestConnectACLsConnectDemoNamespaced(f *framework.F) {
t := f.T()

t.Log("test register Connect job w/ ACLs enabled w/ operator token")

// === Setup ACL policy within a namespace and mint Operator token ===

// create a namespace
namespace := tc.createConsulNamespace("ns-"+uuid.Short(), f)
tc.consulNamespace = namespace
t.Log("created namespace:", namespace)

// create a policy allowing writes of services "count-api" and "count-dashboard"
policyID := tc.createConsulPolicy(consulPolicy{
Name: "nomad-operator-policy-" + uuid.Short(),
Rules: `service "count-api" { policy = "write" } service "count-dashboard" { policy = "write" }`,
Namespace: namespace,
}, f)
t.Log("created operator policy:", policyID)

// create a Consul "operator token" blessed with the above policy
operatorToken := tc.createOperatorTokenNamespaced(policyID, namespace, f)
t.Log("created operator token:", operatorToken)

jobID := connectJobID()
tc.jobIDs = append(tc.jobIDs, jobID)

allocs := e2eutil.RegisterAndWaitForAllocs(t, tc.Nomad(), demoConnectJob, jobID, operatorToken)
f.Equal(2, len(allocs), "expected 2 allocs for connect demo", allocs)
allocIDs := e2eutil.AllocIDsFromAllocationListStubs(allocs)
f.Equal(2, len(allocIDs), "expected 2 allocIDs for connect demo", allocIDs)
e2eutil.WaitForAllocsRunning(t, tc.Nomad(), allocIDs)

// === Check Consul SI tokens were generated for sidecars ===
foundSITokens := tc.countSITokens(t)
f.Equal(2, len(foundSITokens), "expected 2 SI tokens total: %v", foundSITokens)
f.Equal(1, foundSITokens["connect-proxy-count-api"], "expected 1 SI token for connect-proxy-count-api: %v", foundSITokens)
f.Equal(1, foundSITokens["connect-proxy-count-dashboard"], "expected 1 SI token for connect-proxy-count-dashboard: %v", foundSITokens)

t.Log("connect legacy job with ACLs enable finished")
}

func (tc *ConnectACLsE2ETest) TestConnectACLsConnectNativeDemo(f *framework.F) {
t := f.T()

Expand Down
15 changes: 9 additions & 6 deletions nomad/consul_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (c *consulACLsAPI) canReadKeystore(namespace string, token *api.ACLToken) (

// check each policy directly attached to the token
for _, policyRef := range token.Policies {
if allowable, err := c.policyAllowsKeystoreRead(matches, namespace, policyRef.ID); err != nil {
if allowable, err := c.policyAllowsKeystoreRead(matches, namespace, policyRef.ID, token.Namespace); err != nil {
return false, err
} else if allowable {
return true, nil
Expand All @@ -133,13 +133,14 @@ func (c *consulACLsAPI) canReadKeystore(namespace string, token *api.ACLToken) (
for _, roleLink := range token.Roles {
role, _, err := c.aclClient.RoleRead(roleLink.ID, &api.QueryOptions{
AllowStale: false,
Namespace: token.Namespace,
})
if err != nil {
return false, err
}

for _, policyLink := range role.Policies {
allowable, err := c.policyAllowsKeystoreRead(matches, namespace, policyLink.ID)
allowable, err := c.policyAllowsKeystoreRead(matches, namespace, policyLink.ID, token.Namespace)
if err != nil {
return false, err
} else if allowable {
Expand Down Expand Up @@ -173,7 +174,7 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC

// check each policy directly attached to the token
for _, policyRef := range token.Policies {
if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID); err != nil {
if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID, token.Namespace); err != nil {
return false, err
} else if allowable {
return true, nil
Expand All @@ -190,7 +191,7 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC
}

for _, policyLink := range role.Policies {
allowable, wErr := c.policyAllowsServiceWrite(matches, namespace, service, policyLink.ID)
allowable, wErr := c.policyAllowsServiceWrite(matches, namespace, service, policyLink.ID, token.Namespace)
if wErr != nil {
return false, wErr
} else if allowable {
Expand All @@ -202,9 +203,10 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC
return false, nil
}

func (c *consulACLsAPI) policyAllowsServiceWrite(matches bool, namespace, service string, policyID string) (bool, error) {
func (c *consulACLsAPI) policyAllowsServiceWrite(matches bool, namespace, service string, policyID string, tokenNamespace string) (bool, error) {
policy, _, err := c.aclClient.PolicyRead(policyID, &api.QueryOptions{
AllowStale: false,
Namespace: tokenNamespace,
})
if err != nil {
return false, err
Expand Down Expand Up @@ -287,9 +289,10 @@ func (cp *ConsulPolicy) allowsServiceWrite(matches bool, namespace, task string)
return false
}

func (c *consulACLsAPI) policyAllowsKeystoreRead(matches bool, namespace, policyID string) (bool, error) {
func (c *consulACLsAPI) policyAllowsKeystoreRead(matches bool, namespace, policyID string, tokenNamespace string) (bool, error) {
policy, _, err := c.aclClient.PolicyRead(policyID, &api.QueryOptions{
AllowStale: false,
Namespace: tokenNamespace,
})
if err != nil {
return false, err
Expand Down

0 comments on commit c983a8f

Please sign in to comment.