Sign the fwupmgr bootloader about sbctl HOT 17 CLOSED

sbctl sign -s /usr/libexec/fwupd/efi/fwupdx64.efi -o /usr/libexec/fwupd/efi/fwupdx64.efi.signed

Should solve all of it. sbctl would track fwupdx64.efi for changes and write out fwupdx64.efi.signed when you sign.

from sbctl.

I think the latest commits fixes several of the issues mentioned in this issue. Please verify :)

from sbctl.

Poke me if #11 is OK to merge after this or if there are anything missing :)

from sbctl.

Indeed. Thanks for the quick answer!

Actually, it's there in the README, no idea how I missed it at first.

from sbctl.

I did actually intentionally add it to the README.md for this reason :) It's a nice feature and demonstrates the flexibility.

from sbctl.

Ok, I'm fairly sure this has some bug in functionality now...

$ cd /usr/libexec/fwupd/efi
$ sudo sbctl -s fwupdx64.efi -o fwupdx64.efi.signed
-> Signing /usr/libexec/fwupd/efi/fwupdx64.efi...
$ sudo sbctl verify
==> Verifying file database and EFI images in /boot/efi...
  -> /boot/efi/EFI/refind/refind_x64.efi is signed
  -> /boot/efi/EFI/void/vmlinuz-1.0_1 is signed
  -> /boot/efi/EFI/void/vmlinuz-1.0_1.EFI is signed
  -> /boot/efi/EFI/void/vmlinuz-1.1_1 is signed
panic: runtime error: slice bounds out of range [2:1]

goroutine 1 [running]:
github.com/foxboron/sbctl.VerifyESP()
	/home/ericonr/mypro/sbctl/sbctl.go:52 +0x486
main.verifyCmd.func1(0xc000135080, 0x8d06d8, 0x0, 0x0)
	/home/ericonr/mypro/sbctl/cmd/sbctl/main.go:109 +0x20
github.com/spf13/cobra.(*Command).execute(0xc000135080, 0x8d06d8, 0x0, 0x0, 0xc000135080, 0x8d06d8)
	/home/ericonr/.cache/go/pkg/mod/github.com/spf13/[email protected]/command.go:846 +0x29d
github.com/spf13/cobra.(*Command).ExecuteC(0x8a2860, 0xc000055f70, 0x1, 0x1)
	/home/ericonr/.cache/go/pkg/mod/github.com/spf13/[email protected]/command.go:950 +0x349
github.com/spf13/cobra.(*Command).Execute(...)
	/home/ericonr/.cache/go/pkg/mod/github.com/spf13/[email protected]/command.go:887
main.main()
	/home/ericonr/mypro/sbctl/cmd/sbctl/main.go:290 +0x4a7
$ sudo sbctl sign-all
==> /boot/efi/EFI/void/vmlinuz-1.0_1.EFI has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.2_1.EFI has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.2_1 has been signed...
==> /boot/efi/EFI/voidb/vmlinuz-1.2_1 has been signed...
  -> Signing /usr/libexec/fwupd/efi/fwupdx64.efi...
==> /boot/efi/EFI/refind/refind_x64.efi has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.0_1 has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.1_1 has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.1_1.EFI has been signed...

Removing the files with remove-file seems to be enough to stop getting errors. Perhaps it's necessary to mention that files need to have absolute paths? Or make the path logic when using -s more complex.

EDIT: when running sign-all, it creates the file in my current directory, which makes sense, but doesn't feel like reasonable behavior for something that you're saving. I think it makes sense to require absolute paths when saving something.

Doing it with absolute paths seems to work fine, but has weird stuff in sign-all.

$ sudo sbctl sign -s /usr/libexec/fwupd/efi/fwupdx64.efi -o /usr/libexec/fwupd/efi/fwupdx64.efi.signed
  -> Signing /usr/libexec/fwupd/efi/fwupdx64.efi...
