-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Support labeling of volumes to Pod's SELinux label #15323
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ import ( | |
"net/http" | ||
"os" | ||
"path" | ||
"path/filepath" | ||
"sort" | ||
"strings" | ||
"sync" | ||
|
@@ -62,6 +63,7 @@ import ( | |
kubeletutil "k8s.io/kubernetes/pkg/kubelet/util" | ||
"k8s.io/kubernetes/pkg/labels" | ||
"k8s.io/kubernetes/pkg/runtime" | ||
"k8s.io/kubernetes/pkg/securitycontext" | ||
"k8s.io/kubernetes/pkg/types" | ||
"k8s.io/kubernetes/pkg/util" | ||
"k8s.io/kubernetes/pkg/util/bandwidth" | ||
|
@@ -73,6 +75,7 @@ import ( | |
nodeutil "k8s.io/kubernetes/pkg/util/node" | ||
"k8s.io/kubernetes/pkg/util/oom" | ||
"k8s.io/kubernetes/pkg/util/procfs" | ||
"k8s.io/kubernetes/pkg/util/selinux" | ||
"k8s.io/kubernetes/pkg/util/sets" | ||
"k8s.io/kubernetes/pkg/version" | ||
"k8s.io/kubernetes/pkg/volume" | ||
|
@@ -975,6 +978,47 @@ func (kl *Kubelet) syncNodeStatus() { | |
} | ||
} | ||
|
||
// relabelVolumes relabels SELinux volumes to match the pod's | ||
// SELinuxOptions specification. This is only needed if the pod uses | ||
// hostPID or hostIPC. Otherwise relabeling is delegated to docker. | ||
func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap) error { | ||
if pod.Spec.SecurityContext.SELinuxOptions == nil { | ||
return nil | ||
} | ||
|
||
rootDirContext, err := kl.getRootDirContext() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
chconRunner := selinux.NewChconRunner() | ||
// Apply the pod's Level to the rootDirContext | ||
rootDirSELinuxOptions, err := securitycontext.ParseSELinuxOptions(rootDirContext) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
rootDirSELinuxOptions.Level = pod.Spec.SecurityContext.SELinuxOptions.Level | ||
volumeContext := fmt.Sprintf("%s:%s:%s:%s", rootDirSELinuxOptions.User, rootDirSELinuxOptions.Role, rootDirSELinuxOptions.Type, rootDirSELinuxOptions.Level) | ||
|
||
for _, volume := range volumes { | ||
if volume.Builder.SupportsSELinux() && !volume.Builder.IsReadOnly() { | ||
// Relabel the volume and its content to match the 'Level' of the pod | ||
err := filepath.Walk(volume.Builder.GetPath(), func(path string, info os.FileInfo, err error) error { | ||
if err != nil { | ||
return err | ||
} | ||
return chconRunner.SetContext(path, volumeContext) | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
volume.SELinuxLabeled = true | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func makeMounts(pod *api.Pod, podDir string, container *api.Container, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) { | ||
// Kubernetes only mounts on /etc/hosts if : | ||
// - container does not use hostNetwork and | ||
|
@@ -991,11 +1035,21 @@ func makeMounts(pod *api.Pod, podDir string, container *api.Container, podVolume | |
glog.Warningf("Mount cannot be satisified for container %q, because the volume is missing: %q", container.Name, mount) | ||
continue | ||
} | ||
|
||
relabelVolume := false | ||
// If the volume supports SELinux and it has not been | ||
// relabeled already and it is not a read-only volume, | ||
// relabel it and mark it as labeled | ||
if vol.Builder.SupportsSELinux() && !vol.SELinuxLabeled && !vol.Builder.IsReadOnly() { | ||
vol.SELinuxLabeled = true | ||
relabelVolume = true | ||
} | ||
mounts = append(mounts, kubecontainer.Mount{ | ||
Name: mount.Name, | ||
ContainerPath: mount.MountPath, | ||
HostPath: vol.GetPath(), | ||
ReadOnly: mount.ReadOnly, | ||
Name: mount.Name, | ||
ContainerPath: mount.MountPath, | ||
HostPath: vol.Builder.GetPath(), | ||
ReadOnly: mount.ReadOnly, | ||
SELinuxRelabel: relabelVolume, | ||
}) | ||
} | ||
if mountEtcHostsFile { | ||
|
@@ -1080,6 +1134,16 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *api.Pod, container *api.Cont | |
} | ||
|
||
opts.PortMappings = makePortMappings(container) | ||
// Docker does not relabel volumes if the container is running | ||
// in the host pid or ipc namespaces so the kubelet must | ||
// relabel the volumes | ||
if pod.Spec.SecurityContext != nil && (pod.Spec.SecurityContext.HostIPC || pod.Spec.SecurityContext.HostPID) { | ||
err = kl.relabelVolumes(pod, vol) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it a problem that this whole function could fail and the labels are not reverted? I think it is ok, but have to ask. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thockin With this PR it would not be too bad as the next pod will relabel the volume to match its label. So the volume is not rendered unusable. If we change things so that volumes are only relabeled once, only when newly formatted for example, then we might have to handle that. Although TBH I don't think there is value in putting effort into preserving the previous labels; default labels are not usable by pods, and presumably the user does not care about an older pod's labels because the want this pod to use the volume exclusively. |
||
if err != nil { | ||
return nil, err | ||
} | ||
} | ||
|
||
opts.Mounts, err = makeMounts(pod, kl.getPodDir(pod.UID), container, vol) | ||
if err != nil { | ||
return nil, err | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* | ||
Copyright 2015 The Kubernetes Authors All rights reserved. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
// Package selinux contains selinux utility functions. | ||
package selinux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curiosity: if we feel like we know how to do the labelling ourselves, why should we ever defer to docker? Isn't our code net simpler, less coupled, and more fixable if we just do it ourselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. what about non-docker runtimes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thockin I think that we are better off letting docker, or non-docker rumtimes, do the labeling. Because docker is in a better position to know what label the container will run as if one is not provided. In this PR I use the rootContext as a heuristic to decide what the appropriate label 'type' will be. That works well because there are only two answers svirt_sandbox_file_t or "it does not matter because the container is privileged" but if docker changes its defaults we would have to handle that.
In addition I am investigating an issue where containers which run in the hostIPC or hostPID are unconfined by SELinux. That is true with pure docker but not in kube. If I can fix that issue this whole code pathway becomes unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I buy that, but this PR is long overdue, so I'll stop arguing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thockin The chcon runner can definitely be re-used to provide support for rkt and other runtimes. I'll make a follow-up issue.