seccomp: whitelist io-uring related system calls by omegacoleman · Pull Request #39415 · moby/moby

omegacoleman · 2019-06-25T17:18:06Z

closes #39396

- What I did

I added the io-uring related system call introduced in kernel 5.1 to the seccomp whitelist.

- How I did it

This is merely a follow-up to the upstream

- How to verify it

Build docker with the master branch libseccomp, and will be run programs using io-uring without trouble.

With older kernels or older versions of libseccomp, the configure will be omitted.

- Description for the changelog

seccomp: whitelist io-uring related system calls

- A picture of a cute animal (not mandatory but encouraged)

Would be a waste for such a simple change..

thaJeztah · 2019-06-26T00:16:41Z

Looks like CI is failing, because the json version was not updated as part of this PR;

17:32:36 The result of go generate ./profiles/seccomp/ differs
17:32:36 
17:32:36  M profiles/seccomp/default.json
17:32:36 
17:32:36 Please re-run go generate ./profiles/seccomp/

thaJeztah · 2019-06-26T00:16:53Z

ping @justincormack @crosbymichael PTAL

omegacoleman · 2019-06-26T03:30:16Z

Looks like CI is failing, because the json version was not updated as part of this PR;
17:32:36 The result of go generate ./profiles/seccomp/ differs
17:32:36 
17:32:36  M profiles/seccomp/default.json
17:32:36 
17:32:36 Please re-run go generate ./profiles/seccomp/

I made a mistake thinking the json file is manually written, and it should be generated. I have fixed that now but some of the ci seems to be failing because of removing some non-existing folder (C:\go), what does that mean?

thaJeztah · 2019-06-26T06:16:52Z

