Skip to content

Commit 65e47d0

Browse files
jsafranejsafrane
committed
UPSTREAM: 133425: Fix SELinux label comparison
The comparison of SELinux labels in KCM tolerates missing fields - the operating system is going to default them from its defaults, but in KCM we don't know what the defaults are. But the OS won't default the last component, "level", which includes also categories. Make sure that labels with a level set conflicts with level "", that's what will conflict on the OS too. Add also some e2e tests for that
1 parent 44e5bf3 commit 65e47d0

File tree

5 files changed

+147
-24
lines changed

5 files changed

+147
-24
lines changed

‎pkg/controller/volume/selinuxwarning/cache/volumecache_test.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
345345
expectedConflicts: []Conflict{},
346346
},
347347
{
348-
name: "existing volume in a new pod with existing policy and new incomparable label (missing categories)",
348+
name: "existing volume in a new pod with existing policy and new comparable label (missing categories)",
349349
initialPods: existingPods,
350350
podToAdd: podWithVolume{
351351
podNamespace: "testns",
@@ -354,7 +354,16 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
354354
label: "system_u:system_r:label7",
355355
changePolicy: v1.SELinuxChangePolicyMountOption,
356356
},
357-
expectedConflicts: []Conflict{},
357+
expectedConflicts: []Conflict{
358+
{
359+
PropertyName: "SELinuxLabel",
360+
EventReason: "SELinuxLabelConflict",
361+
Pod: cache.ObjectName{Namespace: "testns", Name: "testpod"},
362+
PropertyValue: "system_u:system_r:label7",
363+
OtherPod: cache.ObjectName{Namespace: "ns7", Name: "pod7"},
364+
OtherPropertyValue: "::label7:c0,c1",
365+
},
366+
},
358367
},
359368
{
360369
name: "existing volume in a new pod with existing policy and new incomparable label (missing everything)",
@@ -368,6 +377,27 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) {
368377
},
369378
expectedConflicts: []Conflict{},
370379
},
380+
{
381+
name: "existing volume in a new pod with existing policy and new comparable label (missing everything but categories)",
382+
initialPods: existingPods,
383+
podToAdd: podWithVolume{
384+
podNamespace: "testns",
385+
podName: "testpod",
386+
volumeName: "vol8",
387+
label: "system_u:system_r:label8:c0,c1",
388+
changePolicy: v1.SELinuxChangePolicyMountOption,
389+
},
390+
expectedConflicts: []Conflict{
391+
{
392+
PropertyName: "SELinuxLabel",
393+
EventReason: "SELinuxLabelConflict",
394+
Pod: cache.ObjectName{Namespace: "testns", Name: "testpod"},
395+
PropertyValue: "system_u:system_r:label8:c0,c1",
396+
OtherPod: cache.ObjectName{Namespace: "ns8", Name: "pod8"},
397+
OtherPropertyValue: "",
398+
},
399+
},
400+
},
371401
}
372402
for _, tt := range tests {
373403
t.Run(tt.name, func(t *testing.T) {

‎pkg/controller/volume/selinuxwarning/translator/selinux_translator.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,15 @@ func (c *ControllerSELinuxTranslator) SELinuxOptionsToFileLabel(opts *v1.SELinux
6060
// Conflicts returns true if two SELinux labels conflict.
6161
// These labels must be generated by SELinuxOptionsToFileLabel above
6262
// (the function expects strict nr. of elements in the labels).
63-
// Since this translator cannot default missing components,
64-
// the missing components are treated as incomparable and they do not
65-
// conflict with anything.
63+
// Since this translator cannot default missing label components from the operating system,
64+
// the first three components can be empty. In this case, the empty components don't lead to a
65+
// conflict when compared to a real SELinux label and this function returns false (as no
66+
// conflict can be detected).
67+
// The last component (level) is always compared, as it is not defaulted by the operating system.
6668
// Example: "system_u:system_r:container_t:s0:c1,c2" *does not* conflict with ":::s0:c1,c2",
67-
// because the node that will run such a Pod may expand "":::s0:c1,c2" to "system_u:system_r:container_t:s0:c1,c2".
68-
// However, "system_u:system_r:container_t:s0:c1,c2" *does* conflict with ":::s0:c98,c99".
69+
// because the node that will run such a Pod may expand ":::s0:c1,c2" to "system_u:system_r:container_t:s0:c1,c2".
70+
// However: "system_u:system_r:container_t:s0:c1,c2" *does* conflict with ":::s0:c98,c99".
71+
// And ":::s0:c1,c2" *does* conflict with "" or ":::", because it's never defaulted by the OS.
6972
func (c *ControllerSELinuxTranslator) Conflicts(labelA, labelB string) bool {
7073
partsA := strings.SplitN(labelA, ":", 4)
7174
partsB := strings.SplitN(labelB, ":", 4)
@@ -82,16 +85,20 @@ func (c *ControllerSELinuxTranslator) Conflicts(labelA, labelB string) bool {
8285
if partsA[i] == partsB[i] {
8386
continue
8487
}
88+
if i == 3 {
89+
// The last component must always match
90+
return true
91+
}
92+
// i<3, empty parts are incomparable
8593
if partsA[i] == "" {
86-
// incomparable part, no conflict
8794
continue
8895
}
8996
if partsB[i] == "" {
90-
// incomparable part, no conflict
9197
continue
9298
}
9399
// Parts are not equal and neither of them is "" -> conflict
94100
return true
95101
}
102+
96103
return false
97104
}

‎pkg/controller/volume/selinuxwarning/translator/selinux_translator_test.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,26 +93,32 @@ func TestLabelsConflict(t *testing.T) {
9393
conflict: false,
9494
},
9595
{
96-
name: "empty string don't conflict with anything",
96+
name: "empty strings don't conflict with anything except the level",
9797
a: "",
9898
b: "system_u:system_r:container_t",
9999
conflict: false,
100100
},
101+
{
102+
name: "empty string conflicts with level",
103+
a: "",
104+
b: "system_u:system_r:container_t:s0:c1,c2",
105+
conflict: true,
106+
},
101107
{
102108
name: "empty parts don't conflict with anything",
103-
a: ":::::::::::::",
109+
a: ":::",
104110
b: "system_u:system_r:container_t",
105111
conflict: false,
106112
},
107113
{
108114
name: "different lengths don't conflict if the common parts are the same",
109-
a: "system_u:system_r:container_t:c0,c2",
110-
b: "system_u:system_r:container_t",
115+
a: "system_u:system_r:container_t:",
116+
b: "system_u:system_r",
111117
conflict: false,
112118
},
113119
{
114120
name: "different lengths conflict if the common parts differ",
115-
a: "system_u:system_r:conflict_t:c0,c2",
121+
a: "system_u:system_r:conflict_t:",
116122
b: "system_u:system_r:container_t",
117123
conflict: true,
118124
},
@@ -125,9 +131,15 @@ func TestLabelsConflict(t *testing.T) {
125131
{
126132
name: "non-conflicting empty parts",
127133
a: "system_u::container_t",
128-
b: ":system_r::c0,c2",
134+
b: ":system_r::",
129135
conflict: false,
130136
},
137+
{
138+
name: "empty level conflicts with non-empty level",
139+
a: ":::s0:c1,c2",
140+
b: "",
141+
conflict: true,
142+
},
131143
}
132144
for _, test := range tests {
133145
t.Run(test.name, func(t *testing.T) {

‎test/e2e/storage/csimock/base.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ func (m *mockDriverSetup) createPodWithFSGroup(ctx context.Context, fsGroup *int
463463
return class, claim, pod
464464
}
465465

466-
func (m *mockDriverSetup) createPodWithSELinux(ctx context.Context, accessModes []v1.PersistentVolumeAccessMode, mountOptions []string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) {
466+
func (m *mockDriverSetup) createPodWithSELinux(ctx context.Context, accessModes []v1.PersistentVolumeAccessMode, mountOptions []string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy, privileged bool) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) {
467467
ginkgo.By("Creating pod with SELinux context")
468468
f := m.f
469469
nodeSelection := m.config.ClientNodeSelection
@@ -480,7 +480,7 @@ func (m *mockDriverSetup) createPodWithSELinux(ctx context.Context, accessModes
480480
ReclaimPolicy: m.tp.reclaimPolicy,
481481
}
482482
class, claim := createClaim(ctx, f.ClientSet, scTest, nodeSelection, m.tp.scName, f.Namespace.Name, accessModes)
483-
pod, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, seLinuxOpts, policy)
483+
pod, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, seLinuxOpts, policy, privileged)
484484
framework.ExpectNoError(err, "Failed to create pause pod with SELinux context %s: %v", seLinuxOpts, err)
485485

486486
if class != nil {
@@ -802,7 +802,7 @@ func startBusyBoxPodWithVolumeSource(cs clientset.Interface, volumeSource v1.Vol
802802
return cs.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{})
803803
}
804804

805-
func startPausePodWithSELinuxOptions(cs clientset.Interface, pvc *v1.PersistentVolumeClaim, node e2epod.NodeSelection, ns string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy) (*v1.Pod, error) {
805+
func startPausePodWithSELinuxOptions(cs clientset.Interface, pvc *v1.PersistentVolumeClaim, node e2epod.NodeSelection, ns string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy, privileged bool) (*v1.Pod, error) {
806806
pod := &v1.Pod{
807807
ObjectMeta: metav1.ObjectMeta{
808808
GenerateName: "pvc-volume-tester-",
@@ -816,6 +816,9 @@ func startPausePodWithSELinuxOptions(cs clientset.Interface, pvc *v1.PersistentV
816816
{
817817
Name: "volume-tester",
818818
Image: imageutils.GetE2EImage(imageutils.Pause),
819+
SecurityContext: &v1.SecurityContext{
820+
Privileged: &privileged,
821+
},
819822
VolumeMounts: []v1.VolumeMount{
820823
{
821824
Name: "my-volume",

‎test/e2e/storage/csimock/csi_selinux_mount.go

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount", func() {
298298
// Act
299299
ginkgo.By("Starting the initial pod")
300300
accessModes := []v1.PersistentVolumeAccessMode{t.volumeMode}
301-
_, claim, pod := m.createPodWithSELinux(ctx, accessModes, t.mountOptions, t.firstPodSELinuxOpts, t.firstPodChangePolicy)
301+
_, claim, pod := m.createPodWithSELinux(ctx, accessModes, t.mountOptions, t.firstPodSELinuxOpts, t.firstPodChangePolicy, false /* privileged */)
302302
err := e2epod.WaitForPodNameRunningInNamespace(ctx, m.cs, pod.Name, pod.Namespace)
303303
framework.ExpectNoError(err, "starting the initial pod")
304304

@@ -331,7 +331,7 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount", func() {
331331
pod, err = m.cs.CoreV1().Pods(pod.Namespace).Get(ctx, pod.Name, metav1.GetOptions{})
332332
framework.ExpectNoError(err, "getting the initial pod")
333333
nodeSelection := e2epod.NodeSelection{Name: pod.Spec.NodeName}
334-
pod2, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, t.secondPodSELinuxOpts, t.secondPodChangePolicy)
334+
pod2, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, t.secondPodSELinuxOpts, t.secondPodChangePolicy, false /* privileged */)
335335
framework.ExpectNoError(err, "creating second pod with SELinux context %s", t.secondPodSELinuxOpts)
336336
m.pods = append(m.pods, pod2)
337337

@@ -453,8 +453,10 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount metrics and SELinuxWarningC
453453
csiDriverSELinuxEnabled bool
454454
firstPodSELinuxOpts *v1.SELinuxOptions
455455
firstPodChangePolicy *v1.PodSELinuxChangePolicy
456+
firstPodPrivileged bool
456457
secondPodSELinuxOpts *v1.SELinuxOptions
457458
secondPodChangePolicy *v1.PodSELinuxChangePolicy
459+
secondPodPrivileged bool
458460
volumeMode v1.PersistentVolumeAccessMode
459461
waitForSecondPodStart bool
460462
secondPodFailureEvent string
@@ -599,7 +601,7 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount metrics and SELinuxWarningC
599601
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
600602
},
601603
{
602-
name: "error is not bumped on two Pods with a different policy RWX volume (nil + MountOption)",
604+
name: "error is not bumped on two Pods with the same policy RWX volume (nil + MountOption)",
603605
csiDriverSELinuxEnabled: true,
604606
firstPodSELinuxOpts: &seLinuxOpts1,
605607
firstPodChangePolicy: &mount,
@@ -611,7 +613,7 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount metrics and SELinuxWarningC
611613
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
612614
},
613615
{
614-
name: "error is not bumped on two Pods with a different policy RWX volume (MountOption + MountOption)",
616+
name: "error is not bumped on two Pods with the same policy RWX volume (MountOption + MountOption)",
615617
csiDriverSELinuxEnabled: true,
616618
firstPodSELinuxOpts: &seLinuxOpts1,
617619
firstPodChangePolicy: &mount,
@@ -648,6 +650,75 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount metrics and SELinuxWarningC
648650
expectControllerConflictProperty: "SELinuxLabel",
649651
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
650652
},
653+
{
654+
name: "error is not bumped on two privileged Pods with mount policy RWO volume",
655+
csiDriverSELinuxEnabled: true,
656+
firstPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
657+
firstPodPrivileged: true,
658+
firstPodChangePolicy: &recursive,
659+
secondPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
660+
secondPodPrivileged: true,
661+
secondPodChangePolicy: &recursive,
662+
volumeMode: v1.ReadWriteOnce,
663+
waitForSecondPodStart: true,
664+
expectNodeIncreases: sets.New[string]( /* no metric is increased, admitted_total was already increased when the first pod started */ ),
665+
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
666+
},
667+
{
668+
name: "error is not bumped on two privileged Pods with recursive policy RWO volume",
669+
csiDriverSELinuxEnabled: true,
670+
firstPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
671+
firstPodPrivileged: true,
672+
firstPodChangePolicy: &mount,
673+
secondPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
674+
secondPodPrivileged: true,
675+
secondPodChangePolicy: &mount,
676+
volumeMode: v1.ReadWriteOnce,
677+
waitForSecondPodStart: true,
678+
expectNodeIncreases: sets.New[string]( /* no metric is increased, admitted_total was already increased when the first pod started */ ),
679+
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
680+
},
681+
{
682+
name: "error is not bumped on a privileged and unprivileged Pod with given SELinux context and recursive policy",
683+
csiDriverSELinuxEnabled: true,
684+
firstPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
685+
firstPodPrivileged: true,
686+
secondPodSELinuxOpts: &seLinuxOpts1,
687+
secondPodChangePolicy: &recursive,
688+
secondPodPrivileged: false,
689+
volumeMode: v1.ReadWriteMany,
690+
waitForSecondPodStart: true,
691+
expectNodeIncreases: sets.New[string]( /* no metric is increased, admitted_total was already increased when the first pod started */ ),
692+
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
693+
},
694+
{
695+
name: "error is bumped on a privileged and unprivileged Pod with given SELinux with MountOption policy",
696+
csiDriverSELinuxEnabled: true,
697+
firstPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
698+
firstPodPrivileged: true,
699+
secondPodSELinuxOpts: &seLinuxOpts1,
700+
secondPodChangePolicy: &mount,
701+
secondPodFailureEvent: "conflicting SELinux labels of volume",
702+
volumeMode: v1.ReadWriteOncePod,
703+
waitForSecondPodStart: false,
704+
expectNodeIncreases: sets.New[string]("volume_manager_selinux_volume_context_mismatch_errors_total"),
705+
expectControllerConflictProperty: "SELinuxLabel",
706+
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
707+
},
708+
{
709+
name: "error is bumped on an unprivileged and privileged Pod with given SELinux with MountOption policy",
710+
csiDriverSELinuxEnabled: true,
711+
firstPodSELinuxOpts: &seLinuxOpts1,
712+
firstPodChangePolicy: &mount,
713+
secondPodSELinuxOpts: nil, /* privileged Pods are typically without SELinux context */
714+
secondPodPrivileged: true,
715+
secondPodFailureEvent: "conflicting SELinux labels of volume",
716+
volumeMode: v1.ReadWriteOncePod,
717+
waitForSecondPodStart: false,
718+
expectNodeIncreases: sets.New[string]("volume_manager_selinux_volume_context_mismatch_errors_total"),
719+
expectControllerConflictProperty: "SELinuxLabel",
720+
testTags: []interface{}{framework.WithFeatureGate(features.SELinuxMount)},
721+
},
651722
}
652723
for _, t := range tests {
653724
t := t
@@ -673,7 +744,7 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount metrics and SELinuxWarningC
673744

674745
ginkgo.By("Starting the first pod")
675746
accessModes := []v1.PersistentVolumeAccessMode{t.volumeMode}
676-
_, claim, pod := m.createPodWithSELinux(ctx, accessModes, []string{}, t.firstPodSELinuxOpts, t.firstPodChangePolicy)
747+
_, claim, pod := m.createPodWithSELinux(ctx, accessModes, []string{}, t.firstPodSELinuxOpts, t.firstPodChangePolicy, t.firstPodPrivileged)
677748
err = e2epod.WaitForPodNameRunningInNamespace(ctx, m.cs, pod.Name, pod.Namespace)
678749
framework.ExpectNoError(err, "starting the initial pod")
679750

@@ -688,7 +759,7 @@ var _ = utils.SIGDescribe("CSI Mock selinux on mount metrics and SELinuxWarningC
688759
ginkgo.By("Starting the second pod")
689760
// Skip scheduler, it would block scheduling the second pod with ReadWriteOncePod PV.
690761
nodeSelection := e2epod.NodeSelection{Name: pod.Spec.NodeName}
691-
pod2, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, t.secondPodSELinuxOpts, t.secondPodChangePolicy)
762+
pod2, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, t.secondPodSELinuxOpts, t.secondPodChangePolicy, t.secondPodPrivileged)
692763
framework.ExpectNoError(err, "creating second pod with SELinux context %s", t.secondPodSELinuxOpts)
693764
m.pods = append(m.pods, pod2)
694765

0 commit comments

Comments
 (0)