From 6f6bc24efdb6d6e761656070afc4791de9680f0c Mon Sep 17 00:00:00 2001 From: Chaim Lev-Ari Date: Wed, 22 Jul 2020 21:38:45 +0300 Subject: [PATCH] feat(containers): Ensure users cannot create privileged containers via the API (#3969) (#4077) * feat(containers): Ensure users cannot create privileged containers via the API * feat(containers): add rbac check in stack creation Co-authored-by: Maxime Bajeux --- .../handler/stacks/create_compose_stack.go | 19 +++-- api/http/handler/stacks/create_swarm_stack.go | 19 +++-- api/http/handler/stacks/handler.go | 20 ++++++ api/http/handler/stacks/stack_create.go | 20 ++++-- api/http/proxy/factory/docker/containers.go | 72 +++++++++++++++++++ api/http/proxy/factory/docker/transport.go | 3 +- 6 files changed, 135 insertions(+), 18 deletions(-) diff --git a/api/http/handler/stacks/create_compose_stack.go b/api/http/handler/stacks/create_compose_stack.go index b021190b8..37d8c1d7c 100644 --- a/api/http/handler/stacks/create_compose_stack.go +++ b/api/http/handler/stacks/create_compose_stack.go @@ -283,6 +283,7 @@ type composeStackDeploymentConfig struct { dockerhub *portainer.DockerHub registries []portainer.Registry isAdmin bool + user *portainer.User } func (handler *Handler) createComposeDeployConfig(r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) (*composeStackDeploymentConfig, *httperror.HandlerError) { @@ -302,12 +303,18 @@ func (handler *Handler) createComposeDeployConfig(r *http.Request, stack *portai } filteredRegistries := security.FilterRegistries(registries, securityContext) + user, err := handler.DataStore.User().User(securityContext.UserID) + if err != nil { + return nil, &httperror.HandlerError{http.StatusInternalServerError, "Unable to load user information from the database", err} + } + config := &composeStackDeploymentConfig{ stack: stack, endpoint: endpoint, dockerhub: dockerhub, registries: filteredRegistries, isAdmin: securityContext.IsAdmin, + user: user, } return config, nil @@ -324,7 +331,12 @@ func (handler *Handler) deployComposeStack(config *composeStackDeploymentConfig) return err } - if !settings.AllowBindMountsForRegularUsers && !config.isAdmin { + isAdminOrEndpointAdmin, err := handler.userIsAdminOrEndpointAdmin(config.user, config.endpoint.ID) + if err != nil { + return err + } + + if (!settings.AllowBindMountsForRegularUsers || !settings.AllowPrivilegedModeForRegularUsers) && !isAdminOrEndpointAdmin { composeFilePath := path.Join(config.stack.ProjectPath, config.stack.EntryPoint) stackContent, err := handler.FileService.GetFileContent(composeFilePath) @@ -332,13 +344,10 @@ func (handler *Handler) deployComposeStack(config *composeStackDeploymentConfig) return err } - valid, err := handler.isValidStackFile(stackContent) + err = handler.isValidStackFile(stackContent, settings) if err != nil { return err } - if !valid { - return errors.New("bind-mount disabled for non administrator users") - } } handler.stackCreationMutex.Lock() diff --git a/api/http/handler/stacks/create_swarm_stack.go b/api/http/handler/stacks/create_swarm_stack.go index e28261bed..f9aac664a 100644 --- a/api/http/handler/stacks/create_swarm_stack.go +++ b/api/http/handler/stacks/create_swarm_stack.go @@ -292,6 +292,7 @@ type swarmStackDeploymentConfig struct { registries []portainer.Registry prune bool isAdmin bool + user *portainer.User } func (handler *Handler) createSwarmDeployConfig(r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint, prune bool) (*swarmStackDeploymentConfig, *httperror.HandlerError) { @@ -311,6 +312,11 @@ func (handler *Handler) createSwarmDeployConfig(r *http.Request, stack *portaine } filteredRegistries := security.FilterRegistries(registries, securityContext) + user, err := handler.DataStore.User().User(securityContext.UserID) + if err != nil { + return nil, &httperror.HandlerError{http.StatusInternalServerError, "Unable to load user information from the database", err} + } + config := &swarmStackDeploymentConfig{ stack: stack, endpoint: endpoint, @@ -318,6 +324,7 @@ func (handler *Handler) createSwarmDeployConfig(r *http.Request, stack *portaine registries: filteredRegistries, prune: prune, isAdmin: securityContext.IsAdmin, + user: user, } return config, nil @@ -329,7 +336,12 @@ func (handler *Handler) deploySwarmStack(config *swarmStackDeploymentConfig) err return err } - if !settings.AllowBindMountsForRegularUsers && !config.isAdmin { + isAdminOrEndpointAdmin, err := handler.userIsAdminOrEndpointAdmin(config.user, config.endpoint.ID) + if err != nil { + return err + } + + if !settings.AllowBindMountsForRegularUsers && !isAdminOrEndpointAdmin { composeFilePath := path.Join(config.stack.ProjectPath, config.stack.EntryPoint) stackContent, err := handler.FileService.GetFileContent(composeFilePath) @@ -337,13 +349,10 @@ func (handler *Handler) deploySwarmStack(config *swarmStackDeploymentConfig) err return err } - valid, err := handler.isValidStackFile(stackContent) + err = handler.isValidStackFile(stackContent, settings) if err != nil { return err } - if !valid { - return errors.New("bind-mount disabled for non administrator users") - } } handler.stackCreationMutex.Lock() diff --git a/api/http/handler/stacks/handler.go b/api/http/handler/stacks/handler.go index e560392c3..2be3ddc70 100644 --- a/api/http/handler/stacks/handler.go +++ b/api/http/handler/stacks/handler.go @@ -89,3 +89,23 @@ func (handler *Handler) userCanAccessStack(securityContext *security.RestrictedR } return false, nil } + +func (handler *Handler) userIsAdminOrEndpointAdmin(user *portainer.User, endpointID portainer.EndpointID) (bool, error) { + isAdmin := user.Role == portainer.AdministratorRole + if isAdmin { + return true, nil + } + + rbacExtension, err := handler.DataStore.Extension().Extension(portainer.RBACExtension) + if err != nil && err != bolterrors.ErrObjectNotFound { + return false, errors.New("Unable to verify if RBAC extension is loaded") + } + + if rbacExtension == nil { + return false, nil + } + + _, endpointResourceAccess := user.EndpointAuthorizations[portainer.EndpointID(endpointID)][portainer.EndpointResourcesAccess] + + return endpointResourceAccess, nil +} diff --git a/api/http/handler/stacks/stack_create.go b/api/http/handler/stacks/stack_create.go index 87f358abd..a785ff8a2 100644 --- a/api/http/handler/stacks/stack_create.go +++ b/api/http/handler/stacks/stack_create.go @@ -106,10 +106,10 @@ func (handler *Handler) createSwarmStack(w http.ResponseWriter, r *http.Request, return &httperror.HandlerError{http.StatusBadRequest, "Invalid value for query parameter: method. Value must be one of: string, repository or file", errors.New(request.ErrInvalidQueryParameter)} } -func (handler *Handler) isValidStackFile(stackFileContent []byte) (bool, error) { +func (handler *Handler) isValidStackFile(stackFileContent []byte, settings *portainer.Settings) error { composeConfigYAML, err := loader.ParseYAML(stackFileContent) if err != nil { - return false, err + return err } composeConfigFile := types.ConfigFile{ @@ -126,19 +126,25 @@ func (handler *Handler) isValidStackFile(stackFileContent []byte) (bool, error) options.SkipInterpolation = true }) if err != nil { - return false, err + return err } for key := range composeConfig.Services { service := composeConfig.Services[key] - for _, volume := range service.Volumes { - if volume.Type == "bind" { - return false, nil + if !settings.AllowBindMountsForRegularUsers { + for _, volume := range service.Volumes { + if volume.Type == "bind" { + return errors.New("bind-mount disabled for non administrator users") + } } } + + if !settings.AllowPrivilegedModeForRegularUsers && service.Privileged == true { + return errors.New("privileged mode disabled for non administrator users") + } } - return true, nil + return nil } func (handler *Handler) decorateStackResponse(w http.ResponseWriter, stack *portainer.Stack, userID portainer.UserID) *httperror.HandlerError { diff --git a/api/http/proxy/factory/docker/containers.go b/api/http/proxy/factory/docker/containers.go index dfda0b04a..519282be4 100644 --- a/api/http/proxy/factory/docker/containers.go +++ b/api/http/proxy/factory/docker/containers.go @@ -1,12 +1,18 @@ package docker import ( + "bytes" "context" + "encoding/json" + "errors" + "io/ioutil" "net/http" "github.com/docker/docker/client" portainer "github.com/portainer/portainer/api" + bolterrors "github.com/portainer/portainer/api/bolt/errors" "github.com/portainer/portainer/api/http/proxy/factory/responseutils" + "github.com/portainer/portainer/api/http/security" "github.com/portainer/portainer/api/internal/authorization" ) @@ -148,3 +154,69 @@ func containerHasBlackListedLabel(containerLabels map[string]interface{}, labelB return false } + +func (transport *Transport) decorateContainerCreationOperation(request *http.Request, resourceIdentifierAttribute string, resourceType portainer.ResourceControlType) (*http.Response, error) { + type PartialContainer struct { + HostConfig struct { + Privileged bool `json:"Privileged"` + } `json:"HostConfig"` + } + + tokenData, err := security.RetrieveTokenData(request) + if err != nil { + return nil, err + } + + user, err := transport.dataStore.User().User(tokenData.ID) + if err != nil { + return nil, err + } + + rbacExtension, err := transport.dataStore.Extension().Extension(portainer.RBACExtension) + if err != nil && err != bolterrors.ErrObjectNotFound { + return nil, err + } + + endpointResourceAccess := false + _, ok := user.EndpointAuthorizations[portainer.EndpointID(transport.endpoint.ID)][portainer.EndpointResourcesAccess] + if ok { + endpointResourceAccess = true + } + + if (rbacExtension != nil && !endpointResourceAccess && tokenData.Role != portainer.AdministratorRole) || (rbacExtension == nil && tokenData.Role != portainer.AdministratorRole) { + settings, err := transport.dataStore.Settings().Settings() + if err != nil { + return nil, err + } + + if !settings.AllowPrivilegedModeForRegularUsers { + body, err := ioutil.ReadAll(request.Body) + if err != nil { + return nil, err + } + + partialContainer := &PartialContainer{} + err = json.Unmarshal(body, partialContainer) + if err != nil { + return nil, err + } + + if partialContainer.HostConfig.Privileged { + return nil, errors.New("forbidden to use privileged mode") + } + + request.Body = ioutil.NopCloser(bytes.NewBuffer(body)) + } + } + + response, err := transport.executeDockerRequest(request) + if err != nil { + return response, err + } + + if response.StatusCode == http.StatusCreated { + err = transport.decorateGenericResourceCreationResponse(response, resourceIdentifierAttribute, resourceType, tokenData.ID) + } + + return response, err +} diff --git a/api/http/proxy/factory/docker/transport.go b/api/http/proxy/factory/docker/transport.go index 5be06eb0b..6ecfb4615 100644 --- a/api/http/proxy/factory/docker/transport.go +++ b/api/http/proxy/factory/docker/transport.go @@ -189,7 +189,7 @@ func (transport *Transport) proxyConfigRequest(request *http.Request) (*http.Res func (transport *Transport) proxyContainerRequest(request *http.Request) (*http.Response, error) { switch requestPath := request.URL.Path; requestPath { case "/containers/create": - return transport.decorateGenericResourceCreationOperation(request, containerObjectIdentifier, portainer.ContainerResourceControl) + return transport.decorateContainerCreationOperation(request, containerObjectIdentifier, portainer.ContainerResourceControl) case "/containers/prune": return transport.administratorOperation(request) @@ -629,6 +629,7 @@ func (transport *Transport) createRegistryAccessContext(request *http.Request) ( return nil, err } + accessContext := ®istryAccessContext{ isAdmin: true, userID: tokenData.ID,