Thanks! The RS5 failure can be ignored (see #39403)

omegacoleman · 2019-06-26T10:12:43Z

Thanks! The RS5 failure can be ignored (see #39403)

The only failure left is TestAPISwarmServicesMultipleAgents in janky, which does not seem to be relevant. I read the code but it is not about seccomp.

thaJeztah · 2019-06-26T10:45:11Z

yes, looks like that one is sometimes flaky; #23626

I'll restart CI

omegacoleman · 2019-06-30T10:06:19Z

yes, looks like that one is sometimes flaky; #23626

I'll restart CI

This time it's only a few SKIPs & XFAILs, why is it marked as failing again?

thaJeztah · 2019-07-01T11:04:54Z

Looks to be an issue with the docker-py tests;

15:06:25 _______________________ ExecTest.test_exec_command_demux _______________________
15:06:25 /docker-py/tests/integration/api_exec_test.py:182: in test_exec_command_demux
15:06:25     assert next(exec_log) == (b'hello out\r\n', None)
15:06:25 E   AssertionError: assert ('hello out\r...rr\r\n', None) == ('hello out\r\n', None)
15:06:25 E     At index 0 diff: 'hello out\r\nhello err\r\n' != 'hello out\r\n'
15:06:25 E     Use -v to get the full diff
15:06:25 - generated xml file: /go/src/github.com/docker/docker/bundles/test-docker-py/results.xml -

I'll kick that build

justincormack · 2019-07-01T11:19:58Z

Can you sort these alphabetically like the rest please

justincormack · 2019-07-01T11:21:07Z

Are there any other new syscalls since the last update?

thaJeztah · 2019-09-03T08:55:40Z

ping @omegacoleman could you answer @justincormack's question (and sort the list #39415 (comment))?

omegacoleman · 2019-09-05T16:18:02Z

Are there any other new syscalls since the last update?

The precise answer is "I don't know" since I'm just a network dev not kernel dev, but even though there are, the new system call has to be added to the upstream libseccomp's syscall translation (name -> number) before it could be added to the config here, and these calls were added.

Sorry for the delay as it is well, trival & as the upstream has added the calls I was able to just override the config myself..

codecov · 2019-09-05T16:20:44Z

Codecov Report

❗ No coverage uploaded for pull request base (master@4ce0402). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #39415   +/-   ##
=========================================
  Coverage          ?   37.34%           
=========================================
  Files             ?      609           
  Lines             ?    45200           
  Branches          ?        0           
=========================================
  Hits              ?    16882           
  Misses            ?    26030           
  Partials          ?     2288

GordonTheTurtle · 2019-09-05T16:35:26Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" [email protected]:omegacoleman/moby.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354606984
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

thaJeztah

Thanks for updating; could you try rebasing the PR to get rid of the merge commit?

Signed-off-by: youcai <[email protected]>

omegacoleman · 2019-09-07T07:36:39Z

Thanks for updating; could you try rebasing the PR to get rid of the merge commit?

done

thaJeztah

LGTM, thanks!

thaJeztah · 2019-09-11T00:46:51Z

windows failure is unrelated; let me get this one in

cyphar · 2020-07-15T06:21:39Z

This seems like it's a bad idea -- io_uring allows programs to execute certain syscalls without being limited by seccomp. Currently all of the syscalls you can access through io_uring are permitted in the default profile, but I'm a little worried that this wasn't brought up at the time.

justincormack · 2020-07-15T09:12:15Z

@cyphar it looks like io_uring will get its own restriction mechanisms see eg https://lwn.net/SubscriberLink/826053/c58110f9c3493661/. I really don't think we can restrict access to a syscall that lots of people want to use based on the fact it might in future add support for unsafe actions.

I am reviewing the whole seccomp policy based on what has been useful in the past.

cyphar · 2020-07-15T09:59:29Z

Funnily enough, I found this PR after commenting on that LWN article. The problem with that restriction mechanism is that it doesn't really fulfil the seccomp usecase (as long as you can create a new io_uring ring, you can opt to not enable restrictions for it). But Kees has said he's interested in figuring out how to get seccomp to limit io_uring, so I am hopeful this will happen.

omegacoleman · 2020-07-17T04:06:42Z

Funnily enough, I found this PR after commenting on that LWN article. The problem with that restriction mechanism is that it doesn't really fulfil the seccomp usecase (as long as you can create a new io_uring ring, you can opt to not enable restrictions for it). But Kees has said he's interested in figuring out how to get seccomp to limit io_uring, so I am hopeful this will happen.

From what I got from the news they're working on sth like using eBPF to filter the SQEs upon ktail update. Will try to make that configurable after kernel & libseccomp get that feature.

AkihiroSuda · 2020-07-17T04:15:30Z

Shall we revert this until we find the solution?

cyphar · 2020-07-17T04:59:33Z

I don't think it's necessary to revert. The default profile allows everything that you can do with io_uring -- and custom profiles should be modified accordingly. I was more just surprised that this wasn't mentioned at the time. io_uring will grow support for more syscalls in the future so we should keep an eye on this.

This syncs the seccomp profile with changes made to containerd's default profile in [1]. The original containerd issue and PR mention: > Security experts generally believe io_uring to be unsafe. In fact > Google ChromeOS and Android have turned it off, plus all Google > production servers turn it off. Based on the blog published by Google > below it seems like a bunch of vulnerabilities related to io_uring can > be exploited to breakout of the container. > > [2] > > Other security reaserchers also hold this opinion: see [3] for a > blackhat presentation on io_uring exploits. For the record, these syscalls were added to the allowlist in [4]. [1]: containerd/containerd@a48ddf4 [2]: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html [3]: https://i.blackhat.com/BH-US-23/Presentations/US-23-Lin-bad_io_uring.pdf [4]: moby#39415 Signed-off-by: Albin Kerouanton <[email protected]>

thaJeztah · 2023-12-01T08:52:49Z

for future visitors; these were removed again in;

seccomp: block io_uring_* syscalls in default profile #46762
and in containerd: Update RuntimeDefault seccomp profile to disallow io_uring related syscalls containerd/containerd#9320

This syncs the seccomp profile with changes made to containerd's default profile in [1]. The original containerd issue and PR mention: > Security experts generally believe io_uring to be unsafe. In fact > Google ChromeOS and Android have turned it off, plus all Google > production servers turn it off. Based on the blog published by Google > below it seems like a bunch of vulnerabilities related to io_uring can > be exploited to breakout of the container. > > [2] > > Other security reaserchers also hold this opinion: see [3] for a > blackhat presentation on io_uring exploits. For the record, these syscalls were added to the allowlist in [4]. [1]: containerd/containerd@a48ddf4 [2]: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html [3]: https://i.blackhat.com/BH-US-23/Presentations/US-23-Lin-bad_io_uring.pdf [4]: moby/moby#39415 Signed-off-by: Albin Kerouanton <[email protected]>

GordonTheTurtle added the status/0-triage label Jun 25, 2019

thaJeztah added area/security/seccomp status/2-code-review and removed status/0-triage labels Jun 26, 2019

thaJeztah added the rebuild/janky label Jun 26, 2019

GordonTheTurtle removed the rebuild/janky label Jun 26, 2019

thaJeztah added the rebuild/janky label Jul 1, 2019

GordonTheTurtle removed the rebuild/janky label Jul 1, 2019

justincormack reviewed Jul 1, 2019

View reviewed changes

GordonTheTurtle assigned vieux Jul 10, 2019

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Sep 5, 2019

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Sep 5, 2019

thaJeztah requested changes Sep 5, 2019

View reviewed changes

seccomp: whitelist io-uring related system calls

f4d41f1

Signed-off-by: youcai <[email protected]>

omegacoleman requested a review from thaJeztah September 7, 2019 07:37

justincormack approved these changes Sep 10, 2019

View reviewed changes

thaJeztah approved these changes Sep 10, 2019

View reviewed changes

thaJeztah added impact/changelog status/4-merge and removed status/2-code-review labels Sep 10, 2019

thaJeztah merged commit 746eab2 into moby:master Sep 11, 2019

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

AkihiroSuda mentioned this pull request Jul 17, 2020

Revert "seccomp: whitelist io-uring related system calls" #41223

Closed

thaJeztah mentioned this pull request Aug 24, 2020

seccomp: allow io-uring related system calls containerd/containerd#4493

Merged

achimnol mentioned this pull request Feb 24, 2021

Update seccomp profiles lablup/backend.ai-jail#10

Open

thaJeztah mentioned this pull request Nov 2, 2023

Consider removing io_uring syscalls in from RuntimeDefault containerd/containerd#9048

Closed

akerouanton mentioned this pull request Nov 2, 2023

seccomp: block io_uring_* syscalls in default profile #46762

Merged

smiyc mentioned this pull request Feb 10, 2026

[Docs]: io_method: "io_uring" not allowed by default seccomp profile cloudnative-pg/cloudnative-pg#9948

Open

2 tasks

Conversation

omegacoleman commented Jun 25, 2019

Uh oh!

thaJeztah commented Jun 26, 2019

Uh oh!

thaJeztah commented Jun 26, 2019

Uh oh!

omegacoleman commented Jun 26, 2019

Uh oh!

thaJeztah commented Jun 26, 2019

Uh oh!

omegacoleman commented Jun 26, 2019

Uh oh!

thaJeztah commented Jun 26, 2019

Uh oh!

omegacoleman commented Jun 30, 2019

Uh oh!

thaJeztah commented Jul 1, 2019

Uh oh!

justincormack Jul 1, 2019

Choose a reason for hiding this comment

Uh oh!

omegacoleman Sep 5, 2019

Choose a reason for hiding this comment

Uh oh!

justincormack commented Jul 1, 2019

Uh oh!

thaJeztah commented Sep 3, 2019

Uh oh!

omegacoleman commented Sep 5, 2019

Uh oh!

codecov bot commented Sep 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GordonTheTurtle commented Sep 5, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

omegacoleman commented Sep 7, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Sep 11, 2019

Uh oh!

cyphar commented Jul 15, 2020

Uh oh!

justincormack commented Jul 15, 2020

Uh oh!

cyphar commented Jul 15, 2020

Uh oh!

omegacoleman commented Jul 17, 2020

Uh oh!

AkihiroSuda commented Jul 17, 2020

Uh oh!

cyphar commented Jul 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Dec 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov bot commented Sep 5, 2019 •

edited

Loading

cyphar commented Jul 17, 2020 •

edited

Loading