From 8857822260c0ed62948f216f7e0ffbff5601a011 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Tue, 13 Jun 2017 11:52:04 +0100 Subject: [PATCH 1/2] authz: eliminate race during plugin removal from middleware Also, this removes the use of a questionable golang range feature which corrects for mutation of a slice during iteration over that slice. This makes the filter operation easier to read and reason about. Signed-off-by: David Sheets Upstream-commit: 7da3986297e04b419ce08b19766633dba36b7d30 Component: engine --- components/engine/pkg/authorization/middleware.go | 13 +++++++++++++ components/engine/plugin/backend_linux.go | 9 +-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/components/engine/pkg/authorization/middleware.go b/components/engine/pkg/authorization/middleware.go index 05130121e9..595a06d53c 100644 --- a/components/engine/pkg/authorization/middleware.go +++ b/components/engine/pkg/authorization/middleware.go @@ -46,6 +46,19 @@ func (m *Middleware) SetPlugins(names []string) { m.mu.Unlock() } +// RemovePlugin removes a single plugin from this authz middleware chain +func (m *Middleware) RemovePlugin(name string) { + m.mu.Lock() + defer m.mu.Unlock() + plugins := m.plugins[:0] + for _, authPlugin := range m.plugins { + if authPlugin.Name() != name { + plugins = append(plugins, authPlugin) + } + } + m.plugins = plugins +} + // WrapHandler returns a new handler function wrapping the previous one in the request chain. func (m *Middleware) WrapHandler(handler func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error) func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { return func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { diff --git a/components/engine/plugin/backend_linux.go b/components/engine/plugin/backend_linux.go index 1d7f3a838c..012f6cf22b 100644 --- a/components/engine/plugin/backend_linux.go +++ b/components/engine/plugin/backend_linux.go @@ -60,14 +60,7 @@ func (pm *Manager) Disable(refOrID string, config *types.PluginDisableConfig) er for _, typ := range p.GetTypes() { if typ.Capability == authorization.AuthZApiImplements { - authzList := pm.config.AuthzMiddleware.GetAuthzPlugins() - for i, authPlugin := range authzList { - if authPlugin.Name() == p.Name() { - // Remove plugin from authzmiddleware chain - authzList = append(authzList[:i], authzList[i+1:]...) - pm.config.AuthzMiddleware.SetAuthzPlugins(authzList) - } - } + pm.config.AuthzMiddleware.RemovePlugin(p.Name()) } } From 142f748495776b274719f575ae16fd16b80cc5c4 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Tue, 13 Jun 2017 12:12:42 +0100 Subject: [PATCH 2/2] authz: remove and hide unused and local-only methods respectively Signed-off-by: David Sheets Upstream-commit: 24264697c54843ea8dbd30ac37652409943e7bf4 Component: engine --- components/engine/pkg/authorization/middleware.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/components/engine/pkg/authorization/middleware.go b/components/engine/pkg/authorization/middleware.go index 595a06d53c..7789a758df 100644 --- a/components/engine/pkg/authorization/middleware.go +++ b/components/engine/pkg/authorization/middleware.go @@ -25,20 +25,12 @@ func NewMiddleware(names []string, pg plugingetter.PluginGetter) *Middleware { } } -// GetAuthzPlugins gets authorization plugins -func (m *Middleware) GetAuthzPlugins() []Plugin { +func (m *Middleware) getAuthzPlugins() []Plugin { m.mu.Lock() defer m.mu.Unlock() return m.plugins } -// SetAuthzPlugins sets authorization plugins -func (m *Middleware) SetAuthzPlugins(plugins []Plugin) { - m.mu.Lock() - m.plugins = plugins - m.mu.Unlock() -} - // SetPlugins sets the plugin used for authorization func (m *Middleware) SetPlugins(names []string) { m.mu.Lock() @@ -62,7 +54,7 @@ func (m *Middleware) RemovePlugin(name string) { // WrapHandler returns a new handler function wrapping the previous one in the request chain. func (m *Middleware) WrapHandler(handler func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error) func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { return func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - plugins := m.GetAuthzPlugins() + plugins := m.getAuthzPlugins() if len(plugins) == 0 { return handler(ctx, w, r, vars) } @@ -96,7 +88,7 @@ func (m *Middleware) WrapHandler(handler func(ctx context.Context, w http.Respon // There's a chance that the authCtx.plugins was updated. One of the reasons // this can happen is when an authzplugin is disabled. - plugins = m.GetAuthzPlugins() + plugins = m.getAuthzPlugins() if len(plugins) == 0 { logrus.Debug("There are no authz plugins in the chain") return nil