Skip to content

Strictly validate lctl set_param parameter keys against allowlist in upcall server - #608

Merged
tyuchn merged 2 commits into
GoogleCloudPlatform:mainfrom
amacaskill:fix-bug-524246779
Aug 26, 2026
Merged

tyuchn merged 2 commits into
GoogleCloudPlatform:mainfrom
amacaskill:fix-bug-524246779

Conversation

@amacaskill

@amacaskill amacaskill commented Aug 4, 2026

Copy link
Copy Markdown

What this PR does

Enforces strict security allowlist validation in the upcall IPC server for lctl set_param parameter keys executed on behalf of the host Lustre kernel/udev proxy on Container-Optimized OS (COS) nodes.

Context & Motivation

On COS nodes, Lustre kernel udev events and IAM upcalls communicate with the privileged CSI driver container over a Unix domain socket (ipc.sock). While lctl set_param is an authorized subcommand, unrestricted parameter setting poses a security risk because attackers could configure kernel upcall binary paths (e.g. ptlrpc.*.upcall, mdt.*.identity_upcall, osc.*.upcall) that execute as root on the host.

Implementation Details

Following review and confirmation with Lustre performance experts:

  1. Allowlisted Subsystems: Parameter keys are restricted to the four core client subsystems:
    • osc.*: Object Storage Client tunables (e.g. osc.*.max_dirty_mb, max_rpcs_in_flight, checksums) and dynamic mount target connections (osc.<fs>-<target>-osc-<addr>.import).
    • mdc.*: Metadata Client tunables (mdc.*.max_rpcs_in_flight) and dynamic target connections (mdc.<fs>-<target>-mdc-<addr>.import).
    • llite.*: Client VFS/caching tunables (llite.*.max_read_ahead_mb, enable_compression, opencache_*, statahead_*).
    • ldlm.*: Lustre Distributed Lock Manager namespace tunables (ldlm.namespaces.*.lru_size, ldlm.*.*mdc*.lru_max_age).
  2. Flag Disallowal: Options/flags (such as -n and -P) are rejected because client nodes on GKE do not require them (-P is exclusive to MGS persistent configs).
  3. Defense in Depth: Explicitly blocks any parameter key containing the upcall keyword.
  4. Test Coverage: Unit test and manual local test

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces strict argument validation for the lctl set_param command to prevent unauthorized upcall configurations and path traversals. Feedback focuses on a critical security vulnerability where attackers could bypass the upcall substring check using globbing/wildcard characters or backslashes. It is recommended to disallow backslashes in the parameter key and restrict globbing characters in the final component of the parameter name, as well as to add robust test cases covering these bypass scenarios.

Comment thread pkg/upcall/server.go Outdated
Comment thread pkg/upcall/server_test.go
@amacaskill
amacaskill force-pushed the fix-bug-524246779 branch 5 times, most recently from 078533c to d54c5e7 Compare August 5, 2026 01:28
Comment thread pkg/upcall/server.go Outdated
@tyuchn
tyuchn force-pushed the fix-bug-524246779 branch from d54c5e7 to d7247ab Compare August 25, 2026 22:27
…upcall server

Enforce strict allowlist validation in the upcall IPC server for
lctl set_param parameter keys (supporting osc.*, mdc.*, llite.*, and
ldlm.*) while disallowing flags and upcall hook parameters.

Bug: b/524246779
@tyuchn
tyuchn force-pushed the fix-bug-524246779 branch from d7247ab to 5150750 Compare August 25, 2026 22:32
@tyuchn

tyuchn commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

/gemini review

@tyuchn
tyuchn requested a review from Rishita-Golla August 25, 2026 22:36

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces stricter argument validation for the lctl set_param command by restricting parameter keys to specific allowed prefixes (osc., mdc., llite., ldlm.) and blocking keys containing the substring "upcall". It also adds comprehensive unit tests to verify these validation rules. The review feedback identifies a critical security vulnerability where the "upcall" block can be bypassed using wildcard characters (e.g., up*all) that lctl expands. It is recommended to explicitly disallow wildcard characters in the final parameter name component and to add corresponding test cases to verify this behavior.

Comment thread pkg/upcall/server.go
Comment thread pkg/upcall/server_test.go
…meter name

Disallow wildcard characters (*, ?, [, ]) and backslashes in the leaf
component of lctl set_param parameter keys to prevent bypassing the upcall
block. Add corresponding test cases for wildcard, question mark, character
class, and escape bypass attempts.
@tyuchn
tyuchn merged commit 2425260 into GoogleCloudPlatform:main Aug 26, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants