From 1b8051a7835f9897b00cadfc4ec624368a0eb64b Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Mon, 6 Jul 2015 18:28:09 -0600 Subject: [PATCH 01/10] acl: initial pass at keyring ACLs --- acl/acl.go | 54 +++++++++++++++++++++++++++++++++++++ acl/policy.go | 26 ++++++++++++++++++ consul/internal_endpoint.go | 26 ++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/acl/acl.go b/acl/acl.go index 24b62d0b42..1a8a6d2bc8 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -58,6 +58,13 @@ type ACL interface { // EventWrite determines if a specific event may be fired. EventWrite(string) bool + // KeyringRead determines if the encryption keyring used in + // the gossip layer can be read. + KeyringRead() bool + + // KeyringWrite determines if the keyring can be manipulated + KeyringWrite() bool + // ACLList checks for permission to list all the ACLs ACLList() bool @@ -101,6 +108,14 @@ func (s *StaticACL) EventWrite(string) bool { return s.defaultAllow } +func (s *StaticACL) KeyringRead() bool { + return s.defaultAllow +} + +func (s *StaticACL) KeyringWrite() bool { + return s.defaultAllow +} + func (s *StaticACL) ACLList() bool { return s.allowManage } @@ -153,6 +168,11 @@ type PolicyACL struct { // eventRules contains the user event policies eventRules *radix.Tree + + // keyringRules contains the keyring policies. The keyring has + // a very simple yes/no without prefix mathing, so here we + // don't need to use a radix tree. + keyringRules map[string]struct{} } // New is used to construct a policy based ACL from a set of policies @@ -163,6 +183,7 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { keyRules: radix.New(), serviceRules: radix.New(), eventRules: radix.New(), + keyringRules: make(map[string]struct{}), } // Load the key policy @@ -180,6 +201,11 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { p.eventRules.Insert(ep.Event, ep.Policy) } + // Load the keyring policy + for _, krp := range policy.Keyring { + p.keyringRules[krp.Policy] = struct{}{} + } + return p, nil } @@ -321,6 +347,34 @@ func (p *PolicyACL) EventWrite(name string) bool { return p.parent.EventWrite(name) } +// KeyringRead is used to determine if the keyring can be +// read by the current ACL token. +func (p *PolicyACL) KeyringRead() bool { + // First check for an explicit deny + if _, ok := p.keyringRules[KeyringPolicyDeny]; ok { + return false + } + + // Now check for read or write. Write implies read. + _, ok := p.keyringRules[KeyringPolicyRead] + if !ok { + _, ok = p.keyringRules[KeyringPolicyWrite] + } + return ok +} + +// KeyringWrite determines if the keyring can be manipulated. +func (p *PolicyACL) KeyringWrite() bool { + // First check for an explicit deny + if _, ok := p.keyringRules[KeyringPolicyDeny]; ok { + return false + } + + // Check for read permission + _, ok := p.keyringRules[KeyringPolicyWrite] + return ok +} + // ACLList checks if listing of ACLs is allowed func (p *PolicyACL) ACLList() bool { return p.parent.ACLList() diff --git a/acl/policy.go b/acl/policy.go index 1b14b61ac6..0327b2ef2b 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -16,6 +16,9 @@ const ( EventPolicyRead = "read" EventPolicyWrite = "write" EventPolicyDeny = "deny" + KeyringPolicyWrite = "write" + KeyringPolicyRead = "read" + KeyringPolicyDeny = "deny" ) // Policy is used to represent the policy specified by @@ -25,6 +28,7 @@ type Policy struct { Keys []*KeyPolicy `hcl:"key,expand"` Services []*ServicePolicy `hcl:"service,expand"` Events []*EventPolicy `hcl:"event,expand"` + Keyring []*KeyringPolicy `hcl:"keyring"` } // KeyPolicy represents a policy for a key @@ -57,6 +61,17 @@ func (e *EventPolicy) GoString() string { return fmt.Sprintf("%#v", *e) } +// KeyringPolicy represents a policy for the encryption keyring. +type KeyringPolicy struct { + // We only need a single field for the keyring, since access + // is binary (allowed or disallowed) and no prefix is respected. + Policy string +} + +func (k *KeyringPolicy) GoString() string { + return fmt.Sprintf("%#v", *k) +} + // Parse is used to parse the specified ACL rules into an // intermediary set of policies, before being compiled into // the ACL @@ -105,5 +120,16 @@ func Parse(rules string) (*Policy, error) { } } + // Validate the keyring policy + for _, krp := range p.Keyring { + switch krp.Policy { + case KeyringPolicyRead: + case KeyringPolicyWrite: + case KeyringPolicyDeny: + default: + return nil, fmt.Errorf("Invalid keyring policy: %#v", krp) + } + } + return p, nil } diff --git a/consul/internal_endpoint.go b/consul/internal_endpoint.go index 639083416f..1806104f66 100644 --- a/consul/internal_endpoint.go +++ b/consul/internal_endpoint.go @@ -1,6 +1,8 @@ package consul import ( + "fmt" + "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/serf/serf" ) @@ -80,6 +82,30 @@ func (m *Internal) KeyringOperation( args *structs.KeyringRequest, reply *structs.KeyringResponses) error { + // Check ACLs + acl, err := m.srv.resolveToken(args.Token) + if err != nil { + return err + } + if acl != nil { + switch args.Operation { + case structs.KeyringList: + if !acl.KeyringRead() { + return fmt.Errorf("Reading keyring denied by ACLs") + } + case structs.KeyringInstall: + fallthrough + case structs.KeyringUse: + fallthrough + case structs.KeyringRemove: + if !acl.KeyringWrite() { + return fmt.Errorf("Modifying keyring denied due to ACLs") + } + default: + panic("Invalid keyring operation") + } + } + // Only perform WAN keyring querying and RPC forwarding once if !args.Forwarded { args.Forwarded = true From bffc0861cc4bb2369ed75b6deecdb9c57613b11b Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 10:30:34 -0600 Subject: [PATCH 02/10] agent: read-level keyring ACLs work --- command/agent/keyring.go | 3 ++- command/agent/rpc.go | 8 +++++--- command/agent/rpc_client.go | 3 ++- command/keyring.go | 15 +++++++++------ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/command/agent/keyring.go b/command/agent/keyring.go index 07bd19b0c8..4d44991967 100644 --- a/command/agent/keyring.go +++ b/command/agent/keyring.go @@ -121,8 +121,9 @@ func (a *Agent) keyringProcess(args *structs.KeyringRequest) (*structs.KeyringRe // ListKeys lists out all keys installed on the collective Consul cluster. This // includes both servers and clients in all DC's. -func (a *Agent) ListKeys() (*structs.KeyringResponses, error) { +func (a *Agent) ListKeys(token string) (*structs.KeyringResponses, error) { args := structs.KeyringRequest{Operation: structs.KeyringList} + args.Token = token return a.keyringProcess(&args) } diff --git a/command/agent/rpc.go b/command/agent/rpc.go index 56d9bc7a3d..0e64549a39 100644 --- a/command/agent/rpc.go +++ b/command/agent/rpc.go @@ -78,6 +78,7 @@ var msgpackHandle = &codec.MsgpackHandle{ type requestHeader struct { Command string Seq uint64 + Token string } // Response header is sent before each response @@ -365,6 +366,7 @@ func (i *AgentRPC) handleRequest(client *rpcClient, reqHeader *requestHeader) er // Look for a command field command := reqHeader.Command seq := reqHeader.Seq + token := reqHeader.Token // Ensure the handshake is performed before other commands if command != handshakeCommand && client.version == 0 { @@ -406,7 +408,7 @@ func (i *AgentRPC) handleRequest(client *rpcClient, reqHeader *requestHeader) er return i.handleReload(client, seq) case installKeyCommand, useKeyCommand, removeKeyCommand, listKeysCommand: - return i.handleKeyring(client, seq, command) + return i.handleKeyring(client, seq, command, token) default: respHeader := responseHeader{Seq: seq, Error: unsupportedCommand} @@ -618,7 +620,7 @@ func (i *AgentRPC) handleReload(client *rpcClient, seq uint64) error { return client.Send(&resp, nil) } -func (i *AgentRPC) handleKeyring(client *rpcClient, seq uint64, cmd string) error { +func (i *AgentRPC) handleKeyring(client *rpcClient, seq uint64, cmd, token string) error { var req keyringRequest var queryResp *structs.KeyringResponses var r keyringResponse @@ -632,7 +634,7 @@ func (i *AgentRPC) handleKeyring(client *rpcClient, seq uint64, cmd string) erro switch cmd { case listKeysCommand: - queryResp, err = i.agent.ListKeys() + queryResp, err = i.agent.ListKeys(token) case installKeyCommand: queryResp, err = i.agent.InstallKey(req.Key) case useKeyCommand: diff --git a/command/agent/rpc_client.go b/command/agent/rpc_client.go index cbc9689cfb..4bcf4a4b89 100644 --- a/command/agent/rpc_client.go +++ b/command/agent/rpc_client.go @@ -188,10 +188,11 @@ func (c *RPCClient) WANMembers() ([]Member, error) { return resp.Members, err } -func (c *RPCClient) ListKeys() (keyringResponse, error) { +func (c *RPCClient) ListKeys(token string) (keyringResponse, error) { header := requestHeader{ Command: listKeysCommand, Seq: c.getSeq(), + Token: token, } var resp keyringResponse err := c.genericRPC(&header, nil, &resp) diff --git a/command/keyring.go b/command/keyring.go index ee072b8792..50645230f4 100644 --- a/command/keyring.go +++ b/command/keyring.go @@ -16,7 +16,7 @@ type KeyringCommand struct { } func (c *KeyringCommand) Run(args []string) int { - var installKey, useKey, removeKey string + var installKey, useKey, removeKey, token string var listKeys bool cmdFlags := flag.NewFlagSet("keys", flag.ContinueOnError) @@ -26,6 +26,7 @@ func (c *KeyringCommand) Run(args []string) int { cmdFlags.StringVar(&useKey, "use", "", "use key") cmdFlags.StringVar(&removeKey, "remove", "", "remove key") cmdFlags.BoolVar(&listKeys, "list", false, "list keys") + cmdFlags.StringVar(&token, "token", "", "acl token") rpcAddr := RPCAddrFlag(cmdFlags) if err := cmdFlags.Parse(args); err != nil { @@ -65,7 +66,7 @@ func (c *KeyringCommand) Run(args []string) int { if listKeys { c.Ui.Info("Gathering installed encryption keys...") - r, err := client.ListKeys() + r, err := client.ListKeys(token) if err != nil { c.Ui.Error(fmt.Sprintf("error: %s", err)) return 1 @@ -199,13 +200,15 @@ Options: -install= Install a new encryption key. This will broadcast the new key to all members in the cluster. - -use= Change the primary encryption key, which is used to - encrypt messages. The key must already be installed - before this operation can succeed. + -list List all keys currently in use within the cluster. -remove= Remove the given key from the cluster. This operation may only be performed on keys which are not currently the primary key. - -list List all keys currently in use within the cluster. + -token="" ACL token to use during requests. Defaults to that + of the agent. + -use= Change the primary encryption key, which is used to + encrypt messages. The key must already be installed + before this operation can succeed. -rpc-addr=127.0.0.1:8400 RPC address of the Consul agent. ` return strings.TrimSpace(helpText) From 5c65bc7df24d4daf28fbae48e8fb7c5f45498125 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 10:36:51 -0600 Subject: [PATCH 03/10] agent: write-level keyring ACLs work --- command/agent/keyring.go | 9 ++++++--- command/agent/rpc.go | 6 +++--- command/agent/rpc_client.go | 9 ++++++--- command/keyring.go | 6 +++--- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/command/agent/keyring.go b/command/agent/keyring.go index 4d44991967..f51b04c0c3 100644 --- a/command/agent/keyring.go +++ b/command/agent/keyring.go @@ -128,19 +128,22 @@ func (a *Agent) ListKeys(token string) (*structs.KeyringResponses, error) { } // InstallKey installs a new gossip encryption key -func (a *Agent) InstallKey(key string) (*structs.KeyringResponses, error) { +func (a *Agent) InstallKey(key, token string) (*structs.KeyringResponses, error) { args := structs.KeyringRequest{Key: key, Operation: structs.KeyringInstall} + args.Token = token return a.keyringProcess(&args) } // UseKey changes the primary encryption key used to encrypt messages -func (a *Agent) UseKey(key string) (*structs.KeyringResponses, error) { +func (a *Agent) UseKey(key, token string) (*structs.KeyringResponses, error) { args := structs.KeyringRequest{Key: key, Operation: structs.KeyringUse} + args.Token = token return a.keyringProcess(&args) } // RemoveKey will remove a gossip encryption key from the keyring -func (a *Agent) RemoveKey(key string) (*structs.KeyringResponses, error) { +func (a *Agent) RemoveKey(key, token string) (*structs.KeyringResponses, error) { args := structs.KeyringRequest{Key: key, Operation: structs.KeyringRemove} + args.Token = token return a.keyringProcess(&args) } diff --git a/command/agent/rpc.go b/command/agent/rpc.go index 0e64549a39..dd2d376c60 100644 --- a/command/agent/rpc.go +++ b/command/agent/rpc.go @@ -636,11 +636,11 @@ func (i *AgentRPC) handleKeyring(client *rpcClient, seq uint64, cmd, token strin case listKeysCommand: queryResp, err = i.agent.ListKeys(token) case installKeyCommand: - queryResp, err = i.agent.InstallKey(req.Key) + queryResp, err = i.agent.InstallKey(req.Key, token) case useKeyCommand: - queryResp, err = i.agent.UseKey(req.Key) + queryResp, err = i.agent.UseKey(req.Key, token) case removeKeyCommand: - queryResp, err = i.agent.RemoveKey(req.Key) + queryResp, err = i.agent.RemoveKey(req.Key, token) default: respHeader := responseHeader{Seq: seq, Error: unsupportedCommand} client.Send(&respHeader, nil) diff --git a/command/agent/rpc_client.go b/command/agent/rpc_client.go index 4bcf4a4b89..3ce90b1634 100644 --- a/command/agent/rpc_client.go +++ b/command/agent/rpc_client.go @@ -199,10 +199,11 @@ func (c *RPCClient) ListKeys(token string) (keyringResponse, error) { return resp, err } -func (c *RPCClient) InstallKey(key string) (keyringResponse, error) { +func (c *RPCClient) InstallKey(key, token string) (keyringResponse, error) { header := requestHeader{ Command: installKeyCommand, Seq: c.getSeq(), + Token: token, } req := keyringRequest{key} var resp keyringResponse @@ -210,10 +211,11 @@ func (c *RPCClient) InstallKey(key string) (keyringResponse, error) { return resp, err } -func (c *RPCClient) UseKey(key string) (keyringResponse, error) { +func (c *RPCClient) UseKey(key, token string) (keyringResponse, error) { header := requestHeader{ Command: useKeyCommand, Seq: c.getSeq(), + Token: token, } req := keyringRequest{key} var resp keyringResponse @@ -221,10 +223,11 @@ func (c *RPCClient) UseKey(key string) (keyringResponse, error) { return resp, err } -func (c *RPCClient) RemoveKey(key string) (keyringResponse, error) { +func (c *RPCClient) RemoveKey(key, token string) (keyringResponse, error) { header := requestHeader{ Command: removeKeyCommand, Seq: c.getSeq(), + Token: token, } req := keyringRequest{key} var resp keyringResponse diff --git a/command/keyring.go b/command/keyring.go index 50645230f4..3a47cb9358 100644 --- a/command/keyring.go +++ b/command/keyring.go @@ -80,7 +80,7 @@ func (c *KeyringCommand) Run(args []string) int { if installKey != "" { c.Ui.Info("Installing new gossip encryption key...") - r, err := client.InstallKey(installKey) + r, err := client.InstallKey(installKey, token) if err != nil { c.Ui.Error(fmt.Sprintf("error: %s", err)) return 1 @@ -90,7 +90,7 @@ func (c *KeyringCommand) Run(args []string) int { if useKey != "" { c.Ui.Info("Changing primary gossip encryption key...") - r, err := client.UseKey(useKey) + r, err := client.UseKey(useKey, token) if err != nil { c.Ui.Error(fmt.Sprintf("error: %s", err)) return 1 @@ -100,7 +100,7 @@ func (c *KeyringCommand) Run(args []string) int { if removeKey != "" { c.Ui.Info("Removing gossip encryption key...") - r, err := client.RemoveKey(removeKey) + r, err := client.RemoveKey(removeKey, token) if err != nil { c.Ui.Error(fmt.Sprintf("error: %s", err)) return 1 From 47a33e3f1aa0aea91a98bcb25d1b0421ffc7b148 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 10:45:38 -0600 Subject: [PATCH 04/10] acl: keyring policy uses a flat string --- acl/acl.go | 29 +++++++---------------------- acl/policy.go | 27 +++++++-------------------- 2 files changed, 14 insertions(+), 42 deletions(-) diff --git a/acl/acl.go b/acl/acl.go index 1a8a6d2bc8..29a7569d81 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -172,7 +172,7 @@ type PolicyACL struct { // keyringRules contains the keyring policies. The keyring has // a very simple yes/no without prefix mathing, so here we // don't need to use a radix tree. - keyringRules map[string]struct{} + keyringRule string } // New is used to construct a policy based ACL from a set of policies @@ -183,7 +183,6 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { keyRules: radix.New(), serviceRules: radix.New(), eventRules: radix.New(), - keyringRules: make(map[string]struct{}), } // Load the key policy @@ -202,9 +201,7 @@ func New(parent ACL, policy *Policy) (*PolicyACL, error) { } // Load the keyring policy - for _, krp := range policy.Keyring { - p.keyringRules[krp.Policy] = struct{}{} - } + p.keyringRule = policy.Keyring return p, nil } @@ -350,29 +347,17 @@ func (p *PolicyACL) EventWrite(name string) bool { // KeyringRead is used to determine if the keyring can be // read by the current ACL token. func (p *PolicyACL) KeyringRead() bool { - // First check for an explicit deny - if _, ok := p.keyringRules[KeyringPolicyDeny]; ok { + switch p.keyringRule { + case KeyringPolicyRead, KeyringPolicyWrite: + return true + default: return false } - - // Now check for read or write. Write implies read. - _, ok := p.keyringRules[KeyringPolicyRead] - if !ok { - _, ok = p.keyringRules[KeyringPolicyWrite] - } - return ok } // KeyringWrite determines if the keyring can be manipulated. func (p *PolicyACL) KeyringWrite() bool { - // First check for an explicit deny - if _, ok := p.keyringRules[KeyringPolicyDeny]; ok { - return false - } - - // Check for read permission - _, ok := p.keyringRules[KeyringPolicyWrite] - return ok + return p.keyringRule == KeyringPolicyWrite } // ACLList checks if listing of ACLs is allowed diff --git a/acl/policy.go b/acl/policy.go index 0327b2ef2b..d9e62792e8 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -28,7 +28,7 @@ type Policy struct { Keys []*KeyPolicy `hcl:"key,expand"` Services []*ServicePolicy `hcl:"service,expand"` Events []*EventPolicy `hcl:"event,expand"` - Keyring []*KeyringPolicy `hcl:"keyring"` + Keyring string `hcl:"keyring"` } // KeyPolicy represents a policy for a key @@ -61,17 +61,6 @@ func (e *EventPolicy) GoString() string { return fmt.Sprintf("%#v", *e) } -// KeyringPolicy represents a policy for the encryption keyring. -type KeyringPolicy struct { - // We only need a single field for the keyring, since access - // is binary (allowed or disallowed) and no prefix is respected. - Policy string -} - -func (k *KeyringPolicy) GoString() string { - return fmt.Sprintf("%#v", *k) -} - // Parse is used to parse the specified ACL rules into an // intermediary set of policies, before being compiled into // the ACL @@ -121,14 +110,12 @@ func Parse(rules string) (*Policy, error) { } // Validate the keyring policy - for _, krp := range p.Keyring { - switch krp.Policy { - case KeyringPolicyRead: - case KeyringPolicyWrite: - case KeyringPolicyDeny: - default: - return nil, fmt.Errorf("Invalid keyring policy: %#v", krp) - } + switch p.Keyring { + case KeyringPolicyRead: + case KeyringPolicyWrite: + case KeyringPolicyDeny: + default: + return nil, fmt.Errorf("Invalid keyring policy: %#v", p.Keyring) } return p, nil From 7e50a457d9fdda119f62e698d1771ae0757a96fb Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 11:07:37 -0600 Subject: [PATCH 05/10] acl: allow omitting keyring policy, add tests --- acl/acl.go | 9 +++++++-- acl/acl_test.go | 30 ++++++++++++++++++++++++++++++ acl/policy.go | 1 + acl/policy_test.go | 6 +++++- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/acl/acl.go b/acl/acl.go index 29a7569d81..f18be42b55 100644 --- a/acl/acl.go +++ b/acl/acl.go @@ -350,14 +350,19 @@ func (p *PolicyACL) KeyringRead() bool { switch p.keyringRule { case KeyringPolicyRead, KeyringPolicyWrite: return true - default: + case KeyringPolicyDeny: return false + default: + return p.parent.KeyringRead() } } // KeyringWrite determines if the keyring can be manipulated. func (p *PolicyACL) KeyringWrite() bool { - return p.keyringRule == KeyringPolicyWrite + if p.keyringRule == KeyringPolicyWrite { + return true + } + return p.parent.KeyringWrite() } // ACLList checks if listing of ACLs is allowed diff --git a/acl/acl_test.go b/acl/acl_test.go index 5bd77dc8b2..6872b04f13 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -47,6 +47,18 @@ func TestStaticACL(t *testing.T) { if !all.ServiceWrite("foobar") { t.Fatalf("should allow") } + if !all.EventRead("foobar") { + t.Fatalf("should allow") + } + if !all.EventWrite("foobar") { + t.Fatalf("should allow") + } + if !all.KeyringRead() { + t.Fatalf("should allow") + } + if !all.KeyringWrite() { + t.Fatalf("should allow") + } if all.ACLList() { t.Fatalf("should not allow") } @@ -78,6 +90,12 @@ func TestStaticACL(t *testing.T) { if none.EventWrite("") { t.Fatalf("should not allow") } + if none.KeyringRead() { + t.Fatalf("should now allow") + } + if none.KeyringWrite() { + t.Fatalf("should not allow") + } if none.ACLList() { t.Fatalf("should not allow") } @@ -97,6 +115,18 @@ func TestStaticACL(t *testing.T) { if !manage.ServiceWrite("foobar") { t.Fatalf("should allow") } + if !manage.EventRead("foobar") { + t.Fatalf("should allow") + } + if !manage.EventWrite("foobar") { + t.Fatalf("should allow") + } + if !manage.KeyringRead() { + t.Fatalf("should allow") + } + if !manage.KeyringWrite() { + t.Fatalf("should allow") + } if !manage.ACLList() { t.Fatalf("should allow") } diff --git a/acl/policy.go b/acl/policy.go index d9e62792e8..9009ee76b5 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -114,6 +114,7 @@ func Parse(rules string) (*Policy, error) { case KeyringPolicyRead: case KeyringPolicyWrite: case KeyringPolicyDeny: + case "": // Special case to allow omitting the keyring policy default: return nil, fmt.Errorf("Invalid keyring policy: %#v", p.Keyring) } diff --git a/acl/policy_test.go b/acl/policy_test.go index 11f815da2b..043dc9c946 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -34,6 +34,7 @@ event "foo" { event "bar" { policy = "deny" } +keyring = "deny" ` exp := &Policy{ Keys: []*KeyPolicy{ @@ -78,6 +79,7 @@ event "bar" { Policy: EventPolicyDeny, }, }, + Keyring: KeyringPolicyDeny, } out, err := Parse(inp) @@ -124,7 +126,8 @@ func TestParse_JSON(t *testing.T) { "bar": { "policy": "deny" } - } + }, + "keyring": "deny" }` exp := &Policy{ Keys: []*KeyPolicy{ @@ -169,6 +172,7 @@ func TestParse_JSON(t *testing.T) { Policy: EventPolicyDeny, }, }, + Keyring: KeyringPolicyDeny, } out, err := Parse(inp) From 02b49058a2e904610b809235072e1ce0435084f4 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 11:21:27 -0600 Subject: [PATCH 06/10] acl: more keyring tests --- acl/acl_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/acl/acl_test.go b/acl/acl_test.go index 6872b04f13..1b83c81dec 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -245,6 +245,7 @@ func TestPolicyACL(t *testing.T) { } } + // Test the events type eventcase struct { inp string read bool @@ -369,3 +370,30 @@ func TestPolicyACL_Parent(t *testing.T) { } } } + +func TestPolicyACL_Keyring(t *testing.T) { + // Test keyring ACLs + type keyringcase struct { + inp string + read bool + write bool + } + keyringcases := []keyringcase{ + {"", false, false}, + {KeyringPolicyRead, true, false}, + {KeyringPolicyWrite, true, true}, + {KeyringPolicyDeny, false, false}, + } + for _, c := range keyringcases { + acl, err := New(DenyAll(), &Policy{Keyring: c.inp}) + if err != nil { + t.Fatalf("bad: %s", err) + } + if acl.KeyringRead() != c.read { + t.Fatalf("bad: %#v", c) + } + if acl.KeyringWrite() != c.write { + t.Fatalf("bad: %#v", c) + } + } +} From 38175f450bec2d32617576bc71096c7e3937c621 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 11:45:49 -0600 Subject: [PATCH 07/10] website: docs for keyring ACLs --- .../source/docs/internals/acl.html.markdown | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/website/source/docs/internals/acl.html.markdown b/website/source/docs/internals/acl.html.markdown index ff3eb47ffc..c1daa63738 100644 --- a/website/source/docs/internals/acl.html.markdown +++ b/website/source/docs/internals/acl.html.markdown @@ -18,8 +18,8 @@ on tokens to which fine grained rules can be applied. It is very similar to When the ACL system was launched in Consul 0.4, it was only possible to specify policies for the KV store. In Consul 0.5, ACL policies were extended to service -registrations. In Consul 0.6, ACL's were further extended to restrict the -service discovery mechanisms and user events.. +registrations. In Consul 0.6, ACL's were further extended to restrict service +discovery mechanisms, user events, and encryption keyring operations. ## ACL Design @@ -147,6 +147,27 @@ event "" { As always, the more secure way to handle user events is to explicitly grant access to each API token based on the events they should be able to fire. +### Blacklist mode and Keyring Operations + +Consul 0.6 and later supports securing the encryption keyring operations using +ACL's. Encryption is an optional component of the gossip layer. More information +about Consul's keyring operations can be found on the [keyring +command](/docs/commands/keyring.html) documentation page. + +If your [`acl_default_policy`](/docs/agent/options.html#acl_default_policy) is +set to `deny`, then the `anonymous` token will not have access to read or write +to the encryption keyring. The keyring policy is yet another first-class citizen +in the ACL syntax. You can configure the anonymous token to have free reign over +the keyring using a policy like the following: + +``` +keyring = "write" +``` + +Encryption keyring operations are sensitive and should be properly secured. It +is recommended that instead of configuring a wide-open policy like above, a +per-token policy is applied to maximize security. + ### Bootstrapping ACLs Bootstrapping the ACL system is done by providing an initial [`acl_master_token` @@ -229,6 +250,9 @@ event "" { event "destroy-" { policy = "deny" } + +# Read-only mode for the encryption keyring by default (list only) +keyring = "read" ``` This is equivalent to the following JSON input: @@ -261,7 +285,8 @@ This is equivalent to the following JSON input: "destroy-": { "policy": "deny" } - } + }, + "keyring": "read" } ``` From 58c26497a92dcae2fbef45e190ba58ee9a3db709 Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 14:46:41 -0600 Subject: [PATCH 08/10] acl: adding negative tests for bad policy --- acl/policy_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/acl/policy_test.go b/acl/policy_test.go index 043dc9c946..eb0528ffc9 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -2,6 +2,7 @@ package acl import ( "reflect" + "strings" "testing" ) @@ -184,3 +185,18 @@ func TestParse_JSON(t *testing.T) { t.Fatalf("bad: %#v %#v", out, exp) } } + +func TestACLPolicy_badPolicy(t *testing.T) { + cases := []string{ + `key "" { policy = "nope" }`, + `service "" { policy = "nope" }`, + `event "" { policy = "nope" }`, + `keyring = "nope"`, + } + for _, c := range cases { + _, err := Parse(c) + if err == nil || !strings.Contains(err.Error(), "Invalid") { + t.Fatalf("expected policy error, got: %#v", err) + } + } +} From 79ac4f3512ec6682cce41f6443b897221196d68d Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Tue, 7 Jul 2015 15:14:06 -0600 Subject: [PATCH 09/10] agent: testing keyring ACLs --- command/agent/keyring_test.go | 66 ++++++++++++++++++++++++++++++++ command/agent/rpc_client_test.go | 14 +++---- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/command/agent/keyring_test.go b/command/agent/keyring_test.go index 558c71f5dc..f364b6fa86 100644 --- a/command/agent/keyring_test.go +++ b/command/agent/keyring_test.go @@ -5,7 +5,10 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" + + "github.com/hashicorp/consul/testutil" ) func TestAgent_LoadKeyrings(t *testing.T) { @@ -113,3 +116,66 @@ func TestAgent_InitKeyring(t *testing.T) { t.Fatalf("bad: %s", content) } } + +func TestAgentKeyring_ACL(t *testing.T) { + key1 := "tbLJg26ZJyJ9pK3qhc9jig==" + key2 := "4leC33rgtXKIVUr9Nr0snQ==" + + conf := nextConfig() + conf.ACLDatacenter = "dc1" + conf.ACLMasterToken = "root" + conf.ACLDefaultPolicy = "deny" + dir, agent := makeAgentKeyring(t, conf, key1) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + testutil.WaitForLeader(t, agent.RPC, "dc1") + + // List keys without access fails + _, err := agent.ListKeys("") + if err == nil || !strings.Contains(err.Error(), "denied") { + t.Fatalf("expected denied error, got: %#v", err) + } + + // List keys with access works + _, err = agent.ListKeys("root") + if err != nil { + t.Fatalf("err: %s", err) + } + + // Install without access fails + _, err = agent.InstallKey(key2, "") + if err == nil || !strings.Contains(err.Error(), "denied") { + t.Fatalf("expected denied error, got: %#v", err) + } + + // Install with access works + _, err = agent.InstallKey(key2, "root") + if err != nil { + t.Fatalf("err: %s", err) + } + + // Use without access fails + _, err = agent.UseKey(key2, "") + if err == nil || !strings.Contains(err.Error(), "denied") { + t.Fatalf("expected denied error, got: %#v", err) + } + + // Use with access works + _, err = agent.UseKey(key2, "root") + if err != nil { + t.Fatalf("err: %s", err) + } + + // Remove without access fails + _, err = agent.RemoveKey(key1, "") + if err == nil || !strings.Contains(err.Error(), "denied") { + t.Fatalf("expected denied error, got: %#v", err) + } + + // Remove with access works + _, err = agent.RemoveKey(key1, "root") + if err != nil { + t.Fatalf("err: %s", err) + } +} diff --git a/command/agent/rpc_client_test.go b/command/agent/rpc_client_test.go index 48d8335649..31ad1ee03b 100644 --- a/command/agent/rpc_client_test.go +++ b/command/agent/rpc_client_test.go @@ -361,7 +361,7 @@ func TestRPCClientInstallKey(t *testing.T) { }) // install key2 - r, err := p1.client.InstallKey(key2) + r, err := p1.client.InstallKey(key2, "") if err != nil { t.Fatalf("err: %s", err) } @@ -391,7 +391,7 @@ func TestRPCClientUseKey(t *testing.T) { defer p1.Close() // add a second key to the ring - r, err := p1.client.InstallKey(key2) + r, err := p1.client.InstallKey(key2, "") if err != nil { t.Fatalf("err: %s", err) } @@ -412,21 +412,21 @@ func TestRPCClientUseKey(t *testing.T) { }) // can't remove key1 yet - r, err = p1.client.RemoveKey(key1) + r, err = p1.client.RemoveKey(key1, "") if err != nil { t.Fatalf("err: %s", err) } keyringError(t, r) // change primary key - r, err = p1.client.UseKey(key2) + r, err = p1.client.UseKey(key2, "") if err != nil { t.Fatalf("err: %s", err) } keyringSuccess(t, r) // can remove key1 now - r, err = p1.client.RemoveKey(key1) + r, err = p1.client.RemoveKey(key1, "") if err != nil { t.Fatalf("err: %s", err) } @@ -437,7 +437,7 @@ func TestRPCClientKeyOperation_encryptionDisabled(t *testing.T) { p1 := testRPCClient(t) defer p1.Close() - r, err := p1.client.ListKeys() + r, err := p1.client.ListKeys("") if err != nil { t.Fatalf("err: %s", err) } @@ -445,7 +445,7 @@ func TestRPCClientKeyOperation_encryptionDisabled(t *testing.T) { } func listKeys(t *testing.T, c *RPCClient) map[string]map[string]int { - resp, err := c.ListKeys() + resp, err := c.ListKeys("") if err != nil { t.Fatalf("err: %s", err) } From 7aa8539c108a672a54cd8460503221ae017390de Mon Sep 17 00:00:00 2001 From: Ryan Uber Date: Thu, 23 Jul 2015 17:09:33 -0700 Subject: [PATCH 10/10] agent: disable ACLs for RPC client tests --- command/agent/rpc_client_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/command/agent/rpc_client_test.go b/command/agent/rpc_client_test.go index 31ad1ee03b..af1af23ea2 100644 --- a/command/agent/rpc_client_test.go +++ b/command/agent/rpc_client_test.go @@ -325,6 +325,7 @@ func TestRPCClientListKeys(t *testing.T) { p1 := testRPCClientWithConfig(t, func(c *Config) { c.EncryptKey = key1 c.Datacenter = "dc1" + c.ACLDatacenter = "" }) defer p1.Close() @@ -343,6 +344,7 @@ func TestRPCClientInstallKey(t *testing.T) { key2 := "xAEZ3uVHRMZD9GcYMZaRQw==" p1 := testRPCClientWithConfig(t, func(c *Config) { c.EncryptKey = key1 + c.ACLDatacenter = "" }) defer p1.Close() @@ -387,6 +389,7 @@ func TestRPCClientUseKey(t *testing.T) { key2 := "xAEZ3uVHRMZD9GcYMZaRQw==" p1 := testRPCClientWithConfig(t, func(c *Config) { c.EncryptKey = key1 + c.ACLDatacenter = "" }) defer p1.Close() @@ -434,7 +437,9 @@ func TestRPCClientUseKey(t *testing.T) { } func TestRPCClientKeyOperation_encryptionDisabled(t *testing.T) { - p1 := testRPCClient(t) + p1 := testRPCClientWithConfig(t, func(c *Config) { + c.ACLDatacenter = "" + }) defer p1.Close() r, err := p1.client.ListKeys("")