Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
namespace: Allow container to join pid namespace of agent
Browse files Browse the repository at this point in the history
This is a debug-only feature and is disabled by default reflected
by the value of "enable_agent_pidns" in the configuration.toml
which is set to false by default.

Allow a container to join the pid namespace of the agent.
This is allowed using an environment variable "AGENT_PIDNS".
Only if this variable is set and configuration explicitly enabled to
allow this, does the runtime set the "agentPidNs" flag in the
CreateContainer grpc request sent to the agent.

Fixes #2633

Signed-off-by: Archana Shinde <[email protected]>
  • Loading branch information
amshinde committed Aug 3, 2020
1 parent 50085ca commit cb49a57
Show file tree
Hide file tree
Showing 13 changed files with 90 additions and 4 deletions.
7 changes: 7 additions & 0 deletions cli/config/configuration-acrn.toml.in
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,10 @@ sandbox_cgroup_only=@DEFSANDBOXCGROUPONLY@
# Supported experimental features:
# (default: [])
experimental=@DEFAULTEXPFEATURES@

# If enabled, containers are allowed to join the pid namespace of the agent
# when the env variable KATA_AGENT_PIDNS is set for a container.
# Use this with caution and only when required, as this option allows the container
# to access the agent process. It is recommended to enable this option
# only in debug scenarios and with containers with lowered priveleges.
#enable_agent_pidns = true
7 changes: 7 additions & 0 deletions cli/config/configuration-clh.toml.in
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,10 @@ sandbox_cgroup_only=@DEFSANDBOXCGROUPONLY@
# Supported experimental features:
# (default: [])
experimental=@DEFAULTEXPFEATURES@

# If enabled, containers are allowed to join the pid namespace of the agent
# when the env variable KATA_AGENT_PIDNS is set for a container.
# Use this with caution and only when required, as this option allows the container
# to access the agent process. It is recommended to enable this option
# only in debug scenarios and with containers with lowered priveleges.
#enable_agent_pidns = true
7 changes: 7 additions & 0 deletions cli/config/configuration-fc.toml.in
Original file line number Diff line number Diff line change
Expand Up @@ -344,3 +344,10 @@ sandbox_cgroup_only=@DEFSANDBOXCGROUPONLY@
# Supported experimental features:
# (default: [])
experimental=@DEFAULTEXPFEATURES@

# If enabled, containers are allowed to join the pid namespace of the agent
# when the env variable KATA_AGENT_PIDNS is set for a container.
# Use this with caution and only when required, as this option allows the container
# to access the agent process. It is recommended to enable this option
# only in debug scenarios and with containers with lowered priveleges.
#enable_agent_pidns = true
9 changes: 9 additions & 0 deletions cli/config/configuration-qemu.toml.in
Original file line number Diff line number Diff line change
Expand Up @@ -470,3 +470,12 @@ sandbox_cgroup_only=@DEFSANDBOXCGROUPONLY@
# Supported experimental features:
# (default: [])
experimental=@DEFAULTEXPFEATURES@


# If enabled, containers are allowed to join the pid namespace of the agent
# when the env variable KATA_AGENT_PIDNS is set for a container.
# Use this with caution and only when required, as this option allows the container
# to access the agent process. It is recommended to enable this option
# only in debug scenarios and with containers with lowered priveleges.
#enable_agent_pidns = true

4 changes: 3 additions & 1 deletion pkg/katatestutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type RuntimeConfigOptions struct {
EnableIOThreads bool
HotplugVFIOOnRootBus bool
DisableNewNetNs bool
EnableAgentPidNs bool
HypervisorDebug bool
RuntimeDebug bool
RuntimeTrace bool
Expand Down Expand Up @@ -89,5 +90,6 @@ func MakeRuntimeConfigFileData(config RuntimeConfigOptions) string {
[runtime]
enable_debug = ` + strconv.FormatBool(config.RuntimeDebug) + `
enable_tracing = ` + strconv.FormatBool(config.RuntimeTrace) + `
disable_new_netns= ` + strconv.FormatBool(config.DisableNewNetNs)
disable_new_netns= ` + strconv.FormatBool(config.DisableNewNetNs) + `
enable_agent_pidns= ` + strconv.FormatBool(config.EnableAgentPidNs)
}
6 changes: 6 additions & 0 deletions pkg/katautils/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ type runtime struct {
DisableNewNetNs bool `toml:"disable_new_netns"`
DisableGuestSeccomp bool `toml:"disable_guest_seccomp"`
SandboxCgroupOnly bool `toml:"sandbox_cgroup_only"`
EnableAgentPidNs bool `toml:"enable_agent_pidns"`
Experimental []string `toml:"experimental"`
InterNetworkModel string `toml:"internetworking_model"`
}
Expand Down Expand Up @@ -1220,6 +1221,11 @@ func LoadConfiguration(configPath string, ignoreLogging, builtIn bool) (resolved

config.SandboxCgroupOnly = tomlConf.Runtime.SandboxCgroupOnly
config.DisableNewNetNs = tomlConf.Runtime.DisableNewNetNs
config.EnableAgentPidNs = tomlConf.Runtime.EnableAgentPidNs
if config.EnableAgentPidNs {
kataUtilsLogger.Warn("Feature to allow containers to share PID namespace with the agent has been enabled. Please understand this has security implications and should only be used for debug purposes")
}

for _, f := range tomlConf.Runtime.Experimental {
feature := exp.Get(f)
if feature == nil {
Expand Down
5 changes: 4 additions & 1 deletion pkg/katautils/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf
disableNewNetNs := false
sharedFS := "virtio-9p"
virtioFSdaemon := path.Join(dir, "virtiofsd")
enableAgentPidNs := true

configFileOptions := ktu.RuntimeConfigOptions{
Hypervisor: "qemu",
Expand Down Expand Up @@ -119,6 +120,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf
AgentTrace: agentTrace,
SharedFS: sharedFS,
VirtioFSDaemon: virtioFSdaemon,
EnableAgentPidNs: enableAgentPidNs,
}

runtimeConfigFileData := ktu.MakeRuntimeConfigFileData(configFileOptions)
Expand Down Expand Up @@ -210,7 +212,8 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf
NetmonConfig: netmonConfig,
DisableNewNetNs: disableNewNetNs,

FactoryConfig: factoryConfig,
EnableAgentPidNs: enableAgentPidNs,
FactoryConfig: factoryConfig,
}

err = SetKernelParams(&runtimeConfig)
Expand Down
29 changes: 29 additions & 0 deletions virtcontainers/kata_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ const (

// path to vfio devices
vfioPath = "/dev/vfio/"

agentPidEnv = "KATA_AGENT_PIDNS"
)

var (
Expand Down Expand Up @@ -1468,6 +1470,15 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,

sharedPidNs := k.handlePidNamespace(grpcSpec, sandbox)

agentPidNs := k.checkAgentPidNs(c)
if agentPidNs {
if !sandbox.config.EnableAgentPidNs {
agentPidNs = false
k.Logger().Warn("Env variable for sharing container pid namespace with the agent set, but the runtime configuration does not allow this")
} else {
k.Logger().Warn("Container will share PID namespace with the agent")
}
}
passSeccomp := !sandbox.config.DisableGuestSeccomp && sandbox.seccompSupported

// We need to constraint the spec to make sure we're not passing
Expand All @@ -1481,6 +1492,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
Devices: ctrDevices,
OCI: grpcSpec,
SandboxPidns: sharedPidNs,
AgentPidns: agentPidNs,
}

if _, err = k.sendReq(req); err != nil {
Expand Down Expand Up @@ -1706,6 +1718,23 @@ func (k *kataAgent) handlePidNamespace(grpcSpec *grpc.Spec, sandbox *Sandbox) bo
return sharedPidNs
}

// checkAgentPidNs checks if environment variable KATA_AGENT_PIDNS has been set for a containers
// This variable is used to indicate if the containers pid namespace should be shared
// with the agent pidns. This approach was taken due to the lack of support for container level annotations.
func (k *kataAgent) checkAgentPidNs(container *Container) bool {
agentPidNs := false

for _, env := range container.config.Cmd.Envs {
if env.Var == agentPidEnv {
if val, err := strconv.ParseBool(env.Value); err == nil {
agentPidNs = val
}
}
}

return agentPidNs
}

func (k *kataAgent) startContainer(sandbox *Sandbox, c *Container) error {
span, _ := k.trace("startContainer")
defer span.Finish()
Expand Down
2 changes: 2 additions & 0 deletions virtcontainers/persist.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) {
Stateful: sconfig.Stateful,
SystemdCgroup: sconfig.SystemdCgroup,
SandboxCgroupOnly: sconfig.SandboxCgroupOnly,
EnableAgentPidNs: sconfig.EnableAgentPidNs,
DisableGuestSeccomp: sconfig.DisableGuestSeccomp,
Cgroups: sconfig.Cgroups,
}
Expand Down Expand Up @@ -487,6 +488,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) {
Stateful: savedConf.Stateful,
SystemdCgroup: savedConf.SystemdCgroup,
SandboxCgroupOnly: savedConf.SandboxCgroupOnly,
EnableAgentPidNs: savedConf.EnableAgentPidNs,
DisableGuestSeccomp: savedConf.DisableGuestSeccomp,
Cgroups: savedConf.Cgroups,
}
Expand Down
3 changes: 3 additions & 0 deletions virtcontainers/persist/api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ type SandboxConfig struct {
// SandboxCgroupOnly enables cgroup only at podlevel in the host
SandboxCgroupOnly bool

// Determines if containers are allowed to join the pid namespace of the kata agent
EnableAgentPidNs bool

DisableGuestSeccomp bool

// Experimental enables experimental features
Expand Down
7 changes: 5 additions & 2 deletions virtcontainers/persist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ func TestSandboxRestore(t *testing.T) {
var err error
assert := assert.New(t)
sconfig := SandboxConfig{
ID: "test-exp",
Experimental: []exp.Feature{persist.NewStoreFeature},
ID: "test-exp",
Experimental: []exp.Feature{persist.NewStoreFeature},
EnableAgentPidNs: true,
}
container := make(map[string]*Container)
container["test-exp"] = &Container{}
Expand Down Expand Up @@ -56,6 +57,7 @@ func TestSandboxRestore(t *testing.T) {
assert.Equal(sandbox.state.State, types.StateString(""))
assert.Equal(sandbox.state.GuestMemoryBlockSizeMB, uint32(0))
assert.Equal(len(sandbox.state.BlockIndexMap), 0)
assert.Equal(sandbox.config.EnableAgentPidNs, true)

// set state data and save again
sandbox.state.State = types.StateString("running")
Expand All @@ -78,4 +80,5 @@ func TestSandboxRestore(t *testing.T) {
assert.Equal(sandbox.state.GuestMemoryBlockSizeMB, uint32(1024))
assert.Equal(len(sandbox.state.BlockIndexMap), 1)
assert.Equal(sandbox.state.BlockIndexMap[2], struct{}{})
assert.Equal(sandbox.config.EnableAgentPidNs, true)
}
5 changes: 5 additions & 0 deletions virtcontainers/pkg/oci/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ type RuntimeConfig struct {
//Determines kata processes are managed only in sandbox cgroup
SandboxCgroupOnly bool

//Determines if containers are allowed to join the pid namespace of the kata agent
EnableAgentPidNs bool

//Experimental features enabled
Experimental []exp.Feature
}
Expand Down Expand Up @@ -852,6 +855,8 @@ func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, c

SandboxCgroupOnly: runtime.SandboxCgroupOnly,

EnableAgentPidNs: runtime.EnableAgentPidNs,

DisableGuestSeccomp: runtime.DisableGuestSeccomp,

// Q: Is this really necessary? @weizhang555
Expand Down
3 changes: 3 additions & 0 deletions virtcontainers/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ type SandboxConfig struct {
// SandboxCgroupOnly enables cgroup only at podlevel in the host
SandboxCgroupOnly bool

// EnableAgentPidNs allows containers to share pid namespace with the agent
EnableAgentPidNs bool

DisableGuestSeccomp bool

// Experimental features enabled
Expand Down

0 comments on commit cb49a57

Please sign in to comment.