[PATCH 1/2] Add support for GPG signature enforcement on booted

Samuel Mendoza-Jonas sam at ozlabs.au.ibm.com
Tue Aug 2 14:00:35 AEST 2016


On Mon, 2016-08-01 at 12:09 -0500, Timothy Pearson wrote:
>  kernels and related blobs
> 
> This can be used to implement a form of organization-controlled secure boot,
> whereby kernels may be loaded from a variety of sources but they will only
> boot if a valid signature file is found for each component, and only if the
> signature is listed in the /etc/pb-lockdown file.

Cool! This all looks pretty interesting - I have some general comments
lower down, but in particular:

- Some work with Secure Boot is also being done by Nayna Jain, but with
a different approach. I don't think either version steps on the other's
toes but something to look at.
(http://patchwork.ozlabs.org/patch/629157/ etc)

- It looks like you've based your changes on and older/separate version
of Petitboot. Current upstream (and especially for OpenPOWER) can be
found here: http://git.ozlabs.org/?p=petitboot, and also at
https://github.com/open-power/petitboot

> 
> This patch also disables direct command line access when the /etc/pb-lockdown
> file is present.
> 
> Signed-off-by: Timothy Pearson <tpearson at raptorengineering.com>
> ---
>  configure.ac         |   62 ++++++++
>  discover/Makefile.am |    3 +-
>  discover/boot.c      |  381 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  lib/url/url.c        |    2 +-
>  lib/url/url.h        |    1 +
>  ui/ncurses/nc-cui.c  |   16 ++-
>  6 files changed, 454 insertions(+), 11 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index b540819..f6d23c2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -150,6 +150,68 @@ AS_IF(
>  	]
>  )
>  
> +AC_ARG_WITH(
> +	[signed-boot],
> +	[AS_HELP_STRING([--with-signed-boot],
> +		[build kernel signature checking support [default=yes]]
> +	)],
> +	[],
> +	[with_signed_boot=yes]
> +)
> +AM_CONDITIONAL([WITH_SIGNED_BOOT], [test "x$with_signed_boot" = "xyes"])
> +
> +AM_CONDITIONAL(
> +	[WITH_SIGNED_BOOT],
> +	[test "x$with_signed_boot" = "xyes"])
> +
> +AS_IF(
> +	[test "x$with_signed_boot" = "xyes"],
> +	[PKG_CHECK_MODULES(
> +		[GPGME],
> +		[gpgme >= 1.0.0],
> +		[SAVE_LIBS="$LIBS" LIBS="$LIBS $gpgme_LIBS"
> +			AC_CHECK_LIB(
> +				[gpgme],
> +				[gpgme_op_verify],
> +				[],
> +				[AC_MSG_FAILURE([--with-signed-boot was given but the test for gpgme failed.])]
> +			)
> +			LIBS="$SAVE_LIBS"
> +		],
> +		[AM_PATH_GPGME([1.0.0], [SAVE_LIBS="$LIBS" LIBS="$LIBS $gpgme_LIBS"
> +			AC_CHECK_LIB(
> +				[gpgme],
> +				[gpgme_op_verify],
> +				[],
> +				[AC_MSG_FAILURE([--with-signed-boot was given but the test for gpgme failed.])]
> +			)
> +			LIBS="$SAVE_LIBS"],
> +			[AC_MSG_RESULT([$gpgme_PKG_ERRORS])
> +				AC_MSG_FAILURE([ Consider adjusting PKG_CONFIG_PATH environment variable])
> +			])
> +		]
> +	)]
> +)
> +
> +AS_IF(
> +	[test "x$with_signed_boot" = "xyes"],
> +	[SAVE_CPPFLAGS="$CPPFLAGS" CPPFLAGS="$CPPFLAGS $gpgme_CFLAGS"
> +		AC_CHECK_HEADERS(
> +			[gpgme.h],
> +			[],
> +			[AC_MSG_FAILURE([ --with-signed-boot given but gpgme.h not found])]
> +		)
> +		CPPFLAGS="$SAVE_CPPFLAGS"
> +	]
> +)
> +
> +AC_ARG_VAR(
> +	[lockdown_file],
> +	[Location of authorized signature file [default = "/etc/pb-lockdown"]]
> +)
> +AS_IF([test "x$lockdown_file" = x], [lockdown_file="/etc/pb-lockdown"])
> +AC_DEFINE_UNQUOTED(LOCKDOWN_FILE, "$lockdown_file", [Lockdown file location])
> +
>  AC_ARG_ENABLE(
>  	[busybox],
>  	[AS_HELP_STRING(
> diff --git a/discover/Makefile.am b/discover/Makefile.am
> index 5d0f6e2..44ef5e5 100644
> --- a/discover/Makefile.am
> +++ b/discover/Makefile.am
> @@ -56,7 +56,8 @@ discover_pb_discover_LDADD = \
>  	discover/grub2/grub2-parser.ro \
>  	discover/platform.ro \
>  	$(core_lib) \
> -	$(UDEV_LIBS)
> +	$(UDEV_LIBS) \
> +	$(GPGME_LIBS)
>  
>  discover_pb_discover_CPPFLAGS = \
>  	$(AM_CPPFLAGS) \
> diff --git a/discover/boot.c b/discover/boot.c
> index 6e7fda6..eb65f31 100644
> --- a/discover/boot.c
> +++ b/discover/boot.c
> @@ -20,6 +20,10 @@
>  #include <util/util.h>
>  #include <i18n/i18n.h>
>  
> +#if defined(HAVE_LIBGPGME)
> +#include <gpgme.h>
> +#endif
> +
>  #include "device-handler.h"
>  #include "boot.h"
>  #include "paths.h"
> @@ -31,6 +35,11 @@ enum {
>  	BOOT_HOOK_EXIT_UPDATE	= 2,
>  };
>  
> +enum {
> +	KEXEC_LOAD_SIG_SETUP_INVALID = 253,
> +	KEXEC_LOAD_SIGNATURE_FAILURE = 254,
> +};
> +
>  struct boot_task {
>  	struct load_url_result *image;
>  	struct load_url_result *initrd;
> @@ -43,8 +52,189 @@ struct boot_task {
>  	void *status_arg;
>  	bool dry_run;
>  	bool cancelled;
> +#if defined(HAVE_LIBGPGME)
> +	bool verify_signature;
> +	struct load_url_result *image_signature;
> +	struct load_url_result *initrd_signature;
> +	struct load_url_result *dtb_signature;
> +	const char *local_image_signature;
> +	const char *local_initrd_signature;
> +	const char *local_dtb_signature;
> +#endif
>  };
>  
> +static int copy_file_to_destination(const char * source_file, char * destination_file, int max_dest_filename_size) {

It's not immediately clear to me why the boot files need to be copied to
tmp before verifying them. Is there some quirk of gpgme that requires it?

> +	int result = 0;
> +	char template[21] = "/tmp/petitbootXXXXXX";
> +	FILE *source_handle = fopen(source_file, "r");
> +	int destination_fd = mkstemp(template);
> +	FILE *destination_handle = fdopen(destination_fd, "w");
> +	if(!source_handle || !(destination_handle)) {
> +		// handle open error
> +		result = 1;

Do you mean to return here?

> +		pb_log("%s: failed: unable to open source file '%s'\n", __func__, source_file);
> +	}
> +
> +	size_t l1;
> +	unsigned char buffer[8192];

Any particular reason for this size?

> +
> +	/* Copy data */
> +	while ((l1 = fread(buffer, 1, sizeof buffer, source_handle)) > 0) {
> +		size_t l2 = fwrite(buffer, 1, l1, destination_handle);
> +		if (l2 < l1) {
> +			if (ferror(destination_handle)) {
> +				/* General error */
> +				result = 1;
> +				pb_log("%s: failed: unknown fault\n", __func__);
> +			}
> +			else {
> +				/* No space on destination device */
> +				result = 2;
> +				pb_log("%s: failed: temporary storage full\n", __func__);
> +			}
> +		}
> +	}
> +
> +	if (result) {
> +		destination_file[0] = 0;

'0' or '\0'?

> +	}
> +	else {
> +		ssize_t r;
> +		char readlink_buffer[8192];
> +		snprintf(readlink_buffer, 8192, "/proc/self/fd/%d", destination_fd);

Style stuff - unless there's a particular reason for it we generally
prefer that all variables are defined at the start of a function.
Additionally you could probably flip this around and have
	if (result) {
		...
		return result;
	}
to lose the indent for this block.

> +		r = readlink(readlink_buffer, destination_file, max_dest_filename_size);
> +		if (r < 0) {
> +			/* readlink failed */
> +			result = 3;
> +			pb_log("%s: failed: unable to obtain temporary filename\n", __func__);
> +		}
> +		destination_file[r] = 0;
> +	}
> +
> +	fclose(source_handle);
> +	fclose(destination_handle);
> +
> +	return result;
> +}
> +
> +#if defined(HAVE_LIBGPGME)

I think there's officially enough code depending on libgpgme that you
could justify moving it into it's own .c file instead of interspersing
it within boot.c. Then instead from wrapping everything in #if defines,
you could decide somewhere further up whether to boot normally or with
signatures (eg. by stat-ing /etc/pb-lockdown). Thoughts?

> +static int verify_file_signature(const char * plaintext_filename, const char * signature_filename, FILE * authorized_signatures_handle, const char * keyring_path)
> +{
> +	int result = 0;
> +	int valid = 0;
> +
> +	if (signature_filename == NULL)
> +		return 10;

I noticed in a few places increasing numbers are returned for errors, but
it doesn't look like any callers check for specific numbers - do these
have any particular meaning?

> +
> +	gpgme_ctx_t gpg_context;
> +	gpgme_error_t err;
> +	gpgme_data_t plaintext_data;
> +	gpgme_data_t signature_data;
> +	gpgme_engine_info_t enginfo;
> +	gpgme_verify_result_t verification_result;
> +	gpgme_signature_t verification_signatures;

Style - we order variable declarations in decreasing size for clarity, eg:

	gpgme_signature_t verification_signatures;
	gpgme_verify_result_t verification_result;
	gpgme_engine_info_t enginfo;
	etc...

> +
> +	/* Initialize gpgme */
> +	setlocale (LC_ALL, "");
> +	gpgme_check_version(NULL);
> +	gpgme_set_locale(NULL, LC_CTYPE, setlocale (LC_CTYPE, NULL));
> +	err = gpgme_engine_check_version(GPGME_PROTOCOL_OpenPGP);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: OpenPGP support not available\n", __func__);
> +		result = 1;
> +		return result;

return 1; ?

> +	}
> +	err = gpgme_get_engine_info(&enginfo);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG engine failed to initialize\n", __func__);
> +		result = 2;
> +		return result;
> +	}
> +	err = gpgme_new(&gpg_context);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG context could not be created\n", __func__);
> +		result = 3;
> +		return result;
> +	}
> +	err = gpgme_set_protocol(gpg_context, GPGME_PROTOCOL_OpenPGP);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG protocol could not be set\n", __func__);
> +		result = 4;
> +		return result;
> +	}
> +	if (keyring_path)
> +		err = gpgme_ctx_set_engine_info (gpg_context, GPGME_PROTOCOL_OpenPGP, enginfo->file_name, keyring_path);
> +	else
> +		err = gpgme_ctx_set_engine_info (gpg_context, GPGME_PROTOCOL_OpenPGP, enginfo->file_name, enginfo->home_dir);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not set GPG engine information\n", __func__);
> +		result = 5;
> +		return result;
> +	}
> +	err = gpgme_data_new_from_file(&plaintext_data, plaintext_filename, 1);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not create GPG plaintext data buffer from file '%s'\n", __func__, plaintext_filename);
> +		result = 6;
> +		return result;
> +	}
> +	err = gpgme_data_new_from_file(&signature_data, signature_filename, 1);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not create GPG signature data buffer from file '%s'\n", __func__, signature_filename);
> +		result = 7;
> +		return result;
> +	}
> +
> +	/* Check signature */
> +	err = gpgme_op_verify(gpg_context, signature_data, plaintext_data, NULL);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not verify GPG signature\n", __func__);
> +		result = 8;
> +		return result;
> +	}
> +	verification_result = gpgme_op_verify_result(gpg_context);
> +	verification_signatures = verification_result->signatures;
> +	while (verification_signatures) {
> +		if (verification_signatures->status == GPG_ERR_NO_ERROR) {
> +			pb_log("%s: Good signature for key ID '%s' ('%s')\n", __func__, verification_signatures->fpr, signature_filename);
> +			/* Verify fingerprint is present in authorized signatures file */
> +			char *auth_sig_line = NULL;
> +			size_t auth_sig_len = 0;
> +			ssize_t auth_sig_read;
> +			rewind(authorized_signatures_handle);
> +			while ((auth_sig_read = getline(&auth_sig_line, &auth_sig_len, authorized_signatures_handle)) != -1) {
> +				auth_sig_len = strlen(auth_sig_line);
> +				while ((auth_sig_line[auth_sig_len-1] == '\n') || (auth_sig_line[auth_sig_len-1] == '\r'))
> +					auth_sig_len--;
> +				auth_sig_line[auth_sig_len] = 0;
> +				if (strcmp(auth_sig_line, verification_signatures->fpr) == 0)
> +					valid = 1;
> +			}
> +			free(auth_sig_line);
> +		}
> +		else {
> +			pb_log("%s: Signature for key ID '%s' ('%s') invalid.  Status: %08x\n", __func__, verification_signatures->fpr, signature_filename, verification_signatures->status);
> +		}
> +		verification_signatures = verification_signatures->next;
> +	}
> +
> +	/* Clean up */
> +	gpgme_data_release(plaintext_data);
> +	gpgme_data_release(signature_data);
> +	gpgme_release(gpg_context);
> +
> +	if (!valid) {
> +		pb_log("%s: Incorrect GPG signature\n", __func__);
> +		result = 9;
> +		return result;
> +	}
> +	else {
> +		pb_log("%s: GPG signature '%s' for file '%s' verified\n", __func__, signature_filename, plaintext_filename);
> +	}
> +
> +	return result;
> +}
> +#endif
> +
>  /**
>   * kexec_load - kexec load helper.
>   */
> @@ -57,20 +247,98 @@ 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;
> +
> +#if defined(HAVE_LIBGPGME)
> +	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;
> +
> +	if (boot_task->verify_signature) {
> +		int max_filename_size = 8192;
> +		char kernel_filename[max_filename_size];
> +		char initrd_filename[max_filename_size];
> +		char dtb_filename[max_filename_size];
> +
> +		kernel_filename[0] = 0;
> +		initrd_filename[0] = 0;
> +		dtb_filename[0] = 0;
> +
> +		FILE *authorized_signatures_handle = NULL;
> +
> +		/* Load authorized signatures file */
> +		authorized_signatures_handle = fopen(LOCKDOWN_FILE, "r");
> +		if (!authorized_signatures_handle) {
> +			pb_log("%s: unable to read lockdown file\n", __func__);
> +			result = KEXEC_LOAD_SIG_SETUP_INVALID;
> +			return result;
> +		}
> +
> +		/* Copy files to temporary directory for verification / boot */
> +		result = copy_file_to_destination(boot_task->local_image, kernel_filename, max_filename_size);
> +		if (result) {
> +			pb_log("%s: image copy failed: (%d)\n", __func__, result);
> +			return result;
> +		}
> +		if (boot_task->local_initrd) {
> +			result = copy_file_to_destination(boot_task->local_initrd, initrd_filename, max_filename_size);
> +			if (result) {
> +				pb_log("%s: initrd copy failed: (%d)\n", __func__, result);
> +				unlink(local_image);
> +				return result;
> +			}
> +		}
> +		if (boot_task->local_dtb) {
> +			result = copy_file_to_destination(boot_task->local_dtb, dtb_filename, max_filename_size);
> +			if (result) {
> +				pb_log("%s: dtb copy failed: (%d)\n", __func__, result);
> +				unlink(local_image);
> +				if (local_initrd)
> +					unlink(local_initrd);
> +				return result;
> +			}
> +		}
> +		local_image = strdup(kernel_filename);

Use talloc_strndup() instead of strdup()

> +		if (boot_task->local_initrd)
> +			local_initrd = strdup(initrd_filename);
> +		if (boot_task->local_dtb)
> +			local_dtb = strdup(dtb_filename);
> +
> +		/* Check signatures */
> +		if (verify_file_signature(kernel_filename, local_image_signature, authorized_signatures_handle, "/etc/gpg"))
> +			result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +		if (boot_task->local_initrd_signature)
> +			if (verify_file_signature(initrd_filename, local_initrd_signature, authorized_signatures_handle, "/etc/gpg"))
> +				result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +		if (boot_task->local_dtb_signature)
> +			if (verify_file_signature(dtb_filename, local_dtb_signature, authorized_signatures_handle, "/etc/gpg"))
> +				result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +
> +		fclose(authorized_signatures_handle);
> +
> +		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> +			pb_log("%s: Aborting kexec due to signature verification failure\n", __func__);
> +			goto abort_kexec;
> +		}
> +	}
> +#endif
> +
>  	p = argv;
>  	*p++ = pb_system_apps.kexec;	/* 1 */
>  	*p++ = "-l";			/* 2 */
>  
> -	if (boot_task->local_initrd) {
> +	if (local_initrd) {
>  		s_initrd = talloc_asprintf(boot_task, "--initrd=%s",
> -				boot_task->local_initrd);
> +				local_initrd);
>  		assert(s_initrd);
>  		*p++ = s_initrd;	 /* 3 */
>  	}
>  
> -	if (boot_task->local_dtb) {
> +	if (local_dtb) {
>  		s_dtb = talloc_asprintf(boot_task, "--dtb=%s",
> -						boot_task->local_dtb);
> +						local_dtb);
>  		assert(s_dtb);
>  		*p++ = s_dtb;		 /* 4 */
>  	}
> @@ -82,7 +350,7 @@ static int kexec_load(struct boot_task *boot_task)
>  		*p++ = s_args;		/* 5 */
>  	}
>  
> -	*p++ = boot_task->local_image;	/* 6 */
> +	*p++ = local_image;	/* 6 */
>  	*p++ = NULL;			/* 7 */
>  
>  	result = process_run_simple_argv(boot_task, argv);
> @@ -90,6 +358,23 @@ static int kexec_load(struct boot_task *boot_task)
>  	if (result)
>  		pb_log("%s: failed: (%d)\n", __func__, result);
>  
> +#if defined(HAVE_LIBGPGME)
> +abort_kexec:
> +	if (boot_task->verify_signature) {
> +		unlink(local_image);
> +		if (local_initrd)
> +			unlink(local_initrd);
> +		if (local_dtb)
> +			unlink(local_dtb);
> +
> +		free((char*)local_image);
> +		if (local_initrd)
> +			free((char*)local_initrd);
> +		if (local_dtb)
> +			free((char*)local_dtb);
> +	}
> +#endif
> +
>  	return result;
>  }
>  
> @@ -373,23 +658,53 @@ static void boot_process(struct load_url_result *result, void *data)
>  			check_load(task, "dtb", task->dtb))
>  		goto no_load;
>  
> +#if defined(HAVE_LIBGPGME)
> +	if (task->verify_signature) {
> +		if (check_load(task, "kernel image signature", task->image_signature) ||
> +				check_load(task, "initrd signature", task->initrd_signature) ||
> +				check_load(task, "dtb signature", task->dtb_signature))
> +			goto no_sig_load;
> +	}
> +#endif
> +
>  	/* we make a copy of the local paths, as the boot hooks might update
>  	 * and/or create these */
>  	task->local_image = task->image ? task->image->local : NULL;
>  	task->local_initrd = task->initrd ? task->initrd->local : NULL;
>  	task->local_dtb = task->dtb ? task->dtb->local : NULL;
>  
> +#if defined(HAVE_LIBGPGME)
> +	if (task->verify_signature) {
> +		task->local_image_signature = task->image_signature ? task->image_signature->local : NULL;
> +		task->local_initrd_signature = task->initrd_signature ? task->initrd_signature->local : NULL;
> +		task->local_dtb_signature = task->dtb_signature ? task->dtb_signature->local : NULL;
> +	}
> +#endif
> +
>  	run_boot_hooks(task);
>  
>  	update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO,
>  			_("performing kexec_load"));
>  
>  	rc = kexec_load(task);
> -	if (rc) {
> +	if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
> +		update_status(task->status_fn, task->status_arg,
> +				BOOT_STATUS_ERROR, _("signature verification failed"));
> +	}
> +	else if (rc == KEXEC_LOAD_SIG_SETUP_INVALID) {
> +		update_status(task->status_fn, task->status_arg,
> +				BOOT_STATUS_ERROR, _("invalid signature configuration"));
> +	}
> +	else if (rc) {
>  		update_status(task->status_fn, task->status_arg,
>  				BOOT_STATUS_ERROR, _("kexec load failed"));
>  	}
>  
> +no_sig_load:
> +	cleanup_load(task->image_signature);
> +	cleanup_load(task->initrd_signature);
> +	cleanup_load(task->dtb_signature);
> +
>  no_load:
>  	cleanup_load(task->image);
>  	cleanup_load(task->initrd);
> @@ -434,6 +749,19 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  	const char *boot_desc;
>  	int rc;
>  
> +#if defined(HAVE_LIBGPGME)
> +	struct pb_url *image_sig = NULL, *initrd_sig = NULL, *dtb_sig = NULL;
> +
> +	int max_filename_size = 8192;
> +	char kernel_signature_filename[max_filename_size];
> +	char initrd_signature_filename[max_filename_size];
> +	char dtb_signature_filename[max_filename_size];
> +
> +	kernel_signature_filename[0] = 0;
> +	initrd_signature_filename[0] = 0;
> +	dtb_signature_filename[0] = 0;
> +#endif
> +
>  	if (opt && opt->option->name)
>  		boot_desc = opt->option->name;
>  	else if (cmd && cmd->boot_image_file)
> @@ -471,6 +799,12 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  	boot_task->dry_run = dry_run;
>  	boot_task->status_fn = status_fn;
>  	boot_task->status_arg = status_arg;
> +#if defined(HAVE_LIBGPGME)
> +	if (access(LOCKDOWN_FILE, F_OK) == -1)
> +		boot_task->verify_signature = false;
> +	else
> +		boot_task->verify_signature = true;
> +#endif
>  
>  	if (cmd && cmd->boot_args) {
>  		boot_task->args = talloc_strdup(boot_task, cmd->boot_args);
> @@ -485,6 +819,41 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  	rc = start_url_load(boot_task, "kernel image", image, &boot_task->image)
>  	  || start_url_load(boot_task, "initrd", initrd, &boot_task->initrd)
>  	  || start_url_load(boot_task, "dtb", dtb, &boot_task->dtb);
> +#if defined(HAVE_LIBGPGME)
> +	if (boot_task->verify_signature) {
> +		/* Generate names of associated signature files and load */
> +		if (image) {
> +			snprintf(kernel_signature_filename, max_filename_size, "%s%s", image->file, ".sig");
> +			image_sig = pb_url_copy(ctx, image);
> +			talloc_free(image_sig->file);
> +			image_sig->file = talloc_strdup(image_sig, kernel_signature_filename);
> +			snprintf(kernel_signature_filename, max_filename_size, "%s%s", image->path, ".sig");
> +			talloc_free(image_sig->path);
> +			image_sig->path = talloc_strdup(image_sig, kernel_signature_filename);
> +			rc |= start_url_load(boot_task, "kernel image signature", image_sig, &boot_task->image_signature);

In other spots too, but keep to an 80-character limit

> +		}
> +		if (initrd) {
> +			snprintf(initrd_signature_filename, max_filename_size, "%s%s", initrd->file, ".sig");
> +			initrd_sig = pb_url_copy(ctx, initrd);
> +			talloc_free(initrd_sig->file);
> +			initrd_sig->file = talloc_strdup(initrd_sig, initrd_signature_filename);
> +			snprintf(initrd_signature_filename, max_filename_size, "%s%s", initrd->path, ".sig");
> +			talloc_free(initrd_sig->path);
> +			initrd_sig->path = talloc_strdup(initrd_sig, initrd_signature_filename);
> +			rc |= start_url_load(boot_task, "initrd signature", initrd_sig, &boot_task->initrd_signature);
> +		}
> +		if (dtb) {
> +			snprintf(dtb_signature_filename, max_filename_size, "%s%s", dtb->file, ".sig");
> +			dtb_sig = pb_url_copy(ctx, dtb);
> +			talloc_free(dtb_sig->file);
> +			dtb_sig->file = talloc_strdup(dtb_sig, dtb_signature_filename);
> +			snprintf(dtb_signature_filename, max_filename_size, "%s%s", dtb->path, ".sig");
> +			talloc_free(dtb_sig->path);
> +			dtb_sig->path = talloc_strdup(dtb_sig, dtb_signature_filename);
> +			rc |= start_url_load(boot_task, "dtb signature", dtb_sig, &boot_task->dtb_signature);
> +		}
> +	}
> +#endif
>  
>  	/* If all URLs are local, we may be done. */
>  	if (rc) {
> diff --git a/lib/url/url.c b/lib/url/url.c
> index 7202f49..6eeced3 100644
> --- a/lib/url/url.c
> +++ b/lib/url/url.c
> @@ -246,7 +246,7 @@ static void pb_url_update_full(struct pb_url *url)
>  	url->full = pb_url_to_string(url);
>  }
>  
> -static struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url)
> +struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url)
>  {
>  	struct pb_url *new_url;
>  
> diff --git a/lib/url/url.h b/lib/url/url.h
> index 25e1ad8..9043615 100644
> --- a/lib/url/url.h
> +++ b/lib/url/url.h
> @@ -62,6 +62,7 @@ struct pb_url {
>  
>  bool is_url(const char *str);
>  struct pb_url *pb_url_parse(void *ctx, const char *url_str);
> +struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url);
>  struct pb_url *pb_url_join(void *ctx, const struct pb_url *url, const char *s);
>  char *pb_url_to_string(struct pb_url *url);
>  
> diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
> index 3f1e0a2..0d027a8 100644
> --- a/ui/ncurses/nc-cui.c
> +++ b/ui/ncurses/nc-cui.c
> @@ -746,6 +746,9 @@ static struct pmenu *main_menu_init(struct cui *cui)
>  	struct pmenu_item *i;
>  	struct pmenu *m;
>  	int result;
> +	bool lockdown = false;
> +	if (access(LOCKDOWN_FILE, F_OK) != -1)
> +		lockdown = true;
>  
>  	m = pmenu_init(cui, 7, cui_on_exit);
>  	if (!m) {
> @@ -789,9 +792,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);
> +	}

The problem here is that while the "Exit to shell" button is replaced,
users can still exit with 'x' or potentially some other key combination
that causes ncurses to exit.
Additionally it looks like the "Reboot" button just calls the exit
callback anyway :)

>  
>  	result = pmenu_setup(m);
>  



More information about the Petitboot mailing list