$ sudo sbctl sign-all
==> /boot/efi/EFI/void/vmlinuz-1.0_1.EFI has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.2_1.EFI has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.2_1 has been signed...
==> /boot/efi/EFI/voidb/vmlinuz-1.2_1 has been signed...
  -> Signing /usr/libexec/fwupd/efi/fwupdx64.efi...
==> /boot/efi/EFI/refind/refind_x64.efi has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.0_1 has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.1_1 has been signed...
==> /boot/efi/EFI/void/vmlinuz-1.1_1.EFI has been signed...

It is only regenerating the signed version, not touching the original one, so it isn't a huge bug, but it is always regenerating the signed version.

As a PS, disabling color output when piping into something and/or with a command line option would be a welcome feature, but I can try to implement it later.

from sbctl.

Ah, i see the issue. In an attempt at not verifying files twice I did some basic caching of the checked files.

It assumes there is some path to the output, and can probably be made more clever. But we should probably verify or have a warning when people add files without absolute paths.

It is only regenerating the signed version, not touching the original one, so it isn't a huge bug, but it is always regenerating the signed version.

Right, because we don't really know when the file has changed when we save it with -o. We could store a checksum of the original file?

from sbctl.

As a PS, disabling color output when piping into something and/or with a command line option would be a welcome feature, but I can try to implement it later.

The entire logging code is a giant hack as I wanted to have the same output format as efi-roller. Mostly because I didn't want to spend time inventing a new formatting to the output. Suggestions for something more sane, and or help redesigning the output would be welcome

https://github.com/Foxboron/sbctl/blob/master/log.go

from sbctl.

It assumes there is some path to the output, and can probably be made more clever. But we should probably verify or have a warning when people add files without absolute paths.

Intuitively, I think it would work like:

path: either relative or absolute

1. sbctl sign <path>: replaces file
2. sbctl sign -s <path>: replaces file, saves the absolute path
3. sbctl sign <path> -o <path>: generates new file
4. sbctl sign -s <path> -o <path>: generates new file, saves the absolute path for both original file and output file

The 2nd and 4th options can instead be errors if used without an absolute path.

Right, because we don't really know when the file has changed when we save it with -o. We could store a checksum of the original file?

I think that could work, but why not store checksums for both files? Do you think a user could have some other program make changes to the signed file and still want to use it?

The entire logging code is a giant hack as I wanted to have the same output format as efi-roller. Mostly because I didn't want to spend time inventing a new formatting to the output. Suggestions for something more sane, and or help redesigning the output would be welcome

I will take a look at it later!

from sbctl.

The 2nd and 4th options can instead be errors if used without an absolute path.

Hmm, this seems complicated. It would be better to enforce absolute paths across the board and be consistent I think?

I think that could work, but why not store checksums for both files? Do you think a user could have some other program make changes to the signed file and still want to use it?

Good point. They could very well sign with multiple db keys, and currently we would just override it. So maybe we should have some logic if the outfile is defined? Ensure we have signed it if it exists? and only create it if it doesn't exist?

from sbctl.

Hmm, this seems complicated. It would be better to enforce absolute paths across the board and be consistent I think?

I don't see a problem with it. Would just make the usecase of generating a signed file to use elsewhere a bit trickier, but no big deal.

Ensure we have signed it if it exists? and only create it if it doesn't exist?

We'd need proper diagnostics messages for these, I think. But yeah, that sounds ok.

from sbctl.

If the original file ceases existing, it bugs out as well.

from sbctl.

So local code now runs filesystem.Abs on the output files with sign, and create-bundle which is the one we store in the database. I think this should ensure we are always having absolute paths.

I'll fix the missing file issue as well.

from sbctl.

I think you forgot to commit the ChecksumFile function.

from sbctl.

Woopsie, that is correct! fixed :)

from sbctl.

Found two issues, fixed in #15

from sbctl.

I think this can be closed, thanks!

from sbctl.