Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconstruct SELinux mount label #113596

Merged
merged 5 commits into from Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/kubelet/volumemanager/cache/desired_state_of_world.go
Expand Up @@ -290,7 +290,7 @@ func (dsw *desiredStateOfWorld) AddPodToVolume(
if err != nil {
return "", err
}
klog.V(4).InfoS("volume final SELinux label decided", "volume", volumeSpec.Name(), "label", seLinuxFileLabel)
klog.V(4).InfoS("expected volume SELinux label context", "volume", volumeSpec.Name(), "label", seLinuxFileLabel)

if vol, volumeExists := dsw.volumesToMount[volumeName]; !volumeExists {
var sizeLimit *resource.Quantity
Expand All @@ -309,6 +309,7 @@ func (dsw *desiredStateOfWorld) AddPodToVolume(
}
if !util.VolumeSupportsSELinuxMount(volumeSpec) {
// Clear SELinux label for the volume with unsupported access modes.
klog.V(4).InfoS("volume does not support SELinux context mount, clearing the expected label", "volume", volumeSpec.Name())
seLinuxFileLabel = ""
}
if seLinuxFileLabel != "" {
Expand Down
1 change: 0 additions & 1 deletion pkg/kubelet/volumemanager/reconciler/reconstruct.go
Expand Up @@ -160,7 +160,6 @@ func (rc *reconciler) updateStates(volumesNeedUpdate map[v1.UniqueVolumeName]*gl
klog.ErrorS(err, "Could not find device mount path for volume", "volumeName", gvl.volumeName)
continue
}
// TODO(jsafrane): add reconstructed SELinux context
err = rc.actualStateOfWorld.MarkDeviceAsMounted(gvl.volumeName, gvl.devicePath, deviceMountPath, "")
if err != nil {
klog.ErrorS(err, "Could not mark device is mounted to actual state of world", "volume", gvl.volumeName)
Expand Down
11 changes: 8 additions & 3 deletions pkg/kubelet/volumemanager/reconciler/reconstruct_common.go
Expand Up @@ -56,6 +56,7 @@ type reconstructedVolume struct {
mounter volumepkg.Mounter
deviceMounter volumepkg.DeviceMounter
blockVolumeMapper volumepkg.BlockVolumeMapper
seLinuxMountContext string
}

// globalVolumeInfo stores reconstructed volume information
Expand Down Expand Up @@ -211,6 +212,9 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
return nil, err
}
volumeSpec := reconstructed.Spec
if volumeSpec == nil {
return nil, fmt.Errorf("failed to reconstruct volume for plugin %q (spec.Name: %q) pod %q (UID: %q): got nil", volume.pluginName, volume.volumeSpecName, volume.podName, pod.UID)
}

// We have to find the plugins by volume spec (NOT by plugin name) here
// in order to correctly reconstruct ephemeral volume types.
Expand Down Expand Up @@ -312,9 +316,10 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
volumeGidValue: "",
// devicePath is updated during updateStates() by checking node status's VolumesAttached data.
// TODO: get device path directly from the volume mount path.
devicePath: "",
mounter: volumeMounter,
blockVolumeMapper: volumeMapper,
devicePath: "",
mounter: volumeMounter,
blockVolumeMapper: volumeMapper,
seLinuxMountContext: reconstructed.SELinuxMountContext,
}
return reconstructedVolume, nil
}
7 changes: 5 additions & 2 deletions pkg/kubelet/volumemanager/reconciler/reconstruct_new.go
Expand Up @@ -112,6 +112,7 @@ func (rc *reconciler) updateStatesNew(reconstructedVolumes map[v1.UniqueVolumeNa
klog.ErrorS(err, "Could not add volume information to actual state of world", "volumeName", gvl.volumeName)
continue
}
var seLinuxMountContext string
for _, volume := range gvl.podVolumes {
markVolumeOpts := operationexecutor.MarkVolumeOpts{
PodName: volume.podName,
Expand All @@ -123,14 +124,16 @@ func (rc *reconciler) updateStatesNew(reconstructedVolumes map[v1.UniqueVolumeNa
VolumeGidVolume: volume.volumeGidValue,
VolumeSpec: volume.volumeSpec,
VolumeMountState: operationexecutor.VolumeMountUncertain,
SELinuxMountContext: volume.seLinuxMountContext,
}

_, err = rc.actualStateOfWorld.CheckAndMarkVolumeAsUncertainViaReconstruction(markVolumeOpts)
if err != nil {
klog.ErrorS(err, "Could not add pod to volume information to actual state of world", "pod", klog.KObj(volume.pod))
continue
}
klog.V(2).InfoS("Volume is marked as uncertain and added into the actual state", "pod", klog.KObj(volume.pod), "podName", volume.podName, "volumeName", volume.volumeName)
seLinuxMountContext = volume.seLinuxMountContext
klog.V(2).InfoS("Volume is marked as uncertain and added into the actual state", "pod", klog.KObj(volume.pod), "podName", volume.podName, "volumeName", volume.volumeName, "seLinuxMountContext", volume.seLinuxMountContext)
}
// If the volume has device to mount, we mark its device as uncertain.
if gvl.deviceMounter != nil || gvl.blockVolumeMapper != nil {
Expand All @@ -139,7 +142,7 @@ func (rc *reconciler) updateStatesNew(reconstructedVolumes map[v1.UniqueVolumeNa
klog.ErrorS(err, "Could not find device mount path for volume", "volumeName", gvl.volumeName)
continue
}
err = rc.actualStateOfWorld.MarkDeviceAsUncertain(gvl.volumeName, gvl.devicePath, deviceMountPath, "")
err = rc.actualStateOfWorld.MarkDeviceAsUncertain(gvl.volumeName, gvl.devicePath, deviceMountPath, seLinuxMountContext)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have tests that verify if selinux information is recorded as expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit test added

klog.ErrorS(err, "Could not mark device is uncertain to actual state of world", "volumeName", gvl.volumeName, "deviceMountPath", deviceMountPath)
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to verify(and I forgot some details), but is staging (ie. MountDevice) and Setup create different json files or they create same files? Are we consistently saving selinux context in both places? Is that feature gated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They create different jsons and I think SELinux is only in the SetUp one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it other way around - only MountDevice seems to be setting - volDataKey.seLinuxMountContext: deviceMounterArgs.SELinuxLabel, and nothing I could see in Setup. IMO that also should be feature gated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction: it was in MountDevice, I added it to both + both feature-gated.

Expand Down
3 changes: 2 additions & 1 deletion pkg/volume/csi/csi_mounter_test.go
Expand Up @@ -60,7 +60,7 @@ var (
testAccount = "test-service-account"
)

func prepareVolumeInfoFile(mountPath string, plug *csiPlugin, specVolumeName, volumeID, driverName, lifecycleMode string) error {
func prepareVolumeInfoFile(mountPath string, plug *csiPlugin, specVolumeName, volumeID, driverName, lifecycleMode, seLinuxMountContext string) error {
nodeName := string(plug.host.GetNodeName())
volData := map[string]string{
volDataKey.specVolID: specVolumeName,
Expand All @@ -69,6 +69,7 @@ func prepareVolumeInfoFile(mountPath string, plug *csiPlugin, specVolumeName, vo
volDataKey.nodeName: nodeName,
volDataKey.attachmentID: getAttachmentName(volumeID, driverName, nodeName),
volDataKey.volumeLifecycleMode: lifecycleMode,
volDataKey.seLinuxMountContext: seLinuxMountContext,
}
if err := os.MkdirAll(mountPath, 0755); err != nil {
return fmt.Errorf("failed to create dir for volume info file: %s", err)
Expand Down
17 changes: 9 additions & 8 deletions pkg/volume/csi/csi_plugin.go
Expand Up @@ -455,22 +455,23 @@ func (p *csiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.Re
if err != nil {
return volume.ReconstructedVolume{}, errors.New(log("plugin.ConstructVolumeSpec failed loading volume data using [%s]: %v", mountPath, err))
}

klog.V(4).Info(log("plugin.ConstructVolumeSpec extracted [%#v]", volData))

var spec *volume.Spec
var ret volume.ReconstructedVolume
if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) {
ret.SELinuxMountContext = volData[volDataKey.seLinuxMountContext]
}

// If mode is VolumeLifecycleEphemeral, use constructVolSourceSpec
// to construct volume source spec. If mode is VolumeLifecyclePersistent,
// use constructPVSourceSpec to construct volume construct pv source spec.
if storage.VolumeLifecycleMode(volData[volDataKey.volumeLifecycleMode]) == storage.VolumeLifecycleEphemeral {
spec = p.constructVolSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName])
return volume.ReconstructedVolume{Spec: spec}, nil
ret.Spec = p.constructVolSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName])
return ret, nil
}

spec = p.constructPVSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName], volData[volDataKey.volHandle])
return volume.ReconstructedVolume{
Spec: spec,
}, nil
ret.Spec = p.constructPVSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName], volData[volDataKey.volHandle])
return ret, nil
}

// constructVolSourceSpec constructs volume.Spec with CSIVolumeSource
Expand Down
54 changes: 44 additions & 10 deletions pkg/volume/csi/csi_plugin_test.go
Expand Up @@ -29,9 +29,12 @@ import (
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
fakeclient "k8s.io/client-go/kubernetes/fake"
utiltesting "k8s.io/client-go/util/testing"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/volume"
volumetest "k8s.io/kubernetes/pkg/volume/testing"
)
Expand Down Expand Up @@ -316,16 +319,20 @@ func TestPluginConstructVolumeSpec(t *testing.T) {
defer os.RemoveAll(tmpDir)

testCases := []struct {
name string
originSpec *volume.Spec
specVolID string
volHandle string
podUID types.UID
name string
seLinuxMountEnabled bool
originSpec *volume.Spec
originSELinuxMountContext string
specVolID string
volHandle string
expectedSELinuxContext string
podUID types.UID
}{
{
name: "construct spec1 from original persistent spec",
specVolID: "test.vol.id",
volHandle: "testvol-handle1",
name: "construct spec1 from original persistent spec",
specVolID: "test.vol.id",
volHandle: "testvol-handle1",

originSpec: volume.NewSpecFromPersistentVolume(makeTestPV("test.vol.id", 20, testDriver, "testvol-handle1"), true),
podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
},
Expand All @@ -336,12 +343,35 @@ func TestPluginConstructVolumeSpec(t *testing.T) {
originSpec: volume.NewSpecFromPersistentVolume(makeTestPV("spec2", 20, testDriver, "handle2"), true),
podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
},
{
name: "construct SELinux context from original persistent spec when the feature is enabled",
seLinuxMountEnabled: true,
specVolID: "spec3",
volHandle: "handle3",
originSELinuxMountContext: "system_u:object_r:container_file_t:s0:c314,c894",
originSpec: volume.NewSpecFromPersistentVolume(makeTestPV("spec3", 20, testDriver, "handle3"), true),
expectedSELinuxContext: "system_u:object_r:container_file_t:s0:c314,c894",
podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
},
{
name: "construct empty SELinux from original persistent spec when the feature is disabled",
seLinuxMountEnabled: false,
specVolID: "spec4",
volHandle: "handle4",
originSELinuxMountContext: "system_u:object_r:container_file_t:s0:c314,c894",
originSpec: volume.NewSpecFromPersistentVolume(makeTestPV("spec4", 20, testDriver, "handle4"), true),
expectedSELinuxContext: "", // The context is cleared when the feature gate is off
podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
},
}

registerFakePlugin(testDriver, "endpoint", []string{"1.0.0"}, t)

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ReadWriteOncePod, tc.seLinuxMountEnabled)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SELinuxMountReadWriteOncePod, tc.seLinuxMountEnabled)()

