From 0ed6b1bb18da8a8487132627241803057addf501 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Sat, 10 Dec 2016 11:19:08 -0800 Subject: [PATCH 1/9] Bans anonymous queries that aren't tied to a session. This gets us coverage of PQ creation under the existing service policy or the soon-to-be-added session policy. --- api/prepared_query_test.go | 1 + command/agent/dns_test.go | 13 +++ consul/prepared_query_endpoint.go | 12 ++- consul/prepared_query_endpoint_test.go | 129 ++++++++++++++++++------- 4 files changed, 118 insertions(+), 37 deletions(-) diff --git a/api/prepared_query_test.go b/api/prepared_query_test.go index d16f007c9d..c9adb5b2c2 100644 --- a/api/prepared_query_test.go +++ b/api/prepared_query_test.go @@ -45,6 +45,7 @@ func TestPreparedQuery(t *testing.T) { // Create a simple prepared query. def := &PreparedQueryDefinition{ + Name: "test", Service: ServiceQuery{ Service: "redis", }, diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index be9c56c2a7..8eabfa804f 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -569,6 +569,7 @@ func TestDNS_ServiceLookup(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -1021,6 +1022,7 @@ func TestDNS_ServiceLookup_ServiceAddress(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -1115,6 +1117,7 @@ func TestDNS_ServiceLookup_ServiceAddressIPV6(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -1236,6 +1239,7 @@ func TestDNS_ServiceLookup_WanAddress(t *testing.T) { Datacenter: "dc2", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -1641,6 +1645,7 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -1745,6 +1750,7 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -2040,6 +2046,7 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -2163,6 +2170,7 @@ func TestDNS_ServiceLookup_OnlyFailing(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, @@ -2283,6 +2291,7 @@ func TestDNS_ServiceLookup_OnlyPassing(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", OnlyPassing: true, @@ -2356,6 +2365,7 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "web", }, @@ -2450,6 +2460,7 @@ func TestDNS_ServiceLookup_Truncate(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "web", }, @@ -2770,6 +2781,7 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "search", }, @@ -4342,6 +4354,7 @@ func TestDNS_Compression_Query(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "db", }, diff --git a/consul/prepared_query_endpoint.go b/consul/prepared_query_endpoint.go index 47b6fe1a77..b73a4c848a 100644 --- a/consul/prepared_query_endpoint.go +++ b/consul/prepared_query_endpoint.go @@ -96,7 +96,7 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) // Parse the query and prep it for the state store. switch args.Op { case structs.PreparedQueryCreate, structs.PreparedQueryUpdate: - if err := parseQuery(args.Query); err != nil { + if err := parseQuery(args.Query, p.srv.config.ACLEnforceVersion8); err != nil { return fmt.Errorf("Invalid prepared query: %v", err) } @@ -125,7 +125,7 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string) // update operation. Some of the fields are not checked or are partially // checked, as noted in the comments below. This also updates all the parsed // fields of the query. -func parseQuery(query *structs.PreparedQuery) error { +func parseQuery(query *structs.PreparedQuery, enforceVersion8 bool) error { // We skip a few fields: // - ID is checked outside this fn. // - Name is optional with no restrictions, except for uniqueness which @@ -133,10 +133,16 @@ func parseQuery(query *structs.PreparedQuery) error { // names do not overlap with IDs, which is also checked during the // transaction. Otherwise, people could "steal" queries that they don't // have proper ACL rights to change. - // - Session is optional and checked for integrity during the transaction. // - Template is checked during the transaction since that's where we // compile it. + // Anonymous queries require a session or need to be part of a template. + if enforceVersion8 { + if query.Name == "" && query.Template.Type == "" && query.Session == "" { + return fmt.Errorf("Must be bound to a session") + } + } + // Token is checked when the query is executed, but we do make sure the // user hasn't accidentally pasted-in the special redacted token name, // which if we allowed in would be super hard to debug and understand. diff --git a/consul/prepared_query_endpoint_test.go b/consul/prepared_query_endpoint_test.go index d23b627b9c..f3120892e3 100644 --- a/consul/prepared_query_endpoint_test.go +++ b/consul/prepared_query_endpoint_test.go @@ -32,6 +32,7 @@ func TestPreparedQuery_Apply(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "redis", }, @@ -515,6 +516,7 @@ func TestPreparedQuery_Apply_ForwardLeader(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "redis", }, @@ -531,53 +533,77 @@ func TestPreparedQuery_Apply_ForwardLeader(t *testing.T) { func TestPreparedQuery_parseQuery(t *testing.T) { query := &structs.PreparedQuery{} - err := parseQuery(query) + err := parseQuery(query, true) + if err == nil || !strings.Contains(err.Error(), "Must be bound to a session") { + t.Fatalf("bad: %v", err) + } + + query.Session = "adf4238a-882b-9ddc-4a9d-5b6758e4159e" + err = parseQuery(query, true) if err == nil || !strings.Contains(err.Error(), "Must provide a Service") { t.Fatalf("bad: %v", err) } - query.Service.Service = "foo" - if err := parseQuery(query); err != nil { - t.Fatalf("err: %v", err) - } - - query.Token = redactedToken - err = parseQuery(query) - if err == nil || !strings.Contains(err.Error(), "Bad Token") { + query.Session = "" + query.Template.Type = "some-kind-of-template" + err = parseQuery(query, true) + if err == nil || !strings.Contains(err.Error(), "Must provide a Service") { t.Fatalf("bad: %v", err) } - query.Token = "adf4238a-882b-9ddc-4a9d-5b6758e4159e" - if err := parseQuery(query); err != nil { - t.Fatalf("err: %v", err) - } - - query.Service.Failover.NearestN = -1 - err = parseQuery(query) - if err == nil || !strings.Contains(err.Error(), "Bad NearestN") { + query.Template.Type = "" + err = parseQuery(query, false) + if err == nil || !strings.Contains(err.Error(), "Must provide a Service") { t.Fatalf("bad: %v", err) } - query.Service.Failover.NearestN = 3 - if err := parseQuery(query); err != nil { - t.Fatalf("err: %v", err) - } + // None of the rest of these care about version 8 ACL enforcement. + for _, version8 := range []bool{true, false} { + query = &structs.PreparedQuery{} + query.Session = "adf4238a-882b-9ddc-4a9d-5b6758e4159e" + query.Service.Service = "foo" + if err := parseQuery(query, version8); err != nil { + t.Fatalf("err: %v", err) + } - query.DNS.TTL = "two fortnights" - err = parseQuery(query) - if err == nil || !strings.Contains(err.Error(), "Bad DNS TTL") { - t.Fatalf("bad: %v", err) - } + query.Token = redactedToken + err = parseQuery(query, version8) + if err == nil || !strings.Contains(err.Error(), "Bad Token") { + t.Fatalf("bad: %v", err) + } - query.DNS.TTL = "-3s" - err = parseQuery(query) - if err == nil || !strings.Contains(err.Error(), "must be >=0") { - t.Fatalf("bad: %v", err) - } + query.Token = "adf4238a-882b-9ddc-4a9d-5b6758e4159e" + if err := parseQuery(query, version8); err != nil { + t.Fatalf("err: %v", err) + } - query.DNS.TTL = "3s" - if err := parseQuery(query); err != nil { - t.Fatalf("err: %v", err) + query.Service.Failover.NearestN = -1 + err = parseQuery(query, version8) + if err == nil || !strings.Contains(err.Error(), "Bad NearestN") { + t.Fatalf("bad: %v", err) + } + + query.Service.Failover.NearestN = 3 + if err := parseQuery(query, version8); err != nil { + t.Fatalf("err: %v", err) + } + + query.DNS.TTL = "two fortnights" + err = parseQuery(query, version8) + if err == nil || !strings.Contains(err.Error(), "Bad DNS TTL") { + t.Fatalf("bad: %v", err) + } + + query.DNS.TTL = "-3s" + err = parseQuery(query, version8) + if err == nil || !strings.Contains(err.Error(), "must be >=0") { + t.Fatalf("bad: %v", err) + } + + query.DNS.TTL = "3s" + if err := parseQuery(query, version8); err != nil { + t.Fatalf("err: %v", err) + } } } @@ -917,9 +943,25 @@ func TestPreparedQuery_Get(t *testing.T) { } } + // Create a session. + var session string + { + req := structs.SessionRequest{ + Datacenter: "dc1", + Op: structs.SessionCreate, + Session: structs.Session{ + Node: s1.config.NodeName, + }, + } + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &req, &session); err != nil { + t.Fatalf("err: %v", err) + } + } + // Now update the query to take away its name. query.Op = structs.PreparedQueryUpdate query.Query.Name = "" + query.Query.Session = session if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } @@ -1170,9 +1212,25 @@ func TestPreparedQuery_List(t *testing.T) { } } + // Create a session. + var session string + { + req := structs.SessionRequest{ + Datacenter: "dc1", + Op: structs.SessionCreate, + Session: structs.Session{ + Node: s1.config.NodeName, + }, + } + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &req, &session); err != nil { + t.Fatalf("err: %v", err) + } + } + // Now take away the query name. query.Op = structs.PreparedQueryUpdate query.Query.Name = "" + query.Query.Session = session if err := msgpackrpc.CallWithCodec(codec, "PreparedQuery.Apply", &query, &reply); err != nil { t.Fatalf("err: %v", err) } @@ -1451,6 +1509,7 @@ func TestPreparedQuery_Execute(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "foo", }, @@ -2314,6 +2373,7 @@ func TestPreparedQuery_Execute_ForwardLeader(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Service: "redis", }, @@ -2577,6 +2637,7 @@ func (m *mockQueryServer) ForwardDC(method, dc string, args interface{}, reply i func TestPreparedQuery_queryFailover(t *testing.T) { query := &structs.PreparedQuery{ + Name: "test", Service: structs.ServiceQuery{ Failover: structs.QueryDatacenterOptions{ NearestN: 0, From 0139bbb9634065a4cbed283d8e5826bfb1e2a135 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Sun, 11 Dec 2016 11:24:44 -0800 Subject: [PATCH 2/9] Adds support for a new "acl_agent_token" which is used for internal catalog operations. --- command/agent/agent.go | 7 ++-- command/agent/config.go | 20 ++++++++++++ command/agent/config_test.go | 32 ++++++++++++++++++- command/agent/local.go | 4 +-- consul/config.go | 18 +++++++++++ consul/config_test.go | 20 ++++++++++++ consul/leader.go | 6 ++-- .../source/docs/agent/options.html.markdown | 8 +++++ 8 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 consul/config_test.go diff --git a/command/agent/agent.go b/command/agent/agent.go index 23c7f4d557..7e464b2fa4 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -227,8 +227,8 @@ func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter, Port: agent.config.Ports.Server, Tags: []string{}, } - // TODO (slackpad) - Plumb the "acl_agent_token" into here. - agent.state.AddService(&consulService, "") + + agent.state.AddService(&consulService, agent.config.GetTokenForAgent()) } else { err = agent.setupClient() agent.state.SetIface(agent.client) @@ -364,6 +364,9 @@ func (a *Agent) consulConfig() *consul.Config { if a.config.ACLToken != "" { base.ACLToken = a.config.ACLToken } + if a.config.ACLAgentToken != "" { + base.ACLAgentToken = a.config.ACLAgentToken + } if a.config.ACLMasterToken != "" { base.ACLMasterToken = a.config.ACLMasterToken } diff --git a/command/agent/config.go b/command/agent/config.go index fc80b4bf1a..c4c4c06e29 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -488,6 +488,11 @@ type Config struct { // token is not provided. If not configured the 'anonymous' token is used. ACLToken string `mapstructure:"acl_token" json:"-"` + // ACLAgentToken is the default token used to make requests for the agent + // itself, such as for registering itself with the catalog. If not + // configured, the 'acl_token' will be used. + ACLAgentToken string `mapstructure:"acl_agent_token" json:"-"` + // ACLMasterToken is used to bootstrap the ACL system. It should be specified // on the servers in the ACLDatacenter. When the leader comes online, it ensures // that the Master token is available. This provides the initial token. @@ -756,6 +761,18 @@ func (c *Config) ClientListener(override string, port int) (net.Addr, error) { return &net.TCPAddr{IP: ip, Port: port}, nil } +// GetTokenForAgent returns the token the agent should use for its own internal +// operations, such as registering itself with the catalog. +func (c *Config) GetTokenForAgent() string { + if c.ACLAgentToken != "" { + return c.ACLAgentToken + } else if c.ACLToken != "" { + return c.ACLToken + } else { + return "" + } +} + // DecodeConfig reads the configuration from the given reader in JSON // format and decodes it into a proper Config structure. func DecodeConfig(r io.Reader) (*Config, error) { @@ -1466,6 +1483,9 @@ func MergeConfig(a, b *Config) *Config { if b.ACLToken != "" { result.ACLToken = b.ACLToken } + if b.ACLAgentToken != "" { + result.ACLAgentToken = b.ACLAgentToken + } if b.ACLMasterToken != "" { result.ACLMasterToken = b.ACLMasterToken } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 48a615f974..58f97213b3 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -643,7 +643,7 @@ func TestDecodeConfig(t *testing.T) { } // ACLs - input = `{"acl_token": "1234", "acl_datacenter": "dc2", + input = `{"acl_token": "1234", "acl_agent_token": "5678", "acl_datacenter": "dc2", "acl_ttl": "60s", "acl_down_policy": "deny", "acl_default_policy": "deny", "acl_master_token": "2345", "acl_replication_token": "8675309"}` @@ -655,6 +655,9 @@ func TestDecodeConfig(t *testing.T) { if config.ACLToken != "1234" { t.Fatalf("bad: %#v", config) } + if config.ACLAgentToken != "5678" { + t.Fatalf("bad: %#v", config) + } if config.ACLMasterToken != "2345" { t.Fatalf("bad: %#v", config) } @@ -674,6 +677,32 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + // ACL token precedence. + input = `{}` + config, err = DecodeConfig(bytes.NewReader([]byte(input))) + if err != nil { + t.Fatalf("err: %s", err) + } + if token := config.GetTokenForAgent(); token != "" { + t.Fatalf("bad: %s", token) + } + input = `{"acl_token": "hello"}` + config, err = DecodeConfig(bytes.NewReader([]byte(input))) + if err != nil { + t.Fatalf("err: %s", err) + } + if token := config.GetTokenForAgent(); token != "hello" { + t.Fatalf("bad: %s", token) + } + input = `{"acl_agent_token": "world", "acl_token": "hello"}` + config, err = DecodeConfig(bytes.NewReader([]byte(input))) + if err != nil { + t.Fatalf("err: %s", err) + } + if token := config.GetTokenForAgent(); token != "world" { + t.Fatalf("bad: %s", token) + } + // ACL flag for Consul version 0.8 features (broken out since we will // eventually remove this). We first verify this is opt-out. config = DefaultConfig() @@ -1561,6 +1590,7 @@ func TestMergeConfig(t *testing.T) { CheckUpdateInterval: 8 * time.Minute, CheckUpdateIntervalRaw: "8m", ACLToken: "1234", + ACLAgentToken: "5678", ACLMasterToken: "2345", ACLDatacenter: "dc2", ACLTTL: 15 * time.Second, diff --git a/command/agent/local.go b/command/agent/local.go index 475fe56d9c..c324a15361 100644 --- a/command/agent/local.go +++ b/command/agent/local.go @@ -400,7 +400,7 @@ func (l *localState) setSyncState() error { req := structs.NodeSpecificRequest{ Datacenter: l.config.Datacenter, Node: l.config.NodeName, - QueryOptions: structs.QueryOptions{Token: l.config.ACLToken}, + QueryOptions: structs.QueryOptions{Token: l.config.GetTokenForAgent()}, } var out1 structs.IndexedNodeServices var out2 structs.IndexedHealthChecks @@ -709,7 +709,7 @@ func (l *localState) syncNodeInfo() error { Node: l.config.NodeName, Address: l.config.AdvertiseAddr, TaggedAddresses: l.config.TaggedAddresses, - WriteRequest: structs.WriteRequest{Token: l.config.ACLToken}, + WriteRequest: structs.WriteRequest{Token: l.config.GetTokenForAgent()}, } var out struct{} err := l.iface.RPC("Catalog.Register", &req, &out) diff --git a/consul/config.go b/consul/config.go index 0259b3d31d..c10252ed26 100644 --- a/consul/config.go +++ b/consul/config.go @@ -155,6 +155,11 @@ type Config struct { // backwards compatibility as well. ACLToken string + // ACLAgentToken is the default token used to make requests for the agent + // itself, such as for registering itself with the catalog. If not + // configured, the ACLToken will be used. + ACLAgentToken string + // ACLMasterToken is used to bootstrap the ACL system. It should be specified // on the servers in the ACLDatacenter. When the leader comes online, it ensures // that the Master token is available. This provides the initial token. @@ -370,6 +375,7 @@ func (c *Config) ScaleRaft(raftMultRaw uint) { c.RaftConfig.LeaderLeaseTimeout = raftMult * def.LeaderLeaseTimeout } +// tlsConfig maps this config into a tlsutil config. func (c *Config) tlsConfig() *tlsutil.Config { tlsConf := &tlsutil.Config{ VerifyIncoming: c.VerifyIncoming, @@ -384,3 +390,15 @@ func (c *Config) tlsConfig() *tlsutil.Config { } return tlsConf } + +// GetTokenForAgent returns the token the agent should use for its own internal +// operations, such as registering itself with the catalog. +func (c *Config) GetTokenForAgent() string { + if c.ACLAgentToken != "" { + return c.ACLAgentToken + } else if c.ACLToken != "" { + return c.ACLToken + } else { + return "" + } +} diff --git a/consul/config_test.go b/consul/config_test.go new file mode 100644 index 0000000000..e0d1cb93a8 --- /dev/null +++ b/consul/config_test.go @@ -0,0 +1,20 @@ +package consul + +import ( + "testing" +) + +func TestConfig_GetTokenForAgent(t *testing.T) { + config := DefaultConfig() + if token := config.GetTokenForAgent(); token != "" { + t.Fatalf("bad: %s", token) + } + config.ACLToken = "hello" + if token := config.GetTokenForAgent(); token != "hello" { + t.Fatalf("bad: %s", token) + } + config.ACLAgentToken = "world" + if token := config.GetTokenForAgent(); token != "world" { + t.Fatalf("bad: %s", token) + } +} diff --git a/consul/leader.go b/consul/leader.go index 375b01ae5c..43cde0666a 100644 --- a/consul/leader.go +++ b/consul/leader.go @@ -428,7 +428,7 @@ AFTER_CHECK: Status: structs.HealthPassing, Output: SerfCheckAliveOutput, }, - WriteRequest: structs.WriteRequest{Token: s.config.ACLToken}, + WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, } var out struct{} return s.endpoints.Catalog.Register(&req, &out) @@ -469,7 +469,7 @@ func (s *Server) handleFailedMember(member serf.Member) error { Status: structs.HealthCritical, Output: SerfCheckFailedOutput, }, - WriteRequest: structs.WriteRequest{Token: s.config.ACLToken}, + WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, } var out struct{} return s.endpoints.Catalog.Register(&req, &out) @@ -612,7 +612,7 @@ func (s *Server) reapTombstones(index uint64) { Datacenter: s.config.Datacenter, Op: structs.TombstoneReap, ReapIndex: index, - WriteRequest: structs.WriteRequest{Token: s.config.ACLToken}, + WriteRequest: structs.WriteRequest{Token: s.config.GetTokenForAgent()}, } _, err := s.raftApply(structs.TombstoneRequestType, &req) if err != nil { diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index ea6fb252c6..59118d9401 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -377,6 +377,14 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass all operations, and "extend-cache" allows any cached ACLs to be used, ignoring their TTL values. If a non-cached ACL is used, "extend-cache" acts like "deny". +* `acl_agent_token` - Used for clients + and servers to perform internal operations to the service catalog. If this isn't specified, then + the `acl_token` will be used. This was added in Consul 0.7.2. +

