Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

acl: ACL Tokens can now be assigned an optional set of service identities #5390

Merged
merged 1 commit into from Apr 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
104 changes: 97 additions & 7 deletions agent/consul/acl.go
Expand Up @@ -4,10 +4,11 @@ import (
"fmt"
"log"
"os"
"sort"
"sync"
"time"

"github.com/armon/go-metrics"
metrics "github.com/armon/go-metrics"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
Expand Down Expand Up @@ -133,7 +134,7 @@ type ACLResolverConfig struct {
// - Resolving policies remotely via an ACL.PolicyResolve RPC
//
// Remote Resolution:
// Remote resolution can be done syncrhonously or asynchronously depending
// Remote resolution can be done synchronously or asynchronously depending
// on the ACLDownPolicy in the Config passed to the resolver.
//
// When the down policy is set to async-cache and we have already cached values
Expand Down Expand Up @@ -503,7 +504,9 @@ func (r *ACLResolver) filterPoliciesByScope(policies structs.ACLPolicies) struct

func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (structs.ACLPolicies, error) {
policyIDs := identity.PolicyIDs()
if len(policyIDs) == 0 {
serviceIdentities := identity.ServiceIdentityList()

if len(policyIDs) == 0 && len(serviceIdentities) == 0 {
policy := identity.EmbeddedPolicy()
if policy != nil {
return []*structs.ACLPolicy{policy}, nil
Expand All @@ -513,9 +516,96 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (
return nil, nil
}

syntheticPolicies := r.synthesizePoliciesForServiceIdentities(serviceIdentities)

// For the new ACLs policy replication is mandatory for correct operation on servers. Therefore
// we only attempt to resolve policies locally
policies := make([]*structs.ACLPolicy, 0, len(policyIDs))
policies, err := r.collectPoliciesForIdentity(identity, policyIDs, len(syntheticPolicies))
if err != nil {
return nil, err
}

policies = append(policies, syntheticPolicies...)
filtered := r.filterPoliciesByScope(policies)
return filtered, nil
}

func (r *ACLResolver) synthesizePoliciesForServiceIdentities(serviceIdentities []*structs.ACLServiceIdentity) []*structs.ACLPolicy {
if len(serviceIdentities) == 0 {
return nil
}

// Collect and dedupe service identities. Prefer increasing datacenter scope.
serviceIdentities = dedupeServiceIdentities(serviceIdentities)

syntheticPolicies := make([]*structs.ACLPolicy, 0, len(serviceIdentities))
for _, s := range serviceIdentities {
syntheticPolicies = append(syntheticPolicies, s.SyntheticPolicy())
}

return syntheticPolicies
}

func dedupeServiceIdentities(in []*structs.ACLServiceIdentity) []*structs.ACLServiceIdentity {
// From: https://github.com/golang/go/wiki/SliceTricks#in-place-deduplicate-comparable

if len(in) <= 1 {
return in
}

sort.Slice(in, func(i, j int) bool {
return in[i].ServiceName < in[j].ServiceName
})

j := 0
for i := 1; i < len(in); i++ {
if in[j].ServiceName == in[i].ServiceName {
// Prefer increasing scope.
if len(in[j].Datacenters) == 0 || len(in[i].Datacenters) == 0 {
in[j].Datacenters = nil
} else {
in[j].Datacenters = mergeStringSlice(in[j].Datacenters, in[i].Datacenters)
}
continue
}
j++
in[j] = in[i]
}

// Discard the skipped items.
for i := j + 1; i < len(in); i++ {
in[i] = nil
}

return in[:j+1]
}

func mergeStringSlice(a, b []string) []string {
out := make([]string, 0, len(a)+len(b))
out = append(out, a...)
out = append(out, b...)
return dedupeStringSlice(out)
}

func dedupeStringSlice(in []string) []string {
// From: https://github.com/golang/go/wiki/SliceTricks#in-place-deduplicate-comparable

sort.Strings(in)

j := 0
for i := 1; i < len(in); i++ {
if in[j] == in[i] {
continue
}
j++
in[j] = in[i]
}

return in[:j+1]
}

func (r *ACLResolver) collectPoliciesForIdentity(identity structs.ACLIdentity, policyIDs []string, extraCap int) ([]*structs.ACLPolicy, error) {
policies := make([]*structs.ACLPolicy, 0, len(policyIDs)+extraCap)

// Get all associated policies
var missing []string
Expand Down Expand Up @@ -559,7 +649,7 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (

// Hot-path if we have no missing or expired policies
if len(missing)+len(expired) == 0 {
return r.filterPoliciesByScope(policies), nil
return policies, nil
}

hasMissing := len(missing) > 0
Expand All @@ -579,7 +669,7 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (
if !waitForResult {
// waitForResult being false requires that all the policies were cached already
policies = append(policies, expired...)
return r.filterPoliciesByScope(policies), nil
return policies, nil
}

res := <-waitChan
Expand All @@ -596,7 +686,7 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (
}
}

return r.filterPoliciesByScope(policies), nil
return policies, nil
}

func (r *ACLResolver) resolveTokenToPolicies(token string) (structs.ACLPolicies, error) {
Expand Down
44 changes: 36 additions & 8 deletions agent/consul/acl_endpoint.go
Expand Up @@ -8,13 +8,13 @@ import (
"regexp"
"time"

"github.com/armon/go-metrics"
metrics "github.com/armon/go-metrics"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/consul/state"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/go-memdb"
"github.com/hashicorp/go-uuid"
memdb "github.com/hashicorp/go-memdb"
uuid "github.com/hashicorp/go-uuid"
)

const (
Expand All @@ -24,7 +24,11 @@ const (
)

// Regex for matching
var validPolicyName = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,128}$`)
var (
validPolicyName = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,128}$`)
validServiceIdentityName = regexp.MustCompile(`^[a-z0-9]([a-z0-9\-_]*[a-z0-9])?$`)
serviceIdentityNameMaxLength = 256
)

// ACL endpoint is used to manipulate ACLs
type ACL struct {
Expand Down Expand Up @@ -275,10 +279,11 @@ func (a *ACL) TokenClone(args *structs.ACLTokenSetRequest, reply *structs.ACLTok
cloneReq := structs.ACLTokenSetRequest{
Datacenter: args.Datacenter,
ACLToken: structs.ACLToken{
Policies: token.Policies,
Local: token.Local,
Description: token.Description,
ExpirationTime: token.ExpirationTime,
Policies: token.Policies,
ServiceIdentities: token.ServiceIdentities,
Local: token.Local,
Description: token.Description,
ExpirationTime: token.ExpirationTime,
},
WriteRequest: args.WriteRequest,
}
Expand Down Expand Up @@ -450,6 +455,18 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
}
token.Policies = policies

for _, svcid := range token.ServiceIdentities {
if svcid.ServiceName == "" {
return fmt.Errorf("Service identity is missing the service name field on this token")
}
if token.Local && len(svcid.Datacenters) > 0 {
return fmt.Errorf("Service identity %q cannot specify a list of datacenters on a local token", svcid.ServiceName)
}
if !isValidServiceIdentityName(svcid.ServiceName) {
return fmt.Errorf("Service identity %q has an invalid name. Only alphanumeric characters, '-' and '_' are allowed", svcid.ServiceName)
}
}

if token.Rules != "" {
return fmt.Errorf("Rules cannot be specified for this token")
}
Expand Down Expand Up @@ -487,6 +504,17 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
return nil
}

// isValidServiceIdentityName returns true if the provided name can be used as
// an ACLServiceIdentity ServiceName. This is more restrictive than standard
// catalog registration, which basically takes the view that "everything is
// valid".
func isValidServiceIdentityName(name string) bool {
if len(name) < 1 || len(name) > serviceIdentityNameMaxLength {
return false
}
return validServiceIdentityName.MatchString(name)
}

func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) error {
if err := a.aclPreCheck(); err != nil {
return err
Expand Down
118 changes: 118 additions & 0 deletions agent/consul/acl_endpoint_test.go
Expand Up @@ -919,6 +919,124 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
require.Len(t, token.Policies, 0)
})

t.Run("Create it with invalid service identity (empty)", func(t *testing.T) {
req := structs.ACLTokenSetRequest{
Datacenter: "dc1",
ACLToken: structs.ACLToken{
Description: "foobar",
Policies: nil,
Local: false,
ServiceIdentities: []*structs.ACLServiceIdentity{
&structs.ACLServiceIdentity{ServiceName: ""},
},
},
WriteRequest: structs.WriteRequest{Token: "root"},
}

resp := structs.ACLToken{}

err := acl.TokenSet(&req, &resp)
requireErrorContains(t, err, "Service identity is missing the service name field")
})

t.Run("Create it with invalid service identity (too large)", func(t *testing.T) {
long := strings.Repeat("x", serviceIdentityNameMaxLength+1)
req := structs.ACLTokenSetRequest{
Datacenter: "dc1",
ACLToken: structs.ACLToken{
Description: "foobar",
Policies: nil,
Local: false,
ServiceIdentities: []*structs.ACLServiceIdentity{
&structs.ACLServiceIdentity{ServiceName: long},
},
},
WriteRequest: structs.WriteRequest{Token: "root"},
}

resp := structs.ACLToken{}

err := acl.TokenSet(&req, &resp)
require.NotNil(t, err)
})

for _, test := range []struct {
name string
ok bool
}{
{"-abc", false},
{"abc-", false},
{"a-bc", true},
{"_abc", false},
{"abc_", false},
{"a_bc", true},
{":abc", false},
{"abc:", false},
{"a:bc", false},
{"Abc", false},
{"aBc", false},
{"abC", false},
{"0abc", true},
{"abc0", true},
{"a0bc", true},
} {
var testName string
if test.ok {
testName = "Create it with valid service identity (by regex): " + test.name
} else {
testName = "Create it with invalid service identity (by regex): " + test.name
}
t.Run(testName, func(t *testing.T) {
req := structs.ACLTokenSetRequest{
Datacenter: "dc1",
ACLToken: structs.ACLToken{
Description: "foobar",
Policies: nil,
Local: false,
ServiceIdentities: []*structs.ACLServiceIdentity{
&structs.ACLServiceIdentity{ServiceName: test.name},
},
},
WriteRequest: structs.WriteRequest{Token: "root"},
}

resp := structs.ACLToken{}

err := acl.TokenSet(&req, &resp)
if test.ok {
require.NoError(t, err)

// Get the token directly to validate that it exists
tokenResp, err := retrieveTestToken(codec, "root", "dc1", resp.AccessorID)
require.NoError(t, err)
token := tokenResp.Token
require.ElementsMatch(t, req.ACLToken.ServiceIdentities, token.ServiceIdentities)
} else {
require.NotNil(t, err)
}
})
}

t.Run("Create it with invalid service identity (datacenters set on local token)", func(t *testing.T) {
req := structs.ACLTokenSetRequest{
Datacenter: "dc1",
ACLToken: structs.ACLToken{
Description: "foobar",
Policies: nil,
Local: true,
ServiceIdentities: []*structs.ACLServiceIdentity{
&structs.ACLServiceIdentity{ServiceName: "foo", Datacenters: []string{"dc2"}},
},
},
WriteRequest: structs.WriteRequest{Token: "root"},
}

resp := structs.ACLToken{}

err := acl.TokenSet(&req, &resp)
requireErrorContains(t, err, "cannot specify a list of datacenters on a local token")
})

for _, test := range []struct {
name string
offset time.Duration
Expand Down