[PATCH 1/2] [V3] Add support for GPG signature enforcement on booted
Samuel Mendoza-Jonas
sam at mendozajonas.com
Fri Aug 12 11:46:12 AEST 2016
> struct boot_task {
> struct load_url_result *image;
> struct load_url_result *initrd;
> @@ -43,6 +50,15 @@ struct boot_task {
> void *status_arg;
> bool dry_run;
> bool cancelled;
> + bool verify_signature;
> + struct load_url_result *image_signature;
> + struct load_url_result *initrd_signature;
> + struct load_url_result *dtb_signature;
> + struct load_url_result *cmdline_signature;
> + const char *local_image_signature;
> + const char *local_initrd_signature;
> + const char *local_dtb_signature;
> + const char *local_cmdline_signature;
> };
>
> /**
> @@ -57,20 +73,153 @@ static int kexec_load(struct boot_task *boot_task)
> char *s_dtb = NULL;
> char *s_args = NULL;
>
> + const char* local_initrd = boot_task->local_initrd;
> + const char* local_dtb = boot_task->local_dtb;
> + const char* local_image = boot_task->local_image;
> +
> + const char* local_initrd_signature = (boot_task->verify_signature) ?
> + boot_task->local_initrd_signature : NULL;
> + const char* local_dtb_signature = (boot_task->verify_signature) ?
> + boot_task->local_dtb_signature : NULL;
> + const char* local_image_signature = (boot_task->verify_signature) ?
> + boot_task->local_image_signature : NULL;
> + const char* local_cmdline_signature = (boot_task->verify_signature) ?
> + boot_task->local_cmdline_signature : NULL;
> +
..etc.
While overall the function of the signature checking/etc looks pretty
good to me, this still adds a /whole/ lot of code to a fairly generic file.
Would it make more sense to do something like
if (task->verify_signature) {
rc = do_gpg_stuff() // possibly asynchronously?
if (rc)
//cancel boot
}
So that all the signature-specific code is self contained in gpg.c, and
boot.c just calls out to it if needed?
We could also avoid adding all the foo_signature and local_foo_signature
fields to the boot_task struct, and assuming all checks passed gpg.c can
just update the existing local paths of the kernel/initrd/etc..
> @@ -789,9 +804,16 @@ static struct pmenu *main_menu_init(struct cui *cui)
> i->on_execute = menu_add_url_execute;
> pmenu_item_insert(m, i, 5);
>
> - i = pmenu_item_create(m, _("Exit to shell"));
> - i->on_execute = pmenu_exit_cb;
> - pmenu_item_insert(m, i, 6);
> + if (lockdown) {
> + i = pmenu_item_create(m, _("Reboot"));
> + i->on_execute = pmenu_exit_cb;
> + pmenu_item_insert(m, i, 6);
> + }
> + else {
> + i = pmenu_item_create(m, _("Exit to shell"));
> + i->on_execute = pmenu_exit_cb;
> + pmenu_item_insert(m, i, 6);
> + }
>
> result = pmenu_setup(m);
>
If we're just changing the item name but not the callback, this could
just be
if (lockdown)
i = pmenu_item_create(m, _("Reboot"));
else
i = pmenu_item_create(m, _("Exit to shell"));
i->on_execute.....
Since these UI changes aren't related to the changes to booting, they
would be better of as a separate patch.
More information about the Petitboot
mailing list