+ For clients, this token must at least have write access to the node name it will register as. For + servers, this must have write access to all nodes that are expected to join the cluster, as well + as write access to the "consul" service, which will be registered automatically on its behalf. + * `acl_enforce_version_8` - Used for clients and servers to determine if enforcement should occur for new ACL policies being previewed before Consul 0.8. Added in Consul 0.7.2, this will default to false in versions of From bcf1ffad994830d9c054f70bc6b42e6c3adb8696 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 11:58:31 -0800 Subject: [PATCH 3/9] Adds complete ACL coverage for /v1/coordinate/nodes and Coordinate.Update RPC. --- command/agent/agent.go | 5 +- consul/acl.go | 25 ++- consul/acl_test.go | 35 +++++ consul/coordinate_endpoint.go | 14 ++ consul/coordinate_endpoint_test.go | 238 +++++++++++++++++++++++++++-- 5 files changed, 297 insertions(+), 20 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 7e464b2fa4..6b9bcaa50d 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -818,14 +818,11 @@ func (a *Agent) sendCoordinate() { continue } - // TODO - Consider adding a distance check so we don't send - // an update if the position hasn't changed by more than a - // threshold. req := structs.CoordinateUpdateRequest{ Datacenter: a.config.Datacenter, Node: a.config.NodeName, Coord: c, - WriteRequest: structs.WriteRequest{Token: a.config.ACLToken}, + WriteRequest: structs.WriteRequest{Token: a.config.GetTokenForAgent()}, } var reply struct{} if err := a.RPC("Coordinate.Update", &req, &reply); err != nil { diff --git a/consul/acl.go b/consul/acl.go index cfbf744f77..ba51e2a8f2 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -413,6 +413,22 @@ func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) { *nodes = csn } +// filterCoordinates is used to filter nodes in a coordinate dump based on ACL +// rules. +func (f *aclFilter) filterCoordinates(coords *structs.Coordinates) { + c := *coords + for i := 0; i < len(c); i++ { + node := c[i].Node + if f.allowNode(node) { + continue + } + f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node) + c = append(c[:i], c[i+1:]...) + i-- + } + *coords = c +} + // filterNodeDump is used to filter through all parts of a node dump and // remove elements the provided ACL token cannot access. func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) { @@ -448,14 +464,10 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) { // filterNodes is used to filter through all parts of a node list and remove // elements the provided ACL token cannot access. func (f *aclFilter) filterNodes(nodes *structs.Nodes) { - if !f.enforceVersion8 { - return - } - n := *nodes for i := 0; i < len(n); i++ { node := n[i].Node - if f.acl.NodeRead(node) { + if f.allowNode(node) { continue } f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node) @@ -548,6 +560,9 @@ func (s *Server) filterACL(token string, subj interface{}) error { case *structs.IndexedCheckServiceNodes: filt.filterCheckServiceNodes(&v.Nodes) + case *structs.IndexedCoordinates: + filt.filterCoordinates(&v.Coordinates) + case *structs.IndexedHealthChecks: filt.filterHealthChecks(&v.HealthChecks) diff --git a/consul/acl_test.go b/consul/acl_test.go index 221e09fe97..9a6834c9df 100644 --- a/consul/acl_test.go +++ b/consul/acl_test.go @@ -1012,6 +1012,41 @@ func TestACL_filterCheckServiceNodes(t *testing.T) { } } +func TestACL_filterCoordinates(t *testing.T) { + // Create some coordinates. + coords := structs.Coordinates{ + &structs.Coordinate{ + Node: "node1", + Coord: generateRandomCoordinate(), + }, + &structs.Coordinate{ + Node: "node2", + Coord: generateRandomCoordinate(), + }, + } + + // Try permissive filtering. + filt := newAclFilter(acl.AllowAll(), nil, false) + filt.filterCoordinates(&coords) + if len(coords) != 2 { + t.Fatalf("bad: %#v", coords) + } + + // Try restrictive filtering without version 8 ACL enforcement. + filt = newAclFilter(acl.DenyAll(), nil, false) + filt.filterCoordinates(&coords) + if len(coords) != 2 { + t.Fatalf("bad: %#v", coords) + } + + // Try restrictive filtering with version 8 ACL enforcement. + filt = newAclFilter(acl.DenyAll(), nil, true) + filt.filterCoordinates(&coords) + if len(coords) != 0 { + t.Fatalf("bad: %#v", coords) + } +} + func TestACL_filterNodeDump(t *testing.T) { // Create a node dump dump := structs.NodeDump{ diff --git a/consul/coordinate_endpoint.go b/consul/coordinate_endpoint.go index 9e0df58212..6a30baa452 100644 --- a/consul/coordinate_endpoint.go +++ b/consul/coordinate_endpoint.go @@ -119,6 +119,17 @@ func (c *Coordinate) Update(args *structs.CoordinateUpdateRequest, reply *struct return fmt.Errorf("rejected bad coordinate: %v", args.Coord) } + // Fetch the ACL token, if any, and enforce the node policy if enabled. + acl, err := c.srv.resolveToken(args.Token) + if err != nil { + return err + } + if acl != nil && c.srv.config.ACLEnforceVersion8 { + if !acl.NodeWrite(args.Node) { + return permissionDeniedErr + } + } + // Add the coordinate to the map of pending updates. c.updatesLock.Lock() c.updates[args.Node] = args.Coord @@ -173,6 +184,9 @@ func (c *Coordinate) ListNodes(args *structs.DCSpecificRequest, reply *structs.I } reply.Index, reply.Coordinates = index, coords + if err := c.srv.filterACL(args.Token, reply); err != nil { + return err + } return nil }) } diff --git a/consul/coordinate_endpoint_test.go b/consul/coordinate_endpoint_test.go index 88171a7c8f..42e41c0dcb 100644 --- a/consul/coordinate_endpoint_test.go +++ b/consul/coordinate_endpoint_test.go @@ -187,6 +187,87 @@ func TestCoordinate_Update(t *testing.T) { } } +func TestCoordinate_Update_ACLDeny(t *testing.T) { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = false + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testutil.WaitForLeader(t, s1.RPC, "dc1") + + // Register some nodes. + nodes := []string{"node1", "node2"} + for _, node := range nodes { + req := structs.RegisterRequest{ + Datacenter: "dc1", + Node: node, + Address: "127.0.0.1", + } + var reply struct{} + if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + } + + // Send an update for the first node. This should go through since we + // don't have version 8 ACLs enforced yet. + req := structs.CoordinateUpdateRequest{ + Datacenter: "dc1", + Node: "node1", + Coord: generateRandomCoordinate(), + } + var out struct{} + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &req, &out); err != nil { + t.Fatalf("err: %v", err) + } + + // Now turn on version 8 enforcement and try again. + s1.config.ACLEnforceVersion8 = true + err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &req, &out) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Create an ACL that can write to the node. + arg := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: ` +node "node1" { + policy = "write" +} +`, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + var id string + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &id); err != nil { + t.Fatalf("err: %v", err) + } + + // With the token, it should now go through. + req.Token = id + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &req, &out); err != nil { + t.Fatalf("err: %v", err) + } + + // But it should be blocked for the other node. + req.Node = "node2" + err = msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &req, &out) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } +} + func TestCoordinate_ListDatacenters(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) @@ -240,8 +321,7 @@ func TestCoordinate_ListNodes(t *testing.T) { } } - // Send coordinate updates for a few nodes, waiting a little while for - // the batch update to run. + // Send coordinate updates for a few nodes. arg1 := structs.CoordinateUpdateRequest{ Datacenter: "dc1", Node: "foo", @@ -269,9 +349,120 @@ func TestCoordinate_ListNodes(t *testing.T) { if err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &arg3, &out); err != nil { t.Fatalf("err: %v", err) } - time.Sleep(3 * s1.config.CoordinateUpdatePeriod) // Now query back for all the nodes. + testutil.WaitForResult(func() (bool, error) { + arg := structs.DCSpecificRequest{ + Datacenter: "dc1", + } + resp := structs.IndexedCoordinates{} + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.ListNodes", &arg, &resp); err != nil { + t.Fatalf("err: %v", err) + } + if len(resp.Coordinates) != 3 || + resp.Coordinates[0].Node != "bar" || + resp.Coordinates[1].Node != "baz" || + resp.Coordinates[2].Node != "foo" { + return false, fmt.Errorf("bad: %v", resp.Coordinates) + } + verifyCoordinatesEqual(t, resp.Coordinates[0].Coord, arg2.Coord) // bar + verifyCoordinatesEqual(t, resp.Coordinates[1].Coord, arg3.Coord) // baz + verifyCoordinatesEqual(t, resp.Coordinates[2].Coord, arg1.Coord) // foo + return true, nil + }, func(err error) { t.Fatalf("err: %v", err) }) +} + +func TestCoordinate_ListNodes_ACLFilter(t *testing.T) { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = false + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testutil.WaitForLeader(t, s1.RPC, "dc1") + + // Register some nodes. + nodes := []string{"foo", "bar", "baz"} + for _, node := range nodes { + req := structs.RegisterRequest{ + Datacenter: "dc1", + Node: node, + Address: "127.0.0.1", + WriteRequest: structs.WriteRequest{ + Token: "root", + }, + } + var reply struct{} + if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + } + + // Send coordinate updates for a few nodes. + arg1 := structs.CoordinateUpdateRequest{ + Datacenter: "dc1", + Node: "foo", + Coord: generateRandomCoordinate(), + WriteRequest: structs.WriteRequest{ + Token: "root", + }, + } + var out struct{} + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &arg1, &out); err != nil { + t.Fatalf("err: %v", err) + } + + arg2 := structs.CoordinateUpdateRequest{ + Datacenter: "dc1", + Node: "bar", + Coord: generateRandomCoordinate(), + WriteRequest: structs.WriteRequest{ + Token: "root", + }, + } + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &arg2, &out); err != nil { + t.Fatalf("err: %v", err) + } + + arg3 := structs.CoordinateUpdateRequest{ + Datacenter: "dc1", + Node: "baz", + Coord: generateRandomCoordinate(), + WriteRequest: structs.WriteRequest{ + Token: "root", + }, + } + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.Update", &arg3, &out); err != nil { + t.Fatalf("err: %v", err) + } + + // Wait for all the coordinate updates to apply. Since we aren't + // enforcing version 8 ACLs, this should also allow us to read + // everything back without a token. + testutil.WaitForResult(func() (bool, error) { + arg := structs.DCSpecificRequest{ + Datacenter: "dc1", + } + resp := structs.IndexedCoordinates{} + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.ListNodes", &arg, &resp); err != nil { + t.Fatalf("err: %v", err) + } + if len(resp.Coordinates) == 3 { + return true, nil + } + return false, fmt.Errorf("bad: %v", resp.Coordinates) + }, func(err error) { t.Fatalf("err: %v", err) }) + + // Now that we've waited for the batch processing to ingest the + // coordinates we can do the rest of the requests without the loop. We + // will start by turning on version 8 ACL support which should block + // everything. + s1.config.ACLEnforceVersion8 = true arg := structs.DCSpecificRequest{ Datacenter: "dc1", } @@ -279,13 +470,38 @@ func TestCoordinate_ListNodes(t *testing.T) { if err := msgpackrpc.CallWithCodec(codec, "Coordinate.ListNodes", &arg, &resp); err != nil { t.Fatalf("err: %v", err) } - if len(resp.Coordinates) != 3 || - resp.Coordinates[0].Node != "bar" || - resp.Coordinates[1].Node != "baz" || - resp.Coordinates[2].Node != "foo" { - t.Fatalf("bad: %v", resp.Coordinates) + if len(resp.Coordinates) != 0 { + t.Fatalf("bad: %#v", resp.Coordinates) + } + + // Create an ACL that can read one of the nodes. + var id string + { + req := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: ` +node "foo" { + policy = "read" +} +`, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &id); err != nil { + t.Fatalf("err: %v", err) + } + } + + // With the token, it should now go through. + arg.Token = id + if err := msgpackrpc.CallWithCodec(codec, "Coordinate.ListNodes", &arg, &resp); err != nil { + t.Fatalf("err: %v", err) + } + if len(resp.Coordinates) != 1 || resp.Coordinates[0].Node != "foo" { + t.Fatalf("bad: %#v", resp.Coordinates) } - verifyCoordinatesEqual(t, resp.Coordinates[0].Coord, arg2.Coord) // bar - verifyCoordinatesEqual(t, resp.Coordinates[1].Coord, arg3.Coord) // baz - verifyCoordinatesEqual(t, resp.Coordinates[2].Coord, arg1.Coord) // foo } From 35475a66dfd284506710731fd404348040287e32 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 16:28:52 -0800 Subject: [PATCH 4/9] Adds full ACL coverage for /v1/health endpoints. --- consul/acl.go | 4 +- consul/acl_test.go | 232 ++++++++++++++++++++++++++------- consul/health_endpoint_test.go | 24 ++++ 3 files changed, 211 insertions(+), 49 deletions(-) diff --git a/consul/acl.go b/consul/acl.go index ba51e2a8f2..790639e2c3 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -350,7 +350,7 @@ func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) { hc := *checks for i := 0; i < len(hc); i++ { check := hc[i] - if f.allowService(check.ServiceName) { + if f.allowNode(check.Node) && f.allowService(check.ServiceName) { continue } f.logger.Printf("[DEBUG] consul: dropping check %q from result due to ACLs", check.CheckID) @@ -403,7 +403,7 @@ func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) { csn := *nodes for i := 0; i < len(csn); i++ { node := csn[i] - if f.allowService(node.Service.Service) { + if f.allowNode(node.Node.Node) && f.allowService(node.Service.Service) { continue } f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node.Node.Node) diff --git a/consul/acl_test.go b/consul/acl_test.go index 9a6834c9df..431683106a 100644 --- a/consul/acl_test.go +++ b/consul/acl_test.go @@ -808,27 +808,93 @@ func TestACL_MultiDC_Found(t *testing.T) { } func TestACL_filterHealthChecks(t *testing.T) { - // Create some health checks - hc := structs.HealthChecks{ - &structs.HealthCheck{ - Node: "node1", - CheckID: "check1", - ServiceName: "foo", - }, + // Create some health checks. + fill := func() structs.HealthChecks { + return structs.HealthChecks{ + &structs.HealthCheck{ + Node: "node1", + CheckID: "check1", + ServiceName: "foo", + }, + } } - // Try permissive filtering - filt := newAclFilter(acl.AllowAll(), nil, false) - filt.filterHealthChecks(&hc) - if len(hc) != 1 { - t.Fatalf("bad: %#v", hc) + // Try permissive filtering. + { + hc := fill() + filt := newAclFilter(acl.AllowAll(), nil, false) + filt.filterHealthChecks(&hc) + if len(hc) != 1 { + t.Fatalf("bad: %#v", hc) + } } - // Try restrictive filtering - filt = newAclFilter(acl.DenyAll(), nil, false) - filt.filterHealthChecks(&hc) - if len(hc) != 0 { - t.Fatalf("bad: %#v", hc) + // Try restrictive filtering. + { + hc := fill() + filt := newAclFilter(acl.DenyAll(), nil, false) + filt.filterHealthChecks(&hc) + if len(hc) != 0 { + t.Fatalf("bad: %#v", hc) + } + } + + // Allowed to see the service but not the node. + policy, err := acl.Parse(` +service "foo" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err := acl.New(acl.DenyAll(), policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // This will work because version 8 ACLs aren't being enforced. + { + hc := fill() + filt := newAclFilter(perms, nil, false) + filt.filterHealthChecks(&hc) + if len(hc) != 1 { + t.Fatalf("bad: %#v", hc) + } + } + + // But with version 8 the node will block it. + { + hc := fill() + filt := newAclFilter(perms, nil, true) + filt.filterHealthChecks(&hc) + if len(hc) != 0 { + t.Fatalf("bad: %#v", hc) + } + } + + // Chain on access to the node. + policy, err = acl.Parse(` +node "node1" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err = acl.New(perms, policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Now it should go through. + { + hc := fill() + filt := newAclFilter(perms, nil, true) + filt.filterHealthChecks(&hc) + if len(hc) != 1 { + t.Fatalf("bad: %#v", hc) + } } } @@ -899,7 +965,7 @@ service "foo" { t.Fatalf("err: %v", err) } - /// This will work because version 8 ACLs aren't being enforced. + // This will work because version 8 ACLs aren't being enforced. { nodes := fill() filt := newAclFilter(perms, nil, false) @@ -974,41 +1040,113 @@ func TestACL_filterNodeServices(t *testing.T) { } func TestACL_filterCheckServiceNodes(t *testing.T) { - // Create some nodes - nodes := structs.CheckServiceNodes{ - structs.CheckServiceNode{ - Node: &structs.Node{ - Node: "node1", - }, - Service: &structs.NodeService{ - ID: "foo", - Service: "foo", - }, - Checks: structs.HealthChecks{ - &structs.HealthCheck{ - Node: "node1", - CheckID: "check1", - ServiceName: "foo", + // Create some nodes. + fill := func() structs.CheckServiceNodes { + return structs.CheckServiceNodes{ + structs.CheckServiceNode{ + Node: &structs.Node{ + Node: "node1", + }, + Service: &structs.NodeService{ + ID: "foo", + Service: "foo", + }, + Checks: structs.HealthChecks{ + &structs.HealthCheck{ + Node: "node1", + CheckID: "check1", + ServiceName: "foo", + }, }, }, - }, + } } - // Try permissive filtering - filt := newAclFilter(acl.AllowAll(), nil, false) - filt.filterCheckServiceNodes(&nodes) - if len(nodes) != 1 { - t.Fatalf("bad: %#v", nodes) - } - if len(nodes[0].Checks) != 1 { - t.Fatalf("bad: %#v", nodes[0].Checks) + // Try permissive filtering. + { + nodes := fill() + filt := newAclFilter(acl.AllowAll(), nil, false) + filt.filterCheckServiceNodes(&nodes) + if len(nodes) != 1 { + t.Fatalf("bad: %#v", nodes) + } + if len(nodes[0].Checks) != 1 { + t.Fatalf("bad: %#v", nodes[0].Checks) + } } - // Try restrictive filtering - filt = newAclFilter(acl.DenyAll(), nil, false) - filt.filterCheckServiceNodes(&nodes) - if len(nodes) != 0 { - t.Fatalf("bad: %#v", nodes) + // Try restrictive filtering. + { + nodes := fill() + filt := newAclFilter(acl.DenyAll(), nil, false) + filt.filterCheckServiceNodes(&nodes) + if len(nodes) != 0 { + t.Fatalf("bad: %#v", nodes) + } + } + + // Allowed to see the service but not the node. + policy, err := acl.Parse(` +service "foo" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err := acl.New(acl.DenyAll(), policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // This will work because version 8 ACLs aren't being enforced. + { + nodes := fill() + filt := newAclFilter(perms, nil, false) + filt.filterCheckServiceNodes(&nodes) + if len(nodes) != 1 { + t.Fatalf("bad: %#v", nodes) + } + if len(nodes[0].Checks) != 1 { + t.Fatalf("bad: %#v", nodes[0].Checks) + } + } + + // But with version 8 the node will block it. + { + nodes := fill() + filt := newAclFilter(perms, nil, true) + filt.filterCheckServiceNodes(&nodes) + if len(nodes) != 0 { + t.Fatalf("bad: %#v", nodes) + } + } + + // Chain on access to the node. + policy, err = acl.Parse(` +node "node1" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err = acl.New(perms, policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Now it should go through. + { + nodes := fill() + filt := newAclFilter(perms, nil, true) + filt.filterCheckServiceNodes(&nodes) + if len(nodes) != 1 { + t.Fatalf("bad: %#v", nodes) + } + if len(nodes[0].Checks) != 1 { + t.Fatalf("bad: %#v", nodes[0].Checks) + } } } diff --git a/consul/health_endpoint_test.go b/consul/health_endpoint_test.go index 23a063de3a..b5db5b9f21 100644 --- a/consul/health_endpoint_test.go +++ b/consul/health_endpoint_test.go @@ -507,6 +507,12 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) { if !found { t.Fatalf("bad: %#v", reply.HealthChecks) } + + // We've already proven that we call the ACL filtering function so we + // test node filtering down in acl.go for node cases. This also proves + // that we respect the version 8 ACL flag, since the test server sets + // that to false (the regression value of *not* changing this is better + // for now until we change the sense of the version 8 ACL flag). } func TestHealth_ServiceChecks_FilterACL(t *testing.T) { @@ -543,6 +549,12 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) { if len(reply.HealthChecks) != 0 { t.Fatalf("bad: %#v", reply.HealthChecks) } + + // We've already proven that we call the ACL filtering function so we + // test node filtering down in acl.go for node cases. This also proves + // that we respect the version 8 ACL flag, since the test server sets + // that to false (the regression value of *not* changing this is better + // for now until we change the sense of the version 8 ACL flag). } func TestHealth_ServiceNodes_FilterACL(t *testing.T) { @@ -572,6 +584,12 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) { if len(reply.Nodes) != 0 { t.Fatalf("bad: %#v", reply.Nodes) } + + // We've already proven that we call the ACL filtering function so we + // test node filtering down in acl.go for node cases. This also proves + // that we respect the version 8 ACL flag, since the test server sets + // that to false (the regression value of *not* changing this is better + // for now until we change the sense of the version 8 ACL flag). } func TestHealth_ChecksInState_FilterACL(t *testing.T) { @@ -602,4 +620,10 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) { if !found { t.Fatalf("missing service 'foo': %#v", reply.HealthChecks) } + + // We've already proven that we call the ACL filtering function so we + // test node filtering down in acl.go for node cases. This also proves + // that we respect the version 8 ACL flag, since the test server sets + // that to false (the regression value of *not* changing this is better + // for now until we change the sense of the version 8 ACL flag). } From 9c785c70227b7815f4cecf7b7b929bc63be8883f Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 16:53:31 -0800 Subject: [PATCH 5/9] Fixes implementation of node ACLs for /v1/catalog/node/. This would return a "permission denied" error, but this changes it to return the same response as a node that doesn't exist (as was originally intended and written in the code comments). --- consul/acl.go | 19 ++++-- consul/acl_test.go | 116 ++++++++++++++++++++++++++------ consul/catalog_endpoint.go | 14 ---- consul/catalog_endpoint_test.go | 15 ++++- 4 files changed, 122 insertions(+), 42 deletions(-) diff --git a/consul/acl.go b/consul/acl.go index 790639e2c3..d50876c76e 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -388,13 +388,22 @@ func (f *aclFilter) filterServiceNodes(nodes *structs.ServiceNodes) { } // filterNodeServices is used to filter services on a given node base on ACLs. -func (f *aclFilter) filterNodeServices(services *structs.NodeServices) { - for svc, _ := range services.Services { +func (f *aclFilter) filterNodeServices(services **structs.NodeServices) { + if *services == nil { + return + } + + if !f.allowNode((*services).Node.Node) { + *services = nil + return + } + + for svc, _ := range (*services).Services { if f.allowService(svc) { continue } f.logger.Printf("[DEBUG] consul: dropping service %q from result due to ACLs", svc) - delete(services.Services, svc) + delete((*services).Services, svc) } } @@ -573,9 +582,7 @@ func (s *Server) filterACL(token string, subj interface{}) error { filt.filterNodes(&v.Nodes) case *structs.IndexedNodeServices: - if v.NodeServices != nil { - filt.filterNodeServices(v.NodeServices) - } + filt.filterNodeServices(&v.NodeServices) case *structs.IndexedServiceNodes: filt.filterServiceNodes(&v.ServiceNodes) diff --git a/consul/acl_test.go b/consul/acl_test.go index 431683106a..3dc85c7cde 100644 --- a/consul/acl_test.go +++ b/consul/acl_test.go @@ -1011,31 +1011,107 @@ node "node1" { } func TestACL_filterNodeServices(t *testing.T) { - // Create some node services - services := structs.NodeServices{ - Node: &structs.Node{ - Node: "node1", - }, - Services: map[string]*structs.NodeService{ - "foo": &structs.NodeService{ - ID: "foo", - Service: "foo", + // Create some node services. + fill := func() *structs.NodeServices { + return &structs.NodeServices{ + Node: &structs.Node{ + Node: "node1", }, - }, + Services: map[string]*structs.NodeService{ + "foo": &structs.NodeService{ + ID: "foo", + Service: "foo", + }, + }, + } } - // Try permissive filtering - filt := newAclFilter(acl.AllowAll(), nil, false) - filt.filterNodeServices(&services) - if len(services.Services) != 1 { - t.Fatalf("bad: %#v", services.Services) + // Try nil, which is a possible input. + { + var services *structs.NodeServices + filt := newAclFilter(acl.AllowAll(), nil, false) + filt.filterNodeServices(&services) + if services != nil { + t.Fatalf("bad: %#v", services) + } } - // Try restrictive filtering - filt = newAclFilter(acl.DenyAll(), nil, false) - filt.filterNodeServices(&services) - if len(services.Services) != 0 { - t.Fatalf("bad: %#v", services.Services) + // Try permissive filtering. + { + services := fill() + filt := newAclFilter(acl.AllowAll(), nil, false) + filt.filterNodeServices(&services) + if len(services.Services) != 1 { + t.Fatalf("bad: %#v", services.Services) + } + } + + // Try restrictive filtering. + { + services := fill() + filt := newAclFilter(acl.DenyAll(), nil, false) + filt.filterNodeServices(&services) + if len((*services).Services) != 0 { + t.Fatalf("bad: %#v", (*services).Services) + } + } + + // Allowed to see the service but not the node. + policy, err := acl.Parse(` +service "foo" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err := acl.New(acl.DenyAll(), policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // This will work because version 8 ACLs aren't being enforced. + { + services := fill() + filt := newAclFilter(perms, nil, false) + filt.filterNodeServices(&services) + if len((*services).Services) != 1 { + t.Fatalf("bad: %#v", (*services).Services) + } + } + + // But with version 8 the node will block it. + { + services := fill() + filt := newAclFilter(perms, nil, true) + filt.filterNodeServices(&services) + if services != nil { + t.Fatalf("bad: %#v", services) + } + } + + // Chain on access to the node. + policy, err = acl.Parse(` +node "node1" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err = acl.New(perms, policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Now it should go through. + { + services := fill() + filt := newAclFilter(perms, nil, false) + filt.filterNodeServices(&services) + if len((*services).Services) != 1 { + t.Fatalf("bad: %#v", (*services).Services) + } } } diff --git a/consul/catalog_endpoint.go b/consul/catalog_endpoint.go index 35d00dd470..1383b89165 100644 --- a/consul/catalog_endpoint.go +++ b/consul/catalog_endpoint.go @@ -271,20 +271,6 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs return err } - // Node read access is required with version 8 ACLs. We - // just return the same response as if the node doesn't - // exist, which is consistent with how the rest of the - // catalog filtering works and doesn't disclose the node. - acl, err := c.srv.resolveToken(args.Token) - if err != nil { - return err - } - if acl != nil && c.srv.config.ACLEnforceVersion8 { - if !acl.NodeRead(args.Node) { - return permissionDeniedErr - } - } - reply.Index, reply.NodeServices = index, services return c.srv.filterACL(args.Token, reply) }) diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 6593fdf848..c0a329af36 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -1537,10 +1537,12 @@ func TestCatalog_NodeServices_ACLDeny(t *testing.T) { // Now turn on version 8 enforcement and try again. s1.config.ACLEnforceVersion8 = true - err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply) - if err == nil || !strings.Contains(err.Error(), permissionDenied) { + if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil { t.Fatalf("err: %v", err) } + if reply.NodeServices != nil { + t.Fatalf("should not nil") + } // Create an ACL that can read the node. arg := structs.ACLRequest{ @@ -1570,6 +1572,15 @@ node "%s" { if reply.NodeServices == nil { t.Fatalf("should not be nil") } + + // Make sure an unknown node doesn't cause trouble. + args.Node = "nope" + if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &reply); err != nil { + t.Fatalf("err: %v", err) + } + if reply.NodeServices != nil { + t.Fatalf("should not nil") + } } func TestCatalog_NodeServices_FilterACL(t *testing.T) { From 2404af6f94856ddc566efd5525c18b21a88075e3 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 17:27:22 -0800 Subject: [PATCH 6/9] Adds complete ACL support for /v1/query//execute. This was already supported by previous changes to the ACL filter, so we just added a test to show it working. --- consul/prepared_query_endpoint_test.go | 53 ++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/consul/prepared_query_endpoint_test.go b/consul/prepared_query_endpoint_test.go index f3120892e3..d6516c09d5 100644 --- a/consul/prepared_query_endpoint_test.go +++ b/consul/prepared_query_endpoint_test.go @@ -1417,6 +1417,7 @@ func TestPreparedQuery_Execute(t *testing.T) { c.ACLDatacenter = "dc1" c.ACLMasterToken = "root" c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = false }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -2138,6 +2139,58 @@ func TestPreparedQuery_Execute(t *testing.T) { } } + // Turn on version 8 ACLs, which will start to filter even with the exec + // token. + s1.config.ACLEnforceVersion8 = true + { + req := structs.PreparedQueryExecuteRequest{ + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, + } + + var reply structs.PreparedQueryExecuteResponse + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + if len(reply.Nodes) != 0 || + reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { + t.Fatalf("bad: %v", reply) + } + } + + // Revert version 8 ACLs and make sure the query works again. + s1.config.ACLEnforceVersion8 = false + { + req := structs.PreparedQueryExecuteRequest{ + Datacenter: "dc1", + QueryIDOrName: query.Query.ID, + QueryOptions: structs.QueryOptions{Token: execToken}, + } + + var reply structs.PreparedQueryExecuteResponse + if err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply); err != nil { + t.Fatalf("err: %v", err) + } + + if len(reply.Nodes) != 8 || + reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || + !reflect.DeepEqual(reply.DNS, query.Query.DNS) || + !reply.QueryMeta.KnownLeader { + t.Fatalf("bad: %v", reply) + } + for _, node := range reply.Nodes { + if node.Node.Node == "node1" || node.Node.Node == "node3" { + t.Fatalf("bad: %v", node) + } + } + } + // Now fail everything in dc1 and we should get an empty list back. for i := 0; i < 10; i++ { setHealth(fmt.Sprintf("node%d", i+1), structs.HealthCritical) From 99e810f9c77b8c9e5254a1b75b5efc0c2b8193b3 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 18:21:00 -0800 Subject: [PATCH 7/9] Adds complete ACL coverage for /v1/internal/ui/node endpoints. --- consul/acl.go | 24 +++-- consul/acl_test.go | 152 +++++++++++++++++++++++-------- consul/internal_endpoint_test.go | 12 +++ 3 files changed, 143 insertions(+), 45 deletions(-) diff --git a/consul/acl.go b/consul/acl.go index d50876c76e..7def53c3ad 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -445,26 +445,34 @@ func (f *aclFilter) filterNodeDump(dump *structs.NodeDump) { for i := 0; i < len(nd); i++ { info := nd[i] + // Filter nodes + if node := info.Node; !f.allowNode(node) { + f.logger.Printf("[DEBUG] consul: dropping node %q from result due to ACLs", node) + nd = append(nd[:i], nd[i+1:]...) + i-- + continue + } + // Filter services - for i := 0; i < len(info.Services); i++ { - svc := info.Services[i].Service + for j := 0; j < len(info.Services); j++ { + svc := info.Services[j].Service if f.allowService(svc) { continue } f.logger.Printf("[DEBUG] consul: dropping service %q from result due to ACLs", svc) - info.Services = append(info.Services[:i], info.Services[i+1:]...) - i-- + info.Services = append(info.Services[:j], info.Services[j+1:]...) + j-- } // Filter checks - for i := 0; i < len(info.Checks); i++ { - chk := info.Checks[i] + for j := 0; j < len(info.Checks); j++ { + chk := info.Checks[j] if f.allowService(chk.ServiceName) { continue } f.logger.Printf("[DEBUG] consul: dropping check %q from result due to ACLs", chk.CheckID) - info.Checks = append(info.Checks[:i], info.Checks[i+1:]...) - i-- + info.Checks = append(info.Checks[:j], info.Checks[j+1:]...) + j-- } } *dump = nd diff --git a/consul/acl_test.go b/consul/acl_test.go index 3dc85c7cde..52a458195a 100644 --- a/consul/acl_test.go +++ b/consul/acl_test.go @@ -1107,7 +1107,7 @@ node "node1" { // Now it should go through. { services := fill() - filt := newAclFilter(perms, nil, false) + filt := newAclFilter(perms, nil, true) filt.filterNodeServices(&services) if len((*services).Services) != 1 { t.Fatalf("bad: %#v", (*services).Services) @@ -1262,50 +1262,128 @@ func TestACL_filterCoordinates(t *testing.T) { } func TestACL_filterNodeDump(t *testing.T) { - // Create a node dump - dump := structs.NodeDump{ - &structs.NodeInfo{ - Node: "node1", - Services: []*structs.NodeService{ - &structs.NodeService{ - ID: "foo", - Service: "foo", + // Create a node dump. + fill := func() structs.NodeDump { + return structs.NodeDump{ + &structs.NodeInfo{ + Node: "node1", + Services: []*structs.NodeService{ + &structs.NodeService{ + ID: "foo", + Service: "foo", + }, + }, + Checks: []*structs.HealthCheck{ + &structs.HealthCheck{ + Node: "node1", + CheckID: "check1", + ServiceName: "foo", + }, }, }, - Checks: []*structs.HealthCheck{ - &structs.HealthCheck{ - Node: "node1", - CheckID: "check1", - ServiceName: "foo", - }, - }, - }, + } } - // Try permissive filtering - filt := newAclFilter(acl.AllowAll(), nil, false) - filt.filterNodeDump(&dump) - if len(dump) != 1 { - t.Fatalf("bad: %#v", dump) - } - if len(dump[0].Services) != 1 { - t.Fatalf("bad: %#v", dump[0].Services) - } - if len(dump[0].Checks) != 1 { - t.Fatalf("bad: %#v", dump[0].Checks) + // Try permissive filtering. + { + dump := fill() + filt := newAclFilter(acl.AllowAll(), nil, false) + filt.filterNodeDump(&dump) + if len(dump) != 1 { + t.Fatalf("bad: %#v", dump) + } + if len(dump[0].Services) != 1 { + t.Fatalf("bad: %#v", dump[0].Services) + } + if len(dump[0].Checks) != 1 { + t.Fatalf("bad: %#v", dump[0].Checks) + } } - // Try restrictive filtering - filt = newAclFilter(acl.DenyAll(), nil, false) - filt.filterNodeDump(&dump) - if len(dump) != 1 { - t.Fatalf("bad: %#v", dump) + // Try restrictive filtering. + { + dump := fill() + filt := newAclFilter(acl.DenyAll(), nil, false) + filt.filterNodeDump(&dump) + if len(dump) != 1 { + t.Fatalf("bad: %#v", dump) + } + if len(dump[0].Services) != 0 { + t.Fatalf("bad: %#v", dump[0].Services) + } + if len(dump[0].Checks) != 0 { + t.Fatalf("bad: %#v", dump[0].Checks) + } } - if len(dump[0].Services) != 0 { - t.Fatalf("bad: %#v", dump[0].Services) + + // Allowed to see the service but not the node. + policy, err := acl.Parse(` +service "foo" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) } - if len(dump[0].Checks) != 0 { - t.Fatalf("bad: %#v", dump[0].Checks) + perms, err := acl.New(acl.DenyAll(), policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // This will work because version 8 ACLs aren't being enforced. + { + dump := fill() + filt := newAclFilter(perms, nil, false) + filt.filterNodeDump(&dump) + if len(dump) != 1 { + t.Fatalf("bad: %#v", dump) + } + if len(dump[0].Services) != 1 { + t.Fatalf("bad: %#v", dump[0].Services) + } + if len(dump[0].Checks) != 1 { + t.Fatalf("bad: %#v", dump[0].Checks) + } + } + + // But with version 8 the node will block it. + { + dump := fill() + filt := newAclFilter(perms, nil, true) + filt.filterNodeDump(&dump) + if len(dump) != 0 { + t.Fatalf("bad: %#v", dump) + } + } + + // Chain on access to the node. + policy, err = acl.Parse(` +node "node1" { + policy = "read" +} +`) + if err != nil { + t.Fatalf("err %v", err) + } + perms, err = acl.New(perms, policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Now it should go through. + { + dump := fill() + filt := newAclFilter(perms, nil, true) + filt.filterNodeDump(&dump) + if len(dump) != 1 { + t.Fatalf("bad: %#v", dump) + } + if len(dump[0].Services) != 1 { + t.Fatalf("bad: %#v", dump[0].Services) + } + if len(dump[0].Checks) != 1 { + t.Fatalf("bad: %#v", dump[0].Checks) + } } } diff --git a/consul/internal_endpoint_test.go b/consul/internal_endpoint_test.go index 94a59cafbd..042ddd8e37 100644 --- a/consul/internal_endpoint_test.go +++ b/consul/internal_endpoint_test.go @@ -284,6 +284,12 @@ func TestInternal_NodeInfo_FilterACL(t *testing.T) { t.Fatalf("bad: %#v", info.Services) } } + + // We've already proven that we call the ACL filtering function so we + // test node filtering down in acl.go for node cases. This also proves + // that we respect the version 8 ACL flag, since the test server sets + // that to false (the regression value of *not* changing this is better + // for now until we change the sense of the version 8 ACL flag). } func TestInternal_NodeDump_FilterACL(t *testing.T) { @@ -327,6 +333,12 @@ func TestInternal_NodeDump_FilterACL(t *testing.T) { t.Fatalf("bad: %#v", info.Services) } } + + // We've already proven that we call the ACL filtering function so we + // test node filtering down in acl.go for node cases. This also proves + // that we respect the version 8 ACL flag, since the test server sets + // that to false (the regression value of *not* changing this is better + // for now until we change the sense of the version 8 ACL flag). } func TestInternal_EventFire_Token(t *testing.T) { From 60d4322c4920c84d50626e7a0a5cf57d75c9d0f0 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 20:20:28 -0800 Subject: [PATCH 8/9] Adds support to ACL package for session policies. --- acl/acl.go | 60 +++++++++++++++++++++++++++ acl/acl_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++++ acl/policy.go | 19 +++++++++ acl/policy_test.go | 35 ++++++++++++++++ 4 files changed, 215 insertions(+) diff --git a/acl/acl.go b/acl/acl.go index 5b2be3f989..46288a9d7d 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -95,6 +95,13 @@ type ACL interface { // service ServiceWrite(string) bool + // SessionRead checks for permission to read sessions for a given node. + SessionRead(string) bool + + // SessionWrite checks for permission to create sessions for a given + // node. + SessionWrite(string) bool + // Snapshot checks for permission to take and restore snapshots. Snapshot() bool } @@ -175,6 +182,14 @@ func (s *StaticACL) ServiceWrite(string) bool { return s.defaultAllow } +func (s *StaticACL) SessionRead(string) bool { + return s.defaultAllow +} + +func (s *StaticACL) SessionWrite(string) bool { + return s.defaultAllow +} + func (s *StaticACL) Snapshot() bool { return s.allowManage } @@ -224,6 +239,9 @@ type PolicyACL struct { // serviceRules contains the service policies serviceRules *radix.Tree + // sessionRules contains the session policies + sessionRules *radix.Tree + // eventRules contains the user event policies eventRules *radix.Tree @@ -247,6 +265,7 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { keyRules: radix.New(), nodeRules: radix.New(), serviceRules: radix.New(), + sessionRules: radix.New(), eventRules: radix.New(), preparedQueryRules: radix.New(), } @@ -266,6 +285,11 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { p.serviceRules.Insert(sp.Name, sp.Policy) } + // Load the session policy + for _, sp := range policy.Sessions { + p.sessionRules.Insert(sp.Node, sp.Policy) + } + // Load the event policy for _, ep := range policy.Events { p.eventRules.Insert(ep.Event, ep.Policy) @@ -547,3 +571,39 @@ func (p *PolicyACL) ServiceWrite(name string) bool { // No matching rule, use the parent. return p.parent.ServiceWrite(name) } + +// SessionRead checks for permission to read sessions for a given node. +func (p *PolicyACL) SessionRead(node string) bool { + // Check for an exact rule or catch-all + _, rule, ok := p.sessionRules.LongestPrefix(node) + + if ok { + switch rule { + case PolicyRead, PolicyWrite: + return true + default: + return false + } + } + + // No matching rule, use the parent. + return p.parent.SessionRead(node) +} + +// SessionWrite checks for permission to create sessions for a given node. +func (p *PolicyACL) SessionWrite(node string) bool { + // Check for an exact rule or catch-all + _, rule, ok := p.sessionRules.LongestPrefix(node) + + if ok { + switch rule { + case PolicyWrite: + return true + default: + return false + } + } + + // No matching rule, use the parent. + return p.parent.SessionWrite(node) +} diff --git a/acl/acl_test.go b/acl/acl_test.go index e398e50c90..197f9390cf 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -83,6 +83,12 @@ func TestStaticACL(t *testing.T) { if !all.ServiceWrite("foobar") { t.Fatalf("should allow") } + if !all.SessionRead("foobar") { + t.Fatalf("should allow") + } + if !all.SessionWrite("foobar") { + t.Fatalf("should allow") + } if all.Snapshot() { t.Fatalf("should not allow") } @@ -141,6 +147,12 @@ func TestStaticACL(t *testing.T) { if none.ServiceWrite("foobar") { t.Fatalf("should not allow") } + if none.SessionRead("foobar") { + t.Fatalf("should not allow") + } + if none.SessionWrite("foobar") { + t.Fatalf("should not allow") + } if none.Snapshot() { t.Fatalf("should not allow") } @@ -193,6 +205,12 @@ func TestStaticACL(t *testing.T) { if !manage.ServiceWrite("foobar") { t.Fatalf("should allow") } + if !manage.SessionRead("foobar") { + t.Fatalf("should allow") + } + if !manage.SessionWrite("foobar") { + t.Fatalf("should allow") + } if !manage.Snapshot() { t.Fatalf("should allow") } @@ -661,3 +679,86 @@ func TestPolicyACL_Node(t *testing.T) { } } } + +func TestPolicyACL_Session(t *testing.T) { + deny := DenyAll() + policyRoot := &Policy{ + Sessions: []*SessionPolicy{ + &SessionPolicy{ + Node: "root-nope", + Policy: PolicyDeny, + }, + &SessionPolicy{ + Node: "root-ro", + Policy: PolicyRead, + }, + &SessionPolicy{ + Node: "root-rw", + Policy: PolicyWrite, + }, + &SessionPolicy{ + Node: "override", + Policy: PolicyDeny, + }, + }, + } + root, err := New(deny, policyRoot) + if err != nil { + t.Fatalf("err: %v", err) + } + + policy := &Policy{ + Sessions: []*SessionPolicy{ + &SessionPolicy{ + Node: "child-nope", + Policy: PolicyDeny, + }, + &SessionPolicy{ + Node: "child-ro", + Policy: PolicyRead, + }, + &SessionPolicy{ + Node: "child-rw", + Policy: PolicyWrite, + }, + &SessionPolicy{ + Node: "override", + Policy: PolicyWrite, + }, + }, + } + acl, err := New(root, policy) + if err != nil { + t.Fatalf("err: %v", err) + } + + type sessioncase struct { + inp string + read bool + write bool + } + cases := []sessioncase{ + {"nope", false, false}, + {"root-nope", false, false}, + {"root-ro", true, false}, + {"root-rw", true, true}, + {"root-nope-prefix", false, false}, + {"root-ro-prefix", true, false}, + {"root-rw-prefix", true, true}, + {"child-nope", false, false}, + {"child-ro", true, false}, + {"child-rw", true, true}, + {"child-nope-prefix", false, false}, + {"child-ro-prefix", true, false}, + {"child-rw-prefix", true, true}, + {"override", true, true}, + } + for _, c := range cases { + if c.read != acl.SessionRead(c.inp) { + t.Fatalf("Read fail: %#v", c) + } + if c.write != acl.SessionWrite(c.inp) { + t.Fatalf("Write fail: %#v", c) + } + } +} diff --git a/acl/policy.go b/acl/policy.go index c01dbaea18..13d0d99124 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -19,6 +19,7 @@ type Policy struct { Keys []*KeyPolicy `hcl:"key,expand"` Nodes []*NodePolicy `hcl:"node,expand"` Services []*ServicePolicy `hcl:"service,expand"` + Sessions []*SessionPolicy `hcl:"session,expand"` Events []*EventPolicy `hcl:"event,expand"` PreparedQueries []*PreparedQueryPolicy `hcl:"query,expand"` Keyring string `hcl:"keyring"` @@ -55,6 +56,17 @@ func (s *ServicePolicy) GoString() string { return fmt.Sprintf("%#v", *s) } +// SessionPolicy represents a policy for making sessions tied to specific node +// name prefixes. +type SessionPolicy struct { + Node string `hcl:",key"` + Policy string +} + +func (s *SessionPolicy) GoString() string { + return fmt.Sprintf("%#v", *s) +} + // EventPolicy represents a user event policy. type EventPolicy struct { Event string `hcl:",key"` @@ -125,6 +137,13 @@ func Parse(rules string) (*Policy, error) { } } + // Validate the session policies + for _, sp := range p.Sessions { + if !isPolicyValid(sp.Policy) { + return nil, fmt.Errorf("Invalid session policy: %#v", sp) + } + } + // Validate the user event policies for _, ep := range p.Events { if !isPolicyValid(ep.Policy) { diff --git a/acl/policy_test.go b/acl/policy_test.go index 23504ddbec..9120049168 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -46,6 +46,12 @@ service "" { service "foo" { policy = "read" } +session "foo" { + policy = "write" +} +session "bar" { + policy = "deny" +} query "" { policy = "read" } @@ -129,6 +135,16 @@ query "bar" { Policy: PolicyRead, }, }, + Sessions: []*SessionPolicy{ + &SessionPolicy{ + Node: "foo", + Policy: PolicyWrite, + }, + &SessionPolicy{ + Node: "bar", + Policy: PolicyDeny, + }, + }, } out, err := Parse(inp) @@ -199,6 +215,14 @@ func TestACLPolicy_Parse_JSON(t *testing.T) { "foo": { "policy": "read" } + }, + "session": { + "foo": { + "policy": "write" + }, + "bar": { + "policy": "deny" + } } }` exp := &Policy{ @@ -274,6 +298,16 @@ func TestACLPolicy_Parse_JSON(t *testing.T) { Policy: PolicyRead, }, }, + Sessions: []*SessionPolicy{ + &SessionPolicy{ + Node: "foo", + Policy: PolicyWrite, + }, + &SessionPolicy{ + Node: "bar", + Policy: PolicyDeny, + }, + }, } out, err := Parse(inp) @@ -331,6 +365,7 @@ func TestACLPolicy_Bad_Policy(t *testing.T) { `operator = "nope"`, `query "" { policy = "nope" }`, `service "" { policy = "nope" }`, + `session "" { policy = "nope" }`, } for _, c := range cases { _, err := Parse(c) From 8b67991ef7b2495f899a4420e86f434312a93091 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 12 Dec 2016 21:59:22 -0800 Subject: [PATCH 9/9] Adds complete ACL coverage for /v1/session endpoints. --- command/agent/session_endpoint.go | 2 + consul/acl.go | 27 +++ consul/acl_test.go | 33 +++ consul/session_endpoint.go | 64 +++++- consul/session_endpoint_test.go | 357 +++++++++++++++++++++++++++++- 5 files changed, 468 insertions(+), 15 deletions(-) diff --git a/command/agent/session_endpoint.go b/command/agent/session_endpoint.go index 4049cf7d6b..92ed9c6b14 100644 --- a/command/agent/session_endpoint.go +++ b/command/agent/session_endpoint.go @@ -46,6 +46,7 @@ func (s *HTTPServer) SessionCreate(resp http.ResponseWriter, req *http.Request) }, } s.parseDC(req, &args.Datacenter) + s.parseToken(req, &args.Token) // Handle optional request body if req.ContentLength > 0 { @@ -117,6 +118,7 @@ func (s *HTTPServer) SessionDestroy(resp http.ResponseWriter, req *http.Request) Op: structs.SessionDestroy, } s.parseDC(req, &args.Datacenter) + s.parseToken(req, &args.Token) // Pull out the session id args.Session.ID = strings.TrimPrefix(req.URL.Path, "/v1/session/destroy/") diff --git a/consul/acl.go b/consul/acl.go index 7def53c3ad..36135d9ac6 100644 --- a/consul/acl.go +++ b/consul/acl.go @@ -344,6 +344,15 @@ func (f *aclFilter) allowService(service string) bool { return f.acl.ServiceRead(service) } +// allowSession is used to determine if a session for a node is accessible for +// an ACL. +func (f *aclFilter) allowSession(node string) bool { + if !f.enforceVersion8 { + return true + } + return f.acl.SessionRead(node) +} + // filterHealthChecks is used to filter a set of health checks down based on // the configured ACL rules for a token. func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) { @@ -422,6 +431,21 @@ func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) { *nodes = csn } +// filterSessions is used to filter a set of sessions based on ACLs. +func (f *aclFilter) filterSessions(sessions *structs.Sessions) { + s := *sessions + for i := 0; i < len(s); i++ { + session := s[i] + if f.allowSession(session.Node) { + continue + } + f.logger.Printf("[DEBUG] consul: dropping session %q from result due to ACLs", session.ID) + s = append(s[:i], s[i+1:]...) + i-- + } + *sessions = s +} + // filterCoordinates is used to filter nodes in a coordinate dump based on ACL // rules. func (f *aclFilter) filterCoordinates(coords *structs.Coordinates) { @@ -598,6 +622,9 @@ func (s *Server) filterACL(token string, subj interface{}) error { case *structs.IndexedServices: filt.filterServices(v.Services) + case *structs.IndexedSessions: + filt.filterSessions(&v.Sessions) + case *structs.IndexedPreparedQueries: filt.filterPreparedQueries(&v.Queries) diff --git a/consul/acl_test.go b/consul/acl_test.go index 52a458195a..65ed6f7aa3 100644 --- a/consul/acl_test.go +++ b/consul/acl_test.go @@ -1261,6 +1261,39 @@ func TestACL_filterCoordinates(t *testing.T) { } } +func TestACL_filterSessions(t *testing.T) { + // Create a session list. + sessions := structs.Sessions{ + &structs.Session{ + Node: "foo", + }, + &structs.Session{ + Node: "bar", + }, + } + + // Try permissive filtering. + filt := newAclFilter(acl.AllowAll(), nil, true) + filt.filterSessions(&sessions) + if len(sessions) != 2 { + t.Fatalf("bad: %#v", sessions) + } + + // Try restrictive filtering but with version 8 enforcement turned off. + filt = newAclFilter(acl.DenyAll(), nil, false) + filt.filterSessions(&sessions) + if len(sessions) != 2 { + t.Fatalf("bad: %#v", sessions) + } + + // Try restrictive filtering with version 8 enforcement turned on. + filt = newAclFilter(acl.DenyAll(), nil, true) + filt.filterSessions(&sessions) + if len(sessions) != 0 { + t.Fatalf("bad: %#v", sessions) + } +} + func TestACL_filterNodeDump(t *testing.T) { // Create a node dump. fill := func() structs.NodeDump { diff --git a/consul/session_endpoint.go b/consul/session_endpoint.go index c6ddbc75c3..65272d422d 100644 --- a/consul/session_endpoint.go +++ b/consul/session_endpoint.go @@ -30,6 +30,33 @@ func (s *Session) Apply(args *structs.SessionRequest, reply *string) error { return fmt.Errorf("Must provide Node") } + // Fetch the ACL token, if any, and apply the policy. + acl, err := s.srv.resolveToken(args.Token) + if err != nil { + return err + } + if acl != nil && s.srv.config.ACLEnforceVersion8 { + switch args.Op { + case structs.SessionDestroy: + state := s.srv.fsm.State() + _, existing, err := state.SessionGet(args.Session.ID) + if err != nil { + return fmt.Errorf("Unknown session %q", args.Session.ID) + } + if !acl.SessionWrite(existing.Node) { + return permissionDeniedErr + } + + case structs.SessionCreate: + if !acl.SessionWrite(args.Session.Node) { + return permissionDeniedErr + } + + default: + return fmt.Errorf("Invalid session operation %q, args.Op") + } + } + // Ensure that the specified behavior is allowed switch args.Session.Behavior { case "": @@ -130,6 +157,9 @@ func (s *Session) Get(args *structs.SessionSpecificRequest, } else { reply.Sessions = nil } + if err := s.srv.filterACL(args.Token, reply); err != nil { + return err + } return nil }) } @@ -154,6 +184,9 @@ func (s *Session) List(args *structs.DCSpecificRequest, } reply.Index, reply.Sessions = index, sessions + if err := s.srv.filterACL(args.Token, reply); err != nil { + return err + } return nil }) } @@ -178,6 +211,9 @@ func (s *Session) NodeSessions(args *structs.NodeSpecificRequest, } reply.Index, reply.Sessions = index, sessions + if err := s.srv.filterACL(args.Token, reply); err != nil { + return err + } return nil }) } @@ -190,21 +226,35 @@ func (s *Session) Renew(args *structs.SessionSpecificRequest, } defer metrics.MeasureSince([]string{"consul", "session", "renew"}, time.Now()) - // Get the session, from local state + // Get the session, from local state. state := s.srv.fsm.State() index, session, err := state.SessionGet(args.Session) if err != nil { return err } - // Reset the session TTL timer reply.Index = index - if session != nil { - reply.Sessions = structs.Sessions{session} - if err := s.srv.resetSessionTimer(args.Session, session); err != nil { - s.srv.logger.Printf("[ERR] consul.session: Session renew failed: %v", err) - return err + if session == nil { + return nil + } + + // Fetch the ACL token, if any, and apply the policy. + acl, err := s.srv.resolveToken(args.Token) + if err != nil { + return err + } + if acl != nil && s.srv.config.ACLEnforceVersion8 { + if !acl.SessionWrite(session.Node) { + return permissionDeniedErr } } + + // Reset the session TTL timer. + reply.Sessions = structs.Sessions{session} + if err := s.srv.resetSessionTimer(args.Session, session); err != nil { + s.srv.logger.Printf("[ERR] consul.session: Session renew failed: %v", err) + return err + } + return nil } diff --git a/consul/session_endpoint_test.go b/consul/session_endpoint_test.go index e5e0bed184..275a53aadb 100644 --- a/consul/session_endpoint_test.go +++ b/consul/session_endpoint_test.go @@ -2,6 +2,7 @@ package consul import ( "os" + "strings" "testing" "time" @@ -11,7 +12,7 @@ import ( "github.com/hashicorp/net-rpc-msgpackrpc" ) -func TestSessionEndpoint_Apply(t *testing.T) { +func TestSession_Apply(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -70,7 +71,7 @@ func TestSessionEndpoint_Apply(t *testing.T) { } } -func TestSessionEndpoint_DeleteApply(t *testing.T) { +func TestSession_DeleteApply(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -133,7 +134,101 @@ func TestSessionEndpoint_DeleteApply(t *testing.T) { } } -func TestSessionEndpoint_Get(t *testing.T) { +func TestSession_Apply_ACLDeny(t *testing.T) { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = false + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testutil.WaitForLeader(t, s1.RPC, "dc1") + + // Create the ACL. + req := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: ` +session "foo" { + policy = "write" +} +`, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + var token string + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &token); err != nil { + t.Fatalf("err: %v", err) + } + + // Just add a node. + s1.fsm.State().EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"}) + + // Try to create without a token, which will go through since version 8 + // enforcement isn't enabled. + arg := structs.SessionRequest{ + Datacenter: "dc1", + Op: structs.SessionCreate, + Session: structs.Session{ + Node: "foo", + Name: "my-session", + }, + } + var id1 string + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &id1); err != nil { + t.Fatalf("err: %v", err) + } + + // Now turn on version 8 enforcement and try again, it should be denied. + var id2 string + s1.config.ACLEnforceVersion8 = true + err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &id2) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Now set a token and try again. This should go through. + arg.Token = token + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &id2); err != nil { + t.Fatalf("err: %v", err) + } + + // Do a delete on the first session with version 8 enforcement off and + // no token. This should go through. + var out string + s1.config.ACLEnforceVersion8 = false + arg.Op = structs.SessionDestroy + arg.Token = "" + arg.Session.ID = id1 + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &out); err != nil { + t.Fatalf("err: %v", err) + } + + // Turn on version 8 enforcement and make sure the delete of the second + // session fails. + s1.config.ACLEnforceVersion8 = true + arg.Session.ID = id2 + err = msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &out) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Now set a token and try again. This should go through. + arg.Token = token + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &out); err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestSession_Get(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -176,7 +271,7 @@ func TestSessionEndpoint_Get(t *testing.T) { } } -func TestSessionEndpoint_List(t *testing.T) { +func TestSession_List(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -227,7 +322,175 @@ func TestSessionEndpoint_List(t *testing.T) { } } -func TestSessionEndpoint_ApplyTimers(t *testing.T) { +func TestSession_Get_List_NodeSessions_ACLFilter(t *testing.T) { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = false + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testutil.WaitForLeader(t, s1.RPC, "dc1") + + // Create the ACL. + req := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: ` +session "foo" { + policy = "read" +} +`, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + var token string + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &token); err != nil { + t.Fatalf("err: %v", err) + } + + // Create a node and a session. + s1.fsm.State().EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"}) + arg := structs.SessionRequest{ + Datacenter: "dc1", + Op: structs.SessionCreate, + Session: structs.Session{ + Node: "foo", + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + var out string + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &out); err != nil { + t.Fatalf("err: %v", err) + } + + // Perform all the read operations, which should go through since version + // 8 ACL enforcement isn't enabled. + getR := structs.SessionSpecificRequest{ + Datacenter: "dc1", + Session: out, + } + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.Get", &getR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 1 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + listR := structs.DCSpecificRequest{ + Datacenter: "dc1", + } + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.List", &listR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 1 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + nodeR := structs.NodeSpecificRequest{ + Datacenter: "dc1", + Node: "foo", + } + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.NodeSessions", &nodeR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 1 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + + // Now turn on version 8 enforcement and make sure everything is empty. + s1.config.ACLEnforceVersion8 = true + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.Get", &getR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 0 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + { + var sessions structs.IndexedSessions + + if err := msgpackrpc.CallWithCodec(codec, "Session.List", &listR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 0 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.NodeSessions", &nodeR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 0 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + + // Finally, supply the token and make sure the reads are allowed. + getR.Token = token + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.Get", &getR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 1 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + listR.Token = token + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.List", &listR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 1 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + nodeR.Token = token + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.NodeSessions", &nodeR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 1 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } + + // Try to get a session that doesn't exist to make sure that's handled + // correctly by the filter (it will get passed a nil slice). + getR.Session = "adf4238a-882b-9ddc-4a9d-5b6758e4159e" + { + var sessions structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.Get", &getR, &sessions); err != nil { + t.Fatalf("err: %v", err) + } + if len(sessions.Sessions) != 0 { + t.Fatalf("bad: %v", sessions.Sessions) + } + } +} + +func TestSession_ApplyTimers(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -268,7 +531,7 @@ func TestSessionEndpoint_ApplyTimers(t *testing.T) { } } -func TestSessionEndpoint_Renew(t *testing.T) { +func TestSession_Renew(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -428,7 +691,85 @@ func TestSessionEndpoint_Renew(t *testing.T) { } } -func TestSessionEndpoint_NodeSessions(t *testing.T) { +func TestSession_Renew_ACLDeny(t *testing.T) { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + c.ACLEnforceVersion8 = false + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testutil.WaitForLeader(t, s1.RPC, "dc1") + + // Create the ACL. + req := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: ` +session "foo" { + policy = "write" +} +`, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + + var token string + if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &req, &token); err != nil { + t.Fatalf("err: %v", err) + } + + // Just add a node. + s1.fsm.State().EnsureNode(1, &structs.Node{Node: "foo", Address: "127.0.0.1"}) + + // Create a session. The token won't matter here since we don't have + // version 8 ACL enforcement on yet. + arg := structs.SessionRequest{ + Datacenter: "dc1", + Op: structs.SessionCreate, + Session: structs.Session{ + Node: "foo", + Name: "my-session", + }, + } + var id string + if err := msgpackrpc.CallWithCodec(codec, "Session.Apply", &arg, &id); err != nil { + t.Fatalf("err: %v", err) + } + + // Renew without a token should go through without version 8 ACL + // enforcement. + renewR := structs.SessionSpecificRequest{ + Datacenter: "dc1", + Session: id, + } + var session structs.IndexedSessions + if err := msgpackrpc.CallWithCodec(codec, "Session.Renew", &renewR, &session); err != nil { + t.Fatalf("err: %v", err) + } + + // Now turn on version 8 enforcement and the renew should be rejected. + s1.config.ACLEnforceVersion8 = true + err := msgpackrpc.CallWithCodec(codec, "Session.Renew", &renewR, &session) + if err == nil || !strings.Contains(err.Error(), permissionDenied) { + t.Fatalf("err: %v", err) + } + + // Set the token and it should go through. + renewR.Token = token + if err := msgpackrpc.CallWithCodec(codec, "Session.Renew", &renewR, &session); err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestSession_NodeSessions(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -486,7 +827,7 @@ func TestSessionEndpoint_NodeSessions(t *testing.T) { } } -func TestSessionEndpoint_Apply_BadTTL(t *testing.T) { +func TestSession_Apply_BadTTL(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown()