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

systemd: Restore access to TTYs for reboot delay #21

Merged
merged 1 commit into from
May 10, 2023

Conversation

pothos
Copy link
Member

@pothos pothos commented May 4, 2023

When sessions are active, locksmith write a message to the TTYs of the sessions and then delays the reboot for 5 minutes to give the user time to stop the reboot or finish the work.
The commit for cgroup memory and CPU limits also brought in a change to disallow /dev/tty* access which broke the delay for console users except SSH sessions which are under /dev/pts/*.
Allow device access to have a delay of 5 minutes when sessions are active. This includes the autologin session even if no interaction was done there recently. Still, the reboot delay doesn't hurt and since update-engine has a random delay for pulling updates, there is no big difference in the end.

How to use

Update ref in ebuild

Testing done

In the demo @tormath1 prepared, locksmith didn't wait. We looked at the possible issue and found this.
I just tried echo A >> /dev/tty1 with sudo systemd-run -S -P --property=PrivateDevices=true -G and it failed while it works with PrivateDevices=false, so I think this will fix the issue. @tormath1 did a test with PrivateDevices=true removed and the delay worked.

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

When sessions are active, locksmith write a message to the TTYs of the
sessions and then delays the reboot for 5 minutes to give the user time
to stop the reboot or finish the work.
The commit for cgroup memory and CPU limits also brought in a change to
disallow /dev/tty* access which broke the delay for console users except
SSH sessions which are under /dev/pts/*.
Allow device access to have a delay of 5 minutes when sessions are
active. This includes the autologin session even if no interaction was
done there recently. Still, the reboot delay doesn't hurt and since
update-engine has a random delay for pulling updates, there is no big
difference in the end.
@pothos pothos requested a review from a team May 4, 2023 08:28
pothos added a commit to flatcar/scripts that referenced this pull request May 4, 2023
This pulls in
flatcar/locksmith#21
to have working reboot warnings and delays for non-SSH sessions.
This seems to have been broken for a longer time.
Copy link
Member

@jepio jepio left a comment

Choose a reason for hiding this comment

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

Could also be (gpt generated suggestion):

DevicePolicy=closed
DeviceAllow=/dev/tty*
DeviceAllow=/dev/pts/*

@pothos
Copy link
Member Author

pothos commented May 4, 2023

I thought about keeping the block devices unavailable but the commit introducing it wasn't really aimed at providing a security benefit and if we want to go this route we should also not leave the whole root partition writable.

@pothos
Copy link
Member Author

pothos commented May 4, 2023

DeviceAllow=/dev/tty*

The suggestion doesn't work yet, /dev/tty1: Operation not permitted

@pothos pothos merged commit b5fb134 into flatcar-master May 10, 2023
@pothos pothos deleted the kai/reboot-delay branch May 10, 2023 09:41
pothos added a commit to flatcar/scripts that referenced this pull request May 10, 2023
This pulls in
flatcar/locksmith#21
to have working reboot warnings and delays for non-SSH sessions.
This seems to have been broken for a longer time.
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