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.

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

Add support for Seccomp and AppArmor profiles. #3152

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

dperny
Copy link
Collaborator

@dperny dperny commented Aug 11, 2023

Adds support for seccomp profiles, AppArmor profiles, and the no-new-privileges security options.

This rounds out the present functionality of the --security-opt flag present for Docker containers.

@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2023

Codecov Report

Merging #3152 (5e3d974) into master (1983e41) will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3152      +/-   ##
==========================================
+ Coverage   61.67%   61.79%   +0.11%     
==========================================
  Files         155      155              
  Lines       31143    31143              
==========================================
+ Hits        19207    19244      +37     
+ Misses      10389    10346      -43     
- Partials     1547     1553       +6     

api/types.proto Outdated
Comment on lines 1160 to 1167
// Apparmor is the Apparmor profile to be applied to the container, if any.
// The node running the container must have the profile present.
string apparmor = 3;

// Seccomp is the seccomp profile to be used as a seccomp filter for the
// container, or "unconfined" if none. The profile must be present on the
// node running the container.
string seccomp = 4;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps make it explicit; apparmor_profile and seccomp_profile ?

api/types.proto Outdated
string seccomp_profile = 4;

// NoNewPrivileges, if set to true, disables the container from gaining new
// privileges.
Copy link
Member

Choose a reason for hiding this comment

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

It might not hurt to link to It might not hurt to link to https://docs.kernel.org/userspace-api/no_new_privs.html here here.

api/types.proto Outdated
}
ApparmorMode mode = 1;
}
ApparmorOpts apparmor = 4;
Copy link
Member

@neersighted neersighted Aug 21, 2023

Choose a reason for hiding this comment

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

(All instances) probably should be AppArmor

@@ -1156,6 +1156,33 @@ message Privileges {
string level = 5;
}
SELinuxContext selinux_context = 2 [(gogoproto.customname) = "SELinuxContext"];

message SeccompOpts {
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a doc-comment describing the field, and maybe linking to the Docker seccomp docs

api/types.proto Outdated
}
SeccompOpts seccomp = 3;

message ApparmorOpts {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto above

Adds support for seccomp profiles, AppArmor profiles, and the
no-new-privileges security options.

This rounds out the present functionality of the --security-opt flag
present for Docker containers.

Signed-off-by: Drew Erny <derny@mirantis.com>
Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

LGTM, is this ready to come out of draft?

@dperny dperny marked this pull request as ready for review August 23, 2023 15:55
@dperny dperny merged commit 12f0c24 into moby:master Aug 23, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants