Skip to content

Commit

Permalink
migration: match SELinux level of source pod on target pod
Browse files Browse the repository at this point in the history
Signed-off-by: Jed Lejosne <jed@redhat.com>
  • Loading branch information
jean-edouard committed May 24, 2023
1 parent 64a7cd1 commit 6314e05
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 1 deletion.
4 changes: 4 additions & 0 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -17542,6 +17542,10 @@
"type": "integer",
"format": "int64"
},
"disableSELinuxLevelOnTarget": {
"description": "By default, the SELinux level of target virt-launcher pods is forced to the level of the source virt-launcher. When set to true, DisableSELinuxLevelOnTarget lets the CRI auto-assign a random level to the target. That will ensure the target virt-launcher doesn't share categories with another pod on the node. However, migrations will fail when using RWX storage that doesn't automatically deal with SELinux levels.",
"type": "boolean"
},
"disableTLS": {
"description": "When set to true, DisableTLS will disable the additional layer of live migration encryption provided by KubeVirt. This is usually a bad idea. Defaults to false",
"type": "boolean"
Expand Down
20 changes: 20 additions & 0 deletions manifests/generated/kv-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,16 @@ spec:
unless AllowPostCopy is true. Defaults to 800
format: int64
type: integer
disableSELinuxLevelOnTarget:
description: By default, the SELinux level of target virt-launcher
pods is forced to the level of the source virt-launcher.
When set to true, DisableSELinuxLevelOnTarget lets the CRI
auto-assign a random level to the target. That will ensure
the target virt-launcher doesn't share categories with another
pod on the node. However, migrations will fail when using
RWX storage that doesn't automatically deal with SELinux
levels.
type: boolean
disableTLS:
description: When set to true, DisableTLS will disable the
additional layer of live migration encryption provided by
Expand Down Expand Up @@ -3299,6 +3309,16 @@ spec:
unless AllowPostCopy is true. Defaults to 800
format: int64
type: integer
disableSELinuxLevelOnTarget:
description: By default, the SELinux level of target virt-launcher
pods is forced to the level of the source virt-launcher.
When set to true, DisableSELinuxLevelOnTarget lets the CRI
auto-assign a random level to the target. That will ensure
the target virt-launcher doesn't share categories with another
pod on the node. However, migrations will fail when using
RWX storage that doesn't automatically deal with SELinux
levels.
type: boolean
disableTLS:
description: When set to true, DisableTLS will disable the
additional layer of live migration encryption provided by
Expand Down
1 change: 1 addition & 0 deletions pkg/virt-controller/watch/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ go_library(
"//vendor/github.com/emicklei/go-restful:go_default_library",
"//vendor/github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1:go_default_library",
"//vendor/github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1:go_default_library",
"//vendor/github.com/opencontainers/selinux/go-selinux:go_default_library",
"//vendor/github.com/pborman/uuid:go_default_library",
"//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
"//vendor/github.com/prometheus/client_golang/prometheus/promhttp:go_default_library",
Expand Down
28 changes: 28 additions & 0 deletions pkg/virt-controller/watch/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (
"sync"
"time"

"github.com/opencontainers/selinux/go-selinux"

"kubevirt.io/api/migrations/v1alpha1"

k8sv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -619,6 +621,32 @@ func (c *MigrationController) createTargetPod(migration *virtv1.VirtualMachineIn
}
}

disableLevelOnTarget := c.clusterConfig.GetMigrationConfiguration().DisableSELinuxLevelOnTarget
if disableLevelOnTarget == nil || !*disableLevelOnTarget {
// The target pod may share resources with the sources pod (RWX disks for example)
// Therefore, it needs to share the same SELinux categories to inherit the same permissions
// Note: there is a small probablility that the target pod will share the same categories as another pod on its node.
// It is a slight security concern, but not as bad as removing categories on all shared objects for the duration of the migration.
if vmi.Status.SelinuxContext == "" {
log.Log.Object(vmi).Warningf("SELinux context not set on VMI status")
} else {
seContext, err := selinux.NewContext(vmi.Status.SelinuxContext)
if err != nil {
return err
}
level, exists := seContext["level"]
if exists && level != "" {
// The SELinux context looks like "system_u:object_r:container_file_t:s0:c1,c2", we care about "s0:c1,c2"
if templatePod.Spec.SecurityContext == nil {
templatePod.Spec.SecurityContext = &k8sv1.PodSecurityContext{}
}
templatePod.Spec.SecurityContext.SELinuxOptions = &k8sv1.SELinuxOptions{
Level: level,
}
}
}
}

// This is used by the functional test to simulate failures
computeImageOverride, ok := migration.Annotations[virtv1.FuncTestMigrationTargetImageOverrideAnnotation]
if ok && computeImageOverride != "" {
Expand Down
2 changes: 1 addition & 1 deletion pkg/virt-controller/watch/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,6 @@ var _ = Describe("Migration watcher", func() {
shouldExpectPodCreation(vmi.UID, migration.UID, 1, 0, 0)
controller.Execute()
testutils.ExpectEvents(recorder, SuccessfulCreatePodReason)

})
It("should create target pod", func() {
vmi := newVirtualMachine("testvmi", virtv1.Running)
Expand Down Expand Up @@ -1890,6 +1889,7 @@ func newVirtualMachine(name string, phase virtv1.VirtualMachineInstancePhase) *v
vmi.UID = types.UID(name)
vmi.Status.Phase = phase
vmi.Status.NodeName = "tefwegwrerg"
vmi.Status.SelinuxContext = "system_u:object_r:container_file_t:s0:c1,c2"
vmi.ObjectMeta.Labels = make(map[string]string)
// This would be set by mutation webhook
vmi.Status.RuntimeUser = 107
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,15 @@ var CRDsValidation map[string]string = map[string]string{
true. Defaults to 800
format: int64
type: integer
disableSELinuxLevelOnTarget:
description: By default, the SELinux level of target virt-launcher
pods is forced to the level of the source virt-launcher. When
set to true, DisableSELinuxLevelOnTarget lets the CRI auto-assign
a random level to the target. That will ensure the target virt-launcher
doesn't share categories with another pod on the node. However,
migrations will fail when using RWX storage that doesn't automatically
deal with SELinux levels.
type: boolean
disableTLS:
description: When set to true, DisableTLS will disable the additional
layer of live migration encryption provided by KubeVirt. This
Expand Down Expand Up @@ -11201,6 +11210,15 @@ var CRDsValidation map[string]string = map[string]string{
true. Defaults to 800
format: int64
type: integer
disableSELinuxLevelOnTarget:
description: By default, the SELinux level of target virt-launcher
pods is forced to the level of the source virt-launcher. When
set to true, DisableSELinuxLevelOnTarget lets the CRI auto-assign
a random level to the target. That will ensure the target virt-launcher
doesn't share categories with another pod on the node. However,
migrations will fail when using RWX storage that doesn't automatically
deal with SELinux levels.
type: boolean
disableTLS:
description: When set to true, DisableTLS will disable the additional
layer of live migration encryption provided by KubeVirt. This
Expand Down Expand Up @@ -11580,6 +11598,15 @@ var CRDsValidation map[string]string = map[string]string{
true. Defaults to 800
format: int64
type: integer
disableSELinuxLevelOnTarget:
description: By default, the SELinux level of target virt-launcher
pods is forced to the level of the source virt-launcher. When
set to true, DisableSELinuxLevelOnTarget lets the CRI auto-assign
a random level to the target. That will ensure the target virt-launcher
doesn't share categories with another pod on the node. However,
migrations will fail when using RWX storage that doesn't automatically
deal with SELinux levels.
type: boolean
disableTLS:
description: When set to true, DisableTLS will disable the additional
layer of live migration encryption provided by KubeVirt. This
Expand Down
5 changes: 5 additions & 0 deletions staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions staging/src/kubevirt.io/api/core/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2456,6 +2456,11 @@ type MigrationConfiguration struct {
// Network is the name of the CNI network to use for live migrations. By default, migrations go
// through the pod network.
Network *string `json:"network,omitempty"`
// By default, the SELinux level of target virt-launcher pods is forced to the level of the source virt-launcher.
// When set to true, DisableSELinuxLevelOnTarget lets the CRI auto-assign a random level to the target.
// That will ensure the target virt-launcher doesn't share categories with another pod on the node.
// However, migrations will fail when using RWX storage that doesn't automatically deal with SELinux levels.
DisableSELinuxLevelOnTarget *bool `json:"disableSELinuxLevelOnTarget,omitempty"`
}

// DiskVerification holds container disks verification limits
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions staging/src/kubevirt.io/client-go/api/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions tests/security_features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,56 @@ var _ = Describe("[Serial][sig-compute]SecurityFeatures", Serial, decorators.Sig
}, 30*time.Second, 10*time.Second).Should(BeNil())
})
})
Context("The VMI SELinux context status", func() {
It("Should get set and stay the the same after a migration", decorators.RequiresTwoSchedulableNodes, func() {
vmi := libvmi.NewAlpine(libvmi.WithMasqueradeNetworking()...)

By("Starting a New VMI")
vmi, err = virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(nil)).Create(context.Background(), vmi)
Expect(err).NotTo(HaveOccurred())
libwait.WaitForSuccessfulVMIStart(vmi)

By("Ensuring VMI is running by logging in")
libwait.WaitUntilVMIReady(vmi, console.LoginToAlpine)

By("Ensuring the VMI SELinux context status gets set")
seContext := ""
Eventually(func() string {
vmi, err = virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(nil)).Get(context.Background(), vmi.Name, &metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
seContext = vmi.Status.SelinuxContext
return seContext
}, 30*time.Second, 10*time.Second).ShouldNot(BeEmpty(), "VMI SELinux context status never got set")

By("Ensuring the VMI SELinux context status matches the virt-launcher pod files")
stdout := tests.RunCommandOnVmiPod(vmi, []string{"ls", "-lZd", "/"})
Expect(stdout).To(ContainSubstring(seContext))

By("Migrating the VMI")
migration := tests.NewRandomMigration(vmi.Name, vmi.Namespace)
tests.RunMigrationAndExpectCompletion(virtClient, migration, tests.MigrationWaitTime)

By("Ensuring the VMI SELinux context status didn't change")
vmi, err = virtClient.VirtualMachineInstance(testsuite.GetTestNamespace(nil)).Get(context.Background(), vmi.Name, &metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(vmi.Status.SelinuxContext).To(Equal(seContext))

By("Fetching virt-launcher Pod")
pod, err := virtClient.CoreV1().Pods(vmi.Namespace).Get(context.Background(), vmi.Status.MigrationState.TargetPod, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

By("Ensuring the right SELinux context is set on the target pod")
Expect(pod.Spec.SecurityContext).NotTo(BeNil())
Expect(pod.Spec.SecurityContext.SELinuxOptions).NotTo(BeNil(), fmt.Sprintf("%#v", pod.Spec.SecurityContext))
ctx := strings.Split(seContext, ":")
Expect(ctx).To(HaveLen(5))
Expect(pod.Spec.SecurityContext.SELinuxOptions.Level).To(Equal(strings.Join(ctx[3:], ":")))

By("Ensuring the target virt-launcher has the same SELinux context as the source")
stdout = tests.RunCommandOnVmiPod(vmi, []string{"ls", "-lZd", "/"})
Expect(stdout).To(ContainSubstring(seContext))
})
})
})

func runOnAllSchedulableNodes(virtClient kubecli.KubevirtClient, command []string, forbiddenString string) error {
Expand Down

0 comments on commit 6314e05

Please sign in to comment.