From dbb35411f80a9abb68a77b653698678855807e97 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 7 Jun 2021 10:16:34 +0200 Subject: [PATCH 1/2] configs/validator: move cgroup validation to the list of checks Signed-off-by: Sebastiaan van Stijn --- libcontainer/configs/validate/validator.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 0b38b8bdec7..65fe2618866 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -29,6 +29,7 @@ type check func(config *configs.Config) error func (v *ConfigValidator) Validate(config *configs.Config) error { checks := []check{ + v.cgroups, v.rootfs, v.network, v.hostname, @@ -45,10 +46,6 @@ func (v *ConfigValidator) Validate(config *configs.Config) error { return err } } - if err := v.cgroups(config); err != nil { - return err - } - return nil } From b31a9340f92bf02d7fc1a8a9513885561e2fbc47 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 7 Jun 2021 10:24:06 +0200 Subject: [PATCH 2/2] libcontainer: relax validation for absolute paths Commits 1f1e91b1a09b5b22eb414ba7e5ad8c3534526216 and 2192670a2430054c1e539ca7a3bf0ddc43b0bc3d added validation for mountpoints to be an absolute path, to match the OCI specs. Unfortunately, the old behavior (accepting the path to be a relative path) has been around for a long time, and although "not according to the spec", various higher level runtimes rely on this behavior. While higher level runtime have been updated to address this requirement, there will be a transition period before all runtimes are updated to carry these fixes. This patch relaxes the validation, to generate a WARNING instead of failing, allowing runtimes to update (but allowing them to update runc to the current version, which includes security fixes). We can remove this exception in a future patch release. Signed-off-by: Sebastiaan van Stijn --- libcontainer/configs/validate/validator.go | 11 ++++++++++- libcontainer/configs/validate/validator_test.go | 10 ++++++---- libcontainer/specconv/spec_linux.go | 5 ++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index 65fe2618866..b0254600244 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -12,6 +12,7 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/intelrdt" selinux "github.com/opencontainers/selinux/go-selinux" + "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -39,13 +40,21 @@ func (v *ConfigValidator) Validate(config *configs.Config) error { v.sysctl, v.intelrdt, v.rootlessEUID, - v.mounts, } for _, c := range checks { if err := c(config); err != nil { return err } } + // Relaxed validation rules for backward compatibility + warns := []check{ + v.mounts, // TODO (runc v1.x.x): make this an error instead of a warning + } + for _, c := range warns { + if err := c(config); err != nil { + logrus.WithError(err).Warnf("invalid configuration") + } + } return nil } diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go index 7d1ef0f6b3d..687ce1b09a4 100644 --- a/libcontainer/configs/validate/validator_test.go +++ b/libcontainer/configs/validate/validator_test.go @@ -323,10 +323,12 @@ func TestValidateMounts(t *testing.T) { isErr bool dest string }{ - {isErr: true, dest: "not/an/abs/path"}, - {isErr: true, dest: "./rel/path"}, - {isErr: true, dest: "./rel/path"}, - {isErr: true, dest: "../../path"}, + // TODO (runc v1.x.x): make these relative paths an error. See https://github.com/opencontainers/runc/pull/3004 + {isErr: false, dest: "not/an/abs/path"}, + {isErr: false, dest: "./rel/path"}, + {isErr: false, dest: "./rel/path"}, + {isErr: false, dest: "../../path"}, + {isErr: false, dest: "/abs/path"}, {isErr: false, dest: "/abs/but/../unclean"}, } diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index ac83123df54..8474769c940 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -333,7 +333,10 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { func createLibcontainerMount(cwd string, m specs.Mount) (*configs.Mount, error) { if !filepath.IsAbs(m.Destination) { - return nil, fmt.Errorf("mount destination %s not absolute", m.Destination) + // Relax validation for backward compatibility + // TODO (runc v1.x.x): change warning to an error + // return nil, fmt.Errorf("mount destination %s is not absolute", m.Destination) + logrus.Warnf("mount destination %s is not absolute. Support for non-absolute mount destinations will be removed in a future release.", m.Destination) } flags, pgflags, data, ext := parseMountOptions(m.Options) source := m.Source