mounter, err := plug.NewMounter(
tc.originSpec,
&api.Pod{ObjectMeta: meta.ObjectMeta{UID: tc.podUID, Namespace: testns}},
Expand All @@ -356,7 +386,7 @@ func TestPluginConstructVolumeSpec(t *testing.T) {
csiMounter := mounter.(*csiMountMgr)

mountPath := filepath.Dir(csiMounter.GetPath())
err = prepareVolumeInfoFile(mountPath, plug, tc.originSpec.Name(), csiMounter.volumeID, testDriver, string(csiMounter.volumeLifecycleMode))
err = prepareVolumeInfoFile(mountPath, plug, tc.originSpec.Name(), csiMounter.volumeID, testDriver, string(csiMounter.volumeLifecycleMode), tc.originSELinuxMountContext)
if err != nil {
t.Fatalf("failed to save fake volume info file: %s", err)
}
Expand Down Expand Up @@ -395,6 +425,10 @@ func TestPluginConstructVolumeSpec(t *testing.T) {
if rec.Spec.Name() != tc.specVolID {
t.Errorf("Unexpected spec name constructed %s", rec.Spec.Name())
}

if rec.SELinuxMountContext != tc.expectedSELinuxContext {
t.Errorf("Expected SELinux context %q, got %q", tc.expectedSELinuxContext, rec.SELinuxMountContext)
}
})
}
}
Expand Down Expand Up @@ -490,7 +524,7 @@ func TestPluginConstructVolumeSpecWithInline(t *testing.T) {
csiMounter := mounter.(*csiMountMgr)

mountPath := filepath.Dir(csiMounter.GetPath())
err = prepareVolumeInfoFile(mountPath, plug, tc.originSpec.Name(), csiMounter.volumeID, testDriver, string(csiMounter.volumeLifecycleMode))
err = prepareVolumeInfoFile(mountPath, plug, tc.originSpec.Name(), csiMounter.volumeID, testDriver, string(csiMounter.volumeLifecycleMode), "")
if err != nil {
t.Fatalf("failed to save fake volume info file: %s", err)
}
Expand Down
17 changes: 16 additions & 1 deletion pkg/volume/fc/fc.go
Expand Up @@ -283,10 +283,25 @@ func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volum
FC: &v1.FCVolumeSource{WWIDs: wwids, Lun: &lun, TargetWWNs: wwns},
},
}

