shim/manager: Retry shim start without userns on clone failure

cloneMntNs sets CLONE_NEWUSER|CLONE_NEWNS on the child, but clone can
fail for reasons the proactive AppArmor sysctl check cannot detect —
seccomp filters, other LSM policies, or EACCES when inherited socket
fds cross the user namespace boundary after exec triggers capability
recomputation.

Return whether namespace flags were set so the caller can distinguish a
namespace-related Start failure from an unrelated one. On failure, rebuild
the command without clone flags and retry, degrading gracefully to no
mount isolation rather than failing the container start entirely.

Signed-off-by: Derek McGowan <derek@mcg.dev>

Author: dmcgowan

internal/shim/manager/manager_unix.go

@@ -183,10 +183,33 @@ func (manager) Start(ctx context.Context, id string, opts shim.StartOpts) (_ shi
 		cmd.ExtraFiles = append(cmd.ExtraFiles, s.f)
 	}
 
-	cloneMntNs(ctx, cmd)
+	userns := cloneMntNs(ctx, cmd)
 
 	if err := cmd.Start(); err != nil {
-		return params, err
+		if !userns {
+			return params, err
+		}
+		// clone(CLONE_NEWUSER) can fail for reasons not covered by the
+		// proactive AppArmor check — e.g. seccomp filters, LSM policies,
+		// or EACCES from the child's capability recomputation when
+		// inherited socket fds cross the user namespace boundary after
+		// exec. Retry without namespace isolation rather than failing
+		// the container start.
+		//
+		// Note: we cannot log here — during "start" the logger output
+		// goes to stderr which containerd captures as part of the
+		// bootstrap response (CombinedOutput), corrupting the JSON.
+		cmd, err = newCommand(ctx, id, opts.Address, opts.TTRPCAddress, opts.Debug)
+		if err != nil {
+			return params, err
+		}
+		cmd.ExtraFiles = append(cmd.ExtraFiles, sockets[0].f)
+		if opts.Debug && len(sockets) > 1 {
+			cmd.ExtraFiles = append(cmd.ExtraFiles, sockets[1].f)
+		}
+		if err := cmd.Start(); err != nil {
+			return params, err
+		}
 	}
 
 	defer func() {

internal/shim/manager/mount_linux.go

@@ -54,17 +54,16 @@ import (
 // If namespace creation is not possible (e.g. AppArmor restricts
 // unprivileged user namespaces), the function logs a warning and the shim
 // will run without mount isolation.
-func cloneMntNs(_ context.Context, cmd *exec.Cmd) {
+// cloneMntNs returns true if user namespace clone flags were set.
+func cloneMntNs(_ context.Context, cmd *exec.Cmd) bool {
 	if restricted, err := apparmorRestrictsUserns(); err != nil {
 		// Failed to check apparmor userns restriction, skipping mount namespace isolation")
 		// We can't log anything here as it will break the TTRPC protocol!
-		// TODO(vvoland): Find a better way to surface this to the user.
-		return
+		return false
 	} else if restricted {
 		// apparmor_restrict_unprivileged_userns=1 prevents user namespace creation; shim will run without mount namespace isolation
 		// We can't log anything here as it will break the TTRPC protocol!
-		// TODO(vvoland): Find a better way to surface this to the user.
-		return
+		return false
 	}
 
 	uid := os.Getuid()
@@ -76,6 +75,7 @@ func cloneMntNs(_ context.Context, cmd *exec.Cmd) {
 	cmd.SysProcAttr.GidMappings = []syscall.SysProcIDMap{
 		{ContainerID: gid, HostID: gid, Size: 1},
 	}
+	return true
 }
 
 // apparmorRestrictsUserns checks if the kernel sysctl

internal/shim/manager/mount_other.go

@@ -23,4 +23,4 @@ import (
 	"os/exec"
 )
 
-func cloneMntNs(_ context.Context, _ *exec.Cmd) {}
+func cloneMntNs(_ context.Context, _ *exec.Cmd) bool { return false }