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

New feature: bootloader signing #152

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pandom79
Copy link

@pandom79 pandom79 commented Oct 8, 2020

Hi,
I added bootloader's signing functionality.
I needed that so i though to create a merge request.
Pratically, i added two options: -d and -t.
-d to set key file. ( es. -d /keys/db.key )
-t to set cert file ( es. -t /keys/db.crt )

These options are complementary. It hasn't sence to set one without other.
If both are set, i also check sbsign command.
These controls are execute before the packages installation and initramfs generation.
If the user entered wrong data, it is useless go on.

Following, show the steps to get void linux usb key bootable under secure boot enabled:

  1. sudo ./mklive.sh -d my.key -t my.crt

  2. Via dd command, i write ISO image on USB stick.

  3. Reboot machine with USB stick plugged

  4. Setup firmware

You'll see a bootloader called BOOTX??.EFI and BOOTX??-signed.EFI ( ?? = 32 or 64 )

  1. Select BOOTX64-signed.EFI

If the entered keys are correct, void linux will startup finely.

If you are interested that , i am available for eventual changes.
Regards

@sgn
Copy link
Member

sgn commented Oct 8, 2020

This is neither a support nor a non-support from me.
But, sbsign has config file in /etc/default/sbsigntool-kernel-hook

mklive.sh.in Outdated
@@ -233,6 +235,13 @@ generate_grub_efi_boot() {
fi
mkdir -p "${GRUB_EFI_TMPDIR}"/EFI/BOOT
cp -f "$VOIDHOSTDIR"/tmp/bootia32.efi "${GRUB_EFI_TMPDIR}"/EFI/BOOT/BOOTIA32.EFI

#Bootloader signing
if ([ $toSign ] && [ -f "${GRUB_EFI_TMPDIR}"/EFI/BOOT/BOOTX32.EFI ]);then
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need sub-shell?

mklive.sh.in Outdated
@@ -244,6 +253,13 @@ generate_grub_efi_boot() {
die "Failed to generate EFI loader"
fi
cp -f "$VOIDHOSTDIR"/tmp/bootx64.efi "${GRUB_EFI_TMPDIR}"/EFI/BOOT/BOOTX64.EFI

#Bootloader signing
if ([ $toSign ] && [ -f "${GRUB_EFI_TMPDIR}"/EFI/BOOT/BOOTX64.EFI ]);then
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member

Choose a reason for hiding this comment

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

And please reset toSign variable in the beginning of script.

mklive.sh.in Outdated
Comment on lines 259 to 264
print_step "Signing BOOTX64.EFI..."
sbsign --key $DBKEY --cert $DBCRT --output "${GRUB_EFI_TMPDIR}"/EFI/BOOT/BOOTX64-signed.EFI "${GRUB_EFI_TMPDIR}"/EFI/BOOT/BOOTX64.EFI
fi
Copy link
Member

Choose a reason for hiding this comment

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

Make them a function, perhaps?

mklive.sh.in Outdated
Comment on lines 358 to 362
#The -d and -t options are complementary. If one exists, the other must also exist.
#If these options are set, I also check sbsign command.
if ([ -z $DBKEY ] && [ ! -z $DBCRT ]) || ([ ! -z $DBKEY ] && [ -z $DBCRT ]); then
die "Must be set a key and certificate via -d and -t option, exiting..."
elif ([ $DBKEY ] && [ $DBCRT ]); then
Copy link
Member

Choose a reason for hiding this comment

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

Ugly! Maybe?

if [ -n "$key" -a -n "$crt" ]; then
    ...
else if [ -n "$key$crt" ]; then
    die "...."
fi

Copy link

Choose a reason for hiding this comment

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

-a and -o are deprecated (according to bash faq), -n gives no much value in general if any, and similarly ! can be basically used instead of -z :D

Copy link
Member

Choose a reason for hiding this comment

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

I was talking about the order of execution ;)

Copy link

Choose a reason for hiding this comment

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

sure, the structure is fine :)

mklive.sh.in Outdated
Comment on lines 363 to 365
if [ $DBKEY ] && [ ! -f $DBKEY ]; then
die "$DBKEY does not exist, exiting..."
elif [ $DBCRT ] && [ ! -f $DBCRT ]; then
Copy link
Member

