Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: failed to validate namespace value #2393

Closed
wants to merge 2 commits into from
Closed

bugfix: failed to validate namespace value #2393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
daaa664
bugfix: failed to validate namespace value
Ace-Tang Oct 31, 2018
2a28f4c
test: add namespace test for run command
Ace-Tang Oct 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions daemon/mgr/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/alibaba/pouch/pkg/errtypes"
"github.com/alibaba/pouch/pkg/meta"
mountutils "github.com/alibaba/pouch/pkg/mount"
ns "github.com/alibaba/pouch/pkg/namespace"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

package named namespace is not suitable, you just move the container's network functions into the common package, in my opinion, named net is better， but I don't agree move then into pkg.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

network function is one part of namespace, and some function are repeated, so make them as a pkg, wdyt?

"github.com/alibaba/pouch/pkg/utils"
"github.com/alibaba/pouch/storage/quota"
volumetypes "github.com/alibaba/pouch/storage/volume/types"
Expand All @@ -38,6 +39,7 @@ import (
"github.com/docker/libnetwork"
"github.com/go-openapi/strfmt"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -392,7 +394,7 @@ func (mgr *ContainerManager) Create(ctx context.Context, name string, config *ty
if len(config.NetworkingConfig.EndpointsConfig) > 0 {
container.NetworkSettings.Networks = config.NetworkingConfig.EndpointsConfig
}
if container.NetworkSettings.Networks == nil && !IsContainer(config.HostConfig.NetworkMode) {
if container.NetworkSettings.Networks == nil && !ns.IsContainer(config.HostConfig.NetworkMode) {
container.NetworkSettings.Networks = make(map[string]*types.EndpointSettings)
container.NetworkSettings.Networks[config.HostConfig.NetworkMode] = new(types.EndpointSettings)
}
Expand Down Expand Up @@ -547,7 +549,7 @@ func (mgr *ContainerManager) prepareContainerNetwork(ctx context.Context, c *Con

networkMode := c.HostConfig.NetworkMode

if IsContainer(networkMode) {
if ns.IsContainer(networkMode) {
var origContainer *Container
origContainer, err := mgr.Get(ctx, strings.SplitN(networkMode, ":", 2)[1])
if err != nil {
Expand All @@ -564,7 +566,7 @@ func (mgr *ContainerManager) prepareContainerNetwork(ctx context.Context, c *Con
}

// initialise host network mode
if IsHost(networkMode) {
if ns.IsHost(networkMode) {
hostname, err := os.Hostname()
if err != nil {
return err
Expand Down Expand Up @@ -1352,7 +1354,7 @@ func (mgr *ContainerManager) Disconnect(ctx context.Context, containerName, netw
networkMode := c.HostConfig.NetworkMode
c.Unlock()

if IsHost(networkMode) && IsHost(network.Mode) {
if ns.IsHost(networkMode) && ns.IsHost(network.Mode) {
return fmt.Errorf("container cannot be disconnected from host network or connected to hostnetwork ")
}

Expand Down Expand Up @@ -1428,13 +1430,13 @@ func (mgr *ContainerManager) openContainerIO(c *Container) (*containerio.IO, err
}

func (mgr *ContainerManager) updateNetworkConfig(container *Container, networkIDOrName string, endpointConfig *types.EndpointSettings) error {
if IsContainer(container.HostConfig.NetworkMode) {
if ns.IsContainer(container.HostConfig.NetworkMode) {
return fmt.Errorf("container sharing network namespace with another container or host cannot be connected to any other network")
}

// TODO check bridge-mode conflict

if IsUserDefined(container.HostConfig.NetworkMode) {
if ns.IsUserDefined(container.HostConfig.NetworkMode) {
if hasUserDefinedIPAddress(endpointConfig) {
return fmt.Errorf("user specified IP address is supported on user defined networks only")
}
Expand Down Expand Up @@ -1470,7 +1472,7 @@ func (mgr *ContainerManager) updateNetworkConfig(container *Container, networkID
}

func (mgr *ContainerManager) connectToNetwork(ctx context.Context, container *Container, networkIDOrName string, epConfig *types.EndpointSettings) (err error) {
if IsContainer(container.HostConfig.NetworkMode) {
if ns.IsContainer(container.HostConfig.NetworkMode) {
return fmt.Errorf("container sharing network namespace with another container or host cannot be connected to any other network")
}

Expand Down Expand Up @@ -1499,7 +1501,7 @@ func (mgr *ContainerManager) updateNetworkSettings(container *Container, n libne
container.NetworkSettings = &types.NetworkSettings{Networks: make(map[string]*types.EndpointSettings)}
}

if !IsHost(container.HostConfig.NetworkMode) && IsHost(n.Type()) {
if !ns.IsHost(container.HostConfig.NetworkMode) && ns.IsHost(n.Type()) {
return fmt.Errorf("container cannot be connected to host network")
}

Expand All @@ -1513,10 +1515,10 @@ func (mgr *ContainerManager) updateNetworkSettings(container *Container, n libne
// Avoid duplicate config
return nil
}
if !IsPrivate(sn.Type) || !IsPrivate(n.Type()) {
if !ns.IsPrivate(specs.NetworkNamespace, sn.Type) || !ns.IsPrivate(specs.NetworkNamespace, n.Type()) {
return fmt.Errorf("container sharing network namespace with another container or host cannot be connected to any other network")
}
if IsNone(sn.Name) || IsNone(n.Name()) {
if ns.IsNone(sn.Name) || ns.IsNone(n.Name()) {
return fmt.Errorf("container cannot be connected to multiple networks with one of the networks in none mode")
}
}
Expand Down Expand Up @@ -1811,7 +1813,7 @@ func (mgr *ContainerManager) buildContainerEndpoint(c *Container, name string) *
ep := BuildContainerEndpoint(c)
ep.Name = name

if !IsUserDefined(name) {
if !ns.IsUserDefined(name) {
ep.DisableResolver = true
}

Expand Down
28 changes: 28 additions & 0 deletions daemon/mgr/container_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import (
"github.com/alibaba/pouch/apis/types"
"github.com/alibaba/pouch/daemon/logger/jsonfile"
"github.com/alibaba/pouch/daemon/logger/syslog"
ns "github.com/alibaba/pouch/pkg/namespace"
"github.com/alibaba/pouch/pkg/system"

specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -74,6 +76,11 @@ func (mgr *ContainerManager) validateConfig(c *Container, update bool) ([]string
c.AppArmorProfile = ""
}

// validate namespace value
if err := validateNSValue(hostConfig); err != nil {
return warnings, err
}

return warnings, nil
}

Expand Down Expand Up @@ -294,3 +301,24 @@ func validateNvidiaDevice(r *types.Resources) error {
}
return nil
}

// validateNSValue valids value of namespace related parameters
func validateNSValue(hostConfig *types.HostConfig) error {
if !ns.Valid(specs.PIDNamespace, hostConfig.PidMode) {
return fmt.Errorf("invalid pid namespace mode %s", hostConfig.PidMode)
}

if !ns.Valid(specs.UTSNamespace, hostConfig.UTSMode) {
return fmt.Errorf("invalid uts namespace mode %s", hostConfig.UTSMode)
}

if !ns.Valid(specs.IPCNamespace, hostConfig.IpcMode) {
return fmt.Errorf("invalid ipc namespace mode %s", hostConfig.IpcMode)
}

if !ns.Valid(specs.UserNamespace, hostConfig.UsernsMode) {
return fmt.Errorf("invalid user namespace mode %s", hostConfig.UsernsMode)
}

return nil
}
5 changes: 3 additions & 2 deletions daemon/mgr/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/alibaba/pouch/network/types"
"github.com/alibaba/pouch/pkg/errtypes"
"github.com/alibaba/pouch/pkg/meta"
ns "github.com/alibaba/pouch/pkg/namespace"
"github.com/alibaba/pouch/pkg/randomid"
"github.com/alibaba/pouch/pkg/utils"

Expand Down Expand Up @@ -81,7 +82,7 @@ func NewNetworkManager(cfg *config.Config, store *meta.Store, ctrMgr ContainerMg
&ContainerListOption{
All: true,
FilterFunc: func(c *Container) bool {
return (c.IsRunning() || c.IsPaused()) && !isContainer(c.HostConfig.NetworkMode)
return (c.IsRunning() || c.IsPaused()) && !ns.IsContainer(c.HostConfig.NetworkMode)
}})
if err != nil {
logrus.Errorf("failed to new network manager: cannot get container list")
Expand Down Expand Up @@ -613,7 +614,7 @@ func buildSandboxOptions(config network.Config, endpoint *types.Endpoint) ([]lib

sandboxOptions = append(sandboxOptions, libnetwork.OptionHostname(string(endpoint.Hostname)), libnetwork.OptionDomainname(endpoint.Domainname))

if IsHost(endpoint.NetworkMode) {
if ns.IsHost(endpoint.NetworkMode) {
sandboxOptions = append(sandboxOptions, libnetwork.OptionUseDefaultSandbox())
if len(endpoint.ExtraHosts) == 0 {
sandboxOptions = append(sandboxOptions, libnetwork.OptionOriginHostsPath("/etc/hosts"))
Expand Down
37 changes: 0 additions & 37 deletions daemon/mgr/network_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,12 @@ package mgr

import (
"fmt"
"strings"

"github.com/alibaba/pouch/apis/types"

"github.com/docker/libnetwork"
)

// IsContainer is used to check if network mode is container mode.
func IsContainer(mode string) bool {
parts := strings.SplitN(mode, ":", 2)
return len(parts) > 1 && parts[0] == "container"
}

// IsHost is used to check if network mode is host mode.
func IsHost(mode string) bool {
Copy link
Contributor Author

@Ace-Tang Ace-Tang Nov 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like these function is repeat with pid or user or .. namespace, this is not suitable in a network_util.go @rudyfly

return mode == "host"
}

// IsNone is used to check if network mode is none mode.
func IsNone(mode string) bool {
return mode == "none"
}

// IsBridge is used to check if network mode is bridge mode.
func IsBridge(mode string) bool {
return mode == "bridge"
}

// IsUserDefined is used to check if network mode is user-created.
func IsUserDefined(mode string) bool {
return !IsBridge(mode) && !IsContainer(mode) && !IsHost(mode) && !IsNone(mode)
}

// IsDefault indicates whether container uses the default network stack.
func IsDefault(mode string) bool {
return mode == "default"
}

// IsPrivate indicates whether container uses its private network stack.
func IsPrivate(mode string) bool {
return !(IsHost(mode) || IsContainer(mode))
}

// hasUserDefinedIPAddress returns whether the passed endpoint configuration contains IP address configuration
func hasUserDefinedIPAddress(epConfig *types.EndpointSettings) bool {
return epConfig != nil && epConfig.IPAMConfig != nil && (len(epConfig.IPAMConfig.IPV4Address) > 0 || len(epConfig.IPAMConfig.IPV6Address) > 0)
Expand Down
99 changes: 26 additions & 73 deletions daemon/mgr/spec_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/alibaba/pouch/apis/opts"
"github.com/alibaba/pouch/apis/types"
ns "github.com/alibaba/pouch/pkg/namespace"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/devices"
Expand Down Expand Up @@ -373,54 +374,6 @@ func setupNamespaces(ctx context.Context, c *Container, specWrapper *SpecWrapper
return setupUtsNamespace(ctx, c, specWrapper)
}

// isEmpty indicates whether namespace mode is empty.
func isEmpty(mode string) bool {
return mode == ""
}

// isNone indicates whether container's namespace mode is set to "none".
func isNone(mode string) bool {
return mode == "none"
}

// isHost indicates whether the container shares the host's corresponding namespace.
func isHost(mode string) bool {
return mode == "host"
}

// isShareable indicates whether the containers namespace can be shared with another container.
func isShareable(mode string) bool {
return mode == "shareable"
}

// isContainer indicates whether the container uses another container's corresponding namespace.
func isContainer(mode string) bool {
parts := strings.SplitN(mode, ":", 2)
return len(parts) > 1 && parts[0] == "container"
}

// isPrivate indicates whether the container uses its own namespace.
func isPrivate(ns specs.LinuxNamespaceType, mode string) bool {
switch ns {
case specs.IPCNamespace:
return mode == "private"
case specs.NetworkNamespace, specs.PIDNamespace:
return !(isHost(mode) || isContainer(mode))
case specs.UserNamespace, specs.UTSNamespace:
return !(isHost(mode))
}
return false
}

// connectedContainer is the id or name of the container whose namespace this container share with.
func connectedContainer(mode string) string {
parts := strings.SplitN(mode, ":", 2)
if len(parts) == 2 {
return parts[1]
}
return ""
}

func getIpcContainer(ctx context.Context, mgr ContainerMgr, id string) (*Container, error) {
// Check whether the container exists.
c, err := mgr.Get(ctx, id)
Expand Down Expand Up @@ -458,10 +411,10 @@ func setupNetworkNamespace(ctx context.Context, c *Container, specWrapper *SpecW
}

s := specWrapper.s
ns := specs.LinuxNamespace{Type: specs.NetworkNamespace}
netns := specs.LinuxNamespace{Type: specs.NetworkNamespace}

networkMode := c.HostConfig.NetworkMode
if IsContainer(networkMode) {
if ns.IsContainer(networkMode) {
origContainer, err := specWrapper.ctrMgr.Get(ctx, strings.SplitN(networkMode, ":", 2)[1])
if err != nil {
return err
Expand All @@ -472,14 +425,14 @@ func setupNetworkNamespace(ctx context.Context, c *Container, specWrapper *SpecW
return fmt.Errorf("can not join network of a non running container: %s", origContainer.ID)
}

ns.Path = fmt.Sprintf("/proc/%d/ns/net", origContainer.State.Pid)
} else if IsHost(networkMode) {
ns.Path = c.NetworkSettings.SandboxKey
netns.Path = fmt.Sprintf("/proc/%d/ns/net", origContainer.State.Pid)
} else if ns.IsHost(networkMode) {
netns.Path = c.NetworkSettings.SandboxKey
}
setNamespace(s, ns)
setNamespace(s, netns)

for _, ns := range s.Linux.Namespaces {
if ns.Type == "network" && ns.Path == "" && !c.Config.NetworkDisabled {
for _, n := range s.Linux.Namespaces {
if n.Type == "network" && n.Path == "" && !c.Config.NetworkDisabled {
target, err := os.Readlink(filepath.Join("/proc", strconv.Itoa(os.Getpid()), "exe"))
if err != nil {
return err
Expand All @@ -499,19 +452,19 @@ func setupIpcNamespace(ctx context.Context, c *Container, specWrapper *SpecWrapp
s := specWrapper.s
ipcMode := c.HostConfig.IpcMode
switch {
case isContainer(ipcMode):
ns := specs.LinuxNamespace{Type: specs.IPCNamespace}
c, err := getIpcContainer(ctx, specWrapper.ctrMgr, connectedContainer(ipcMode))
case ns.IsContainer(ipcMode):
ipcns := specs.LinuxNamespace{Type: specs.IPCNamespace}
c, err := getIpcContainer(ctx, specWrapper.ctrMgr, ns.ConnectedContainer(ipcMode))
if err != nil {
return fmt.Errorf("setup container ipc namespace mode failed: %v", err)
}
ns.Path = fmt.Sprintf("/proc/%d/ns/ipc", c.State.Pid)
setNamespace(s, ns)
case isHost(ipcMode):
ipcns.Path = fmt.Sprintf("/proc/%d/ns/ipc", c.State.Pid)
setNamespace(s, ipcns)
case ns.IsHost(ipcMode):
removeNamespace(s, specs.IPCNamespace)
default:
ns := specs.LinuxNamespace{Type: specs.IPCNamespace}
setNamespace(s, ns)
ipcns := specs.LinuxNamespace{Type: specs.IPCNamespace}
setNamespace(s, ipcns)
}
return nil
}
Expand All @@ -520,19 +473,19 @@ func setupPidNamespace(ctx context.Context, c *Container, specWrapper *SpecWrapp
s := specWrapper.s
pidMode := c.HostConfig.PidMode
switch {
case isContainer(pidMode):
ns := specs.LinuxNamespace{Type: specs.PIDNamespace}
c, err := getPidContainer(ctx, specWrapper.ctrMgr, connectedContainer(pidMode))
case ns.IsContainer(pidMode):
pidns := specs.LinuxNamespace{Type: specs.PIDNamespace}
c, err := getPidContainer(ctx, specWrapper.ctrMgr, ns.ConnectedContainer(pidMode))
if err != nil {
return fmt.Errorf("setup container pid namespace mode failed: %v", err)
}
ns.Path = fmt.Sprintf("/proc/%d/ns/pid", c.State.Pid)
setNamespace(s, ns)
case isHost(pidMode):
pidns.Path = fmt.Sprintf("/proc/%d/ns/pid", c.State.Pid)
setNamespace(s, pidns)
case ns.IsHost(pidMode):
removeNamespace(s, specs.PIDNamespace)
default:
ns := specs.LinuxNamespace{Type: specs.PIDNamespace}
setNamespace(s, ns)
pidns := specs.LinuxNamespace{Type: specs.PIDNamespace}
setNamespace(s, pidns)
}
return nil
}
Expand All @@ -541,7 +494,7 @@ func setupUtsNamespace(ctx context.Context, c *Container, specWrapper *SpecWrapp
s := specWrapper.s
utsMode := c.HostConfig.UTSMode
switch {
case isHost(utsMode):
case ns.IsHost(utsMode):
removeNamespace(s, specs.UTSNamespace)
// remove hostname
s.Hostname = ""
Expand Down
Loading