Skip to content

Commit

Permalink
acl: ACL Tokens can now be assigned an optional set of service identi…
Browse files Browse the repository at this point in the history
…ties

These act like a special cased version of a Policy Template for granting
a token the privileges necessary to register a service and its connect
proxy, and read upstreams from the catalog.
  • Loading branch information
rboyer committed Mar 1, 2019
1 parent 8e9b063 commit 024c5e6
Show file tree
Hide file tree
Showing 14 changed files with 740 additions and 83 deletions.
105 changes: 97 additions & 8 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 @@ -88,7 +89,6 @@ func IsACLRemoteError(err error) bool {
type ACLResolverDelegate interface {
ACLsEnabled() bool
ACLDatacenter(legacy bool) string
// UseLegacyACLs
UseLegacyACLs() bool
ResolveIdentityFromToken(token string) (bool, structs.ACLIdentity, error)
ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy, error)
Expand Down Expand Up @@ -148,7 +148,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 @@ -607,7 +607,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 @@ -617,9 +619,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 @@ -663,7 +752,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
}

fetchIDs := missing
Expand Down Expand Up @@ -704,7 +793,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
}

for i := 0; i < len(fetchIDs); i++ {
Expand Down Expand Up @@ -733,7 +822,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
)

const (
// maxTokenExpirationDuration is the maximum difference allowed between
Expand Down Expand Up @@ -282,10 +286,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 @@ -457,6 +462,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 @@ -494,6 +511,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
128 changes: 128 additions & 0 deletions agent/consul/acl_endpoint_test.go
Expand Up @@ -944,6 +944,134 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
require.Equal(t, policy2.Name, token.Policies[1].Name)
})

clock.Reset()

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")
})

clock.Reset()

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)
})

clock.Reset()

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)
}
})

clock.Reset()
}

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")
})

clock.Reset()

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

0 comments on commit 024c5e6

Please sign in to comment.