Skip to content

Commit 1b675e1

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2417 from jsafrane/4.20-selinux-e2e-driver
OCPBUGS-60946: UPSTREAM: 133425: Fix SELinux label comparison
2 parents a237dcb + fe93594 commit 1b675e1

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)