From 38de272bd4dfea945985b7031cd353ac5f6507c5 Mon Sep 17 00:00:00 2001 From: Anusha Ragunathan Date: Fri, 17 Mar 2017 14:57:23 -0700 Subject: [PATCH] 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 --- cmd/dockerd/daemon.go | 28 ++++++++----- daemon/config/config.go | 40 ++++++++++--------- daemon/daemon.go | 6 ++- .../docker_cli_authz_plugin_v2_test.go | 30 ++++++++++++++ pkg/authorization/middleware.go | 29 ++++++++++++-- pkg/authorization/plugin.go | 6 +++ plugin/backend_linux.go | 14 +++++++ plugin/defs.go | 2 +- plugin/manager.go | 2 + 9 files changed, 120 insertions(+), 37 deletions(-) diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 0eee3f7624..cdc76f93b9 100644 --- a/cmd/dockerd/daemon.go +++ b/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 } diff --git a/daemon/config/config.go b/daemon/config/config.go index ef43a45af2..d00fdd2c6e 100644 --- a/daemon/config/config.go +++ b/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 diff --git a/daemon/daemon.go b/daemon/daemon.go index 4ecd2fa50a..0587154582 100644 --- a/daemon/daemon.go +++ b/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") diff --git a/integration-cli/docker_cli_authz_plugin_v2_test.go b/integration-cli/docker_cli_authz_plugin_v2_test.go index 852289043a..0143f1c0fb 100644 --- a/integration-cli/docker_cli_authz_plugin_v2_test.go +++ b/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 diff --git a/pkg/authorization/middleware.go b/pkg/authorization/middleware.go index 52890dd360..05130121e9 100644 --- a/pkg/authorization/middleware.go +++ b/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 diff --git a/pkg/authorization/plugin.go b/pkg/authorization/plugin.go index 1491a09d22..939f926744 100644 --- a/pkg/authorization/plugin.go +++ b/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) } diff --git a/plugin/backend_linux.go b/plugin/backend_linux.go index 380d0ddaff..14d5748951 100644 --- a/plugin/backend_linux.go +++ b/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 } diff --git a/plugin/defs.go b/plugin/defs.go index 927f639166..cf44c97ec8 100644 --- a/plugin/defs.go +++ b/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)), diff --git a/plugin/manager.go b/plugin/manager.go index a3ee15146f..b8717a9308 100644 --- a/plugin/manager.go +++ b/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.