[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