Skip to content

Commit

Permalink
libcontainer: relax validation for absolute paths
Browse files Browse the repository at this point in the history
Commits 1f1e91b and 2192670
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 <github@gone.nl>
  • Loading branch information
thaJeztah committed Jun 9, 2021
1 parent dbb3541 commit b31a934
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
11 changes: 10 additions & 1 deletion 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 Down Expand Up @@ -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
}

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 b31a934

Please sign in to comment.