Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Whitelist syscalls linked to CAP_SYS_NICE in default seccomp profile #37242

Merged
merged 1 commit into from
Jul 3, 2018
Merged

Whitelist syscalls linked to CAP_SYS_NICE in default seccomp profile #37242

merged 1 commit into from
Jul 3, 2018

Conversation

nvcastet
Copy link
Contributor

@nvcastet nvcastet commented Jun 8, 2018

@thaJeztah
Copy link
Member

ping @justincormack PTAL

@thaJeztah
Copy link
Member

@nvcastet looks like you need to regenerate some files;

16:50:05 The result of go generate ./profiles/seccomp/ differs
16:50:05 
16:50:05  M profiles/seccomp/default.json
16:50:05 
16:50:05 Please re-run go generate ./profiles/seccomp/
16:50:05

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jun 8, 2018
@thaJeztah
Copy link
Member

(and likely squash the two commits)

justincormack
justincormack reviewed Jun 8, 2018
View reviewed changes
profiles/seccomp/default.json Outdated
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you will need to keep the original file ending without a new line to get the validation to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

justincormack
justincormack reviewed Jun 8, 2018
View reviewed changes
profiles/seccomp/seccomp_default.go Outdated
Names: []string{
"get_mempolicy",
"mbind",
"name_to_handle_at",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name_to_handle_at is nothing to do with CAP_SYS_NICE it is gated by CAP_DAC_READ_SEARCH and there are other reasons for excluding it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justincormack I am totally fine removing it. But in that case the documentation would need to be updated at https://docs.docker.com/engine/security/seccomp/. Search for name_to_handle_at, it is mentioned Already gated by CAP_SYS_NICE..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened docker/docs#6854

@nvcastet nvcastet mentioned this pull request Jun 9, 2018
Closed
@nvcastet nvcastet force-pushed the fix_sys_nice_seccomp branch 2 times, most recently from 6d17fd4 to d6b767f Compare June 11, 2018 12:57
@codecov
Copy link

codecov bot commented Jun 11, 2018 •
edited
Loading

Codecov Report

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

@@            Coverage Diff            @@
##             master   #37242   +/-   ##
=========================================
  Coverage          ?   35.32%           
=========================================
  Files             ?      609           
  Lines             ?    45011           
  Branches          ?        0           
=========================================
  Hits              ?    15898           
  Misses            ?    26959           
  Partials          ?     2154

@nvcastet
Copy link
Contributor Author

@thaJeztah Would you know why the DockerSwarmSuite.TestAPISwarmLeaderElection test keeps failing?

14:20:57 FAIL: docker_api_swarm_test.go:296: DockerSwarmSuite.TestAPISwarmLeaderElection
14:20:57 
14:20:57 [d4789021c34ee] waiting for daemon to start
14:20:57 [d4789021c34ee] daemon started
14:20:57 
14:20:57 [da4194801ef22] waiting for daemon to start
14:20:57 [da4194801ef22] daemon started
14:20:57 
14:20:57 [decc49a95fe76] waiting for daemon to start
14:20:57 [decc49a95fe76] daemon started
14:20:57 
14:20:57 [d4789021c34ee] exiting daemon
14:20:57 assertion failed: error is not nil: Error response from daemon: rpc error: code = DeadlineExceeded desc = context deadline exceeded
14:20:57 [da4194801ef22] daemon started
14:20:57 Attempt #2: daemon is still running with pid 22957
14:20:57 [da4194801ef22] exiting daemon
14:20:57 [decc49a95fe76] exiting daemon

@thaJeztah
Copy link
Member

Looks like that one is marked "flaky"; #32673

@thaJeztah thaJeztah added rebuild/* and removed status/failing-ci Indicates that the PR in its current state fails the test suite labels Jun 11, 2018
@nvcastet nvcastet force-pushed the fix_sys_nice_seccomp branch 2 times, most recently from 701d53b to 700b4b4 Compare June 14, 2018 15:01
@nvcastet
Copy link
Contributor Author

@thaJeztah Thanks. Do you know if it is possible to retrigger just the PR jobs that failed (here janky and windowsRS1)?

@nvcastet
Whitelist syscalls linked to CAP_SYS_NICE in default seccomp profile
47dfff6 
* Update profile to match docker documentation at
  https://docs.docker.com/engine/security/seccomp/

Signed-off-by: Nicolas V Castet <nvcastet@us.ibm.com>
@thaJeztah
Copy link
Member

Hm, CI doesn't seem to restart; I asked internally if someone has access to do so

@justincormack PTAL

@thaJeztah
Copy link
Member

Failure on PowerPC can be ignored;

13:56:35 FAIL: docker_cli_daemon_test.go:1800: DockerDaemonSuite.TestDaemonNoSpaceLeftOnDeviceError
13:56:35 
13:56:35 [decb810dce5dc] waiting for daemon to start
13:56:35 [decb810dce5dc] exiting daemon
13:56:35 docker_cli_daemon_test.go:1817:
13:56:35     s.d.Start(c, "--data-root", filepath.Join(testDir, "test-mount"))
13:56:35 /go/src/github.com/docker/docker/internal/test/daemon/daemon.go:203:
13:56:35     t.Fatalf("Error starting daemon with arguments: %v", args)
13:56:35 ... Error: Error starting daemon with arguments: [--data-root /tmp/no-space-left-on-device-test924999940/test-mount]
13:56:35

@n4ss
Copy link

n4ss commented Jun 21, 2018

LGTM !

@nvcastet
Copy link
Contributor Author

@thaJeztah @justincormack Anything else needed to merge this PR?

@justincormack
Copy link
Contributor

Its slightly odd gating these all by CAP_SYS_NICE but I think it makes sense.

thaJeztah
thaJeztah approved these changes Jun 29, 2018
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

@nvcastet will you do a follow up PR in the documentation repo?

@nvcastet
Copy link
Contributor Author

nvcastet commented Jul 2, 2018 •
edited
Loading

@thaJeztah Documentation PR was created at: docker/docs#6861

@thaJeztah
Copy link
Member

Thanks!

Failures look to be flaky tests, so I'll go ahead and merge

@thaJeztah thaJeztah merged commit 6273dff into moby:master Jul 3, 2018
@yosifkit yosifkit mentioned this pull request May 13, 2019
Merged
This was referenced Jul 11, 2022
Closed
Merged
@thaJeztah thaJeztah mentioned this pull request Jan 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justincormack justincormack justincormack approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@stevvooe stevvooe

Labels
area/security/seccomp impact/changelog impact/documentation status/4-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

syscalls linked to CAP_SYS_NICE are not whitelisted in default seccomp profile
7 participants
@nvcastet @thaJeztah @n4ss @justincormack @stevvooe @yongtang @GordonTheTurtle