Skip to content

Commit 8c0988a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#133745 from jsafrane/automated-cherry-pick-of-#133425-release-1.34
Automated cherry pick of kubernetes#133425: Fix SELinux label comparison
2 parents 4fa4783 + 3ebbe57 commit 8c0988a

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
@@ -471,7 +471,7 @@ func (m *mockDriverSetup) createPodWithFSGroup(ctx context.Context, fsGroup *int
471471
return class, claim, pod
472472
}
473473

474-
func (m *mockDriverSetup) createPodWithSELinux(ctx context.Context, accessModes []v1.PersistentVolumeAccessMode, mountOptions []string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) {
474+
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) {
475475
ginkgo.By("Creating pod with SELinux context")
476476
f := m.f
477477
nodeSelection := m.config.ClientNodeSelection
@@ -488,7 +488,7 @@ func (m *mockDriverSetup) createPodWithSELinux(ctx context.Context, accessModes
488488
ReclaimPolicy: m.tp.reclaimPolicy,
489489
}
490490
class, claim := createClaim(ctx, f.ClientSet, scTest, nodeSelection, m.tp.scName, f.Namespace.Name, accessModes)
491-
pod, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, seLinuxOpts, policy)
491+
pod, err := startPausePodWithSELinuxOptions(f.ClientSet, claim, nodeSelection, f.Namespace.Name, seLinuxOpts, policy, privileged)
492492
framework.ExpectNoError(err, "Failed to create pause pod with SELinux context %s: %v", seLinuxOpts, err)
493493

494494
if class != nil {
@@ -826,7 +826,7 @@ func startBusyBoxPodWithVolumeSource(cs clientset.Interface, volumeSource v1.Vol
826826
return cs.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{})
827827
}
828828

829-
func startPausePodWithSELinuxOptions(cs clientset.Interface, pvc *v1.PersistentVolumeClaim, node e2epod.NodeSelection, ns string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy) (*v1.Pod, error) {
829+
func startPausePodWithSELinuxOptions(cs clientset.Interface, pvc *v1.PersistentVolumeClaim, node e2epod.NodeSelection, ns string, seLinuxOpts *v1.SELinuxOptions, policy *v1.PodSELinuxChangePolicy, privileged bool) (*v1.Pod, error) {
830830
pod := &v1.Pod{
831831
ObjectMeta: metav1.ObjectMeta{
832832
GenerateName: "pvc-volume-tester-",
@@ -840,6 +840,9 @@ func startPausePodWithSELinuxOptions(cs clientset.Interface, pvc *v1.PersistentV
840840
{
841841
Name: "volume-tester",
842842
Image: imageutils.GetE2EImage(imageutils.Pause),
843+
SecurityContext: &v1.SecurityContext{
844+
Privileged: &privileged,
845+
},
843846
VolumeMounts: []v1.VolumeMount{
844847
{
845848
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)