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

Add support for enabling/disabling kernel keyring in engines #354

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Add support for enabling/disabling kernel keyring in engines #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion docs/containers.conf.5.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,14 @@ Options are:
`private` Create private IPC Namespace for the container.
`host` Share host IPC Namespace with the container.

**keyring**=true

Indicates whether the container engines create a kernel keyring for use within
rhatdan marked this conversation as resolved.
Show resolved Hide resolved
the container.

**label**=true

Indicates whether the container engines use MAC(SELinux) container separation via via labeling. Flag is ignored on disabled systems.
Indicates whether the container engine uses MAC(SELinux) container separation via labeling. This option is ignored on disabled systems.

**log_driver**="k8s-file"

Expand Down
4 changes: 4 additions & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ type ContainersConfig struct {
// DNSSearches set default DNS search domains.
DNSSearches []string `toml:"dns_searches,omitempty"`

// EnableKeyring tells the container engines whether to create
// a kernel keyring for use within the container
EnableKeyring bool `toml:"keyring,omitempty"`

// EnableLabeling tells the container engines whether to use MAC
// Labeling to separate containers (SELinux)
EnableLabeling bool `toml:"label,omitempty"`
Expand Down
10 changes: 7 additions & 3 deletions pkg/config/containers.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,13 @@ default_sysctls = [
#
# ipcns = "private"

# Flag tells container engine to whether to use container separation using
# MAC(SELinux)labeling or not.
# Flag is ignored on label disabled systems.
# keyring tells the container engine whether to create
# a kernel keyring for use within the container.
# keyring = true
Comment on lines +150 to +151
Copy link
Member

@ashley-cui ashley-cui Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

tiny nit: consistent formatting

otherwise, LGTM once other stuff is addressed

Suggested change
# a kernel keyring for use within the container.
# keyring = true
# a kernel keyring for use within the container.
#
# keyring = true


# label tells the container engine whether to use container separation using
# MAC(SELinux) labeling or not.
# The label flag is ignored on label disabled systems.
#
# label = true

Expand Down
1 change: 1 addition & 0 deletions pkg/config/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func DefaultConfig() (*Config, error) {
DNSServers: []string{},
DNSOptions: []string{},
DNSSearches: []string{},
EnableKeyring: true,
Copy link
Member

Choose a reason for hiding this comment

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

I don't want this to be set unconditionally. This code will likely be greeted on older kernels, where keyrings are not namespaced.

I think we need to run a kernel version check for the built-in default.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hate kernel version checks, becasue they usually are wrong on systems like RHEL that back port fixes. We have been defaulting to on for a long time now without an issue. This has only been asked for inside of a locked down container. I don't believe this is a security issue, and has nothing to do with whether the container can use the kernel keyring. This is the podman/buildah command attempting to create a kernel keyring though conmon.

When people run podman build within a container from Docker, the seccomp rules block this and cause podman to not run, we have been requested to set this field in the podman container to allow it to run with seccomp enabled.

EnableLabeling: selinuxEnabled(),
Env: []string{
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
Expand Down