From cb49a5710ea3263d725643b309503d7bb4c94b66 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Mon, 3 Aug 2020 14:53:07 -0700 Subject: [PATCH] namespace: Allow container to join pid namespace of agent 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 --- cli/config/configuration-acrn.toml.in | 7 +++++++ cli/config/configuration-clh.toml.in | 7 +++++++ cli/config/configuration-fc.toml.in | 7 +++++++ cli/config/configuration-qemu.toml.in | 9 +++++++++ pkg/katatestutils/utils.go | 4 +++- pkg/katautils/config.go | 6 ++++++ pkg/katautils/config_test.go | 5 ++++- virtcontainers/kata_agent.go | 29 +++++++++++++++++++++++++++ virtcontainers/persist.go | 2 ++ virtcontainers/persist/api/config.go | 3 +++ virtcontainers/persist_test.go | 7 +++++-- virtcontainers/pkg/oci/utils.go | 5 +++++ virtcontainers/sandbox.go | 3 +++ 13 files changed, 90 insertions(+), 4 deletions(-) diff --git a/cli/config/configuration-acrn.toml.in b/cli/config/configuration-acrn.toml.in index d1c581243c..cd6eb78860 100644 --- a/cli/config/configuration-acrn.toml.in +++ b/cli/config/configuration-acrn.toml.in @@ -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 diff --git a/cli/config/configuration-clh.toml.in b/cli/config/configuration-clh.toml.in index 6718f4a0df..e249bdd267 100644 --- a/cli/config/configuration-clh.toml.in +++ b/cli/config/configuration-clh.toml.in @@ -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 diff --git a/cli/config/configuration-fc.toml.in b/cli/config/configuration-fc.toml.in index 4bbb19aeb1..b92873cee4 100644 --- a/cli/config/configuration-fc.toml.in +++ b/cli/config/configuration-fc.toml.in @@ -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 diff --git a/cli/config/configuration-qemu.toml.in b/cli/config/configuration-qemu.toml.in index 095ce566c7..9c7af06fab 100644 --- a/cli/config/configuration-qemu.toml.in +++ b/cli/config/configuration-qemu.toml.in @@ -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 + diff --git a/pkg/katatestutils/utils.go b/pkg/katatestutils/utils.go index 999ab64b2f..9560cf48c8 100644 --- a/pkg/katatestutils/utils.go +++ b/pkg/katatestutils/utils.go @@ -34,6 +34,7 @@ type RuntimeConfigOptions struct { EnableIOThreads bool HotplugVFIOOnRootBus bool DisableNewNetNs bool + EnableAgentPidNs bool HypervisorDebug bool RuntimeDebug bool RuntimeTrace bool @@ -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) } diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index fbfba881ba..f909127f61 100644 --- a/pkg/katautils/config.go +++ b/pkg/katautils/config.go @@ -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"` } @@ -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 { diff --git a/pkg/katautils/config_test.go b/pkg/katautils/config_test.go index 4136107f60..f396f2ca73 100644 --- a/pkg/katautils/config_test.go +++ b/pkg/katautils/config_test.go @@ -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", @@ -119,6 +120,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf AgentTrace: agentTrace, SharedFS: sharedFS, VirtioFSDaemon: virtioFSdaemon, + EnableAgentPidNs: enableAgentPidNs, } runtimeConfigFileData := ktu.MakeRuntimeConfigFileData(configFileOptions) @@ -210,7 +212,8 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf NetmonConfig: netmonConfig, DisableNewNetNs: disableNewNetNs, - FactoryConfig: factoryConfig, + EnableAgentPidNs: enableAgentPidNs, + FactoryConfig: factoryConfig, } err = SetKernelParams(&runtimeConfig) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index c604115924..3ba4e3541e 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -54,6 +54,8 @@ const ( // path to vfio devices vfioPath = "/dev/vfio/" + + agentPidEnv = "KATA_AGENT_PIDNS" ) var ( @@ -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 @@ -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 { @@ -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() diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index 6759ed5ce2..9bac39106a 100644 --- a/virtcontainers/persist.go +++ b/virtcontainers/persist.go @@ -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, } @@ -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, } diff --git a/virtcontainers/persist/api/config.go b/virtcontainers/persist/api/config.go index d68eae9b12..45534c1891 100644 --- a/virtcontainers/persist/api/config.go +++ b/virtcontainers/persist/api/config.go @@ -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 diff --git a/virtcontainers/persist_test.go b/virtcontainers/persist_test.go index 52e38ce523..cddd8d93df 100644 --- a/virtcontainers/persist_test.go +++ b/virtcontainers/persist_test.go @@ -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{} @@ -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") @@ -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) } diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index f80c3cc096..99e9a6f554 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -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 } @@ -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 diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 92aa0c12c5..cdd2aff9d4 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -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