Skip to content

Commit

Permalink
merge branch 'pr-3004'
Browse files Browse the repository at this point in the history
Sebastiaan van Stijn (2):
  libcontainer: relax validation for absolute paths
  configs/validator: move cgroup validation to the list of checks

LGTMs: kolyshkin cyphar
Closes #3004
  • Loading branch information
cyphar committed Jun 10, 2021
2 parents dcdf6b6 + b31a934 commit f454bb1
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 9 deletions.
14 changes: 10 additions & 4 deletions libcontainer/configs/validate/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -29,6 +30,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,
Expand All @@ -38,17 +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
}
}
if err := v.cgroups(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
}

Expand Down
10 changes: 6 additions & 4 deletions libcontainer/configs/validate/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
}
Expand Down
5 changes: 4 additions & 1 deletion libcontainer/specconv/spec_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f454bb1

Please sign in to comment.