rpcperms: enforce order of interceptors

In this commit, we let the registered middleware interceptors be stored
in a slice rather than a map so that the order in which the interceptors
are executed is guarenteed to be the same as the order in which they
were registered.
This commit is contained in:
Elle Mouton 2022-07-07 12:42:38 +02:00
parent dc35f78ebc
commit 0d55deac63
No known key found for this signature in database
GPG key ID: D7D916376026F177
2 changed files with 41 additions and 19 deletions

View file

@ -66,6 +66,9 @@
* [Migrate instances of assert.NoError to require.NoError](https://github.com/lightningnetwork/lnd/pull/6636). * [Migrate instances of assert.NoError to require.NoError](https://github.com/lightningnetwork/lnd/pull/6636).
* [Enforce the order of rpc interceptor execution](https://github.com/lightningnetwork/lnd/pull/6709) to be the same as the
order in which they were registered.
### Tooling and documentation ### Tooling and documentation
* An [`.editorconfig` file was * An [`.editorconfig` file was

View file

@ -167,10 +167,19 @@ type InterceptorChain struct {
// rpcsLog is the logger used to log calls to the RPCs intercepted. // rpcsLog is the logger used to log calls to the RPCs intercepted.
rpcsLog btclog.Logger rpcsLog btclog.Logger
// registeredMiddleware is a map of all macaroon permission based RPC // registeredMiddleware is a slice of all macaroon permission based RPC
// middleware clients that are currently registered. The map is keyed // middleware clients that are currently registered. The
// by the middleware's name. // registeredMiddlewareNames can be used to find the index of a specific
registeredMiddleware map[string]*MiddlewareHandler // interceptor within the registeredMiddleware slide using the name of
// the interceptor as the key. The reason for using these two separate
// structures is so that the order in which interceptors are run is
// the same as the order in which they were registered.
registeredMiddleware []*MiddlewareHandler
// registeredMiddlewareNames is a map of registered middleware names
// to the index at which they are stored in the registeredMiddleware
// map.
registeredMiddlewareNames map[string]int
// mandatoryMiddleware is a list of all middleware that is considered to // mandatoryMiddleware is a list of all middleware that is considered to
// be mandatory. If any of them is not registered then all RPC requests // be mandatory. If any of them is not registered then all RPC requests
@ -198,7 +207,7 @@ func NewInterceptorChain(log btclog.Logger, noMacaroons bool,
noMacaroons: noMacaroons, noMacaroons: noMacaroons,
permissionMap: make(map[string][]bakery.Op), permissionMap: make(map[string][]bakery.Op),
rpcsLog: log, rpcsLog: log,
registeredMiddleware: make(map[string]*MiddlewareHandler), registeredMiddlewareNames: make(map[string]int),
mandatoryMiddleware: mandatoryMiddleware, mandatoryMiddleware: mandatoryMiddleware,
quit: make(chan struct{}), quit: make(chan struct{}),
} }
@ -442,25 +451,27 @@ func (r *InterceptorChain) RegisterMiddleware(mw *MiddlewareHandler) error {
defer r.Unlock() defer r.Unlock()
// The name of the middleware is the unique identifier. // The name of the middleware is the unique identifier.
registered, ok := r.registeredMiddleware[mw.middlewareName] _, ok := r.registeredMiddlewareNames[mw.middlewareName]
if ok { if ok {
return fmt.Errorf("a middleware with the name '%s' is already "+ return fmt.Errorf("a middleware with the name '%s' is already "+
"registered", registered.middlewareName) "registered", mw.middlewareName)
} }
// For now, we only want one middleware per custom caveat name. If we // For now, we only want one middleware per custom caveat name. If we
// allowed multiple middlewares handling the same caveat there would be // allowed multiple middlewares handling the same caveat there would be
// a need for extra call chaining logic, and they could overwrite each // a need for extra call chaining logic, and they could overwrite each
// other's responses. // other's responses.
for name, middleware := range r.registeredMiddleware { for _, middleware := range r.registeredMiddleware {
if middleware.customCaveatName == mw.customCaveatName { if middleware.customCaveatName == mw.customCaveatName {
return fmt.Errorf("a middleware is already registered "+ return fmt.Errorf("a middleware is already registered "+
"for the custom caveat name '%s': %v", "for the custom caveat name '%s': %v",
mw.customCaveatName, name) mw.customCaveatName, middleware.middlewareName)
} }
} }
r.registeredMiddleware[mw.middlewareName] = mw r.registeredMiddleware = append(r.registeredMiddleware, mw)
index := len(r.registeredMiddleware) - 1
r.registeredMiddlewareNames[mw.middlewareName] = index
return nil return nil
} }
@ -473,7 +484,15 @@ func (r *InterceptorChain) RemoveMiddleware(middlewareName string) {
log.Debugf("Removing middleware %s", middlewareName) log.Debugf("Removing middleware %s", middlewareName)
delete(r.registeredMiddleware, middlewareName) index, ok := r.registeredMiddlewareNames[middlewareName]
if !ok {
return
}
delete(r.registeredMiddlewareNames, middlewareName)
r.registeredMiddleware = append(
r.registeredMiddleware[:index],
r.registeredMiddleware[index+1:]...,
)
} }
// CustomCaveatSupported makes sure a middleware that handles the given custom // CustomCaveatSupported makes sure a middleware that handles the given custom
@ -888,7 +907,7 @@ func (r *InterceptorChain) checkMandatoryMiddleware(fullMethod string) error {
// Not a white listed call so make sure every mandatory middleware is // Not a white listed call so make sure every mandatory middleware is
// currently connected to lnd. // currently connected to lnd.
for _, name := range r.mandatoryMiddleware { for _, name := range r.mandatoryMiddleware {
if _, ok := r.registeredMiddleware[name]; !ok { if _, ok := r.registeredMiddlewareNames[name]; !ok {
return fmt.Errorf("mandatory middleware '%s' is "+ return fmt.Errorf("mandatory middleware '%s' is "+
"currently not registered, not allowing any "+ "currently not registered, not allowing any "+
"RPC calls", name) "RPC calls", name)