var mountContext string
if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) {
kvh, ok := plugin.host.(volume.KubeletVolumeHost)
if !ok {
return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface")
}
hu := kvh.GetHostUtil()
mountContext, err = hu.GetSELinuxMountContext(mountPath)
if err != nil {
return volume.ReconstructedVolume{}, err
}
}

klog.V(5).Infof("ConstructVolumeSpec: TargetWWNs: %v, Lun: %v, WWIDs: %v",
fcVolume.VolumeSource.FC.TargetWWNs, *fcVolume.VolumeSource.FC.Lun, fcVolume.VolumeSource.FC.WWIDs)
return volume.ReconstructedVolume{
Spec: volume.NewSpecFromVolume(fcVolume),
Spec: volume.NewSpecFromVolume(fcVolume),
SELinuxMountContext: mountContext,
}, nil
}

Expand Down
17 changes: 16 additions & 1 deletion pkg/volume/iscsi/iscsi.go
Expand Up @@ -279,8 +279,23 @@ func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (vo
},
},
}

var mountContext string
if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) {
kvh, ok := plugin.host.(volume.KubeletVolumeHost)
if !ok {
return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface")
}
hu := kvh.GetHostUtil()
mountContext, err = hu.GetSELinuxMountContext(mountPath)
if err != nil {
return volume.ReconstructedVolume{}, err
}
}

