Skip to content

Commit

Permalink
Merge pull request #52 from drachenfels-de/fixes
Browse files Browse the repository at this point in the history 
Fixes for lxcri list, unprivileged runtime and exec namespaces
  • Loading branch information
@r10r
r10r authored May 6, 2021
2 parents 7f340c7 + a2b8f0c commit 3db7b68
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 74 deletions.
81 changes: 43 additions & 38 deletions cmd/lxcri/cli.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func (app *app) configureLogger() error {
return nil
}

func (app *app) loadContainer() (*lxcri.Container, error) {
c, err := clxc.Load(app.cfg.ContainerID)
func (app *app) loadContainer(containerID string) (*lxcri.Container, error) {
c, err := clxc.Load(containerID)
if err != nil {
return c, err
}
Expand Down Expand Up @@ -157,14 +157,14 @@ func main() {
},
&cli.StringFlag{
Name: "root",
Usage: "container runtime root where (logs, init and hook scripts). tmpfs is recommended.",
Usage: "root directory for storage of container runtime state (tmpfs is recommended)",
// exec permissions are not required because init is bind mounted into the root
Value: "/run/lxcri",
Destination: &clxc.Root,
},
&cli.BoolFlag{
Name: "systemd-cgroup",
Usage: "enable support for systemd encoded cgroup path",
Usage: "cgroup path in container spec is systemd encoded and must be expanded",
Destination: &clxc.SystemdCgroup,
},
&cli.StringFlag{
Expand All @@ -176,7 +176,7 @@ func main() {
},
&cli.StringFlag{
Name: "libexec",
Usage: "directory to load runtime executables from",
Usage: "path to directory that contains the runtime executables",
EnvVars: []string{"LXCRI_LIBEXEC"},
Value: libexecDir,
Destination: &clxc.LibexecDir,
Expand Down Expand Up @@ -404,7 +404,7 @@ func doStart(ctxcli *cli.Context) error {
}

func doStartInternal(ctx context.Context) error {
c, err := clxc.loadContainer()
c, err := clxc.loadContainer(clxc.cfg.ContainerID)
if err != nil {
return err
}
Expand All @@ -424,7 +424,7 @@ var stateCmd = cli.Command{
}

func doState(unused *cli.Context) error {
c, err := clxc.loadContainer()
c, err := clxc.loadContainer(clxc.cfg.ContainerID)
if err != nil {
return err
}
Expand Down Expand Up @@ -468,7 +468,7 @@ func doKill(ctxcli *cli.Context) error {
return fmt.Errorf("invalid signal param %q", sig)
}

c, err := clxc.loadContainer()
c, err := clxc.loadContainer(clxc.cfg.ContainerID)
if err != nil {
return err
}
Expand Down Expand Up @@ -628,48 +628,53 @@ func doExec(ctxcli *cli.Context) error {
return err
}

c, err := clxc.loadContainer()
c, err := clxc.loadContainer(clxc.cfg.ContainerID)
if err != nil {
return err
}
defer clxc.releaseContainer(c)

opts := lxcri.ExecOptions{}

if ctxcli.Bool("cgroup") {
opts.Namespaces = append(opts.Namespaces, specs.CgroupNamespace)
}
if ctxcli.Bool("ipc") {
opts.Namespaces = append(opts.Namespaces, specs.IPCNamespace)
}
if ctxcli.Bool("mnt") {
opts.Namespaces = append(opts.Namespaces, specs.MountNamespace)
}
if ctxcli.Bool("net") {
opts.Namespaces = append(opts.Namespaces, specs.NetworkNamespace)
}
if ctxcli.Bool("pid") {
opts.Namespaces = append(opts.Namespaces, specs.PIDNamespace)
}
//if ctxcli.Bool("time") {
// opts.Namespaces = append(opts.Namespaces, specs.TimeNamespace)
//}
if ctxcli.Bool("user") {
opts.Namespaces = append(opts.Namespaces, specs.UserNamespace)
}
if ctxcli.Bool("uts") {
opts.Namespaces = append(opts.Namespaces, specs.UTSNamespace)
}

c.Log.Info().Str("cmd", procSpec.Args[0]).
Uint32("uid", procSpec.User.UID).Uint32("gid", procSpec.User.GID).
Uints32("groups", procSpec.User.AdditionalGids).
Str("namespaces", fmt.Sprintf("%s", opts.Namespaces)).Msg("execute cmd")

if detach {
pid, err := c.ExecDetached(procSpec, nil)
pid, err := c.ExecDetached(procSpec, &opts)
if err != nil {
return err
}
if pidFile != "" {
return createPidFile(pidFile, pid)
}
} else {
opts := lxcri.ExecOptions{}

if ctxcli.Bool("cgroup") {
opts.Namespaces = append(opts.Namespaces, specs.CgroupNamespace)
}
if ctxcli.Bool("ipc") {
opts.Namespaces = append(opts.Namespaces, specs.IPCNamespace)
}
if ctxcli.Bool("mnt") {
opts.Namespaces = append(opts.Namespaces, specs.MountNamespace)
}
if ctxcli.Bool("net") {
opts.Namespaces = append(opts.Namespaces, specs.NetworkNamespace)
}
if ctxcli.Bool("pid") {
opts.Namespaces = append(opts.Namespaces, specs.PIDNamespace)
}
//if ctxcli.Bool("time") {
// opts.Namespaces = append(opts.Namespaces, specs.TimeNamespace)
//}
if ctxcli.Bool("user") {
opts.Namespaces = append(opts.Namespaces, specs.UserNamespace)
}
if ctxcli.Bool("uts") {
opts.Namespaces = append(opts.Namespaces, specs.UTSNamespace)
}

status, err := c.Exec(procSpec, &opts)
if err != nil {
return err
Expand Down Expand Up @@ -757,7 +762,7 @@ func doList(ctxcli *cli.Context) (err error) {
}

func inspectContainer(id string, t *template.Template) error {
c, err := clxc.loadContainer()
c, err := clxc.loadContainer(id)
if err != nil {
return err
}
Expand Down
4 changes: 0 additions & 4 deletions container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,6 @@ func (c *Container) ExecDetached(proc *specs.Process, execOpts *ExecOptions) (pi
return 0, errorf("failed to create attach options: %w", err)
}

c.Log.Info().Strs("args", proc.Args).
Int("uid", opts.UID).Int("gid", opts.GID).
Ints("groups", opts.Groups).Msg("execute cmd")

pid, err = c.LinuxContainer.RunCommandNoWait(proc.Args, opts)
if err != nil {
return pid, errorf("failed to run exec cmd detached: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func configureContainer(rt *Runtime, c *Container) error {
}
newMounts = append(newMounts, m)
}
c.Spec.Mounts = append(c.Spec.Mounts,
newMounts = append(newMounts,
specs.Mount{
Destination: "/dev", Source: "tmpfs", Type: "tmpfs",
Options: []string{"rw", "nosuid", "noexec", "relatime"},
Expand Down
53 changes: 22 additions & 31 deletions runtime_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ func mkdirTemp() (string, error) {
return os.MkdirTemp(tmpRoot, "lxcri-test")
}

func removeAll(t *testing.T, filename string) {
err := os.RemoveAll(filename)
require.NoError(t, err)
}

func newRuntime(t *testing.T) *Runtime {
runtimeRoot, err := mkdirTemp()
require.NoError(t, err)
Expand Down Expand Up @@ -68,10 +73,6 @@ func newConfig(t *testing.T, cmd string, args ...string) *ContainerConfig {
require.NoError(t, err)
t.Logf("container rootfs: %s", rootfs)

// copy test binary to rootfs
//err = exec.Command("cp", cmd, rootfs).Run()
//require.NoError(t, err)

level, err := log.ParseLevel(logLevel)
require.NoError(t, err)

Expand All @@ -83,36 +84,25 @@ func newConfig(t *testing.T, cmd string, args ...string) *ContainerConfig {
id := filepath.Base(rootfs)
cfg := ContainerConfig{
ContainerID: id, Spec: spec,
Log: log.ConsoleLogger(true, level),
Log: log.ConsoleLogger(true, level),
LogFile: "/dev/stderr",
LogLevel: logLevel,
}
cfg.Spec.Linux.CgroupsPath = id + ".slice" // use /proc/self/cgroup"

cfg.Spec.Mounts = append(cfg.Spec.Mounts,
specki.BindMount(cmdAbs, cmdDest),
)

// FIXME /dev/stderr has perms 600
// If container process user is not equal to the
// runtime process user then setting lxc log file will fail
// because of missing permissions.
if runAsRuntimeUser(cfg.Spec) {
cfg.LogFile = "/dev/stderr"
} else {
cfg.LogFile = filepath.Join("/tmp", "log")
}
t.Logf("liblxc log output is written to %s", cfg.LogFile)
cfg.LogLevel = logLevel

return &cfg
}

func TestEmptyNamespaces(t *testing.T) {
t.Parallel()
rt := newRuntime(t)
defer os.RemoveAll(rt.Root)
defer removeAll(t, rt.Root)

cfg := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg.Spec.Root.Path)

// Clearing all namespaces should not work,
// since the mount namespace must never be shared with the host.
Expand All @@ -133,10 +123,10 @@ func TestSharedPIDNamespace(t *testing.T) {
t.Skipf("PID namespace sharing is only permitted as root.")
}
rt := newRuntime(t)
defer os.RemoveAll(rt.Root)
defer removeAll(t, rt.Root)

cfg := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg.Spec.Root.Path)

ctx, cancel := context.WithTimeout(context.Background(), time.Second*3)
defer cancel()
Expand Down Expand Up @@ -170,10 +160,10 @@ func TestSharedPIDNamespace(t *testing.T) {
func TestNonEmptyCgroup(t *testing.T) {
t.Parallel()
rt := newRuntime(t)
defer os.RemoveAll(rt.Root)
defer removeAll(t, rt.Root)

cfg := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg.Spec.Root.Path)

if os.Getuid() != 0 {
cfg.Spec.Linux.UIDMappings = []specs.LinuxIDMapping{
Expand All @@ -195,7 +185,8 @@ func TestNonEmptyCgroup(t *testing.T) {
//time.Sleep(60*time.Second)

cfg2 := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg2.Spec.Root.Path)

cfg2.Spec.Linux.CgroupsPath = cfg.Spec.Linux.CgroupsPath

if os.Getuid() != 0 {
Expand Down Expand Up @@ -232,10 +223,10 @@ func TestRuntimePrivileged(t *testing.T) {
}

rt := newRuntime(t)
defer os.RemoveAll(rt.Root)
defer removeAll(t, rt.Root)

cfg := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg.Spec.Root.Path)

testRuntime(t, rt, cfg)
}
Expand All @@ -256,10 +247,10 @@ func TestRuntimeUnprivileged(t *testing.T) {
}

rt := newRuntime(t)
defer os.RemoveAll(rt.Root)
defer removeAll(t, rt.Root)

cfg := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg.Spec.Root.Path)

// The container UID must have full access to the rootfs.
// MkdirTemp sets directory permissions to 0700.
Expand All @@ -285,10 +276,10 @@ func TestRuntimeUnprivileged(t *testing.T) {
func TestRuntimeUnprivileged2(t *testing.T) {
t.Parallel()
rt := newRuntime(t)
defer os.RemoveAll(rt.Root)
defer removeAll(t, rt.Root)

cfg := newConfig(t, "lxcri-test")
defer os.RemoveAll(cfg.Spec.Root.Path)
defer removeAll(t, cfg.Spec.Root.Path)

if os.Getuid() != 0 {
cfg.Spec.Linux.UIDMappings = []specs.LinuxIDMapping{
Expand Down

0 comments on commit 3db7b68

Please sign in to comment.