Choose a reason for hiding this comment

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

$dbkey and $dbcrt checked above! [ -f "$key" ] is enough.

mklive.sh.in Outdated
die "$DBKEY does not exist, exiting..."
elif [ $DBCRT ] && [ ! -f $DBCRT ]; then
die "$DBCRT does not exist, exiting..."
elif ! [ -x "$(command -v sbsign)" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

What wrong with simple:

elif command -v sbsign; then

Copy link

Choose a reason for hiding this comment

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

command -v sbsign >/dev/null
or
hash sbsign 2>/dev/null
or
type sbsign >/dev/null 2>&1

and if im right, then hash is the fastest, but i think ive only ever seen command -v around void

Copy link
Member

Choose a reason for hiding this comment

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

I don't care about which one is the fastest. I was talking about the uselessness of [ -x ]

Copy link

Choose a reason for hiding this comment

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

the point was the redirection against littering the output, while u r right, just these are again two different things, i hope u didnt get me wrong or got offended or whatever! :)

Copy link
Member

@sgn sgn Oct 9, 2020

Choose a reason for hiding this comment

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

Ah, no, I'm not offended (and I hope you aren't, too).
However, driving out of track is not productive.
You suggestion about appending >/dev/null is good, that's is nice.

Choose a reason for hiding this comment

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

ahh, fine, and thx for the info! :)
however, just for the next time, what do u mean by driving out of track? it was about the same line, is there a way to isolate my stuff better that im not aware of, or i just need a better wording? im not a git/github pro actually :D (also, i often go off topic, but i dont think this was an example of that :D )

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this one is not. The previous one is :-p

Comment on lines +85 to +87
-d <key-file> Set a key file to sign bootloader.
-t <cert-file> Set a certificate file to sign bootloader.
Copy link
Member

Choose a reason for hiding this comment

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

Why d and t? Is it because they're random characters that still available?

@pandom79
Copy link
Author

pandom79 commented Oct 8, 2020

This is neither a support nor a non-support from me.
But, sbsign has config file in /etc/default/sbsigntool-kernel-hook

Sorry, how can i to use that hook to sign bootloader in a custom Void ISO?
The iso image obtainable via mklive command have the efi file in a compressed image called efiboot.img,
therefore i haven't visibility to sign it after creation.
I thought about intervening during its creation to sign it, so I made this change.
My goal is to boot a Voidlinux USB stick with secure boot enabled.
It worked for me.
This patch is just an idea, obviously it can be improved and made more elegant.
If exist a better way to do that, please ignore it.

@ericonr
Copy link
Member

ericonr commented Oct 8, 2020

I'm not sure there's an easy way to use the sbsign hook (which could be run inside the chroot with xbps-reconfigure) without including the private keys in the final ISO, which I don't think is ideal.

