Skip to content

Fix UID/GID overflow, display, and JWT identity override issues - #214

Closed
wiktorn wants to merge 4 commits into
SchedMD:masterfrom
wiktorn:feature/uid-gid-overflow-26.05
Closed

Fix UID/GID overflow, display, and JWT identity override issues#214
wiktorn wants to merge 4 commits into
SchedMD:masterfrom
wiktorn:feature/uid-gid-overflow-26.05

Conversation

@wiktorn

@wiktorn wiktorn commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

GCP OS Login generates UID/GIDs up to 2^32. This PR allows users having UIDs falling between 2^31 and 2^32 to submit jobs via Slurm REST API. This also fixes issue, that user could not impersonate themselves in the POSIX identity passed in JWT token.

  • Fix gid_from_string to allow GIDs up to UINT32_MAX.
  • Remove INT_MAX checks in data_parser plugins to allow UIDs/GIDs in range (2^31..2^32-1) in slurmrestd.
  • Add overflow checks around data_get_int in data_parser plugins to prevent 32-bit wrap-around.
  • Fix JWT identity override check in auth_jwt.c to correctly reject mismatched identities instead of matching ones.
  • Fix squeue display to format UIDs/GIDs as unsigned (%u).

@wiktorn
wiktorn force-pushed the feature/uid-gid-overflow-26.05 branch from 93c00d2 to d1db8d7 Compare August 2, 2026 19:51
@wiktorn

wiktorn commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@wickberg Added sign-off to the commit.

@mcmult mcmult removed the missing-dco label Aug 6, 2026
@mcmult

mcmult commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Hi @wiktorn , Thank you for looking in to this! Would you be able to base this on the master branch and split the commit up into a couple logical groups?
Thanks again!

@wiktorn

wiktorn commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Hey @mcmult, sure. I can rebase this on master. What is the procedure then, to backport that to 26.05? Does this qualifies for backporting?

In terms of splitting up the commits, please confrim this split (each bullet separate commit):

  • libslurm - common/uid.c
  • slurmrestd - data_parser/*/parsers.c
  • auth/jwt plugin - auth/jwt/auth_jwt.c
  • squeue - squeue/print.c

Increase GID limit from INT_MAX to UINT32_MAX in gid_from_string
to support GID values above 2^31 (e.g. from GCP OS Login).

Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
@wiktorn
wiktorn force-pushed the feature/uid-gid-overflow-26.05 branch from d1db8d7 to bd65caf Compare August 6, 2026 16:01
@mcmult

mcmult commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Hi @wiktorn ! I think the proposed split looks good to me.

I expect this to qualify for backporting to 26.05. The actual merge and the backport will happen in gitlab on our side and I'll just have to relay to you here where and how it landed.

For the sake of clarity, it might be better to just make a new PR against master instead of targeting the 26.05 branch and trying to rebase it. I'm sorry about the extra effort involved.

@wiktorn
wiktorn changed the base branch from slurm-26.05 to master August 6, 2026 16:19
@wiktorn

wiktorn commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, I forgot to change target branch. It looks good right now.

@mcmult

mcmult commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Thank you for thi @wiktorn! I had 2 small requests to make on the patches but otherwise I think they look good!

I'd like to use PRId64 instead of %ld, and Id like to add the "static" keyword to the _print_uint() function. I know that isn't there on other functions in the file, but we are trying to push toward that in newly added code.

Thank you so much!

- Remove post-parse checks that rejected UIDs/GIDs >= INT_MAX, to
  correctly pass UIDs/GIDs above 2^31 (for example, from GCP OS Login)
- Add strict bounds checks when parsing 64-bit integer values from JSON
  (DATA_TYPE_INT_64 case) to ensure they fit within 32-bit unsigned range
  (0 to UINT32_MAX).

Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
Allow users to impersonate themselves. Change !xstrcmp to xstrcmp
to correctly reject different usernames instead of same usernames.

Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
- Introduce _print_uint helper function to format values as unsigned (%u).
- Use _print_uint instead of _print_int for printing UIDs and GIDs.

Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
@wiktorn
wiktorn force-pushed the feature/uid-gid-overflow-26.05 branch from bd65caf to f2cdfb6 Compare August 10, 2026 18:38
@wiktorn

wiktorn commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Hey, both changes are in. Let me know, if anything else is needed.

wickberg pushed a commit that referenced this pull request Aug 11, 2026
Increase GID limit from INT_MAX to UINT32_MAX in gid_from_string
to support GID values above 2^31 (e.g. from GCP OS Login).

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
wickberg pushed a commit that referenced this pull request Aug 11, 2026
- Remove post-parse checks that rejected UIDs/GIDs >= INT_MAX, to
  correctly pass UIDs/GIDs above 2^31 (for example, from GCP OS Login)
- Add strict bounds checks when parsing 64-bit integer values from JSON
  (DATA_TYPE_INT_64 case) to ensure they fit within 32-bit unsigned range
  (0 to UINT32_MAX).

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
wickberg pushed a commit that referenced this pull request Aug 11, 2026
Allow users to impersonate themselves. Change !xstrcmp to xstrcmp
to correctly reject different usernames instead of same usernames.

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
wickberg pushed a commit that referenced this pull request Aug 11, 2026
- Introduce _print_uint helper function to format values as unsigned (%u).
- Use _print_uint instead of _print_int for printing UIDs and GIDs.

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
wickberg pushed a commit that referenced this pull request Aug 11, 2026
Changelog: Fix handling of UIDs and GIDs above 2^31 in gid_from_string(),
 the data_parser plugins and squeue output.
Changelog: auth/jwt - Fix the token identity being discarded when the
 requested username matches it, instead of when it differs.
Link: #214
wickberg pushed a commit that referenced this pull request Aug 11, 2026
Increase GID limit from INT_MAX to UINT32_MAX in gid_from_string
to support GID values above 2^31 (e.g. from GCP OS Login).

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
Cherry-picked: 1a5a745
wickberg pushed a commit that referenced this pull request Aug 11, 2026
- Remove post-parse checks that rejected UIDs/GIDs >= INT_MAX, to
  correctly pass UIDs/GIDs above 2^31 (for example, from GCP OS Login)
- Add strict bounds checks when parsing 64-bit integer values from JSON
  (DATA_TYPE_INT_64 case) to ensure they fit within 32-bit unsigned range
  (0 to UINT32_MAX).

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
Cherry-picked: 01b7498
wickberg pushed a commit that referenced this pull request Aug 11, 2026
Allow users to impersonate themselves. Change !xstrcmp to xstrcmp
to correctly reject different usernames instead of same usernames.

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
Cherry-picked: b548355
wickberg pushed a commit that referenced this pull request Aug 11, 2026
- Introduce _print_uint helper function to format values as unsigned (%u).
- Use _print_uint instead of _print_int for printing UIDs and GIDs.

Link: #214
Signed-off-by: Wiktor Niesiobędzki <wiktorn@google.com>
Cherry-picked: a79811b
wickberg pushed a commit that referenced this pull request Aug 11, 2026
Changelog: Fix handling of UIDs and GIDs above 2^31 in gid_from_string(),
 the data_parser plugins and squeue output.
Changelog: auth/jwt - Fix the token identity being discarded when the
 requested username matches it, instead of when it differs.
Link: #214
Cherry-picked: 94d3d28
@mcmult mcmult added the merged label Aug 11, 2026
@mcmult

mcmult commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Thank you for your work on this! We've merged this in and it should be available in 26.05.3.

The merge commit to master is here: c5668fd611

Thanks again for your contribution!

@mcmult mcmult closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants