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

feat: Add notification for secure boot key check #1661

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

jardon
Copy link

@jardon jardon commented Sep 9, 2024

  • Add script that checks secure boot config and handles motd and GUI notifications
  • Add systemd service to run script
  • Enable systemd service
  • Add motd key warning section

Closes #1651

image
image

○ check-sb-key.service - Service to check for secure boot key enrollment
     Loaded: loaded (/usr/lib/systemd/system/check-sb-key.service; enabled; preset: disabled)
    Drop-In: /usr/lib/systemd/system/service.d
             └─10-timeout-abort.conf
     Active: inactive (dead) since Thu 2024-09-19 06:54:01 CDT; 2s ago
   Duration: 67ms
    Process: 6815 ExecStart=/usr/libexec/check-sb-key.sh (code=exited, status=0/SUCCESS)
   Main PID: 6815 (code=exited, status=0/SUCCESS)
        CPU: 11ms

Sep 19 06:54:01 fedora systemd[1]: Started check-sb-key.service - Service to check for secure boot key enrollment.
Sep 19 06:54:01 fedora check-sb-key.sh[6829]: /etc/pki/akmods/certs/akmods-ublue.der is not enrolled
Sep 19 06:54:01 fedora systemd[1]: check-sb-key.service: Deactivated successfully.

@jardon jardon force-pushed the key-dbus-notify branch 11 times, most recently from 4f3bbac to e826c08 Compare September 10, 2024 17:37
@jardon jardon marked this pull request as ready for review September 10, 2024 17:38
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. enhancement New feature or request labels Sep 10, 2024
- Add script to check for sb enabled and key registration
- Add script for notification
- Add systemd service to run script and notify
@bsherman
Copy link
Contributor

@jardon a comment I make to most contributors... once you start getting feedback on PRs, please don't force push. Doing so erases history of comments and their relevance which makes it harder to track the history of change.

We squash merge most/all PRs in the ublue-os repos, so cleaning up commits is not a concern.

Thanks!

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Sep 11, 2024
@jardon jardon marked this pull request as draft September 11, 2024 18:40
@jardon jardon marked this pull request as ready for review September 13, 2024 11:48
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 13, 2024
KyleGospo
KyleGospo previously approved these changes Sep 15, 2024
@castrojo
Copy link
Member

Ok we good on this one? Last call!

@bsherman
Copy link
Contributor

Ok we good on this one? Last call!

i'm actively testing, but i don't think this service works as expected.

fi
USER_ID=$(loginctl list-users --output=json ${JSON_ARG:+$JSON_ARG} | jq -r '.[] | .user')
XDG_DIR=$(loginctl show-user "$USER_ID" | grep RuntimePath | cut -c 13-)
sudo -u "$USER_ID" \
Copy link
Contributor

Choose a reason for hiding this comment

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

This sudo notify-send doesn't work when run from a root systemd service as setup here.

You can test this in a VM by writing the script to /usr/local/sbin/sb-key-notify.sh and the service to /etc/systemd/system/sb-key-notify.service and make sure you change the path for the script in the service file..

Then, systemctl daemon-reload and systemctl enable sb-key-notify ... reboot and/or simply systemctl start sb-key-notify... nothing happens.

I haven't dug deep into why this fails, but there's a more direct approach which is more what I meant to suggest when I suggested a service.

What I'd do is keep all the testing for if secure boot is enabled and if the key is enrolled or not in a script like this... but the script would be sbkey-missing-check.sh or something... and if the conditions warrant a notification, write a file to /run/sbkey-missing-notify.

Then we need this notify-send command NOT with the sudo, to run as the user, and that should get setup with a profile.d/skel combo of script and .desktop file, similar to how we do for the bluefin-firstboot feature.

Copy link
Author

Choose a reason for hiding this comment

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

I'm curious why it doesnt work for you. I've tested it in my environment and it works great. Could you check the output of the service?

I had things more split up beforehand. I'm open to seeing what that looks like to implement it using a desktop file, but I'm a little puzzled how we got here.

Copy link
Author

Choose a reason for hiding this comment

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

more specificially:

bash -x /path/to/script

Copy link
Contributor

@bsherman bsherman Sep 15, 2024

Choose a reason for hiding this comment

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

I'm curious why it doesnt work for you. I've tested it in my environment and it works great. Could you check the output of the service?

I set this up again on Bluefin:GTS (like i referenced in my comment on the Timer section). My sb-key-notify.service is installed in /etc and the sb-key-notify.sh is in /usr/local/sbin

I did see the notification popup when running systemctl start sb-key-notify on this setup, however... upon a reboot.

This is the output from the service:

 journalctl --boot -u sb-key-notify
Sep 15 13:07:53 fedora systemd[1]: Started sb-key-notify.service - Service to check for secure boot key enrollment and send notifications.
Sep 15 13:07:53 fedora sb-key-notify.sh[2952]: /etc/pki/akmods/certs/akmods-ublue.der is not enrolled
Sep 15 13:07:53 fedora loginctl[3023]: Failed to look up user : No such process
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]: usage: sudo -h | -K | -k | -V
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]: usage: sudo -v [-ABkNnS] [-g group] [-h host] [-p prompt] [-u user]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]: usage: sudo -l [-ABkNnS] [-g group] [-h host] [-p prompt] [-U user]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]:             [-u user] [command [arg ...]]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]: usage: sudo [-ABbEHkNnPS] [-r role] [-t type] [-C num] [-D directory]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]:             [-g group] [-h host] [-p prompt] [-R directory] [-T timeout]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]:             [-u user] [VAR=value] [-i | -s] [command [arg ...]]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]: usage: sudo -e [-ABkNnS] [-r role] [-t type] [-C num] [-D directory]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]:             [-g group] [-h host] [-p prompt] [-R directory] [-T timeout]
Sep 15 13:07:53 fedora sb-key-notify.sh[3051]:             [-u user] file ...
Sep 15 13:07:53 fedora systemd[1]: sb-key-notify.service: Deactivated successfully.

