When authz plugin is disabled, remove from authz middleware chain.

When the daemon is configured to run with an authorization-plugin and if
the plugin is disabled, the daemon continues to send API requests to the
plugin and expect it to respond. But the plugin has been disabled. As a
result, all API requests are blocked. Fix this behavior by removing the
disabled plugin from the authz middleware chain.

Tested using riyaz/authz-no-volume-plugin and observed that after
disabling the plugin, API request/response is functional.

Fixes #31836

Signed-off-by: Anusha Ragunathan <anusha.ragunathan@docker.com>

cmd/dockerd/daemon.go

@@ -43,6 +43,7 @@ import (
 	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/system"
+	"github.com/docker/docker/plugin"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/go-connections/tlsconfig"
@@ -264,11 +265,22 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
 	// Notify that the API is active, but before daemon is set up.
 	preNotifySystem()
 
-	d, err := daemon.NewDaemon(cli.Config, registryService, containerdRemote)
+	pluginStore := plugin.NewStore()
+
+	if err := cli.initMiddlewares(api, serverConfig, pluginStore); err != nil {
+		logrus.Fatalf("Error creating middlewares: %v", err)
+	}
+
+	d, err := daemon.NewDaemon(cli.Config, registryService, containerdRemote, pluginStore)
 	if err != nil {
 		return fmt.Errorf("Error starting daemon: %v", err)
 	}
 
+	// validate after NewDaemon has restored enabled plugins. Dont change order.
+	if err := validateAuthzPlugins(cli.Config.AuthorizationPlugins, pluginStore); err != nil {
+		return fmt.Errorf("Error validating authorization plugin: %v", err)
+	}
+
 	if cli.Config.MetricsAddress != "" {
 		if !d.HasExperimental() {
 			return fmt.Errorf("metrics-addr is only supported when experimental is enabled")
@@ -307,10 +319,6 @@ func (cli *DaemonCli) start(opts daemonOptions) (err error) {
 
 	cli.d = d
 
-	// initMiddlewares needs cli.d to be populated. Dont change this init order.
-	if err := cli.initMiddlewares(api, serverConfig); err != nil {
-		logrus.Fatalf("Error creating middlewares: %v", err)
-	}
 	d.SetCluster(c)
 	initRouter(api, d, c)
 
@@ -494,10 +502,10 @@ func initRouter(s *apiserver.Server, d *daemon.Daemon, c *cluster.Cluster) {
 	s.InitRouter(debug.IsEnabled(), routers...)
 }
 
-func (cli *DaemonCli) initMiddlewares(s *apiserver.Server, cfg *apiserver.Config) error {
+func (cli *DaemonCli) initMiddlewares(s *apiserver.Server, cfg *apiserver.Config, pluginStore *plugin.Store) error {
 	v := cfg.Version
 
-	exp := middleware.NewExperimentalMiddleware(cli.d.HasExperimental())
+	exp := middleware.NewExperimentalMiddleware(cli.Config.Experimental)
 	s.UseMiddleware(exp)
 
 	vm := middleware.NewVersionMiddleware(v, api.DefaultVersion, api.MinVersion)
@@ -508,10 +516,8 @@ func (cli *DaemonCli) initMiddlewares(s *apiserver.Server, cfg *apiserver.Config
 		s.UseMiddleware(c)
 	}
 
-	if err := validateAuthzPlugins(cli.Config.AuthorizationPlugins, cli.d.PluginStore); err != nil {
-		return fmt.Errorf("Error validating authorization plugin: %v", err)
-	}
-	cli.authzMiddleware = authorization.NewMiddleware(cli.Config.AuthorizationPlugins, cli.d.PluginStore)
+	cli.authzMiddleware = authorization.NewMiddleware(cli.Config.AuthorizationPlugins, pluginStore)
+	cli.Config.AuthzMiddleware = cli.authzMiddleware
 	s.UseMiddleware(cli.authzMiddleware)
 	return nil
 }

daemon/config/config.go

@@ -16,6 +16,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	daemondiscovery "github.com/docker/docker/daemon/discovery"
 	"github.com/docker/docker/opts"
+	"github.com/docker/docker/pkg/authorization"
 	"github.com/docker/docker/pkg/discovery"
 	"github.com/docker/docker/registry"
 	"github.com/imdario/mergo"
@@ -82,25 +83,26 @@ type CommonTLSOptions struct {
 // It includes json tags to deserialize configuration from a file
 // using the same names that the flags in the command line use.
 type CommonConfig struct {
-	AuthorizationPlugins []string            `json:"authorization-plugins,omitempty"` // AuthorizationPlugins holds list of authorization plugins
-	AutoRestart          bool                `json:"-"`
-	Context              map[string][]string `json:"-"`
-	DisableBridge        bool                `json:"-"`
-	DNS                  []string            `json:"dns,omitempty"`
-	DNSOptions           []string            `json:"dns-opts,omitempty"`
-	DNSSearch            []string            `json:"dns-search,omitempty"`
-	ExecOptions          []string            `json:"exec-opts,omitempty"`
-	GraphDriver          string              `json:"storage-driver,omitempty"`
-	GraphOptions         []string            `json:"storage-opts,omitempty"`
-	Labels               []string            `json:"labels,omitempty"`
-	Mtu                  int                 `json:"mtu,omitempty"`
-	Pidfile              string              `json:"pidfile,omitempty"`
-	RawLogs              bool                `json:"raw-logs,omitempty"`
-	Root                 string              `json:"graph,omitempty"`
-	SocketGroup          string              `json:"group,omitempty"`
-	TrustKeyPath         string              `json:"-"`
-	CorsHeaders          string              `json:"api-cors-header,omitempty"`
-	EnableCors           bool                `json:"api-enable-cors,omitempty"`
+	AuthzMiddleware      *authorization.Middleware `json:"-"`
+	AuthorizationPlugins []string                  `json:"authorization-plugins,omitempty"` // AuthorizationPlugins holds list of authorization plugins
+	AutoRestart          bool                      `json:"-"`
+	Context              map[string][]string       `json:"-"`
+	DisableBridge        bool                      `json:"-"`
+	DNS                  []string                  `json:"dns,omitempty"`
+	DNSOptions           []string                  `json:"dns-opts,omitempty"`
+	DNSSearch            []string                  `json:"dns-search,omitempty"`
+	ExecOptions          []string                  `json:"exec-opts,omitempty"`
+	GraphDriver          string                    `json:"storage-driver,omitempty"`
+	GraphOptions         []string                  `json:"storage-opts,omitempty"`
+	Labels               []string                  `json:"labels,omitempty"`
+	Mtu                  int                       `json:"mtu,omitempty"`
+	Pidfile              string                    `json:"pidfile,omitempty"`
+	RawLogs              bool                      `json:"raw-logs,omitempty"`
+	Root                 string                    `json:"graph,omitempty"`
+	SocketGroup          string                    `json:"group,omitempty"`
+	TrustKeyPath         string                    `json:"-"`
+	CorsHeaders          string                    `json:"api-cors-header,omitempty"`
+	EnableCors           bool                      `json:"api-enable-cors,omitempty"`
 
 	// LiveRestoreEnabled determines whether we should keep containers
 	// alive upon daemon shutdown/start

daemon/daemon.go

@@ -465,7 +465,7 @@ func (daemon *Daemon) IsSwarmCompatible() error {
 
 // NewDaemon sets up everything for the daemon to be able to service
 // requests from the webserver.
-func NewDaemon(config *config.Config, registryService registry.Service, containerdRemote libcontainerd.Remote) (daemon *Daemon, err error) {
+func NewDaemon(config *config.Config, registryService registry.Service, containerdRemote libcontainerd.Remote, pluginStore *plugin.Store) (daemon *Daemon, err error) {
 	setDefaultMtu(config)
 
 	// Ensure that we have a correct root key limit for launching containers.
@@ -562,7 +562,8 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
 	}
 
 	d.RegistryService = registryService
-	d.PluginStore = plugin.NewStore(config.Root) // todo: remove
+	d.PluginStore = pluginStore
+
 	// Plugin system initialization should happen before restore. Do not change order.
 	d.pluginManager, err = plugin.NewManager(plugin.ManagerConfig{
 		Root:               filepath.Join(config.Root, "plugins"),
@@ -572,6 +573,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
 		RegistryService:    registryService,
 		LiveRestoreEnabled: config.LiveRestoreEnabled,
 		LogPluginEvent:     d.LogPluginEvent, // todo: make private
+		AuthzMiddleware:    config.AuthzMiddleware,
 	})
 	if err != nil {
 		return nil, errors.Wrap(err, "couldn't create plugin manager")

integration-cli/docker_cli_authz_plugin_v2_test.go

@@ -75,6 +75,36 @@ func (s *DockerAuthzV2Suite) TestAuthZPluginAllowNonVolumeRequest(c *check.C) {
 	c.Assert(assertContainerList(out, []string{id}), check.Equals, true)
 }
 
+func (s *DockerAuthzV2Suite) TestAuthZPluginDisable(c *check.C) {
+	testRequires(c, DaemonIsLinux, IsAmd64, Network)
+	// Install authz plugin
+	_, err := s.d.Cmd("plugin", "install", "--grant-all-permissions", authzPluginNameWithTag)
+	c.Assert(err, checker.IsNil)
+	// start the daemon with the plugin and load busybox, --net=none build fails otherwise
+	// because it needs to pull busybox
+	s.d.Restart(c, "--authorization-plugin="+authzPluginNameWithTag)
+	c.Assert(s.d.LoadBusybox(), check.IsNil)
+
+	// defer removing the plugin
+	defer func() {
+		s.d.Restart(c)
+		_, err = s.d.Cmd("plugin", "rm", "-f", authzPluginNameWithTag)
+		c.Assert(err, checker.IsNil)
+	}()
+
+	out, err := s.d.Cmd("volume", "create")
+	c.Assert(err, check.NotNil)
+	c.Assert(out, checker.Contains, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
+
+	// disable the plugin
+	_, err = s.d.Cmd("plugin", "disable", authzPluginNameWithTag)
+	c.Assert(err, checker.IsNil)
+
+	// now test to see if the docker api works.
+	_, err = s.d.Cmd("volume", "create")
+	c.Assert(err, checker.IsNil)
+}
+
 func (s *DockerAuthzV2Suite) TestAuthZPluginRejectVolumeRequests(c *check.C) {
 	testRequires(c, DaemonIsLinux, IsAmd64, Network)
 	// Install authz plugin

pkg/authorization/middleware.go

@@ -25,6 +25,20 @@ func NewMiddleware(names []string, pg plugingetter.PluginGetter) *Middleware {
 	}
 }
 
+// GetAuthzPlugins gets authorization plugins
+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()
@@ -35,10 +49,7 @@ func (m *Middleware) SetPlugins(names []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 {
-
-		m.mu.Lock()
-		plugins := m.plugins
-		m.mu.Unlock()
+		plugins := m.GetAuthzPlugins()
 		if len(plugins) == 0 {
 			return handler(ctx, w, r, vars)
 		}
@@ -70,6 +81,16 @@ func (m *Middleware) WrapHandler(handler func(ctx context.Context, w http.Respon
 			logrus.Errorf("Handler for %s %s returned error: %s", r.Method, r.RequestURI, errD)
 		}
 
+		// 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()
+		if len(plugins) == 0 {
+			logrus.Debug("There are no authz plugins in the chain")
+			return nil
+		}
+
+		authCtx.plugins = plugins
+
 		if err := authCtx.AuthZResponse(rw, r); errD == nil && err != nil {
 			logrus.Errorf("AuthZResponse for %s %s returned error: %s", r.Method, r.RequestURI, err)
 			return err

pkg/authorization/plugin.go

@@ -61,6 +61,11 @@ func (a *authorizationPlugin) Name() string {
 	return a.name
 }
 
+// Set the remote for an authz pluginv2
+func (a *authorizationPlugin) SetName(remote string) {
+	a.name = remote
+}
+
 func (a *authorizationPlugin) AuthZRequest(authReq *Request) (*Response, error) {
 	if err := a.initPlugin(); err != nil {
 		return nil, err
@@ -98,6 +103,7 @@ func (a *authorizationPlugin) initPlugin() error {
 
 			if pg := GetPluginGetter(); pg != nil {
 				plugin, e = pg.Get(a.name, AuthZApiImplements, plugingetter.Lookup)
+				a.SetName(plugin.Name())
 			} else {
 				plugin, e = plugins.Get(a.name, AuthZApiImplements)
 			}

plugin/backend_linux.go

@@ -26,6 +26,7 @@ import (
 	"github.com/docker/docker/distribution/xfer"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
+	"github.com/docker/docker/pkg/authorization"
 	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/pools"
@@ -56,6 +57,19 @@ func (pm *Manager) Disable(refOrID string, config *types.PluginDisableConfig) er
 		return fmt.Errorf("plugin %s is in use", p.Name())
 	}
 
+	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)
+				}
+			}
+		}
+	}
+
 	if err := pm.disable(p, c); err != nil {
 		return err
 	}

plugin/defs.go

@@ -18,7 +18,7 @@ type Store struct {
 }
 
 // NewStore creates a Store.
-func NewStore(libRoot string) *Store {
+func NewStore() *Store {
 	return &Store{
 		plugins:  make(map[string]*v2.Plugin),
 		handlers: make(map[string][]func(string, *plugins.Client)),

plugin/manager.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/libcontainerd"
+	"github.com/docker/docker/pkg/authorization"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/plugin/v2"
@@ -49,6 +50,7 @@ type ManagerConfig struct {
 	LogPluginEvent     eventLogger
 	Root               string
 	ExecRoot           string
+	AuthzMiddleware    *authorization.Middleware
 }
 
 // Manager controls the plugin subsystem.