Skip to content

cups/globals: use getauxval(AT_SECURE) for SUID check#1258

Merged
michaelrsweet merged 1 commit intoOpenPrinting:masterfrom
MaxKellermann:at_secure
May 14, 2025
Merged

cups/globals: use getauxval(AT_SECURE) for SUID check#1258
michaelrsweet merged 1 commit intoOpenPrinting:masterfrom
MaxKellermann:at_secure

Conversation

@MaxKellermann
Copy link
Copy Markdown
Contributor

Comparing effective and real uid/gid is not a proper way to check for SUID execution:

  1. this does not consider file capabilities

  2. this check breaks when NO_NEW_PRIVS is used as the Linux kernel resets effective ids during execve(); this means the check is false, but the process still has raised capabilities

For more details about the NO_NEW_PRIVS problem, check this post and the surrounding thread:

https://lore.kernel.org/lkml/20250509184105.840928-1-max.kellermann@ionos.com/

Comparing effective and real uid/gid is not a proper way to check for
SUID execution:

1. this does not consider file capabilities

2. this check breaks when NO_NEW_PRIVS is used as the Linux kernel
   resets effective ids during execve(); this means the check is
   false, but the process still has raised capabilities

For more details about the NO_NEW_PRIVS problem, check this post and
the surrounding thread:

 https://lore.kernel.org/lkml/20250509184105.840928-1-max.kellermann@ionos.com/

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@michaelrsweet michaelrsweet self-assigned this May 10, 2025
@michaelrsweet michaelrsweet added enhancement New feature or request platform issue Issue is specific to an OS or desktop labels May 10, 2025
@michaelrsweet
Copy link
Copy Markdown
Member

Will do a deeper review next week.

WRT comparing effective and real IDs not being "proper", it would be more correct to say that Linux has added functionality that breaks POSIX compliance and requires OS-specific code to fix.

@michaelrsweet michaelrsweet added this to the v2.5 milestone May 10, 2025
@MaxKellermann
Copy link
Copy Markdown
Contributor Author

that Linux has added functionality that breaks POSIX compliance and requires OS-specific code to fix.

I agree with that. My argument in that thread was that Linux needs to fix this ABI breakage, but the Linux kernel developers do not seem to share my opinionn here. It's an ongoing discussion.

But in any case, whatever they decide, AT_SECURE is the better Linux-specific choice.

@MaxKellermann
Copy link
Copy Markdown
Contributor Author

It looks like the discussion has concluded with this (off-list) post from Kees Cook I replied to: https://lore.kernel.org/lkml/CAKPOu+_eL3j9+yaDRMnoCfoyapNSp04rgGaRzpKn9ycbkif_9g@mail.gmail.com/

He wrote:

"classic elevated privilege check" in userspace is just wrong. It's been insufficient to check for euid!=uid for decades.

There is very little support for my proposal to fix the ABI breakage. Sorry, I tried all I could.

Copy link
Copy Markdown
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

Thanks for the clean PR, LGTM (even if the need for it gives me some heartburn...)

@michaelrsweet michaelrsweet merged commit 795ee24 into OpenPrinting:master May 14, 2025
@MaxKellermann MaxKellermann deleted the at_secure branch May 15, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request platform issue Issue is specific to an OS or desktop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants