Skip to content

Commit fe93594

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 fe93594

File tree

6 files changed

+160
-27
lines changed

6 files changed

+160
-27
lines changed

‎openshift-hack/e2e/annotate/generated/zz_generated.annotations.go

Lines changed: 13 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎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",

0 commit comments

Comments
 (0)