ping @Random-Liu @justincormack PTAL

Codecov Report

Merging #3538 into release/1.2 will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           release/1.2    #3538   +/-   ##
============================================
  Coverage        43.66%   43.66%           
============================================
  Files              101      101           
  Lines            10816    10816           
============================================
  Hits              4723     4723           
  Misses            5357     5357           
  Partials           736      736

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eed8acd...8c0ec3c. Read the comment docs.

From the runc libseccomp bump; opencontainers/runc#2074 (comment)

Looks like the fix for the CVE was already merged in opencontainers/runc@03a5a74#diff-c1eca12d097b318b217f891966083c8e as part of opencontainers/runc#1424

The "diff" posted in this PR looks to be between the wrong commits; this is the right link/diff:

seccomp/libseccomp-golang@84e90a9...v0.9.1

The fix is in opencontainers/runc@03a5a74#diff-c1eca12d097b318b217f891966083c8e).

The fix in libseccomp is in commit seccomp/libseccomp-golang@06e7a29

Full diff of libseccomp-golang changes in that runc PR: seccomp/libseccomp-golang@32f571b...84e90a9

@thaJeztah So runc would still need libseccomp 2.3.0+?

That is fine for us, just make it clear, so that others may want to know about it. :)

@thaJeztah So runc would still need libseccomp 2.3.0+?

Current runc master yes, so it could be that at some point we have to raise the minimum requirement, but at least this should bring some time.

I think we should have a look if that can be fixed; haven't looked into it in-depth, but if the offending commit is indeed seccomp/libseccomp-golang@9814e55, then (from the description), it looks like it's only an optimisation that causes the dependency on the newer version; perhaps someone is able to provide a fix upstream to make sure older versions still work?

The code seems to suggest that the seccomp_version should only be used on seccomp 2.3 and above, but perhaps there's a bug?

https://github.com/seccomp/libseccomp-golang/blob/9814e55a2e59df39b8ad4fbff1226585ebb84674/seccomp_internal.go#L123-L138

#if SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 3
unsigned int get_major_version()
{
       return seccomp_version()->major;
}
unsigned int get_minor_version()
{
       return seccomp_version()->minor;
}
unsigned int get_micro_version()
{
       return seccomp_version()->micro;
}
#else

LGTM

@thaJeztah I guess that might be decided at compile time?

So maybe we just need to make sure libseccomp version at built time matches runtime?

It seems that in our CI environment, libseccomp2 was 2.3.1 at compile time, but was 2.2.x at runtime before updating the image kubernetes/test-infra#13639

@thaJeztah I guess that might be decided at compile time?

🤦‍♂ yes, of course, I wasn't thinking; that makes sense