HDDS-15467. Do not fall back to the OM starter user in OMClientRequest#10469
Open
rich7420 wants to merge 1 commit into
Open
HDDS-15467. Do not fall back to the OM starter user in OMClientRequest#10469rich7420 wants to merge 1 commit into
rich7420 wants to merge 1 commit into
Conversation
getUserIfNotExists() rebuilt the request UserInfo from UserGroupInformation.getCurrentUser() (the OM starter/login user) whenever the derived UserInfo was missing a username or remote address. This is a fail-open to an often-privileged identity: any request reaching preExecute without complete user info would silently run as the OM service user. Internal callers that need this (the Trash emptier) already populate their own UserInfo on the request, so getUserInfo() only needs to preserve a caller-supplied host/address when there is no RPC or gRPC context. With that, the getCurrentUser() fallback is redundant and is removed; requests with no identity now fail closed in createUGI().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
OMClientRequest#getUserIfNotExists()rebuilt the requestUserInfofromUserGroupInformation.getCurrentUser()— the OM starter/login user — wheneverthe derived
UserInfowas missing a username or a remote address. Because theresult feeds
createUGI()and the ACL check, this is a fail-open to anoften-privileged identity: any request reaching
preExecute()without completeuser info would silently run as the OM service user. The escalation is latent
today (real RPC and gRPC clients always carry user info), but a future change in
getUserInfo()that dropped the username or remote address for a client pathwould silently grant the OM identity.
The only caller that relies on this fallback is the Trash emptier
(
TrashOzoneFileSystem), and it already populates a completeUserInfo(service user + OM address) on the request it builds. The fallback merely
re-derived the same values, because
getUserInfo()did not carry thecaller-supplied host/address over when there was no RPC/gRPC context.
Dataflow
Before — every write request, on missing user info, is rebuilt as the OM
starter user:
After — no
getCurrentUser()fallback; identity is either derived from theclient context, preserved from a request that already carries it (Trash), or
the request fails closed:
So this PR:
getUserInfo()preserve the host/address already present on the requestwhen neither an RPC nor a gRPC client context is available (mirroring how the
username is already carried over for gRPC s3g requests);
getUserIfNotExists()and thegetCurrentUser()/admin fallback;preExecute()and theOMKeyDeleteRequest/OMKeyRenameRequest/OMAllocateBlockRequestcallers atgetUserInfo().Requests that genuinely have no identity now fail closed in
createUGI()(
UNAUTHORIZED) instead of being granted the OM starter user. Real clientpaths are unchanged (they always carry user info), and the Trash emptier keeps
its explicit service identity.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15467
How was this patch tested?
https://github.com/rich7420/ozone/actions/runs/27190426330