Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(RHEL-35665) CVE-2023-26604 systemd: privilege escalation via the less pager #153

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

brozs
Copy link
Collaborator

@brozs brozs commented Jul 22, 2024

Resolves: RHEL-35665

Based on upstream commits but is rhel-only.

poettering and others added 2 commits July 18, 2024 19:31
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by #5666

(cherry picked from commit 612ebf6)

Related: RHEL-35665
…uested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes #5666.

v2:
- also check $PKEXEC_UID

v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition

Based on: 0a42426

rhel-only

Resolves: RHEL-35665
@github-actions github-actions bot added rhel-7.9 pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Jul 22, 2024
Copy link

github-actions bot commented Jul 22, 2024

Commit validation

Tracker - RHEL-35665

The following commits meet all requirements

commit upstream
6e14037 - pager: set $LESSSECURE whenver we invoke a pager systemd/systemd@612ebf6
a85b8ca - pager: make pager secure when under euid is changed or explicitly requ… rhel-only

Tracker validation

Success

🟢 Tracker RHEL-35665 has set desired product: rhel-7.9.z
🟢 Tracker RHEL-35665 has set desired component: systemd
🟢 Tracker RHEL-35665 has been approved


Pull Request validation

Success

🟡 CI - Waived
🟢 Review - Reviewed by a member
🟢 Approval - Changes were approved


Auto Merge

Failed

🔴 Pull Request has unsupported target branch rhel-7.9, expected branches are: 'main,master'

Success

🟢 Pull Request is not marked as draft and it's not blocked by dont-merge label
🟢 Pull Request meet requirements, title has correct form
🟢 Pull Request meet requirements, mergeable is true
🟠 Pull Request meet requirements, mergeable_state is unstable

Copy link
Member

@dtardon dtardon left a comment

Choose a reason for hiding this comment

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

LGTM

@jamacku
Copy link
Member

jamacku commented Jul 23, 2024

@mrc0mmand It seems that CentOS CI tests have passed, but the overall result is a failure. Could you please double-check? Thanks

@mrc0mmand
Copy link
Member

@mrc0mmand It seems that CentOS CI tests have passed, but the overall result is a failure. Could you please double-check? Thanks

CentOS CI is FUBAR on C7, since C7 is no more (the fact that Duffy gave us a C9S machine in this case is a bug). And, unfortunately, I can't use the same trick as I did with the C8S job, as building C7 systemd on C9S is just impossible. So just ignore the CentOS CI results here; we'll have to rely on internal CI in this case.

@github-actions github-actions bot added pr/needs-manual-merge and removed pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Jul 23, 2024
@lnykryn lnykryn merged commit 974821e into redhat-plumbers:rhel-7.9 Jul 23, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants