fix:[#1651] Add MOTD message for SB keys #1656

jardon · 2024-09-08T20:55:15Z

Add logic to check for SB enrollment and keys
Update motd template

jardon · 2024-09-08T21:46:16Z

These changes are untested. I'll try to get an environment set up to test them before marking the PR as ready.

jardon · 2024-09-09T13:31:31Z

The word wrapping is a little broken, but I believe this is an issue with glow and their handling of hyperlinks. Marking as ready since its out of my control and ugly warnings are better than no warnings at all.

noelmiller

LGTM, I will let others weigh in though.

KyleGospo · 2024-09-09T16:49:57Z

The word wrapping is a little broken, but I believe this is an issue with glow and their handling of hyperlinks. Marking as ready since its out of my control and ugly warnings are better than no warnings at all.

I did a workaround for this, you can use ~ in your text and it will become a new line in the final output.

Good for another PR though, I don't want to hold this up over some formatting. LGTM.

noelmiller · 2024-09-09T16:55:30Z

The word wrapping is a little broken, but I believe this is an issue with glow and their handling of hyperlinks. Marking as ready since its out of my control and ugly warnings are better than no warnings at all.

I did a workaround for this, you can use ~ in your text and it will become a new line in the final output.

Good for another PR though, I don't want to hold this up over some formatting. LGTM.

Is this warning something we would need in Bazzite? Only case I can think of is if someone were to install Bazzite without secure boot on and then turn it on? But then they wouldn't be able to see the message anyway?

KyleGospo · 2024-09-09T16:56:07Z

Missed this, seems there's a typo here?
``if [[ $ENROLL -eq 1 ]] && [[ $SB_ENABLED -eq 0 ]]; then```

$ENROLL doesn't exist, $ENROLLED is created above.

jardon · 2024-09-09T17:04:39Z

I'm not sure how I did that, but its fixed. Should be good after the CI clears. I rechecked it in my environment.

- Add logic to check for SB enrollment and keys - Update motd template

jardon · 2024-09-09T17:08:58Z

The word wrapping is a little broken, but I believe this is an issue with glow and their handling of hyperlinks. Marking as ready since its out of my control and ugly warnings are better than no warnings at all.

I did a workaround for this, you can use ~ in your text and it will become a new line in the final output.

Good for another PR though, I don't want to hold this up over some formatting. LGTM.

Thanks for the tip. I implemented this to clean it up some. Its not perfect, but its better.

castrojo · 2024-09-09T20:09:58Z

Let's merge this tomorrow (after the :stable builds go out), that way we have a week of it in :latest if we need to tweak the text.

jardon · 2024-09-10T21:20:01Z

if this merges first, i can refactor this PR to consolidate the logic

bsherman

Correct me if I'm wrong, but I believe this needs to be fixed.

bsherman · 2024-09-11T16:47:14Z

system_files/shared/usr/libexec/ublue-motd

+# check for secure boot key
+KEY_WARN=""
+FINGERPRINT="2B:E9:91:E3:B1:B5:40:70:F4:3D:80:BB:13:EB:C6:57:E5:A3:78:0D"
+mokutil --list-enrolled | grep -q $FINGERPRINT


I don't think this actually works.

Example:

$ FINGERPRINT="2B:E9:91:E3:B1:B5:40:70:F4:3D:80:BB:13:EB:C6:57:E5:A3:78:0D" $ mokutil --list-enrolled |grep $FINGERPRINT

Notice there is no output because the grep match failed...

I think you want something like this:

$ mokutil --list-enrolled --verbose-listing|grep -i $FINGERPRINT SHA1 Fingerprint: 2b:e9:91:e3:b1:b5:40:70:f4:3d:80:bb:13:eb:c6:57:e5:a3:78:0d

it does work, but not by comparing output. when grep fails to find a match, it gives an exit code of 1. so this just compares the exit code to see if it matches or not.

I've tested it in a VM and it works well. if there's a better option, I'm open to changing it

I understand the grep -q usage, my point is it will always think the key is not enrolled because of the output from mokutil --list-enrolled vs mokutil --list-enrolled --verbose-listing

$ mokutil --list-enrolled 2bb010e24d fedoraca 2be991e3b1 ublue kernel

The output above will never have the fingerprint contents.

Adding --verbose-listing does add the fingerprint output to the listing, however, with a lowercased fingerprint, which won't match what you have here.

$ mokutil --list-enrolled --verbose-listing|grep -i sha1 SHA1 Fingerprint: 2b:b0:10:e2:4d:94:c6:32:24:58:89:ba:aa:9e:d0:f3:d5:ef:1f:68 SHA1 Fingerprint: 2b:e9:91:e3:b1:b5:40:70:f4:3d:80:bb:13:eb:c6:57:e5:a3:78:0d

thats not what the included mokutil command produces

mokutil --list-enrolled | grep Finger SHA1 Fingerprint: 2b:b0:10:e2:4d:94:c6:32:24:58:89:ba:aa:9e:d0:f3:d5:ef:1f:68 SHA1 Fingerprint: 2b:e9:91:e3:b1:b5:40:70:f4:3d:80:bb:13:eb:c6:57:e5:a3:78:0d

$ which mokutil /usr/bin/mokutil $ `which mokutil` --version 0.7.1 $ /usr/bin/mokutil --list-enrolled | grep Finger

Does Bluefin somehow modify mokutil default behavior?

Even if there's a version of mokutil where --list-enrolled does show fingerprint, does --verbose-listing not work?

@jardon and I discovered in a side chat that the difference here is Fedora 39 has mokutil version 0.6.0 and Fedora 40 has version 0.7.1.

And the outputs differ.

bsherman · 2024-09-11T16:50:12Z

system_files/shared/usr/libexec/ublue-motd

+
+# check for secure boot key
+KEY_WARN=""
+FINGERPRINT="2B:E9:91:E3:B1:B5:40:70:F4:3D:80:BB:13:EB:C6:57:E5:A3:78:0D"


I'm not sure if we want to hard code the fingerprint or read it from the cert on-image?

openssl x509 -fingerprint -noout -in /etc/pki/akmods/certs/akmods-ublue.der

this is a good suggestion. ive implemented this in the notify-send PR

jardon · 2024-09-11T19:18:20Z

closing in favor of #1661

jardon force-pushed the key-motd branch 2 times, most recently from 7e56b6b to e927eeb Compare September 8, 2024 21:07

KyleGospo previously approved these changes Sep 8, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 8, 2024

jardon dismissed KyleGospo’s stale review via b2cf183 September 8, 2024 21:08

jardon force-pushed the key-motd branch from e927eeb to b2cf183 Compare September 8, 2024 21:08

jardon force-pushed the key-motd branch 7 times, most recently from c527664 to 6b2d22d Compare September 9, 2024 13:29

jardon marked this pull request as ready for review September 9, 2024 13:31

jardon requested a review from castrojo as a code owner September 9, 2024 13:31

dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Sep 9, 2024

jardon requested a review from KyleGospo September 9, 2024 14:42

noelmiller previously approved these changes Sep 9, 2024

View reviewed changes

KyleGospo added this pull request to the merge queue Sep 9, 2024

KyleGospo previously approved these changes Sep 9, 2024

View reviewed changes

KyleGospo removed this pull request from the merge queue due to a manual request Sep 9, 2024

jardon dismissed stale reviews from KyleGospo and noelmiller via 6a5faaa September 9, 2024 16:58

jardon force-pushed the key-motd branch from 6b2d22d to 6a5faaa Compare September 9, 2024 16:58

fix:[ublue-os#1651] Add MOTD message for SB keys

04016c3

- Add logic to check for SB enrollment and keys - Update motd template

jardon force-pushed the key-motd branch from 6a5faaa to 04016c3 Compare September 9, 2024 17:07

noelmiller approved these changes Sep 9, 2024

View reviewed changes

Merge branch 'main' into key-motd

ebae0ed

bsherman approved these changes Sep 11, 2024

View reviewed changes

bsherman self-requested a review September 11, 2024 16:44

bsherman requested changes Sep 11, 2024

View reviewed changes

dosubot bot removed the lgtm This PR has been approved by a maintainer label Sep 11, 2024

bsherman reviewed Sep 11, 2024

View reviewed changes

jardon closed this Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix:[#1651] Add MOTD message for SB keys #1656

fix:[#1651] Add MOTD message for SB keys #1656

jardon commented Sep 8, 2024 •

edited

Loading

jardon commented Sep 8, 2024

jardon commented Sep 9, 2024

noelmiller left a comment

KyleGospo commented Sep 9, 2024 •

edited

Loading

noelmiller commented Sep 9, 2024

KyleGospo commented Sep 9, 2024

jardon commented Sep 9, 2024

jardon commented Sep 9, 2024

castrojo commented Sep 9, 2024

jardon commented Sep 10, 2024

bsherman left a comment

bsherman Sep 11, 2024

jardon Sep 11, 2024

bsherman Sep 11, 2024 •

edited

Loading

jardon Sep 11, 2024

bsherman Sep 11, 2024 •

edited

Loading

bsherman Sep 11, 2024

bsherman Sep 11, 2024

bsherman Sep 11, 2024

bsherman Sep 11, 2024

jardon Sep 11, 2024

jardon commented Sep 11, 2024

fix:[#1651] Add MOTD message for SB keys #1656

fix:[#1651] Add MOTD message for SB keys #1656

Conversation

jardon commented Sep 8, 2024 • edited Loading

jardon commented Sep 8, 2024

jardon commented Sep 9, 2024

noelmiller left a comment

Choose a reason for hiding this comment

KyleGospo commented Sep 9, 2024 • edited Loading

noelmiller commented Sep 9, 2024

KyleGospo commented Sep 9, 2024

jardon commented Sep 9, 2024

jardon commented Sep 9, 2024

castrojo commented Sep 9, 2024

jardon commented Sep 10, 2024

bsherman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsherman Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsherman Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jardon commented Sep 11, 2024

jardon commented Sep 8, 2024 •

edited

Loading

KyleGospo commented Sep 9, 2024 •

edited

Loading

bsherman Sep 11, 2024 •

edited

Loading

bsherman Sep 11, 2024 •

edited

Loading