luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Refactor config checks oss (#12550) 

Currently the config_entry.go subsystem delegates authorization decisions via the ConfigEntry interface CanRead and CanWrite code. Unfortunately this returns a true/false value and loses the details of the source.

This is not helpful, especially since it the config subsystem can be more complex to understand, since it covers so many domains.

This refactors CanRead/CanWrite to return a structured error message (PermissionDenied or the like) with more details about the reason for denial.

Part of #12241

Signed-off-by: Mark Anderson <manderson@hashicorp.com>
This commit is contained in:
Mark Anderson 2022-03-11 13:45:51 -08:00 committed by GitHub
parent 88accc6c94
commit ab099e5fcb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 69 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
agent/consul/config_endpoint.go
View File

@ -89,8 +89,8 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error
return err
}
if !args.Entry.CanWrite(authz) {
return acl.ErrPermissionDenied // TODO(acl-error-enhancements) Better errors await refactoring of CanWrite above.
if err := args.Entry.CanWrite(authz); err != nil {
return err
}
if args.Op != structs.ConfigEntryUpsert && args.Op != structs.ConfigEntryUpsertCAS {
@ -194,8 +194,8 @@ func (c *ConfigEntry) Get(args *structs.ConfigEntryQuery, reply *structs.ConfigE
}
lookupEntry.GetEnterpriseMeta().Merge(&args.EnterpriseMeta)
if !lookupEntry.CanRead(authz) {
return acl.ErrPermissionDenied
if err := lookupEntry.CanRead(authz); err != nil {
return err
}
return c.srv.blockingQuery(
@ -254,7 +254,8 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe
// Filter the entries returned by ACL permissions.
filteredEntries := make([]structs.ConfigEntry, 0, len(entries))
for _, entry := range entries {
if !entry.CanRead(authz) {
if err := entry.CanRead(authz); err != nil {
// TODO we may wish to extract more details from this error to aid user comprehension
reply.QueryMeta.ResultsFilteredByACLs = true
continue
}
@ -335,7 +336,8 @@ func (c *ConfigEntry) ListAll(args *structs.ConfigEntryListAllRequest, reply *st
// Filter the entries returned by ACL permissions or by the provided kinds.
filteredEntries := make([]structs.ConfigEntry, 0, len(entries))
for _, entry := range entries {
if !entry.CanRead(authz) {
if err := entry.CanRead(authz); err != nil {
// TODO we may wish to extract more details from this error to aid user comprehension
reply.QueryMeta.ResultsFilteredByACLs = true
continue
}
@ -386,8 +388,8 @@ func (c *ConfigEntry) Delete(args *structs.ConfigEntryRequest, reply *structs.Co
return err
}
if !args.Entry.CanWrite(authz) {
return acl.ErrPermissionDenied
if err := args.Entry.CanWrite(authz); err != nil {
return err
}
// Only delete and delete-cas ops are supported. If the caller erroneously

2
agent/http.go
View File

@ -273,7 +273,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler {
// If the token provided does not have the necessary permissions,
// write a forbidden response
// TODO(partitions): should this be possible in a partition?
// TODO((acl-error-enhancements)): We should return error details somehow here.
// TODO(acl-error-enhancements): We should return error details somehow here.
if authz.OperatorRead(nil) != acl.Allow {
resp.WriteHeader(http.StatusForbidden)
return

20
agent/structs/config_entry.go
View File

@ -60,8 +60,8 @@ type ConfigEntry interface {
// CanRead and CanWrite return whether or not the given Authorizer
// has permission to read or write to the config entry, respectively.
CanRead(acl.Authorizer) bool
CanWrite(acl.Authorizer) bool
CanRead(acl.Authorizer) error
CanWrite(acl.Authorizer) error
GetMeta() map[string]string
GetEnterpriseMeta() *EnterpriseMeta
@ -183,16 +183,16 @@ func (e *ServiceConfigEntry) Validate() error {
return validationErr
}
func (e *ServiceConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *ServiceConfigEntry) CanRead(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.ServiceRead(e.Name, &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().ServiceReadAllowed(e.Name, &authzContext)
}
func (e *ServiceConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ServiceConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.ServiceWrite(e.Name, &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().ServiceWriteAllowed(e.Name, &authzContext)
}
func (e *ServiceConfigEntry) GetRaftIndex() *RaftIndex {
@ -306,14 +306,14 @@ func (e *ProxyConfigEntry) Validate() error {
return e.validateEnterpriseMeta()
}
func (e *ProxyConfigEntry) CanRead(authz acl.Authorizer) bool {
return true
func (e *ProxyConfigEntry) CanRead(authz acl.Authorizer) error {
return nil
}
func (e *ProxyConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ProxyConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.MeshWrite(&authzContext) == acl.Allow
return authz.ToAllowAuthorizer().MeshWriteAllowed(&authzContext)
}
func (e *ProxyConfigEntry) GetRaftIndex() *RaftIndex {

28
agent/structs/config_entry_discoverychain.go
View File

@ -251,11 +251,11 @@ func isValidHTTPMethod(method string) bool {
}
}
func (e *ServiceRouterConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *ServiceRouterConfigEntry) CanRead(authz acl.Authorizer) error {
return canReadDiscoveryChain(e, authz)
}
func (e *ServiceRouterConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ServiceRouterConfigEntry) CanWrite(authz acl.Authorizer) error {
return canWriteDiscoveryChain(e, authz)
}
@ -594,11 +594,11 @@ func scaleWeight(v float32) int {
return int(math.Round(float64(v * 100.0)))
}
func (e *ServiceSplitterConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *ServiceSplitterConfigEntry) CanRead(authz acl.Authorizer) error {
return canReadDiscoveryChain(e, authz)
}
func (e *ServiceSplitterConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ServiceSplitterConfigEntry) CanWrite(authz acl.Authorizer) error {
return canWriteDiscoveryChain(e, authz)
}
@ -1069,11 +1069,11 @@ func (e *ServiceResolverConfigEntry) Validate() error {
return nil
}
func (e *ServiceResolverConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *ServiceResolverConfigEntry) CanRead(authz acl.Authorizer) error {
return canReadDiscoveryChain(e, authz)
}
func (e *ServiceResolverConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ServiceResolverConfigEntry) CanWrite(authz acl.Authorizer) error {
return canWriteDiscoveryChain(e, authz)
}
@ -1300,13 +1300,13 @@ type discoveryChainConfigEntry interface {
ListRelatedServices() []ServiceID
}
func canReadDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorizer) bool {
func canReadDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
entry.GetEnterpriseMeta().FillAuthzContext(&authzContext)
return authz.ServiceRead(entry.GetName(), &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().ServiceReadAllowed(entry.GetName(), &authzContext)
}
func canWriteDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorizer) bool {
func canWriteDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorizer) error {
entryID := NewServiceID(entry.GetName(), entry.GetEnterpriseMeta())
var authzContext acl.AuthorizerContext
@ -1314,8 +1314,8 @@ func canWriteDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorize
name := entry.GetName()
if authz.ServiceWrite(name, &authzContext) != acl.Allow {
return false
if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(name, &authzContext); err != nil {
return err
}
for _, svc := range entry.ListRelatedServices() {
@ -1326,11 +1326,11 @@ func canWriteDiscoveryChain(entry discoveryChainConfigEntry, authz acl.Authorize
svc.FillAuthzContext(&authzContext)
// You only need read on related services to redirect traffic flow for
// your own service.
if authz.ServiceRead(svc.ID, &authzContext) != acl.Allow {
return false
if err := authz.ToAllowAuthorizer().ServiceReadAllowed(svc.ID, &authzContext); err != nil {
return err
}
}
return true
return nil
}
// DiscoveryChainRequest is used when requesting the discovery chain for a

8
agent/structs/config_entry_exports.go
View File

@ -136,16 +136,16 @@ func (e *ExportedServicesConfigEntry) Validate() error {
return nil
}
func (e *ExportedServicesConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *ExportedServicesConfigEntry) CanRead(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.MeshRead(&authzContext) == acl.Allow
return authz.ToAllowAuthorizer().MeshReadAllowed(&authzContext)
}
func (e *ExportedServicesConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ExportedServicesConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.MeshWrite(&authzContext) == acl.Allow
return authz.ToAllowAuthorizer().MeshWriteAllowed(&authzContext)
}
func (e *ExportedServicesConfigEntry) GetRaftIndex() *RaftIndex {

16
agent/structs/config_entry_gateways.go
View File

@ -430,16 +430,16 @@ func (e *IngressGatewayConfigEntry) ListRelatedServices() []ServiceID {
return out
}
func (e *IngressGatewayConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *IngressGatewayConfigEntry) CanRead(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.ServiceRead(e.Name, &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().ServiceReadAllowed(e.Name, &authzContext)
}
func (e *IngressGatewayConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *IngressGatewayConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.MeshWrite(&authzContext) == acl.Allow
return authz.ToAllowAuthorizer().MeshWriteAllowed(&authzContext)
}
func (e *IngressGatewayConfigEntry) GetRaftIndex() *RaftIndex {
@ -572,16 +572,16 @@ func (e *TerminatingGatewayConfigEntry) Validate() error {
return nil
}
func (e *TerminatingGatewayConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *TerminatingGatewayConfigEntry) CanRead(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.ServiceRead(e.Name, &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().ServiceReadAllowed(e.Name, &authzContext)
}
func (e *TerminatingGatewayConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *TerminatingGatewayConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.MeshWrite(&authzContext) == acl.Allow
return authz.ToAllowAuthorizer().MeshWriteAllowed(&authzContext)
}
func (e *TerminatingGatewayConfigEntry) GetRaftIndex() *RaftIndex {

8
agent/structs/config_entry_intentions.go
View File

@ -789,16 +789,16 @@ func (e *ServiceIntentionsConfigEntry) GetEnterpriseMeta() *EnterpriseMeta {
return &e.EnterpriseMeta
}
func (e *ServiceIntentionsConfigEntry) CanRead(authz acl.Authorizer) bool {
func (e *ServiceIntentionsConfigEntry) CanRead(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.IntentionRead(e.GetName(), &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().IntentionReadAllowed(e.GetName(), &authzContext)
}
func (e *ServiceIntentionsConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *ServiceIntentionsConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.IntentionWrite(e.GetName(), &authzContext) == acl.Allow
return authz.ToAllowAuthorizer().IntentionWriteAllowed(e.GetName(), &authzContext)
}
func MigrateIntentions(ixns Intentions) []*ServiceIntentionsConfigEntry {

8
agent/structs/config_entry_mesh.go
View File

@ -64,14 +64,14 @@ func (e *MeshConfigEntry) Validate() error {
return e.validateEnterpriseMeta()
}
func (e *MeshConfigEntry) CanRead(authz acl.Authorizer) bool {
return true
func (e *MeshConfigEntry) CanRead(authz acl.Authorizer) error {
return nil
}
func (e *MeshConfigEntry) CanWrite(authz acl.Authorizer) bool {
func (e *MeshConfigEntry) CanWrite(authz acl.Authorizer) error {
var authzContext acl.AuthorizerContext
e.FillAuthzContext(&authzContext)
return authz.MeshWrite(&authzContext) == acl.Allow
return authz.ToAllowAuthorizer().MeshWriteAllowed(&authzContext)
}
func (e *MeshConfigEntry) GetRaftIndex() *RaftIndex {

16
agent/structs/config_entry_test.go
View File

@ -194,8 +194,20 @@ func testConfigEntries_ListRelatedServices_AndACLs(t *testing.T, cases []configE
for _, a := range tc.expectACLs {
require.NotEmpty(t, a.name)
t.Run(a.name, func(t *testing.T) {
require.Equal(t, a.canRead, tc.entry.CanRead(a.authorizer), "unexpected CanRead result")
require.Equal(t, a.canWrite, tc.entry.CanWrite(a.authorizer), "unexpected CanWrite result")
canRead := tc.entry.CanRead(a.authorizer)
if a.canRead {
require.Nil(t, canRead)
} else {
require.Error(t, canRead)
require.True(t, acl.IsErrPermissionDenied(canRead))
}
canWrite := tc.entry.CanWrite(a.authorizer)
if a.canWrite {
require.Nil(t, canWrite)
} else {
require.Error(t, canWrite)
require.True(t, acl.IsErrPermissionDenied(canWrite))
}
})
}
}