return volume.ReconstructedVolume{
Spec: volume.NewSpecFromVolume(iscsiVolume),
Spec: volume.NewSpecFromVolume(iscsiVolume),
SELinuxMountContext: mountContext,
}, nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/volume/plugins.go
Expand Up @@ -573,7 +573,11 @@ type VolumeConfig struct {
// ReconstructedVolume contains information about a volume reconstructed by
// ConstructVolumeSpec().
type ReconstructedVolume struct {
// Spec is the volume spec of a mounted volume
Spec *Spec
// SELinuxMountContext is value of -o context=XYZ mount option.
// If empty, no such mount option is used.
SELinuxMountContext string
}

// NewSpecFromVolume creates an Spec from an v1.Volume
Expand Down
12 changes: 11 additions & 1 deletion pkg/volume/rbd/rbd.go
Expand Up @@ -430,8 +430,18 @@ func (plugin *rbdPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volu
},
},
}

var mountContext string
if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) {
mountContext, err = hu.GetSELinuxMountContext(mountPath)
if err != nil {
return volume.ReconstructedVolume{}, err
}
}

return volume.ReconstructedVolume{
Spec: volume.NewSpecFromVolume(rbdVolume),
Spec: volume.NewSpecFromVolume(rbdVolume),
SELinuxMountContext: mountContext,
}, nil
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/volume/util/hostutil/fake_hostutil.go
Expand Up @@ -116,3 +116,9 @@ func (hu *FakeHostUtil) GetSELinuxSupport(pathname string) (bool, error) {
func (hu *FakeHostUtil) GetMode(pathname string) (os.FileMode, error) {
return 0, errors.New("not implemented")
}

// GetSELinuxMountContext returns value of -o context=XYZ mount option on
// given mount point.
func (hu *FakeHostUtil) GetSELinuxMountContext(pathname string) (string, error) {
return "", errors.New("not implemented")
}
3 changes: 3 additions & 0 deletions pkg/volume/util/hostutil/hostutil.go
Expand Up @@ -68,6 +68,9 @@ type HostUtils interface {
GetSELinuxSupport(pathname string) (bool, error)
// GetMode returns permissions of the path.
GetMode(pathname string) (os.FileMode, error)
// GetSELinuxMountContext returns value of -o context=XYZ mount option on
// given mount point.
GetSELinuxMountContext(pathname string) (string, error)
}

// Compile-time check to ensure all HostUtil implementations satisfy
Expand Down
32 changes: 32 additions & 0 deletions pkg/volume/util/hostutil/hostutil_linux.go
Expand Up @@ -299,3 +299,35 @@ func GetModeLinux(pathname string) (os.FileMode, error) {
}
return info.Mode(), nil
}

// GetSELinuxMountContext returns value of -o context=XYZ mount option on
// given mount point.
func (hu *HostUtil) GetSELinuxMountContext(pathname string) (string, error) {
return getSELinuxMountContext(pathname, procMountInfoPath, selinux.GetEnabled)
}

// getSELinux is common implementation of GetSELinuxSupport on Linux.
// Using an extra function for unit tests.
func getSELinuxMountContext(path string, mountInfoFilename string, selinuxEnabled seLinuxEnabledFunc) (string, error) {
// Skip /proc/mounts parsing if SELinux is disabled.
if !selinuxEnabled() {
return "", nil
}

info, err := findMountInfo(path, mountInfoFilename)
if err != nil {
return "", err
}

for _, opt := range info.SuperOptions {
if !strings.HasPrefix(opt, "context=") {
continue
}
// Remove context=
context := strings.TrimPrefix(opt, "context=")
// Remove double quotes
context = strings.Trim(context, "\"")
return context, nil
}
return "", nil
}