If possible, I would be in favor of signing the kernel in void ISOs with a throwaway key, which allows one to never turn off Secure Boot while installing, even if new keys will have to be added (once for the Void specific one, once for the user's keys).

@pandom79
Copy link
Author

pandom79 commented Oct 8, 2020

I'm not sure there's an easy way to use the sbsign hook (which could be run inside the chroot with xbps-reconfigure) without including the private keys in the final ISO, which I don't think is ideal.

If possible, I would be in favor of signing the kernel in void ISOs with a throwaway key, which allows one to never turn off Secure Boot while installing, even if new keys will have to be added (once for the Void specific one, once for the user's keys).

I just see that hook only signs vmlinuz does not sign the bootloader. Grub is loaded first and then the kernel.
If grub is unsigned it will not boot with secure boot enabled.
However, it worked for me by only signing the efi file, the kernel is unsigned.

@pandom79
Copy link
Author

pandom79 commented Oct 8, 2020

I still have to understand a lot about how void works :)
Let me know if you are interested.
It is useless to make changes

@ericonr
Copy link
Member

ericonr commented Oct 8, 2020

However, it worked for me by only signing the efi file, the kernel is unsigned.

Really? GRUB should verify this, otherwise Secure Boot is useless. rEFInd does proper verification of the kernel.

@pandom79
Copy link
Author

pandom79 commented Oct 9, 2020

However, it worked for me by only signing the efi file, the kernel is unsigned.

Really? GRUB should verify this, otherwise Secure Boot is useless. rEFInd does proper verification of the kernel.

Probably grub handles it differently.
If grub is allowed to start then the rest are too.
As you can see from this example, i sign only efi file. I don't sign initrd.
Perhaps there is some setting....

@pandom79
Copy link
Author

pandom79 commented Oct 9, 2020

About hook /etc/default/sbsigntool-kernel-hook, i guess that this way is more onerous.
First of all, this hook is not installed by default, therefore, we'll need to install sbsigntool package via -p option for additional package or, in signature case, adding it at default packages.
After that, to enable it, we'll need edit it overwriting SBSIGN_EFI_KERNEL from 0 to 1.
The xbps-reconfigure command will call /etc/kernel.d/post-install/40-sbsigntool.
This post installation hook only sign vmlinuz, therefore, we'll need to append efi file sign via command
( for example "echo do_sign path-efi-file >> /etc/kernel.d/post-install/40-sbsigntool" )
Perhaps the keys could not be a problem...after signature we can delete them

Is correct? What do you think?

@sgn
Copy link
Member

sgn commented Oct 9, 2020

Sorry, how can i to use that hook to sign bootloader in a custom Void ISO?
The iso image obtainable via mklive command have the efi file in a compressed image called efiboot.img,
therefore i haven't visibility to sign it after creation.
I thought about intervening during its creation to sign it, so I made this change.

Sorry, I didn't say it clearly. I was trying to say about falling back to those key and cert in those config files

@pandom79
Copy link
Author

pandom79 commented Oct 9, 2020

Sorry, how can i to use that hook to sign bootloader in a custom Void ISO?
The iso image obtainable via mklive command have the efi file in a compressed image called efiboot.img,
therefore i haven't visibility to sign it after creation.
I thought about intervening during its creation to sign it, so I made this change.

Sorry, I didn't say it clearly. I was trying to say about falling back to those key and cert in those config files

Do you say to use the keys mentioned in sbsigntool-kernel-hook ?
EFI_KEY_FILE=/etc/efikeys/db.key
EFI_CERT_FILE=/etc/efikeys/db.crt

@sgn
Copy link
Member

sgn commented Oct 9, 2020 via email

@pandom79
Copy link
Author

pandom79 commented Oct 9, 2020

On 2020-10-09 07:03:59-0700, pandom79 @.***> wrote: > > Sorry, how can i to use that hook to sign bootloader in a custom Void ISO? > > The iso image obtainable via mklive command have the efi file in a compressed image called efiboot.img, > > therefore i haven't visibility to sign it after creation. > > I thought about intervening during its creation to sign it, so I made this change. > > Sorry, I didn't say it clearly. I was trying to say about falling back to those key and cert in those config files Do you say to use the keys mentioned in sbsigntool-kernel-hook ? EFI_KEY_FILE=/etc/efikeys/db.key EFI_CERT_FILE=/etc/efikeys/db.crt
Exactly.

-- Danh

I say that my case it would not works because i don't use sbsigntool-kernel-hook. I sign kernel and bootloader directly in dracut and them path are different.
An other thing go in my mind, i could to create Void ISO on other machine but my keys are on external ssd. How to do.....?
I think that the best way is pass them as arguments. Use them and put in trash.

@pandom79
Copy link
Author

pandom79 commented Oct 9, 2020

Anyway, this is my definitive version.
If you want change something, please justify it with valid reason and write the change completely so i can test it.
Until now the some suggested changes seems don't works for me.

@pandom79
Copy link
Author

Never say definitive...;)
i added the signing and signature verification control.
Someone could set wrong key.
For example :
db.cer in place of db.crt

@@ -282,7 +308,7 @@ generate_iso_image() {
#
# main()
#
while getopts "a:b:r:c:C:T:Kk:l:i:I:s:S:o:p:v:h" opt; do
while getopts "a:b:r:c:C:T:Kk:l:i:I:s:S:o:p:v:d:t:h" opt; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
while getopts "a:b:r:c:C:T:Kk:l:i:I:s:S:o:p:v:d:t:h" opt; do
while getopts "a:b:r:c:C:T:Kk:l:i:I:s:o:p:v:d:t:h" opt; do

Sorry! I have removed the -S option, so there is a conflict now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants