CRI: seccomp format is changed from docker 1.12 to 1.13.

[Random-Liu]

Ref moby/moby#27978.

1. We are currently relying on docker seccomp profile format;
2. We are supporing docker 1.11-1.13.

As CRI runtime, which format should we follow? Or should we support both?

@yujuhong @feiskyer @mrunalp @mikebrow @timstclair
/cc @kubernetes/sig-node-bugs @kubernetes/sig-node-api-reviews

[Random-Liu]

The original profile format https://raw.githubusercontent.com/docker/docker/v1.12.3/profiles/seccomp/default.json is old libcontainerd format https://github.com/opencontainers/runc/blob/5274430fee9bc930598cfd9c9dbd33213f79f96e/libcontainer/configs/config.go#L32.

The new profile format https://github.com/moby/moby/blob/master/profiles/seccomp/default.json is OCI runtime-spec format https://github.com/opencontainers/runtime-spec/blob/master/specs-go/config.go#L558,

Given so, I think CRI should use OCI format, too. And we may want to explicitly define this in CRI that the profile is json marshal of OCI runtime spec.

[mikebrow]

This is a tricky question. CRI only has a path to the profile. The path is a string. The json file itself needs to be in the format supported by the runtime that is parsing the profile. If CRI wants to pick a winner here, I would suggest picking the OCI runtime spec v1.0 that is used by runc.

[Random-Liu]

@mikebrow Makes sense. Yeah, maybe as long as the profile is supported by the container runtime, it will be fine, and we don't need to enforce any format here.

The problem is that we also have plan to define the default list in Kubernetes, given the current CRI, kubelet will need to generate a profile for the runtime with the default seccomp list. If so, we still need to enforce a format, so that kubelet knows how to generate the profile.

Another option is to define seccomp structure in CRI, but it means that all existing runtimes need to change.

[mikebrow]

For reference: the bug pointed to above on moby:

invalid header field value "oci runtime error: container_linux.go:247: starting container process caused \"empty string is not a valid syscall\"\n"

Will have occurred because one of the differences between the two formats is the option for having a single name: string for a syscall was removed leaving only names: []string for a list of sys calls. Thus resulting in an error, after unmarshalling, that there is no valid name for the sys call to go with the sys call action. Old format:

{
     "defaultAction": "SCMP_ACT_ALLOW",
     "syscalls": [
         {
             "name": "chmod",
             "action": "SCMP_ACT_ERRNO"
         }
     ]
}

New format:

{
     "defaultAction": "SCMP_ACT_ALLOW",
     "syscalls": [
         {
             "names": ["chmod"],
             "action": "SCMP_ACT_ERRNO"
         }
     ]
}

[mikebrow]

Hmm.. well the default seccomp list should only be determined after caps are set, and also after you know what golang runtime.GOARCH you are running the container on. Otherwise you run the risk of having your seccomp list filtering out commands that should otherwise be enabled. Additionally, certain commands need arguments to be set based on platform.

I put that new default generator over in containerd/containerd... but maybe the best place for it is over in opencontainers runtime spec. Either way we could bring this code over. See https://github.com/containerd/containerd/blob/master/contrib/seccomp/seccomp_default.go

[resouer]

Another option is to define seccomp structure in CRI, but it means that all existing runtimes need to change.

This leaves some extra burden to the maintenance of runtimes I think ... especially when the spec needs some change. (also, kind of divert from "standard" ...)

Use oci spec SGTM, and I also +1 mike's idea if we can move seccomp_default.go to oci spec.

At least, we may require runtime respect the oci spec and https://github.com/containerd/containerd/blob/master/contrib/seccomp/seccomp_default.go. Seems we can work out a library to handle this.

Note that this may introduce problems for non OCI compatible runtimes (if any, or potential ones).

[feiskyer]

+1 for defining kubernetes own profile format, e.g. same with oci spec. This should be defined in both kubernetes API and CRI.

@Random-Liu For the current problem, we should at least address it in v1.8 release notes.

[pmorie]

Gotta love when APIs change out from under you :)

[tallclair]

To echo what everyone else has already said:

Until we define any Kubernetes profiles, I think it's OK to leave the profile format up to the runtime (especially since seccomp is still in alpha).

Once we add a Kubernetes default profile(s), or when we push seccomp to beta, we should settle on the OCI spec, which seems like the clear path forward.

[tallclair]

Closely related: #39128

EDIT: Actually, that's basically a dupe.

[dims]

Moby seems to be parsing seccomp here - https://github.com/moby/moby/blob/master/profiles/seccomp/seccomp.go

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close