mirror of
https://gitea.com/gitea/act_runner.git
synced 2026-08-06 00:44:22 +02:00
fix!: strip host-escape container options when privileged mode is disabled (#1058)
Workflow-controlled `jobs.<job>.container.options` were merged directly into the
Docker `HostConfig`. When the runner's privileged mode is disabled, only
`Privileged` was forced to `false` — host namespace flags, capability expansion,
security-profile overrides, and device/runtime access from the workflow YAML
survived into the final `HostConfig`. A workflow author could therefore enter
host PID/IPC namespaces and execute commands as root on the runner host:
```yaml
container:
image: ubuntu:22.04
options: >-
--pid=host --ipc=host --cap-add=ALL
--security-opt seccomp=unconfined --security-opt apparmor=unconfined
```
## Fix
`mergeContainerConfigs()` now strips the dangerous options-derived `HostConfig`
fields before merging when privileged mode is off: `PidMode`, `IpcMode`,
`UTSMode`, `CgroupnsMode`, `UsernsMode`, `CapAdd`, `SecurityOpt`, `Devices`,
`DeviceCgroupRules`, `DeviceRequests`, `VolumesFrom`, `Runtime`, `CgroupParent`,
and `Sysctls`. Each strip emits a warning, matching the existing
`--network ignored` handling. Options remain honored when privileged mode is
enabled, since the administrator has already opted into host access.
Reviewed-on: https://gitea.com/gitea/runner/pulls/1058
Reviewed-by: Zettat123 <39446+zettat123@noreply.gitea.com>
This commit is contained in:
@@ -42,6 +42,7 @@ import (
|
|||||||
"github.com/moby/moby/api/types/system"
|
"github.com/moby/moby/api/types/system"
|
||||||
"github.com/moby/moby/client"
|
"github.com/moby/moby/client"
|
||||||
specs "github.com/opencontainers/image-spec/specs-go/v1"
|
specs "github.com/opencontainers/image-spec/specs-go/v1"
|
||||||
|
"github.com/sirupsen/logrus"
|
||||||
)
|
)
|
||||||
|
|
||||||
// drainGracePeriod bounds how long we wait for an output-copy goroutine to
|
// drainGracePeriod bounds how long we wait for an output-copy goroutine to
|
||||||
@@ -500,6 +501,16 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
|
|||||||
return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err)
|
return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// For Gitea
|
||||||
|
// When privileged mode is disabled, container.options is workflow-controlled
|
||||||
|
// untrusted input. Strip the HostConfig fields that would let a workflow break
|
||||||
|
// out of the container (host namespaces, capability expansion, security profile
|
||||||
|
// overrides, device and runtime access). Otherwise these survive into the final
|
||||||
|
// HostConfig even though --privileged is forced off.
|
||||||
|
if !hostConfig.Privileged {
|
||||||
|
sanitizeOptionsHostConfig(logger, containerConfig.HostConfig)
|
||||||
|
}
|
||||||
|
|
||||||
logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config)
|
logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config)
|
||||||
|
|
||||||
err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride, mergo.WithAppendSlice)
|
err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride, mergo.WithAppendSlice)
|
||||||
@@ -1101,6 +1112,77 @@ func (cr *containerReference) wait() common.Executor {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// For Gitea
|
||||||
|
// sanitizeOptionsHostConfig clears the HostConfig fields parsed from a
|
||||||
|
// workflow-controlled container.options string that could be used to escape the
|
||||||
|
// container when privileged mode is disabled. It must only be called when the
|
||||||
|
// runner has privileged mode turned off; with privileged mode enabled the
|
||||||
|
// administrator has already opted into host access.
|
||||||
|
func sanitizeOptionsHostConfig(logger logrus.FieldLogger, hostConfig *container.HostConfig) {
|
||||||
|
warn := func(option string) {
|
||||||
|
logger.Warnf("container option %q is not allowed when privileged mode is disabled and will be ignored", option)
|
||||||
|
}
|
||||||
|
|
||||||
|
if hostConfig.PidMode != "" {
|
||||||
|
warn("--pid")
|
||||||
|
hostConfig.PidMode = ""
|
||||||
|
}
|
||||||
|
if hostConfig.IpcMode != "" {
|
||||||
|
warn("--ipc")
|
||||||
|
hostConfig.IpcMode = ""
|
||||||
|
}
|
||||||
|
if hostConfig.UTSMode != "" {
|
||||||
|
warn("--uts")
|
||||||
|
hostConfig.UTSMode = ""
|
||||||
|
}
|
||||||
|
if hostConfig.CgroupnsMode != "" {
|
||||||
|
warn("--cgroupns")
|
||||||
|
hostConfig.CgroupnsMode = ""
|
||||||
|
}
|
||||||
|
// UsernsMode is set from the runner-controlled input; never let options
|
||||||
|
// override it (e.g. --userns=host disables user namespace remapping).
|
||||||
|
if hostConfig.UsernsMode != "" {
|
||||||
|
warn("--userns")
|
||||||
|
hostConfig.UsernsMode = ""
|
||||||
|
}
|
||||||
|
if len(hostConfig.CapAdd) > 0 {
|
||||||
|
warn("--cap-add")
|
||||||
|
hostConfig.CapAdd = nil
|
||||||
|
}
|
||||||
|
if len(hostConfig.SecurityOpt) > 0 {
|
||||||
|
warn("--security-opt")
|
||||||
|
hostConfig.SecurityOpt = nil
|
||||||
|
}
|
||||||
|
if len(hostConfig.Devices) > 0 {
|
||||||
|
warn("--device")
|
||||||
|
hostConfig.Devices = nil
|
||||||
|
}
|
||||||
|
if len(hostConfig.DeviceCgroupRules) > 0 {
|
||||||
|
warn("--device-cgroup-rule")
|
||||||
|
hostConfig.DeviceCgroupRules = nil
|
||||||
|
}
|
||||||
|
if len(hostConfig.DeviceRequests) > 0 {
|
||||||
|
warn("--gpus")
|
||||||
|
hostConfig.DeviceRequests = nil
|
||||||
|
}
|
||||||
|
if len(hostConfig.VolumesFrom) > 0 {
|
||||||
|
warn("--volumes-from")
|
||||||
|
hostConfig.VolumesFrom = nil
|
||||||
|
}
|
||||||
|
if hostConfig.Runtime != "" {
|
||||||
|
warn("--runtime")
|
||||||
|
hostConfig.Runtime = ""
|
||||||
|
}
|
||||||
|
if hostConfig.CgroupParent != "" {
|
||||||
|
warn("--cgroup-parent")
|
||||||
|
hostConfig.CgroupParent = ""
|
||||||
|
}
|
||||||
|
if len(hostConfig.Sysctls) > 0 {
|
||||||
|
warn("--sysctl")
|
||||||
|
hostConfig.Sysctls = nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// For Gitea
|
// For Gitea
|
||||||
// sanitizeConfig remove the invalid configurations from `config` and `hostConfig`
|
// sanitizeConfig remove the invalid configurations from `config` and `hostConfig`
|
||||||
func (cr *containerReference) sanitizeConfig(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig) {
|
func (cr *containerReference) sanitizeConfig(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig) {
|
||||||
|
|||||||
@@ -669,6 +669,110 @@ func TestCheckVolumes(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSanitizeOptionsHostConfig(t *testing.T) {
|
||||||
|
logger, _ := test.NewNullLogger()
|
||||||
|
|
||||||
|
dangerous := func() *container.HostConfig {
|
||||||
|
return &container.HostConfig{
|
||||||
|
PidMode: "host",
|
||||||
|
IpcMode: "host",
|
||||||
|
UTSMode: "host",
|
||||||
|
CgroupnsMode: "host",
|
||||||
|
UsernsMode: "host",
|
||||||
|
CapAdd: []string{"ALL"},
|
||||||
|
SecurityOpt: []string{"seccomp=unconfined", "apparmor=unconfined"},
|
||||||
|
VolumesFrom: []string{"other"},
|
||||||
|
Runtime: "runc",
|
||||||
|
Resources: container.Resources{
|
||||||
|
CgroupParent: "/custom",
|
||||||
|
Devices: []container.DeviceMapping{{PathOnHost: "/dev/sda", PathInContainer: "/dev/sda", CgroupPermissions: "rwm"}},
|
||||||
|
DeviceCgroupRules: []string{"a *:* rwm"},
|
||||||
|
},
|
||||||
|
Sysctls: map[string]string{"net.ipv4.ip_forward": "1"},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
hostConfig := dangerous()
|
||||||
|
sanitizeOptionsHostConfig(logger, hostConfig)
|
||||||
|
|
||||||
|
assert.Empty(t, string(hostConfig.PidMode))
|
||||||
|
assert.Empty(t, string(hostConfig.IpcMode))
|
||||||
|
assert.Empty(t, string(hostConfig.UTSMode))
|
||||||
|
assert.Empty(t, string(hostConfig.CgroupnsMode))
|
||||||
|
assert.Empty(t, string(hostConfig.UsernsMode))
|
||||||
|
assert.Empty(t, hostConfig.CapAdd)
|
||||||
|
assert.Empty(t, hostConfig.SecurityOpt)
|
||||||
|
assert.Empty(t, hostConfig.Devices)
|
||||||
|
assert.Empty(t, hostConfig.DeviceCgroupRules)
|
||||||
|
assert.Empty(t, hostConfig.VolumesFrom)
|
||||||
|
assert.Empty(t, hostConfig.Runtime)
|
||||||
|
assert.Empty(t, hostConfig.CgroupParent)
|
||||||
|
assert.Empty(t, hostConfig.Sysctls)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMergeContainerConfigsStripsDangerousOptionsWhenUnprivileged(t *testing.T) {
|
||||||
|
// OS-independent options only: --device parsing requires a linux/windows
|
||||||
|
// server OS, which is not guaranteed for the test host.
|
||||||
|
const dangerousOptions = "--pid=host --ipc=host --uts=host --cgroupns=host " +
|
||||||
|
"--userns=host --cap-add=ALL --security-opt seccomp=unconfined " +
|
||||||
|
"--security-opt apparmor=unconfined --volumes-from other " +
|
||||||
|
"--runtime runc --cgroup-parent /custom --sysctl net.ipv4.ip_forward=1"
|
||||||
|
|
||||||
|
t.Run("unprivileged strips host-escape options", func(t *testing.T) {
|
||||||
|
logger, _ := test.NewNullLogger()
|
||||||
|
ctx := common.WithLogger(context.Background(), logger)
|
||||||
|
cr := &containerReference{
|
||||||
|
input: &NewContainerInput{
|
||||||
|
Options: dangerousOptions,
|
||||||
|
NetworkMode: "bridge",
|
||||||
|
UsernsMode: "private",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
_, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{
|
||||||
|
Privileged: false,
|
||||||
|
UsernsMode: container.UsernsMode("private"),
|
||||||
|
NetworkMode: container.NetworkMode("bridge"),
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.False(t, hostConfig.Privileged)
|
||||||
|
assert.Empty(t, string(hostConfig.PidMode))
|
||||||
|
assert.Empty(t, string(hostConfig.IpcMode))
|
||||||
|
assert.Empty(t, string(hostConfig.UTSMode))
|
||||||
|
assert.Empty(t, string(hostConfig.CgroupnsMode))
|
||||||
|
// UsernsMode must keep the runner-controlled value, not the one from options.
|
||||||
|
assert.Equal(t, "private", string(hostConfig.UsernsMode))
|
||||||
|
assert.Empty(t, hostConfig.CapAdd)
|
||||||
|
assert.Empty(t, hostConfig.SecurityOpt)
|
||||||
|
assert.Empty(t, hostConfig.VolumesFrom)
|
||||||
|
assert.Empty(t, hostConfig.Runtime)
|
||||||
|
assert.Empty(t, hostConfig.CgroupParent)
|
||||||
|
assert.Empty(t, hostConfig.Sysctls)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("privileged preserves options", func(t *testing.T) {
|
||||||
|
logger, _ := test.NewNullLogger()
|
||||||
|
ctx := common.WithLogger(context.Background(), logger)
|
||||||
|
cr := &containerReference{
|
||||||
|
input: &NewContainerInput{
|
||||||
|
Options: "--pid=host --cap-add=ALL --security-opt seccomp=unconfined",
|
||||||
|
NetworkMode: "bridge",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
_, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{
|
||||||
|
Privileged: true,
|
||||||
|
NetworkMode: container.NetworkMode("bridge"),
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.Equal(t, "host", string(hostConfig.PidMode))
|
||||||
|
assert.Equal(t, []string{"ALL"}, hostConfig.CapAdd)
|
||||||
|
assert.Equal(t, []string{"seccomp=unconfined"}, hostConfig.SecurityOpt)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func TestCheckVolumesRejectsEscapingHostPaths(t *testing.T) {
|
func TestCheckVolumesRejectsEscapingHostPaths(t *testing.T) {
|
||||||
logger, _ := test.NewNullLogger()
|
logger, _ := test.NewNullLogger()
|
||||||
ctx := common.WithLogger(context.Background(), logger)
|
ctx := common.WithLogger(context.Background(), logger)
|
||||||
|
|||||||
Reference in New Issue
Block a user