Edit: for the record, i get the same behavior when testing on bluefin:stable (Fedora 40)

Copy link
Contributor

Choose a reason for hiding this comment

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

more context:

❯ systemctl cat sb-key-notify.service 
# /etc/systemd/system/sb-key-notify.service
[Unit]
Description=Service to check for secure boot key enrollment and send notifications

[Service]
ExecStart=/usr/local/sbin/sb-key-notify.sh

[Install]
WantedBy=multi-user.target

[Timer]
OnBootSec=1min
OnUnitActiveSec=3h

# /usr/lib/systemd/system/service.d/10-timeout-abort.conf
# This file is part of the systemd package.
# See https://fedoraproject.org/wiki/Changes/Shorter_Shutdown_Timer.
#
# To facilitate debugging when a service fails to stop cleanly,
# TimeoutStopFailureMode=abort is set to "crash" services that fail to stop in
# the time allotted. This will cause the service to be terminated with SIGABRT
# and a coredump to be generated.
#
# To undo this configuration change, create a mask file:
#   sudo mkdir -p /etc/systemd/system/service.d
#   sudo ln -sv /dev/null /etc/systemd/system/service.d/10-timeout-abort.conf

[Service]
TimeoutStopFailureMode=abort

Copy link
Contributor

Choose a reason for hiding this comment

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

can you post the output of bash -x /usr/local/sbin/sb-key-notify.sh, please?

I can, and I will, but I've told you your script works, I do get a notification if running systemctl start sb-key-notify while already logged in.

The problem is notification from the script upon a boot. I'm not sure if you missed where i showed journal output with the failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

root in ~ 
❯ bash -x /usr/local/sbin/sb-key-notify.sh
++ id -u
+ '[' 0 -ne 0 ']'
+ WARNING_MSG='This machine has secure boot turned on, but you haven'\''t enrolled Universal Blue'\''s keys. Failing to enroll these before rebooting **may cause your system to fail to boot**. Follow this link https://docs.projectbluefin.io/introduction#secure-boot ~for instructions on how to enroll the keys.'
+ KEY_WARN_FILE=/run/user-motd-sbkey-warn.md
+ KEY_DER_FILE=/etc/pki/akmods/certs/akmods-ublue.der
+ mokutil --sb-state
+ grep -q enabled
+ SB_ENABLED=0
+ '[' 0 -ne 0 ']'
+ mokutil --test-key /etc/pki/akmods/certs/akmods-ublue.der
/etc/pki/akmods/certs/akmods-ublue.der is not enrolled
+ loginctl --help
+ grep -q json=MODE
++ loginctl list-users --output=json
++ jq -r '.[] | .user'
+ USER_ID=bsherman
++ loginctl show-user bsherman
++ grep RuntimePath
++ cut -c 13-
+ XDG_DIR=/run/user/1000
++ echo 'This machine has secure boot turned on, but you haven'\''t enrolled Universal Blue'\''s keys. Failing to enroll these before rebooting **may cause your system to fail to boot**. Follow this link https://docs.projectbluefin.io/introduction#secure-boot ~for instructions on how to enroll the keys.'
++ tr -d '*~'
+ sudo -u bsherman DISPLAY=:0 DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus notify-send WARNING 'This machine has secure boot turned on, but you haven'\''t enrolled Universal Blue'\''s keys. Failing to enroll these before rebooting may cause your system to fail to boot. Follow this link https://docs.projectbluefin.io/introduction#secure-boot for instructions on how to enroll the keys.' -i dialog-warning -u critical -a mokutil --wait
+ echo '**WARNING**: This machine has secure boot turned on, but you haven'\''t enrolled Universal Blue'\''s keys. Failing to enroll these before rebooting **may cause your system to fail to boot**. Follow this link https://docs.projectbluefin.io/introduction#secure-boot ~for instructions on how to enroll the keys.'

Copy link
Author

Choose a reason for hiding this comment

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

oh i misunderstood. ya so on boot, there is no user logged in so loginctl doesn't list a user to pass to the sudo command. ill look into another solution then

Copy link
Contributor

Choose a reason for hiding this comment

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

oh i misunderstood. ya so on boot, there is no user logged in so loginctl doesn't list a user to pass to the sudo command. ill look into another solution then

yes, sorry it wasn't more clear.

Copy link
Contributor

@bsherman bsherman Sep 15, 2024

Choose a reason for hiding this comment

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

oh i misunderstood. ya so on boot, there is no user logged in so loginctl doesn't list a user to pass to the sudo command. ill look into another solution then

This is why I proposed writing a state file from the service, and then having the user's login process (eg, autostart .desktop file) look for that file and run the notification. Then it would work for ANY user who logs in.

@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Sep 15, 2024
@jardon jardon marked this pull request as draft September 15, 2024 19:05
@jardon
Copy link
Author

jardon commented Sep 18, 2024

If anyone wants to test this in a secure boot enabled environment, rebase onto ghcr.io/jardon/bluefin:key-dbus-notify

@jardon jardon marked this pull request as ready for review September 18, 2024 13:16
@bsherman
Copy link
Contributor

If anyone wants to test this in a secure boot enabled environment, rebase onto ghcr.io/jardon/bluefin:key-dbus-notify

Thank you for sticking with this PR! But also, thanks for going the extra mile and giving us a rebasable image for testing; that's excellent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an motd warning when ublue's keys are not enrolled
4 participants