iscsi: prevent plugin directory path traversal

Project: kubernetes/kubernetes | Language: Go | Concepts: security, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/kubernetes/kubernetes/pull/140258

The problem

The iSCSI volume plugin in Kubernetes accepted API-supplied iSCSI identifiers (like IQN) to construct filesystem paths for storing metadata. If an attacker supplied an IQN containing ../ path traversal segments, the code would create directories or files outside the intended plugin directory, potentially overwriting or exposing sensitive files on the host.

Why it's non-trivial

A naive fix might attempt to sanitize input strings or simply check for .. in the IQN. However, path traversal can be subtle and OS-dependent, especially when path-joining is involved. The real risk is not just in the string, but in the resolved filesystem path after all joins and symlinks are processed. Properly preventing traversal requires validating that the final path is still within the intended base directory before performing any filesystem operations.

The approach

The PR introduces a centralized validation function and applies it at every point where a path derived from user input could be used to create or modify files or directories. It also adds regression tests to ensure the fix is robust.

1. Centralized path validation

A new helper function is added to check that any path is within the plugin directory:

func validateISCSIPluginPath(path, pluginDir string) error {
	if !mount.PathWithinBase(path, pluginDir) {
		return fmt.Errorf(
			"iscsi: resolved path %q escapes plugin directory %q",
			path,
			pluginDir,
		)
	}
	return nil
}

This function uses mount.PathWithinBase to ensure the resolved path is a subpath of the plugin directory. If not, it returns an error.

2. Defensive checks before filesystem operations

This validation is called before any directory is created or file is written. For example, in persistISCSI:

+	var pluginDir string
+	if b.volumeMode == v1.PersistentVolumeBlock {
+		pluginDir = b.iscsiDisk.plugin.host.GetVolumeDevicePluginDir(iscsiPluginName)
+	} else {
+		pluginDir = b.iscsiDisk.plugin.host.GetPluginDir(iscsiPluginName)
+	}
+
+	if err := validateISCSIPluginPath(globalPDPath, pluginDir); err != nil {
+		return err
+	}

This ensures that the path used for persisting metadata is validated before any filesystem operation.

Similarly, validation is added to MountDevice, UnmountDevice, and diskSetUp:

+	pluginDir := attacher.host.GetPluginDir(iscsiPluginName)
+	if err := validateISCSIPluginPath(deviceMountPath, pluginDir); err != nil {
+		return err
+	}

This pattern is repeated for all entry points where a path could be attacker-controlled.

3. Regression tests for traversal attempts

Tests are added to ensure that traversal attempts are detected and blocked, and that no files or directories are created outside the plugin directory:

func TestPersistISCSIRejectsEscapedPluginPath(t *testing.T) {
	// ...
	mounter := iscsiDiskMounter{
		iscsiDisk: &iscsiDisk{
			Portals: []string{"10.0.0.1:3260"},
			Iqn:     "iqn.2026-06.example.com:../../../../../escape",
			// ...
		},
		// ...
	}
	err := (&ISCSIUtil{}).persistISCSI(mounter)
	if err == nil {
		t.Fatal("expected escaped plugin path to be rejected")
	}
	if !strings.Contains(err.Error(), "escapes plugin directory") {
		t.Fatalf("unexpected error: %v", err)
	}
	// ...
}

This test simulates an attack and asserts that the operation is rejected and nothing is created outside the expected directory.

From the review thread

The same problem also affects MountDevice/UnmountDevice in iscsi/attacher.go and diskSetup in isci/disk_manager.go. Can you fix those as well?

I’ve updated the PR to apply the same plugin-directory path validation to MountDevice, UnmountDevice, and diskSetUp. I also added regression tests for the additional paths and centralized the validation so the same boundary check is used consistently across the iSCSI plugin.

Local iSCSI tests and make verify are passing.

Takeaways

• Always validate resolved filesystem paths, not just input strings, before performing any filesystem operations involving user-controlled data.
• Centralize security checks in a helper function to ensure consistent enforcement across all code paths.
• Add regression tests that simulate realistic attack vectors to verify that security boundaries cannot be bypassed.