fix: harden ephemeral mount option handling for inline volumes.#2460
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kapilupadhayay The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Welcome @kapilupadhayay! |
|
Hi @kapilupadhayay. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
andyzhangx
left a comment
There was a problem hiding this comment.
can you sign the easycla first? (select individual contributor option)
|
/ok-to-test |
There was a problem hiding this comment.
Pull request overview
Hardens inline ephemeral CSI volume mounts by stripping security-sensitive blobfuse2 options (--tmp-path and --config-file) supplied via volumeAttributes.mountOptions, preventing root-owned cache writes to attacker-chosen host paths (e.g. /etc/kubernetes/manifests) that could enable static Pod injection and host RCE. PVC/StorageClass-supplied options (admin-controlled) are not affected.
Changes:
- Adds
sanitizeMountOptionsand ablockedEphemeralMountOptionsdenylist inpkg/blob/blob.go. - Calls the sanitizer in
NodeStageVolumebefore joining ephemeral mount options. - Adds
TestSanitizeMountOptionscovering pass-through, blocking, and whitespace cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/blob/blob.go | Introduces denylist and prefix-based sanitization for ephemeral mount options. |
| pkg/blob/nodeserver.go | Sanitizes ephemeral mount options before merging with mountFlags. |
| pkg/blob/blob_test.go | Unit tests for the new sanitizer covering allowed/blocked/whitespace cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Instead of keeping a deny-list of mounts, should we have a list of allowed mount options? |
Agreed, an allow list of safe mount options would improve security, |
I believe deny list is still better. It won't be a breaking change and new options when added in the future need not be maintained in the list. |
|
@kapilupadhayay pls also add |
0d42687 to
ab6e635
Compare
|
/ok-to-test |
ab6e635 to
b55fe25
Compare
|
/test pull-blob-csi-driver-unit |
Harden ephemeral mount option handling for inline volumes.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Blocks the users to specify --tmp-path on ephemeral volumes
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Release note: