Skip to content

Commit

Permalink
Revert "libct/validator: Error out on non-abs paths"
Browse files Browse the repository at this point in the history
This reverts commit 881e92a and adjust
the code so the idmap validations are strict.

Signed-off-by: Rodrigo Campos <[email protected]>
  • Loading branch information
rata committed Aug 7, 2023
1 parent acab6f6 commit 0c196d0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
30 changes: 24 additions & 6 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 @@ -28,13 +29,22 @@ func Validate(config *configs.Config) error {
sysctl,
intelrdtCheck,
rootlessEUIDCheck,
mounts,
mountsStrict,
}
for _, c := range checks {
if err := c(config); err != nil {
return err
}
}
// Relaxed validation rules for backward compatibility
warns := []check{
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).Warn("invalid configuration")
}
}
return nil
}

Expand Down Expand Up @@ -276,17 +286,15 @@ func checkIDMapMounts(config *configs.Config, m *configs.Mount) error {
if !filepath.IsAbs(m.Source) {
return fmt.Errorf("mount source not absolute")
}
if !filepath.IsAbs(m.Destination) {
return fmt.Errorf("mount destination not absolute")
}

return nil
}

func mounts(config *configs.Config) error {
for _, m := range config.Mounts {
// We upgraded this to an error in runc 1.2. We might need to
// revert this change if some users haven't still moved to use
// abs paths, in that please move this check inside
// checkIDMapMounts() as we do want to ensure that for idmap
// mounts anyways.
if !filepath.IsAbs(m.Destination) {
return fmt.Errorf("invalid mount %+v: mount destination not absolute", m)
}
Expand All @@ -298,6 +306,16 @@ func mounts(config *configs.Config) error {
return nil
}

func mountsStrict(config *configs.Config) error {
for _, m := range config.Mounts {
if err := checkIDMapMounts(config, m); err != nil {
return fmt.Errorf("invalid mount %+v: %w", m, err)
}
}

return nil
}

// sameMapping checks if the mappings are the same. If the mappings are the same
// but in different order, it returns false.
func sameMapping(a, b []configs.IDMap) bool {
Expand Down
9 changes: 5 additions & 4 deletions libcontainer/configs/validate/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,11 @@ 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

0 comments on commit 0c196d0

Please sign in to comment.