Skip to content

Commit

Permalink
Merge pull request kubevirt#9246 from jean-edouard/catsontarget
Browse files Browse the repository at this point in the history
migration: match SELinux level of source pod on target pod
  • Loading branch information
kubevirt-bot authored Jun 6, 2023
2 parents 2cfd09e + 8c11802 commit 28a3e16
Show file tree
Hide file tree
Showing 11 changed files with 213 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 @@ -17980,6 +17980,10 @@
"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"
},
"matchSELinuxLevelOnMigration": {
"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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically deal with SELinux levels.",
"type": "boolean"
},
"network": {
"description": "Network is the name of the CNI network to use for live migrations. By default, migrations go through the pod network.",
"type": "string"
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 @@ -542,6 +542,16 @@ spec:
additional layer of live migration encryption provided by
KubeVirt. This is usually a bad idea. Defaults to false
type: boolean
matchSELinuxLevelOnMigration:
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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically deal with
SELinux levels.
type: boolean
network:
description: Network is the name of the CNI network to use
for live migrations. By default, migrations go through the
Expand Down Expand Up @@ -3520,6 +3530,16 @@ spec:
additional layer of live migration encryption provided by
KubeVirt. This is usually a bad idea. Defaults to false
type: boolean
matchSELinuxLevelOnMigration:
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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically deal with
SELinux levels.
type: boolean
network:
description: Network is the name of the CNI network to use
for live migrations. By default, migrations go through the
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 @@ -68,6 +68,7 @@ go_library(
"//vendor/github.com/emicklei/go-restful/v3: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
41 changes: 41 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 @@ -598,6 +600,37 @@ func (c *MigrationController) updateStatus(migration *virtv1.VirtualMachineInsta
return nil
}

func setTargetPodSELinuxLevel(pod *k8sv1.Pod, vmiSeContext string) error {
// 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 vmiSeContext == "none" {
// The SelinuxContext is explicitly set to "none" when SELinux is not present
return nil
}
if vmiSeContext == "" {
return fmt.Errorf("SELinux context not set on VMI status")
} else {
seContext, err := selinux.NewContext(vmiSeContext)
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 pod.Spec.SecurityContext == nil {
pod.Spec.SecurityContext = &k8sv1.PodSecurityContext{}
}
pod.Spec.SecurityContext.SELinuxOptions = &k8sv1.SELinuxOptions{
Level: level,
}
}
}

return nil
}

func (c *MigrationController) createTargetPod(migration *virtv1.VirtualMachineInstanceMigration, vmi *virtv1.VirtualMachineInstance, sourcePod *k8sv1.Pod) error {
templatePod, err := c.templateService.RenderMigrationManifest(vmi, sourcePod)
if err != nil {
Expand Down Expand Up @@ -643,6 +676,14 @@ func (c *MigrationController) createTargetPod(migration *virtv1.VirtualMachineIn
}
}

matchLevelOnTarget := c.clusterConfig.GetMigrationConfiguration().MatchSELinuxLevelOnMigration
if matchLevelOnTarget == nil || *matchLevelOnTarget {
err = setTargetPodSELinuxLevel(templatePod, vmi.Status.SelinuxContext)
if err != nil {
return err
}
}

// This is used by the functional test to simulate failures
computeImageOverride, ok := migration.Annotations[virtv1.FuncTestMigrationTargetImageOverrideAnnotation]
if ok && computeImageOverride != "" {
Expand Down
53 changes: 52 additions & 1 deletion pkg/virt-controller/watch/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,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 @@ -1838,6 +1837,57 @@ var _ = Describe("Migration watcher", func() {
shouldExpectPodCreation(vmi.UID, pendingMigration.UID, 1, 0, 0)
})
})

Context("Migration target SELinux level", func() {
shouldExpectTargetPodWithSELinuxLevel := func(level string) {
// Expect pod creation
kubeClient.Fake.PrependReactor("create", "pods", func(action testing.Action) (handled bool, obj k8sruntime.Object, err error) {
update, ok := action.(testing.CreateAction)
Expect(ok).To(BeTrue())
pod := update.GetObject().(*k8sv1.Pod)
if level != "" {
Expect(pod.Spec.SecurityContext.SELinuxOptions.Level).To(Equal(level))
} else {
if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.SELinuxOptions != nil {
Expect(pod.Spec.SecurityContext.SELinuxOptions.Level).To(BeEmpty())
}
}

return true, update.GetObject(), nil
})
}

It("should be forced to the SELinux level of the source by default", func() {
vmi := newVirtualMachine("testvmi", virtv1.Running)
vmi.Status.SelinuxContext = "system_u:system_r:container_file_t:s0:c1,c2"
migration := newMigration("testmigration", vmi.Name, virtv1.MigrationPending)

addMigration(migration)
addVirtualMachineInstance(vmi)
shouldExpectTargetPodWithSELinuxLevel("s0:c1,c2")

controller.Execute()
testutils.ExpectEvents(recorder, SuccessfulCreatePodReason)
})

It("should not be forced to the SELinux level of the source if the CR option is set to false", func() {
initController(&virtv1.KubeVirtConfiguration{
MigrationConfiguration: &virtv1.MigrationConfiguration{
MatchSELinuxLevelOnMigration: pointer.BoolPtr(false),
},
})
vmi := newVirtualMachine("testvmi", virtv1.Running)
vmi.Status.SelinuxContext = "system_u:system_r:container_file_t:s0:c1,c2"
migration := newMigration("testmigration", vmi.Name, virtv1.MigrationPending)

addMigration(migration)
addVirtualMachineInstance(vmi)
shouldExpectTargetPodWithSELinuxLevel("")

controller.Execute()
testutils.ExpectEvents(recorder, SuccessfulCreatePodReason)
})
})
})

func newPDB(name string, vmi *virtv1.VirtualMachineInstance, pods int) *policyv1.PodDisruptionBudget {
Expand Down Expand Up @@ -1892,6 +1942,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 @@ -1073,6 +1073,15 @@ var CRDsValidation map[string]string = map[string]string{
layer of live migration encryption provided by KubeVirt. This
is usually a bad idea. Defaults to false
type: boolean
matchSELinuxLevelOnMigration:
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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically
deal with SELinux levels.
type: boolean
network:
description: Network is the name of the CNI network to use for live
migrations. By default, migrations go through the pod network.
Expand Down Expand Up @@ -11587,6 +11596,15 @@ var CRDsValidation map[string]string = map[string]string{
layer of live migration encryption provided by KubeVirt. This
is usually a bad idea. Defaults to false
type: boolean
matchSELinuxLevelOnMigration:
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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically
deal with SELinux levels.
type: boolean
network:
description: Network is the name of the CNI network to use for live
migrations. By default, migrations go through the pod network.
Expand Down Expand Up @@ -11966,6 +11984,15 @@ var CRDsValidation map[string]string = map[string]string{
layer of live migration encryption provided by KubeVirt. This
is usually a bad idea. Defaults to false
type: boolean
matchSELinuxLevelOnMigration:
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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically
deal with SELinux levels.
type: boolean
network:
description: Network is the name of the CNI network to use for live
migrations. By default, migrations go through the pod network.
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 @@ -2491,6 +2491,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, MatchSELinuxLevelOnMigration 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 volumes that don't automatically deal with SELinux levels.
MatchSELinuxLevelOnMigration *bool `json:"matchSELinuxLevelOnMigration,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 28a3e16

Please sign in to comment.