-
Notifications
You must be signed in to change notification settings - Fork 106
fix: harden ephemeral mount option handling for inline volumes. #2460
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1110,6 +1110,50 @@ func (d *Driver) useDataPlaneAPI(ctx context.Context, volumeID, accountName stri | |
| return false | ||
| } | ||
|
|
||
| // blockedEphemeralMountOptions is the set of blobfuse2 options that users must not be able to | ||
| // override via volumeAttributes.mountOptions on inline ephemeral volumes. | ||
| // | ||
| // --tmp-path: primary exploit vector (file-cache mode). blobfuse2 writes blob content | ||
| // (attacker-controlled) into this directory as root. Setting it to e.g. | ||
| // /etc/kubernetes/manifests allows the attacker to land an arbitrary static Pod manifest, | ||
| // achieving host-level RCE. | ||
| // | ||
| // --block-cache-path: same exploit vector as --tmp-path but for block-cache mode. blobfuse2 | ||
| // persists downloaded blocks (attacker-controlled blob content) to this directory as root. | ||
| // An ephemeral volume with --block-cache-path=/etc/kubernetes/manifests (optionally combined | ||
| // with --block-cache=true) reproduces the same static-Pod-injection / host RCE primitive. | ||
| // | ||
| // --config-file: two-step bypass for the above blocks. An attacker can upload a blobfuse2 | ||
| // config YAML as a blob (step 1, normal mount caches it to /mnt/<volumeID>/evil.yaml), then | ||
| // point a second ephemeral mount at that cached file via --config-file, which sets | ||
| // file_cache.path or block_cache.path inside the config, reinstating the RCE path. | ||
| var blockedEphemeralMountOptions = []string{ | ||
| "--tmp-path", | ||
|
kapilupadhayay marked this conversation as resolved.
|
||
| "--block-cache-path", | ||
| "--config-file", | ||
| } | ||
|
|
||
| // sanitizeMountOptions removes options from the provided list that are in the | ||
| // blockedEphemeralMountOptions denylist. It is called when processing user-supplied mount options | ||
| // for inline ephemeral CSI volumes before the driver appends its own safe defaults. | ||
| func sanitizeMountOptions(mountOptions []string) []string { | ||
| filtered := make([]string, 0, len(mountOptions)) | ||
| for _, opt := range mountOptions { | ||
| blocked := false | ||
| for _, blockedPrefix := range blockedEphemeralMountOptions { | ||
| if strings.HasPrefix(strings.TrimSpace(opt), blockedPrefix) { | ||
| klog.Warningf("mount option %q is not allowed for ephemeral volumes and will be ignored", opt) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about the short form of these mount options?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made the check exact. None of the options have shorter forms. |
||
| blocked = true | ||
| break | ||
| } | ||
| } | ||
| if !blocked { | ||
| filtered = append(filtered, opt) | ||
| } | ||
| } | ||
| return filtered | ||
| } | ||
|
|
||
| // appendDefaultMountOptions return mount options combined with mountOptions and defaultMountOptions | ||
| func appendDefaultMountOptions(mountOptions []string, tmpPath, containerName string) []string { | ||
| var defaultMountOptions = map[string]string{ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -450,7 +450,10 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe | |
| // Get mountOptions that the volume will be formatted and mounted with | ||
| mountOptions := mountFlags | ||
| if ephemeralVol { | ||
| mountOptions = util.JoinMountOptions(mountOptions, strings.Split(ephemeralVolMountOptions, ",")) | ||
| // Sanitize user-supplied mount options before use: strip options that are | ||
| // security-sensitive (e.g. --tmp-path) and must be driver-controlled. | ||
| sanitized := sanitizeMountOptions(strings.Split(ephemeralVolMountOptions, ",")) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it silently sanitize and mount or should it throw an error saying that it can't mount with provided options?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would prefer this failing with
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, its better to fail with InvalidArg. |
||
| mountOptions = util.JoinMountOptions(mountOptions, sanitized) | ||
| } | ||
| if isHnsEnabled { | ||
| mountOptions = util.JoinMountOptions(mountOptions, []string{"--use-adls=true"}) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.