diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index ee5d1b9b2..057c9f9cb 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -36,8 +36,8 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *struct{}) e if err != nil { return err } - if err := verifyConfigWriteACL(rule, args.Entry.GetKind(), args.Entry.GetName()); err != nil { - return err + if rule != nil && !args.Entry.VerifyWriteACL(rule) { + return acl.ErrPermissionDenied } args.Op = structs.ConfigEntryUpsert @@ -63,9 +63,15 @@ func (c *ConfigEntry) Get(args *structs.ConfigEntryQuery, reply *structs.Indexed if err != nil { return err } - if err := verifyConfigReadACL(rule, args.Kind, args.Name); err != nil { + + // Create a dummy config entry to check the ACL permissions. + lookupEntry, err := structs.MakeConfigEntry(args.Kind, args.Name) + if err != nil { return err } + if rule != nil && !lookupEntry.VerifyReadACL(rule) { + return acl.ErrPermissionDenied + } return c.srv.blockingQuery( &args.QueryOptions, @@ -77,6 +83,11 @@ func (c *ConfigEntry) Get(args *structs.ConfigEntryQuery, reply *structs.Indexed } reply.Index = index + if entry == nil { + return nil + } + + reply.Kind = args.Kind reply.Entries = []structs.ConfigEntry{entry} return nil }) @@ -106,20 +117,15 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe } // Filter the entries returned by ACL permissions. - // TODO(kyhavlov): should we handle the proxy config differently here since - // it's a singleton? filteredEntries := make([]structs.ConfigEntry, 0, len(entries)) for _, entry := range entries { - if err := verifyConfigReadACL(rule, entry.GetKind(), entry.GetName()); err != nil { - if acl.IsErrPermissionDenied(err) { - continue - } else { - return err - } + if rule != nil && !entry.VerifyReadACL(rule) { + continue } filteredEntries = append(filteredEntries, entry) } + reply.Kind = args.Kind reply.Index = index reply.Entries = filteredEntries return nil @@ -143,8 +149,8 @@ func (c *ConfigEntry) Delete(args *structs.ConfigEntryRequest, reply *struct{}) if err != nil { return err } - if err := verifyConfigWriteACL(rule, args.Entry.GetKind(), args.Entry.GetName()); err != nil { - return err + if rule != nil && !args.Entry.VerifyWriteACL(rule) { + return acl.ErrPermissionDenied } args.Op = structs.ConfigEntryDelete @@ -218,49 +224,3 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r return nil }) } - -// verifyConfigReadACL checks whether the given ACL authorizer has permission -// to read the config entry of the given kind/name. -func verifyConfigReadACL(rule acl.Authorizer, kind, name string) error { - if rule == nil { - return nil - } - - switch kind { - case structs.ServiceDefaults: - if !rule.ServiceRead(name) { - return acl.ErrPermissionDenied - } - case structs.ProxyDefaults: - if !rule.OperatorRead() { - return acl.ErrPermissionDenied - } - default: - return fmt.Errorf("unknown config entry type %q", kind) - } - - return nil -} - -// verifyConfigWriteACL checks whether the given ACL authorizer has permission -// to update the config entry of the given kind/name. -func verifyConfigWriteACL(rule acl.Authorizer, kind, name string) error { - if rule == nil { - return nil - } - - switch kind { - case structs.ServiceDefaults: - if !rule.ServiceWrite(name, nil) { - return acl.ErrPermissionDenied - } - case structs.ProxyDefaults: - if !rule.OperatorWrite() { - return acl.ErrPermissionDenied - } - default: - return fmt.Errorf("unknown config entry type %q", kind) - } - - return nil -} diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 0b19c98a8..12c2a43a7 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -200,7 +200,6 @@ operator = "read" t.Fatalf("err: %v", err) } - // Create a dummy service in the state store to look up. // Create some dummy service/proxy configs to be looked up. state := s1.fsm.State() require.NoError(state.EnsureConfigEntry(1, &structs.ProxyConfigEntry{ @@ -233,18 +232,6 @@ operator = "read" require.True(ok) require.Equal("foo", serviceConf.Name) require.Equal(structs.ServiceDefaults, serviceConf.Kind) - - // Try to look up the proxy config with no token. - args.Kind = structs.ProxyDefaults - args.Name = structs.ProxyConfigGlobal - args.QueryOptions.Token = "" - err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.Get", &args, &out) - if !acl.IsErrPermissionDenied(err) { - t.Fatalf("err: %v", err) - } - - args.QueryOptions.Token = id - require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.Get", &args, &out)) } func TestConfigEntry_List(t *testing.T) { @@ -282,6 +269,7 @@ func TestConfigEntry_List(t *testing.T) { var out structs.IndexedConfigEntries require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &out)) + expected.Kind = structs.ServiceDefaults expected.QueryMeta = out.QueryMeta require.Equal(expected, out) } diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index f89154b86..a20432a6a 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/go-msgpack/codec" ) @@ -25,6 +26,11 @@ type ConfigEntry interface { Normalize() error Validate() error + // VerifyReadACL and VerifyWriteACL return whether or not the given Authorizer + // has permission to read or write to the config entry, respectively. + VerifyReadACL(acl.Authorizer) bool + VerifyWriteACL(acl.Authorizer) bool + GetRaftIndex() *RaftIndex } @@ -70,6 +76,14 @@ func (e *ServiceConfigEntry) Validate() error { return nil } +func (e *ServiceConfigEntry) VerifyReadACL(rule acl.Authorizer) bool { + return rule.ServiceRead(e.Name) +} + +func (e *ServiceConfigEntry) VerifyWriteACL(rule acl.Authorizer) bool { + return rule.ServiceWrite(e.Name, nil) +} + func (e *ServiceConfigEntry) GetRaftIndex() *RaftIndex { if e == nil { return &RaftIndex{} @@ -126,6 +140,14 @@ func (e *ProxyConfigEntry) Validate() error { return nil } +func (e *ProxyConfigEntry) VerifyReadACL(rule acl.Authorizer) bool { + return true +} + +func (e *ProxyConfigEntry) VerifyWriteACL(rule acl.Authorizer) bool { + return rule.OperatorWrite() +} + func (e *ProxyConfigEntry) GetRaftIndex() *RaftIndex { if e == nil { return &RaftIndex{} @@ -186,7 +208,7 @@ func (c *ConfigEntryRequest) UnmarshalBinary(data []byte) error { } // Then decode the real thing with appropriate kind of ConfigEntry - entry, err := makeConfigEntry(kind) + entry, err := MakeConfigEntry(kind, "") if err != nil { return err } @@ -206,12 +228,12 @@ func (c *ConfigEntryRequest) UnmarshalBinary(data []byte) error { return nil } -func makeConfigEntry(kind string) (ConfigEntry, error) { +func MakeConfigEntry(kind, name string) (ConfigEntry, error) { switch kind { case ServiceDefaults: - return &ServiceConfigEntry{}, nil + return &ServiceConfigEntry{Name: name}, nil case ProxyDefaults: - return &ProxyConfigEntry{}, nil + return &ProxyConfigEntry{Name: name}, nil default: return nil, fmt.Errorf("invalid config entry kind: %s", kind) } diff --git a/agent/structs/structs.go b/agent/structs/structs.go index ba55da377..214fd0832 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -1143,6 +1143,7 @@ type IndexedNodeDump struct { // IndexedConfigEntries has its own encoding logic which differs from // ConfigEntryRequest as it has to send a slice of ConfigEntry. type IndexedConfigEntries struct { + Kind string Entries []ConfigEntry QueryMeta } @@ -1153,16 +1154,16 @@ func (c *IndexedConfigEntries) MarshalBinary() (data []byte, err error) { bs := make([]byte, 128) enc := codec.NewEncoderBytes(&bs, msgpackHandle) - // Encode kinds of entries first + // Encode length. err = enc.Encode(len(c.Entries)) if err != nil { return nil, err } - for _, entry := range c.Entries { - err = enc.Encode(entry.GetKind()) - if err != nil { - return nil, err - } + + // Encode kind. + err = enc.Encode(c.Kind) + if err != nil { + return nil, err } // Then actual value using alias trick to avoid infinite recursion @@ -1179,23 +1180,23 @@ func (c *IndexedConfigEntries) MarshalBinary() (data []byte, err error) { } func (c *IndexedConfigEntries) UnmarshalBinary(data []byte) error { - // First decode the number of entries + // First decode the number of entries. var numEntries int dec := codec.NewDecoderBytes(data, msgpackHandle) if err := dec.Decode(&numEntries); err != nil { return err } + // Next decode the kind. + var kind string + if err := dec.Decode(&kind); err != nil { + return err + } + + // Then decode the slice of ConfigEntries c.Entries = make([]ConfigEntry, numEntries) for i := 0; i < numEntries; i++ { - // First decode the kind prefix - var kind string - if err := dec.Decode(&kind); err != nil { - return err - } - - // Then decode the real thing with appropriate kind of ConfigEntry - entry, err := makeConfigEntry(kind) + entry, err := MakeConfigEntry(kind, "") if err != nil { return err }