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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove "seccomp" build tag #42501

Merged
merged 1 commit into from
May 13, 2022
Merged

Remove "seccomp" build tag #42501

merged 1 commit into from
May 13, 2022

Conversation

tianon
Copy link
Member

@tianon tianon commented Jun 9, 2021

Similar to the (now removed) apparmor build tag, this build-time toggle existed for users who needed to build without the libseccomp library. That's no longer necessary, and given the importance of seccomp to the overall default security profile of Docker containers, it makes sense that any binary built for Linux should support (and use by default) seccomp if the underlying host does.

(As discussed in #42486 (comment) 馃槃)

This does some changes in vendor/github.com/moby/buildkit so it's not 100% ready to merge as-is. 馃槄

@tianon
Copy link
Member Author

tianon commented Jun 9, 2021

I think the "edge case" here will be a system which supports seccomp, but doesn't have libseccomp, and thus runc is built without libseccomp, and I think that will end up generating an error when it tries to run the container, but IMO that's really the expected behavior (and definitely what I'd want as a user of said setup, so I could correct it either by fixing my runc or by adjusting my container definition to "accept the risk" so to speak).

@cpuguy83
Copy link
Member

As I recall buildkit also needs to drop the build tag.

@thaJeztah
Copy link
Member

This may conflict with #42005

@thaJeztah
Copy link
Member

This may conflict with #42005

Looks like you're lucky; I tried rebasing this locally, and looks like it still applies cleanly 馃槄

@thaJeztah thaJeztah mentioned this pull request Aug 27, 2021
Merged
@tianon tianon marked this pull request as ready for review May 12, 2022 21:09
@tianon tianon requested a review from thaJeztah as a code owner May 12, 2022 21:09
@tianon
Copy link
Member Author

tianon commented May 12, 2022

Thanks to moby/buildkit#2338 and a rebase, I think this is ready for review! 馃

@tianon
Remove "seccomp" build tag
c9e19a2 
Similar to the (now removed) `apparmor` build tag, this build-time toggle existed for users who needed to build without the `libseccomp` library.  That's no longer necessary, and given the importance of seccomp to the overall default security profile of Docker containers, it makes sense that any binary built for Linux should support (and use by default) seccomp if the underlying host does.

Signed-off-by: Tianon Gravi <[email protected]>
@thaJeztah thaJeztah added this to the 22.06.0 milestone May 12, 2022
thaJeztah
thaJeztah approved these changes May 12, 2022
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 (if green)

Completely forgot about this one! Guess we can make some changes in our packaging as well after this is merged 馃憤

@thaJeztah
Copy link
Member

Looks green(ish); some windows failures that are unrelated (gave it another kick)

cpuguy83
cpuguy83 approved these changes May 13, 2022
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit f32b304 into moby:master May 13, 2022
@thaJeztah thaJeztah mentioned this pull request May 13, 2022
Merged
@thaJeztah
Copy link
Member

Opened docker/docker-ce-packaging#693 to remove the obsolete build-tags from our build-scripts

arnout pushed a commit to buildroot/buildroot that referenced this pull request Mar 12, 2023
@ffontaine @jacmet
package/docker-engine: libseccomp is mandatory
1807ef1 
libseccomp is mandatory since bump to version 23.0.0 in commit
485b47e:
moby/moby#42501

Fixes:
 - https://bugs.buildroot.org/show_bug.cgi?id=15321

Signed-off-by: Fabrice Fontaine <[email protected]>
Signed-off-by: Peter Korsgaard <[email protected]>
ricardosalveti added a commit to ricardosalveti/meta-lmp that referenced this pull request May 30, 2023
@ricardosalveti
base: docker.inc: always enable seccomp
be3d6f6 
Since moby/moby#42501 (part of 23.01) seccomp is now
required as a build dependency, so drop optional packageconfig settings.

Signed-off-by: Ricardo Salveti <[email protected]>
ricardosalveti added a commit to ricardosalveti/meta-lmp that referenced this pull request May 31, 2023
@ricardosalveti
base: docker.inc: always enable seccomp
94eaf8a 
Since moby/moby#42501 (part of 23.01) seccomp is now
required as a build dependency, so drop optional packageconfig settings.

Signed-off-by: Ricardo Salveti <[email protected]>
ricardosalveti added a commit to ricardosalveti/meta-lmp that referenced this pull request May 31, 2023
@ricardosalveti
base: docker.inc: always enable seccomp
9640651 
Since moby/moby#42501 (part of 23.01) seccomp is now
required as a build dependency, so drop optional packageconfig settings.

Signed-off-by: Ricardo Salveti <[email protected]>
ricardosalveti added a commit to foundriesio/meta-lmp that referenced this pull request Jun 2, 2023
@ricardosalveti
base: docker.inc: always enable seccomp
9712d1f 
Since moby/moby#42501 (part of 23.01) seccomp is now
required as a build dependency, so drop optional packageconfig settings.

Signed-off-by: Ricardo Salveti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees
No one assigned
Labels
area/packaging area/security/seccomp impact/changelog status/2-code-review
Projects
None yet
Milestone
23.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tianon @cpuguy83 @thaJeztah