[1/3,V5] Add support for GPG signature enforcement on booted
diff mbox

Message ID 1323056151.64551.1471055747097.JavaMail.zimbra@raptorengineeringinc.com
State Superseded
Headers show

Commit Message

Timothy Pearson Aug. 13, 2016, 2:35 a.m. UTC
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.

Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
---
 configure.ac                  |  64 +++++++
 discover/Makefile.am          |  12 +-
 discover/boot.c               | 155 ++++++++++++++---
 discover/boot.h               |  30 ++++
 discover/device-handler.c     |   6 +
 discover/device-handler.h     |   1 +
 discover/gpg.c                | 381 ++++++++++++++++++++++++++++++++++++++++++
 discover/gpg.h                |  73 ++++++++
 discover/grub2/builtins.c     |   8 +
 discover/kboot-parser.c       |   6 +
 discover/pxe-parser.c         |   7 +
 discover/user-event.c         |  16 +-
 discover/yaboot-parser.c      |   7 +
 lib/pb-protocol/pb-protocol.c |  13 ++
 lib/types/types.h             |   2 +
 ui/common/discover-client.c   |   1 +
 ui/common/discover-client.h   |   1 +
 ui/ncurses/nc-boot-editor.c   |  51 +++++-
 ui/ncurses/nc-cui.c           |   2 +
 19 files changed, 807 insertions(+), 29 deletions(-)
 create mode 100644 discover/gpg.c
 create mode 100644 discover/gpg.h

Comments

Murilo Opsfelder Araujo Aug. 16, 2016, 6:50 p.m. UTC | #1
On 08/12/2016 11:35 PM, Timothy Pearson wrote:
[...]
> diff --git a/configure.ac b/configure.ac
> index 00a6113..370511b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -170,6 +170,70 @@ 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"])

Is there any reason to define macro WITH_SIGNED_BOOT twice with
different indentation?  Or the macros are different and I'm overlooking
them?
Timothy Pearson Aug. 16, 2016, 9:14 p.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/16/2016 01:50 PM, Murilo Opsfelder Araújo wrote:
> On 08/12/2016 11:35 PM, Timothy Pearson wrote:
> [...]
>> diff --git a/configure.ac b/configure.ac
>> index 00a6113..370511b 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -170,6 +170,70 @@ 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"])
> 
> Is there any reason to define macro WITH_SIGNED_BOOT twice with
> different indentation?  Or the macros are different and I'm overlooking
> them?
> 

No, I'm thinking this happened without my knowledge during a rebase.
It's exactly the kind of glitch that tends to happen (old line not
wrapped, new line wrapped before rebase).

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXs4IiAAoJEK+E3vEXDOFb9FcIALHkr4tsqfW8WXzkoXX0i5NS
3UX2S8OMhbEWwUezVEss6krq22I6cY2ExKoMNwlbEUwErWg5+vp+Wj5RCa4q5+9c
ZhwodMayYz7LHJ738rD3FfR8hQnKvWXg1EP3nfGBhISvQ8Vyw1jZNg5Z4P27BHl8
NK8X/R9XDzROiCsXiMGFpZfxrdp6TIEbkQp+m3agT9qMyUiShiduuEvvBVmgcago
tXPEDLuNigLCWrgwrrY9ATB+3Yjy0f/yIpCfiXqsYgDlFjwAoC1ubODaIystUIAm
KJiPor78nVCgHdSvDOKzsmr7YCD6sOicgG++hrVbZliiZgoic7wQYEVbPmHgl0M=
=A7DQ
-----END PGP SIGNATURE-----
Timothy Pearson Aug. 16, 2016, 10:29 p.m. UTC | #3
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/16/2016 04:14 PM, Timothy Pearson wrote:
> On 08/16/2016 01:50 PM, Murilo Opsfelder Araújo wrote:
>> Is there any reason to define macro WITH_SIGNED_BOOT twice with
>> different indentation?  Or the macros are different and I'm overlooking
>> them?
> 
> 
> No, I'm thinking this happened without my knowledge during a rebase.
> It's exactly the kind of glitch that tends to happen (old line not
> wrapped, new line wrapped before rebase).
> 

Fixed in patch V6.

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXs5PEAAoJEK+E3vEXDOFbhCEIAJJucSX5sT+n61zpHxJs1bdE
np09Rg9+IzqsVM2qa0M99GJ9ku1tl+KCvd/Yinjs3zr47BFnaRgS08ilj2+9YVPo
6koWNugQ2ryZv3f7almOYFhA0Pmcr92xGGmgRh+GP4nLFvxGvF8ZnZ/s3qNqBN39
R+8Q6dl7YAS32nJb4mS/3rlRBtKpooHfSUrjMZBRkdZz6fsbBANNIT3NytTKu4SR
BFoZBaFb73AfT24Lu1DajlQRYGaYK14Tc6zzef8hdqQVKtEKEutdefWYuETpMFkB
XDuDkeAI8fJRhdf+RhArwkHLivDqPYo154GYkb5ZpE0Ef+9MyPuCVlhSLAKkd4Q=
=sb+Q
-----END PGP SIGNATURE-----
Samuel Mendoza-Jonas Aug. 17, 2016, 3:46 a.m. UTC | #4
On Fri, 2016-08-12 at 21:35 -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.
> 
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> ---
>  configure.ac                  |  64 +++++++
>  discover/Makefile.am          |  12 +-
>  discover/boot.c               | 155 ++++++++++++++---
>  discover/boot.h               |  30 ++++
>  discover/device-handler.c     |   6 +
>  discover/device-handler.h     |   1 +
>  discover/gpg.c                | 381 ++++++++++++++++++++++++++++++++++++++++++
>  discover/gpg.h                |  73 ++++++++
>  discover/grub2/builtins.c     |   8 +
>  discover/kboot-parser.c       |   6 +
>  discover/pxe-parser.c         |   7 +
>  discover/user-event.c         |  16 +-
>  discover/yaboot-parser.c      |   7 +
>  lib/pb-protocol/pb-protocol.c |  13 ++
>  lib/types/types.h             |   2 +
>  ui/common/discover-client.c   |   1 +
>  ui/common/discover-client.h   |   1 +
>  ui/ncurses/nc-boot-editor.c   |  51 +++++-
>  ui/ncurses/nc-cui.c           |   2 +
>  19 files changed, 807 insertions(+), 29 deletions(-)
>  create mode 100644 discover/gpg.c
>  create mode 100644 discover/gpg.h
> 
> diff --git a/configure.ac b/configure.ac
> index 00a6113..370511b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -170,6 +170,70 @@ 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"
> +	]
> +)
> +
> +AM_CONDITIONAL([WITH_GPGME], [test "x$with_signed_boot" = "xyes"])
> +
> +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 899c9a6..4a74da3 100644
> --- a/discover/Makefile.am
> +++ b/discover/Makefile.am
> @@ -15,6 +15,12 @@
>  sbin_PROGRAMS += discover/pb-discover
>  noinst_PROGRAMS += discover/platform.ro
>  
> +if WITH_GPGME
> +gpg_int_SOURCES = discover/gpg.c
> +else
> +gpg_int_SOURCES =
> +endif
> +
>  discover_pb_discover_SOURCES = \
>  	discover/boot.c \
>  	discover/boot.h \
> @@ -52,13 +58,15 @@ discover_pb_discover_SOURCES = \
>  	discover/user-event.h \
>  	discover/kboot-parser.c \
>  	discover/yaboot-parser.c \
> -	discover/pxe-parser.c
> +	discover/pxe-parser.c \
> +	$(gpg_int_SOURCES)
>  
>  discover_pb_discover_LDADD = \
>  	discover/grub2/grub2-parser.ro \
>  	discover/platform.ro \
>  	$(core_lib) \
> -	$(UDEV_LIBS)
> +	$(UDEV_LIBS) \
> +	$(GPGME_LIBS)
>  
>  discover_pb_discover_LDFLAGS = \
>  	$(AM_LDFLAGS) \
> diff --git a/discover/boot.c b/discover/boot.c
> index ba6ce25..38be536 100644
> --- a/discover/boot.c
> +++ b/discover/boot.c
> @@ -25,6 +25,7 @@
>  #include "paths.h"
>  #include "resource.h"
>  #include "platform.h"
> +#include "gpg.h"
>  
>  static const char *boot_hook_dir = PKG_SYSCONF_DIR "/boot.d";
>  enum {
> @@ -32,21 +33,6 @@ enum {
>  	BOOT_HOOK_EXIT_UPDATE	= 2,
>  };
>  
> -struct boot_task {
> -	struct load_url_result *image;
> -	struct load_url_result *initrd;
> -	struct load_url_result *dtb;
> -	const char *local_image;
> -	const char *local_initrd;
> -	const char *local_dtb;
> -	const char *args;
> -	const char *boot_tty;
> -	boot_status_fn status_fn;
> -	void *status_arg;
> -	bool dry_run;
> -	bool cancelled;
> -};
> -
>  /**
>   * kexec_load - kexec load helper.
>   */
> @@ -59,20 +45,33 @@ 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 ((result = gpg_validate_boot_files(boot_task, &local_initrd,

Since we pass boot_task here, is there any need to define the const
chars above? gpg_validate_boot_files can just use boot_task->local_image
etc, and then local_image/etc can be normal char pointers that are
allocated by gpg_validate_boot_files.


> +		&local_dtb, &local_image))) {
> +		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> +			pb_log("%s: Aborting kexec due to"
> +				" signature verification failure\n", __func__);
> +			goto abort_kexec;
> +		}
> +	}
> +
>  	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 */
>  	}
> @@ -84,7 +83,7 @@ static int kexec_load(struct boot_task *boot_task)
>  		*p++ = s_args;		/* 5 */
>  	}
>  
> -	*p++ = boot_task->local_image;	/* 6 */
> +	*p++ = local_image;	/* 6 */

nitpick - indentation

>  	*p++ = NULL;			/* 7 */
>  
>  	result = process_run_simple_argv(boot_task, argv);
> @@ -92,6 +91,10 @@ static int kexec_load(struct boot_task *boot_task)
>  	if (result)
>  		pb_log("%s: failed: (%d)\n", __func__, result);
>  
> +abort_kexec:
> +	gpg_validate_boot_files_cleanup(boot_task, &local_initrd, &local_dtb,
> +					&local_image);
> +
>  	return result;
>  }
>  
> @@ -378,23 +381,69 @@ static void boot_process(struct load_url_result *result, void *data)
>  			check_load(task, "dtb", task->dtb))
>  		goto no_load;
>  
> +	if (task->verify_signature) {
> +		if (load_pending(task->image_signature) ||
> +				load_pending(task->initrd_signature) ||
> +				load_pending(task->dtb_signature) ||
> +				load_pending(task->cmdline_signature))
> +			return;
> +
> +		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) ||
> +				check_load(task, "command line signature",
> +					task->cmdline_signature))
> +			goto no_sig_load;
> +	}
> +
>  	/* 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 (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;
> +		task->local_cmdline_signature = task->cmdline_signature ?
> +			task->cmdline_signature->local : NULL;
> +	}
> +
>  	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, _("kexec load failed"));
> +				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);
> +	cleanup_load(task->cmdline_signature);
> +
>  no_load:
>  	cleanup_load(task->image);
>  	cleanup_load(task->initrd);
> @@ -435,6 +484,8 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  		boot_status_fn status_fn, void *status_arg)
>  {
>  	struct pb_url *image = NULL, *initrd = NULL, *dtb = NULL;
> +	struct pb_url *image_sig = NULL, *initrd_sig = NULL, *dtb_sig = NULL,
> +		*cmdline_sig = NULL;
>  	const struct config *config;
>  	struct boot_task *boot_task;
>  	const char *boot_desc;
> @@ -478,6 +529,8 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  	boot_task->status_fn = status_fn;
>  	boot_task->status_arg = status_arg;
>  
> +	boot_task->verify_signature = (lockdown_status() == PB_LOCKDOWN_SIGN);
> +
>  	if (cmd && cmd->boot_args) {
>  		boot_task->args = talloc_strdup(boot_task, cmd->boot_args);
>  	} else if (opt && opt->option->boot_args) {
> @@ -494,11 +547,69 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  		boot_task->boot_tty = config ? config->boot_tty : NULL;
>  	}
>  
> +	if (boot_task->verify_signature) {
> +		if (cmd && cmd->args_sig_file) {
> +			cmdline_sig = pb_url_parse(opt, cmd->args_sig_file);
> +		} else if (opt && opt->args_sig_file) {
> +			cmdline_sig = opt->args_sig_file->url;
> +		} else {
> +			pb_log("%s: no command line signature file"
> +				" specified\n", __func__);
> +			update_status(status_fn, status_arg, BOOT_STATUS_INFO,
> +					_("Boot failed: no command line"
> +						" signature file specified"));
> +			return NULL;

boot_task should probably get freed here.

> +		}
> +	}
> +
>  	/* start async loads for boot resources */
>  	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 (boot_task->verify_signature) {
> +		/* Generate names of associated signature files and load */
> +		if (image) {
> +			image_sig = pb_url_copy(ctx, image);
> +			talloc_free(image_sig->file);
> +			image_sig->file = talloc_asprintf(image_sig,
> +				"%s.sig", image->file);
> +			talloc_free(image_sig->path);
> +			image_sig->path = talloc_asprintf(image_sig,
> +				"%s.sig", image->path);

This block is the same for each image/initrd/dtb, could it be worth it
to add a function to gpg.c like

			image_sig = gpg_set_signature(ctx, image) ?

> +			rc |= start_url_load(boot_task,
> +				"kernel image signature", image_sig,
> +				&boot_task->image_signature);
> +		}
> +		if (initrd) {
> +			initrd_sig = pb_url_copy(ctx, initrd);
> +			talloc_free(initrd_sig->file);
> +			initrd_sig->file = talloc_asprintf(initrd_sig,
> +				"%s.sig", initrd->file);
> +			talloc_free(initrd_sig->path);
> +			initrd_sig->path = talloc_asprintf(initrd_sig,
> +				"%s.sig", initrd->path);
> +			rc |= start_url_load(boot_task, "initrd signature",
> +				initrd_sig, &boot_task->initrd_signature);
> +		}
> +		if (dtb) {
> +			dtb_sig = pb_url_copy(ctx, dtb);
> +			talloc_free(dtb_sig->file);
> +			dtb_sig->file = talloc_asprintf(dtb_sig,
> +				"%s.sig", dtb->file);
> +			talloc_free(dtb_sig->path);
> +			dtb_sig->path = talloc_asprintf(dtb_sig,
> +				"%s.sig", dtb->path);
> +			rc |= start_url_load(boot_task, "dtb signature",
> +				dtb_sig, &boot_task->dtb_signature);
> +		}
> +
> +		rc |= start_url_load(boot_task,
> +			"kernel command line signature", cmdline_sig,
> +			&boot_task->cmdline_signature);
> +	}
> +
> +	/* If all URLs are local, we may be done. */
>  	if (rc) {
>  		/* Don't call boot_cancel() to preserve the status update */
>  		boot_task->cancelled = true;
> diff --git a/discover/boot.h b/discover/boot.h
> index ec61703..5f6e874 100644
> --- a/discover/boot.h
> +++ b/discover/boot.h
> @@ -11,4 +11,34 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  		boot_status_fn status_fn, void *status_arg);
>  
>  void boot_cancel(struct boot_task *task);
> +
> +struct boot_task {
> +	struct load_url_result *image;
> +	struct load_url_result *initrd;
> +	struct load_url_result *dtb;
> +	const char *local_image;
> +	const char *local_initrd;
> +	const char *local_dtb;
> +	const char *args;
> +	const char *boot_tty;
> +	boot_status_fn status_fn;
> +	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;
> +};
> +
> +enum {
> +	KEXEC_LOAD_SIG_SETUP_INVALID = 253,
> +	KEXEC_LOAD_SIGNATURE_FAILURE = 254,
> +};
> +
>  #endif /* _BOOT_H */
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index c31fcea..f6b6d22 100644
> --- a/discover/device-handler.c
> +++ b/discover/device-handler.c
> @@ -613,6 +613,7 @@ static bool __attribute__((used)) boot_option_is_resolved(
>  	return resource_is_resolved(opt->boot_image) &&
>  		resource_is_resolved(opt->initrd) &&
>  		resource_is_resolved(opt->dtb) &&
> +		resource_is_resolved(opt->args_sig_file) &&
>  		resource_is_resolved(opt->icon);
>  }
>  
> @@ -638,6 +639,8 @@ static bool boot_option_resolve(struct discover_boot_option *opt,
>  	return resource_resolve(opt->boot_image, "boot_image", opt, handler) &&
>  		resource_resolve(opt->initrd, "initrd", opt, handler) &&
>  		resource_resolve(opt->dtb, "dtb", opt, handler) &&
> +		resource_resolve(opt->args_sig_file, "args_sig_file", opt,
> +			handler) &&
>  		resource_resolve(opt->icon, "icon", opt, handler);
>  }
>  
> @@ -652,6 +655,7 @@ static void boot_option_finalise(struct device_handler *handler,
>  	assert(!opt->option->dtb_file);
>  	assert(!opt->option->icon_file);
>  	assert(!opt->option->device_id);
> +	assert(!opt->option->args_sig_file);
>  
>  	if (opt->boot_image)
>  		opt->option->boot_image_file = opt->boot_image->url->full;
> @@ -661,6 +665,8 @@ static void boot_option_finalise(struct device_handler *handler,
>  		opt->option->dtb_file = opt->dtb->url->full;
>  	if (opt->icon)
>  		opt->option->icon_file = opt->icon->url->full;
> +	if (opt->args_sig_file)
> +		opt->option->args_sig_file = opt->args_sig_file->url->full;
>  
>  	opt->option->device_id = opt->device->device->id;
>  
> diff --git a/discover/device-handler.h b/discover/device-handler.h
> index b6f9fd5..f785ccc 100644
> --- a/discover/device-handler.h
> +++ b/discover/device-handler.h
> @@ -48,6 +48,7 @@ struct discover_boot_option {
>  	struct resource		*boot_image;
>  	struct resource		*initrd;
>  	struct resource		*dtb;
> +	struct resource		*args_sig_file;
>  	struct resource		*icon;
>  };
>  
> diff --git a/discover/gpg.c b/discover/gpg.c
> new file mode 100644
> index 0000000..e1c1d04
> --- /dev/null
> +++ b/discover/gpg.c
> @@ -0,0 +1,381 @@
> +/*
> + *  Copyright (C) 2016 Raptor Engineering, LLC
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#if defined(HAVE_CONFIG_H)
> +#include "config.h"
> +#endif
> +
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <assert.h>
> +#include <dirent.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +
> +#include <log/log.h>
> +#include <talloc/talloc.h>
> +#include <url/url.h>
> +#include <util/util.h>
> +#include <i18n/i18n.h>
> +
> +#include "gpg.h"
> +
> +int copy_file_to_destination(const char * source_file,
> +	char * destination_file, int max_dest_filename_size) {

nitpick - char *destination_file, etc

> +	int result = 0;
> +	char template[21] = "/tmp/petitbootXXXXXX";

Don't think you need to explicitly set the array length here.

> +	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
> +		pb_log("%s: failed: unable to open source file '%s'\n",
> +			__func__, source_file);
> +		result = 1;

Probably best to bail out straight away here.

> +	}
> +
> +	size_t l1;
> +	unsigned char buffer[FILE_XFER_BUFFER_SIZE];

talloc() this instead?

> +
> +	/* 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;

Change these last few debug errors to -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__);

Why not just break; here? It looks like even if there's an error we'll
continue to read the source file until finished.

> +			}
> +		}
> +	}
> +
> +	if (result) {
> +		destination_file[0] = '\0';
> +	}
> +	else {
> +		ssize_t r;
> +		char readlink_buffer[MAX_FILENAME_SIZE];
> +		snprintf(readlink_buffer, MAX_FILENAME_SIZE, "/proc/self/fd/%d",
> +			destination_fd);
> +		r = readlink(readlink_buffer, destination_file,
> +			max_dest_filename_size);
> +		if (r < 0) {
> +			/* readlink failed */
> +			result = 3;

Debug error

> +			pb_log("%s: failed: unable to obtain temporary filename"
> +				"\n", __func__);
> +		}
> +		destination_file[r] = '\0';
> +	}
> +
> +	fclose(source_handle);
> +	fclose(destination_handle);
> +
> +	return result;
> +}
> +
> +int verify_file_signature(const char * plaintext_filename,
> +	const char * signature_filename, FILE * authorized_signatures_handle,
> +	const char * keyring_path)

const char *variable

> +{
> +	int valid = 0;
> +
> +	if (signature_filename == NULL)
> +		return -1;
> +
> +	gpgme_signature_t verification_signatures;
> +	gpgme_verify_result_t verification_result;
> +	gpgme_data_t plaintext_data;
> +	gpgme_data_t signature_data;
> +	gpgme_engine_info_t enginfo;
> +	gpgme_ctx_t gpg_context;
> +	gpgme_error_t err;

nitpick - put these before the if (signature..) check

> +
> +	/* 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__);
> +		return -1;
> +	}
> +	err = gpgme_get_engine_info(&enginfo);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG engine failed to initialize\n", __func__);
> +		return -1;
> +	}
> +	err = gpgme_new(&gpg_context);
> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: GPG context could not be created\n", __func__);
> +		return -1;
> +	}
> +	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__);
> +		return -1;
> +	}
> +	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);

Is there ever a time you expect to not have keyring_path?

> +	if (err != GPG_ERR_NO_ERROR) {
> +		pb_log("%s: Could not set GPG engine information\n", __func__);
> +		return -1;
> +	}
> +	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);
> +		return -1;
> +	}
> +	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);
> +		return -1;
> +	}
> +
> +	/* 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 file using GPG signature '%s'\n",
> +			__func__, signature_filename);
> +		return -1;
> +	}
> +	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;

If I'm reading this right, could you instead do
	if (strncmp(auth_sig_line, verfication_signatures->fpr,
		    	strlen(verification_signatures->fpr) == 0)
and skip the while loop?

> +			}
> +			free(auth_sig_line);
> +		}
> +		else {

Would it be worth doing
	if (verification_signatures->status != GPG_ERR_NO_ERROR) {
		pb_log(...);
		continue;
	}
first instead to save yourself an indent of the above block?

> +			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__);
> +		return -1;
> +	}
> +	else {

Don't need this else since we return above

> +		pb_log("%s: GPG signature '%s' for file '%s' verified\n",
> +			__func__, signature_filename, plaintext_filename);
> +	}
> +
> +	return 0;
> +}
> +
> +int gpg_validate_boot_files(struct boot_task *boot_task,
> +	const char** local_initrd, const char** local_dtb,
> +	const char** local_image) {

Assume I nitpick the rest of these too :)

> +	int result = 0;
> +
> +	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;
> +
> +	if (boot_task->verify_signature) {

Since the else case here is do nothing, it looks neater to just do

	if (!boot_task->verify_signature)
		return result;

And save an indent on the rest of the function.

> +		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;

Can just do
	return KEXEC_LOAD_SIG_SETUP_INVALID;

> +		}
> +
> +		/* Copy files to temporary directory for verification / boot */
> +		result = copy_file_to_destination(boot_task->local_image,
> +			kernel_filename, MAX_FILENAME_SIZE);

I wonder if it might not be nicer to just pass a pointer and have
copy_file_to_destination() talloc_strdup() the resulting filename. Then
we don't need to allocate 8*3K for all the names and can drop
MAX_FILENAME_SIZE. Thoughts?

> +		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 gpg_validate_boot_files() fails we'll call
gpg_validate_boot_files_cleanup() afterwards, so I think we're safe to
skip unlink()-ing local_image and local_initrd at this point.

> +			}
> +		}
> +		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 = talloc_strdup(boot_task,
> +			kernel_filename);
> +		if (boot_task->local_initrd)
> +			*local_initrd = talloc_strdup(boot_task,
> +				initrd_filename);
> +		if (boot_task->local_dtb)
> +			*local_dtb = talloc_strdup(boot_task,
> +				dtb_filename);
> +
> +		/* Write command line to temporary file for verification */
> +		char cmdline_template[21] = "/tmp/petitbootXXXXXX";
> +		int cmdline_fd = mkstemp(cmdline_template);
> +		FILE *cmdline_handle = NULL;

(especially if we change to if (!verify_signature) as above), declare
these variables at the start of the function.

> +		if (cmdline_fd < 0) {
> +			// handle mkstemp error

style nitpick - use /* blah */ instead of // blah

> +			pb_log("%s: failed: unable to create command line"
> +				" temporary file for verification\n",
> +				__func__);
> +			result = -1;
> +		}
> +		else {
> +			cmdline_handle = fdopen(cmdline_fd, "w");
> +		}
> +		if (!cmdline_handle) {
> +			// handle open error
> +			pb_log("%s: failed: unable to write command line"
> +				" temporary file for verification\n",
> +				__func__);
> +			result = -1;
> +		}
> +		else {
> +			fwrite(boot_task->args, sizeof(char),
> +				strlen(boot_task->args), cmdline_handle);
> +			fflush(cmdline_handle);
> +		}
> +
> +		/* Check signatures */
> +		if (verify_file_signature(kernel_filename,
> +			local_image_signature,
> +			authorized_signatures_handle, "/etc/gpg"))
> +			result = KEXEC_LOAD_SIGNATURE_FAILURE;
> +		if (verify_file_signature(cmdline_template,
> +			local_cmdline_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;
> +
> +		/* Clean up */
> +		if (cmdline_handle) {
> +			fclose(cmdline_handle);
> +			unlink(cmdline_template);
> +		}
> +		fclose(authorized_signatures_handle);
> +	}
> +
> +	return result;
> +}
> +
> +void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
> +	const char** local_initrd, const char** local_dtb,
> +	const char** local_image) {
> +	if (boot_task->verify_signature) {
> +		unlink(*local_image);
> +		if (*local_initrd)
> +			unlink(*local_initrd);
> +		if (*local_dtb)
> +			unlink(*local_dtb);
> +
> +		talloc_free((char*)*local_image);
> +		if (*local_initrd)
> +			talloc_free((char*)*local_initrd);
> +		if (*local_dtb)
> +			talloc_free((char*)*local_dtb);
> +	}

Related to a comment below, neater if these aren't const chars and
the casts can be dropped.

> +}
> +
> +int lockdown_status() {
> +	if (access(LOCKDOWN_FILE, F_OK) == -1)
> +		return PB_LOCKDOWN_NONE;
> +	else
> +		return PB_LOCKDOWN_SIGN;
> +}
> \ No newline at end of file
> diff --git a/discover/gpg.h b/discover/gpg.h
> new file mode 100644
> index 0000000..9b452b8
> --- /dev/null
> +++ b/discover/gpg.h
> @@ -0,0 +1,73 @@
> +/*
> + *  Copyright (C) 2016 Raptor Engineering, LLC
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#ifndef _PB_GPG_H
> +#define _PB_GPG_H
> +
> +#define MAX_FILENAME_SIZE	8192
> +#define FILE_XFER_BUFFER_SIZE	8192
> +
> +#include "boot.h"
> +
> +enum {
> +	PB_LOCKDOWN_NONE	= 0,
> +	PB_LOCKDOWN_SIGN	= 1,
> +};
> +
> +#if defined(HAVE_LIBGPGME)
> +#include <gpgme.h>
> +#endif /* HAVE_LIBGPGME */
> +
> +int lockdown_status(void);
> +
> +int copy_file_to_destination(const char * source_file,
> +	char * destination_file, int max_dest_filename_size);
> +
> +int verify_file_signature(const char * plaintext_filename,
> +	const char * signature_filename, FILE * authorized_signatures_handle,
> +	const char * keyring_path);
> +
> +int gpg_validate_boot_files(struct boot_task *boot_task,
> +	const char** local_initrd, const char** local_dtb,
> +	const char** local_image);
> +
> +void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
> +	const char** local_initrd, const char** local_dtb,
> +	const char** local_image);
> +
> +#if !defined(HAVE_LIBGPGME)
> +
> +int lockdown_status(void) { return PB_LOCKDOWN_NONE; }
> +
> +int copy_file_to_destination(const char * source_file,
> +	char * destination_file, int max_dest_filename_size) { return -1; }
> +
> +int verify_file_signature(const char * plaintext_filename,
> +	const char * signature_filename, FILE * authorized_signatures_handle,
> +	const char * keyring_path) { return -1; }
> +
> +int gpg_validate_boot_files(struct boot_task *boot_task,
> +	const char** local_initrd, const char** local_dtb,
> +	const char** local_image) { return 0; }
> +
> +void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
> +	const char** local_initrd, const char** local_dtb,
> +	const char** local_image) { }
> +
> +#endif /* HAVE_LIBGPGME */
> +
> +#endif /* _PB_GPG_H */
> \ No newline at end of file
> diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c
> index 8bff732..c16b639 100644
> --- a/discover/grub2/builtins.c
> +++ b/discover/grub2/builtins.c
> @@ -6,7 +6,9 @@
>  #include <types/types.h>
>  #include <talloc/talloc.h>
>  #include <util/util.h>
> +#include <url/url.h>
>  
> +#include "discover/resource.h"
>  #include "discover/parser.h"
>  #include "grub2.h"
>  
> @@ -69,6 +71,12 @@ static int builtin_linux(struct grub2_script *script,
>  		opt->option->boot_args = talloc_asprintf_append(
>  						opt->option->boot_args,
>  						" %s", argv[i]);
> +
> +	char* args_sigfile_default = talloc_asprintf(opt,
> +		"%s.cmdline.sig", argv[1]);
> +	opt->args_sig_file = create_grub2_resource(opt, script->ctx->device,
> +						root, args_sigfile_default);
> +	talloc_free(args_sigfile_default);
>  	return 0;
>  }
>  
> diff --git a/discover/kboot-parser.c b/discover/kboot-parser.c
> index cebe787..f7f75e0 100644
> --- a/discover/kboot-parser.c
> +++ b/discover/kboot-parser.c
> @@ -96,6 +96,12 @@ out_add:
>  	d_opt->boot_image = create_devpath_resource(d_opt,
>  				conf->dc->device, value);
>  
> +	char* args_sigfile_default = talloc_asprintf(d_opt,
> +		"%s.cmdline.sig", value);
> +	d_opt->args_sig_file = create_devpath_resource(d_opt,
> +				conf->dc->device, args_sigfile_default);
> +	talloc_free(args_sigfile_default);
> +
>  	if (root) {
>  		opt->boot_args = talloc_asprintf(opt, "root=%s %s", root, args);
>  		talloc_free(args);
> diff --git a/discover/pxe-parser.c b/discover/pxe-parser.c
> index 4481e5d..4aae8b1 100644
> --- a/discover/pxe-parser.c
> +++ b/discover/pxe-parser.c
> @@ -166,6 +166,13 @@ static void pxe_process_pair(struct conf_context *ctx,
>  		url = pxe_url_join(ctx->dc, ctx->dc->conf_url, value);
>  		opt->boot_image = create_url_resource(opt, url);
>  
> +		char* args_sigfile_default = talloc_asprintf(opt,
> +			"%s.cmdline.sig", value);
> +		url = pxe_url_join(ctx->dc, ctx->dc->conf_url,
> +			args_sigfile_default);
> +		opt->args_sig_file = create_url_resource(opt, url);
> +		talloc_free(args_sigfile_default);
> +
>  	} else if (streq(name, "INITRD")) {
>  		url = pxe_url_join(ctx->dc, ctx->dc->conf_url, value);
>  		opt->initrd = create_url_resource(opt, url);
> diff --git a/discover/user-event.c b/discover/user-event.c
> index 7350b6c..6ea754f 100644
> --- a/discover/user-event.c
> +++ b/discover/user-event.c
> @@ -82,7 +82,7 @@ static void user_event_print_event(struct event __attribute__((unused)) *event)
>  }
>  
>  static struct resource *user_event_resource(struct discover_boot_option *opt,
> -		struct event *event)
> +		struct event *event, bool gen_boot_args_sigfile)

I'm unsure about the changes in user-event. On changing the prototype,
would we be better off adding a new event param rather than adding
gen_boot_args_sigfile.
Since the security model as it stands appears to depend on the user not
accessing the shell, does it even make sense to handle signatures in
user events?

>  {
>  	const char *siaddr, *boot_file;
>  	struct resource *res;
> @@ -101,7 +101,16 @@ static struct resource *user_event_resource(struct discover_boot_option *opt,
>  		return NULL;
>  	}
>  
> -	url_str = talloc_asprintf(opt, "%s%s/%s", "tftp://", siaddr, boot_file);
> +	if (gen_boot_args_sigfile) {
> +		char* args_sigfile_default = talloc_asprintf(opt,
> +			"%s.cmdline.sig", boot_file);
> +		url_str = talloc_asprintf(opt, "%s%s/%s", "tftp://", siaddr,
> +			args_sigfile_default);
> +		talloc_free(args_sigfile_default);
> +	}
> +	else
> +		url_str = talloc_asprintf(opt, "%s%s/%s", "tftp://", siaddr,
> +			boot_file);
>  	url = pb_url_parse(opt, url_str);
>  	talloc_free(url_str);
>  
> @@ -143,12 +152,13 @@ static int parse_user_event(struct discover_context *ctx, struct event *event)
>  	opt->id = talloc_asprintf(opt, "%s#%s", dev->id, val);
>  	opt->name = talloc_strdup(opt, val);
>  
> -	d_opt->boot_image = user_event_resource(d_opt, event);
> +	d_opt->boot_image = user_event_resource(d_opt, event, false);
>  	if (!d_opt->boot_image) {
>  		pb_log("%s: no boot image found for %s!\n", __func__,
>  				opt->name);
>  		goto fail_opt;
>  	}
> +	d_opt->args_sig_file = user_event_resource(d_opt, event, true);
>  
>  	val = event_get_param(event, "rootpath");
>  	if (val) {
> diff --git a/discover/yaboot-parser.c b/discover/yaboot-parser.c
> index aa99392..b62f39d 100644
> --- a/discover/yaboot-parser.c
> +++ b/discover/yaboot-parser.c
> @@ -114,6 +114,13 @@ static void yaboot_finish(struct conf_context *conf)
>  	/* populate the boot option from state data */
>  	state->opt->boot_image = create_yaboot_devpath_resource(state,
>  				conf, state->boot_image);
> +
> +	char* args_sigfile_default = talloc_asprintf(opt,
> +		"%s.cmdline.sig", state->boot_image);
> +	state->opt->args_sig_file = create_yaboot_devpath_resource(state,
> +				conf, args_sigfile_default);
> +	talloc_free(args_sigfile_default);
> +
>  	if (state->initrd) {
>  		state->opt->initrd = create_yaboot_devpath_resource(state,
>  				conf, state->initrd);
> diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
> index 7887fb0..ad7798e 100644
> --- a/lib/pb-protocol/pb-protocol.c
> +++ b/lib/pb-protocol/pb-protocol.c
> @@ -37,6 +37,7 @@
>   *    4-byte len, initrd_file
>   *    4-byte len, dtb_file
>   *    4-byte len, boot_args
> + *    4-byte len, args_sig_file
>   *
>   * action = 0x2: device remove message
>   *  payload:
> @@ -49,6 +50,7 @@
>   *   4-byte len, initrd_file
>   *   4-byte len, dtb_file
>   *   4-byte len, boot_args
> + *   4-byte len, args_sig_file
>   *
>   */
>  
> @@ -72,6 +74,7 @@ void pb_protocol_dump_device(const struct device *dev, const char *text,
>  		fprintf(stream, "%s\t\tinit: %s\n", text, opt->initrd_file);
>  		fprintf(stream, "%s\t\tdtb:  %s\n", text, opt->dtb_file);
>  		fprintf(stream, "%s\t\targs: %s\n", text, opt->boot_args);
> +		fprintf(stream, "%s\t\tasig: %s\n", text, opt->args_sig_file);
>  	}
>  }
>  
> @@ -197,6 +200,7 @@ int pb_protocol_boot_option_len(const struct boot_option *opt)
>  		4 + optional_strlen(opt->initrd_file) +
>  		4 + optional_strlen(opt->dtb_file) +
>  		4 + optional_strlen(opt->boot_args) +
> +		4 + optional_strlen(opt->args_sig_file) +
>  		sizeof(opt->is_default);
>  }
>  
> @@ -207,6 +211,7 @@ int pb_protocol_boot_len(const struct boot_command *boot)
>  		4 + optional_strlen(boot->initrd_file) +
>  		4 + optional_strlen(boot->dtb_file) +
>  		4 + optional_strlen(boot->boot_args) +
> +		4 + optional_strlen(boot->args_sig_file) + 
>  		4 + optional_strlen(boot->tty);
>  }
>  
> @@ -360,6 +365,7 @@ int pb_protocol_serialise_boot_option(const struct boot_option *opt,
>  	pos += pb_protocol_serialise_string(pos, opt->initrd_file);
>  	pos += pb_protocol_serialise_string(pos, opt->dtb_file);
>  	pos += pb_protocol_serialise_string(pos, opt->boot_args);
> +	pos += pb_protocol_serialise_string(pos, opt->args_sig_file);
>  
>  	*(bool *)pos = opt->is_default;
>  	pos += sizeof(bool);
> @@ -380,6 +386,7 @@ int pb_protocol_serialise_boot_command(const struct boot_command *boot,
>  	pos += pb_protocol_serialise_string(pos, boot->initrd_file);
>  	pos += pb_protocol_serialise_string(pos, boot->dtb_file);
>  	pos += pb_protocol_serialise_string(pos, boot->boot_args);
> +	pos += pb_protocol_serialise_string(pos, boot->args_sig_file);
>  	pos += pb_protocol_serialise_string(pos, boot->tty);
>  
>  	assert(pos <= buf + buf_len);
> @@ -750,6 +757,9 @@ int pb_protocol_deserialise_boot_option(struct boot_option *opt,
>  	if (read_string(opt, &pos, &len, &opt->boot_args))
>  		goto out;
>  
> +	if (read_string(opt, &pos, &len, &opt->args_sig_file))
> +		goto out;
> +
>  	if (len < sizeof(bool))
>  		goto out;
>  	opt->is_default = *(bool *)(pos);
> @@ -785,6 +795,9 @@ int pb_protocol_deserialise_boot_command(struct boot_command *cmd,
>  	if (read_string(cmd, &pos, &len, &cmd->boot_args))
>  		goto out;
>  
> +	if (read_string(cmd, &pos, &len, &cmd->args_sig_file))
> +		goto out;
> +
>  	if (read_string(cmd, &pos, &len, &cmd->tty))
>  		goto out;
>  
> diff --git a/lib/types/types.h b/lib/types/types.h
> index 5c5f6ed..6b607cd 100644
> --- a/lib/types/types.h
> +++ b/lib/types/types.h
> @@ -52,6 +52,7 @@ struct boot_option {
>  	char		*initrd_file;
>  	char		*dtb_file;
>  	char		*boot_args;
> +	char		*args_sig_file;
>  	bool		is_default;
>  
>  	struct list_item	list;
> @@ -65,6 +66,7 @@ struct boot_command {
>  	char *initrd_file;
>  	char *dtb_file;
>  	char *boot_args;
> +	char *args_sig_file;
>  	char *tty;
>  };
>  
> diff --git a/ui/common/discover-client.c b/ui/common/discover-client.c
> index 6247dd0..5dbd99b 100644
> --- a/ui/common/discover-client.c
> +++ b/ui/common/discover-client.c
> @@ -312,6 +312,7 @@ static void create_boot_command(struct boot_command *command,
>  	command->initrd_file = data->initrd;
>  	command->dtb_file = data->dtb;
>  	command->boot_args = data->args;
> +	command->args_sig_file = data->args_sig_file;
>  	command->tty = ttyname(STDIN_FILENO);
>  }
>  
> diff --git a/ui/common/discover-client.h b/ui/common/discover-client.h
> index 542a275..59d2df9 100644
> --- a/ui/common/discover-client.h
> +++ b/ui/common/discover-client.h
> @@ -11,6 +11,7 @@ struct pb_boot_data {
>  	char *initrd;
>  	char *dtb;
>  	char *args;
> +	char *args_sig_file;
>  };
>  
>  /**
> diff --git a/ui/ncurses/nc-boot-editor.c b/ui/ncurses/nc-boot-editor.c
> index 4012ec5..9788fef 100644
> --- a/ui/ncurses/nc-boot-editor.c
> +++ b/ui/ncurses/nc-boot-editor.c
> @@ -63,6 +63,8 @@ struct boot_editor {
>  		struct nc_widget_textbox	*dtb_f;
>  		struct nc_widget_label		*args_l;
>  		struct nc_widget_textbox	*args_f;
> +		struct nc_widget_label		*args_sig_file_l;
> +		struct nc_widget_textbox	*args_sig_file_f;
>  		struct nc_widget_button		*ok_b;
>  		struct nc_widget_button		*help_b;
>  		struct nc_widget_button		*cancel_b;
> @@ -73,6 +75,9 @@ struct boot_editor {
>  	char			*initrd;
>  	char			*dtb;
>  	char			*args;
> +	char			*args_sig_file;
> +
> +	bool			use_signature_files;
>  };
>  
>  extern const struct help_text boot_editor_help_text;
> @@ -198,6 +203,12 @@ static struct pb_boot_data *boot_editor_prepare_data(
>  	s = widget_textbox_get_value(boot_editor->widgets.args_f);
>  	bd->args = *s ? talloc_strdup(bd, s) : NULL;
>  
> +	if (boot_editor->use_signature_files) {
> +		s = widget_textbox_get_value(
> +			boot_editor->widgets.args_sig_file_f);
> +		bd->args_sig_file = conditional_prefix(bd, prefix, s);
> +	}
> +
>  	return bd;
>  }
>  
> @@ -323,6 +334,12 @@ static void boot_editor_layout_widgets(struct boot_editor *boot_editor)
>  	y += layout_pair(boot_editor, y, boot_editor->widgets.args_l,
>  					 boot_editor->widgets.args_f);
>  
> +	if (boot_editor->use_signature_files) {
> +		y += layout_pair(boot_editor, y,
> +					boot_editor->widgets.args_sig_file_l,
> +					boot_editor->widgets.args_sig_file_f);
> +	}
> +
>  
>  	y++;
>  	widget_move(widget_button_base(boot_editor->widgets.ok_b), y,
> @@ -445,6 +462,11 @@ static void boot_editor_find_device(struct boot_editor *boot_editor,
>  	if (bd->dtb && !path_on_device(bd_info, bd->dtb))
>  		return;
>  
> +	if (boot_editor->use_signature_files)
> +		if (bd->args_sig_file && !path_on_device(bd_info,
> +			bd->args_sig_file))
> +			return;
> +
>  	/* ok, we match; preselect the device option, and remove the common
>  	 * prefix */
>  	boot_editor->selected_device = bd_info->name;
> @@ -454,6 +476,9 @@ static void boot_editor_find_device(struct boot_editor *boot_editor,
>  		boot_editor->initrd += len;
>  	if (boot_editor->dtb)
>  		boot_editor->dtb += len;
> +	if (boot_editor->use_signature_files)
> +		if (boot_editor->args_sig_file)
> +			boot_editor->args_sig_file += len;
>  }
>  
>  static void boot_editor_setup_widgets(struct boot_editor *boot_editor,
> @@ -501,6 +526,13 @@ static void boot_editor_setup_widgets(struct boot_editor *boot_editor,
>  	boot_editor->widgets.args_f = widget_new_textbox(set, 0, 0,
>  					field_size, boot_editor->args);
>  
> +	if (boot_editor->use_signature_files) {
> +		boot_editor->widgets.args_sig_file_l = widget_new_label(set,
> +				0, 0, _("Argument signature file:"));
> +		boot_editor->widgets.args_sig_file_f = widget_new_textbox(set,
> +				0, 0, field_size, boot_editor->args_sig_file);
> +	}
> +
>  	boot_editor->widgets.ok_b = widget_new_button(set, 0, 0, 10,
>  					_("OK"), ok_click, boot_editor);
>  	boot_editor->widgets.help_b = widget_new_button(set, 0, 0, 10,
> @@ -553,6 +585,15 @@ struct boot_editor *boot_editor_init(struct cui *cui,
>  	if (!boot_editor)
>  		return NULL;
>  
> +#if defined(HAVE_LIBGPGME)
> +	if (access(LOCKDOWN_FILE, F_OK) == -1)
> +		boot_editor->use_signature_files = false;
> +	else
> +		boot_editor->use_signature_files = true;
> +#else
> +	boot_editor->use_signature_files = false;
> +#endif
> +
>  	talloc_set_destructor(boot_editor, boot_editor_destructor);
>  	boot_editor->cui = cui;
>  	boot_editor->item = item;
> @@ -563,9 +604,10 @@ struct boot_editor *boot_editor_init(struct cui *cui,
>  
>  	int ncols1 = strncols(_("Device tree:"));
>  	int ncols2 = strncols(_("Boot arguments:"));
> +	int ncols3 = strncols(_("Argument signature file:"));
>  
>  	boot_editor->label_x = 1;
> -	boot_editor->field_x = 2 + max(ncols1, ncols2);
> +	boot_editor->field_x = 2 + max(max(ncols1, ncols2), ncols3);
>  
>  	nc_scr_init(&boot_editor->scr, pb_boot_editor_sig, 0,
>  			cui, boot_editor_process_key,
> @@ -584,10 +626,15 @@ struct boot_editor *boot_editor_init(struct cui *cui,
>  		boot_editor->initrd = bd->initrd;
>  		boot_editor->dtb = bd->dtb;
>  		boot_editor->args = bd->args;
> +		if (boot_editor->use_signature_files)
> +			boot_editor->args_sig_file = bd->args_sig_file;
> +		else
> +			boot_editor->args_sig_file = "";
>  		boot_editor_find_device(boot_editor, bd, sysinfo);
>  	} else {
>  		boot_editor->image = boot_editor->initrd =
> -			boot_editor->dtb = boot_editor->args = "";
> +			boot_editor->dtb = boot_editor->args =
> +			boot_editor->args_sig_file = "";
>  	}
>  
>  	boot_editor->pad = newpad(
> diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
> index 0c355cc..09b63b0 100644
> --- a/ui/ncurses/nc-cui.c
> +++ b/ui/ncurses/nc-cui.c
> @@ -543,6 +543,7 @@ static int cui_boot_option_add(struct device *dev, struct boot_option *opt,
>  	cod->bd->initrd = talloc_strdup(cod->bd, opt->initrd_file);
>  	cod->bd->dtb = talloc_strdup(cod->bd, opt->dtb_file);
>  	cod->bd->args = talloc_strdup(cod->bd, opt->boot_args);
> +	cod->bd->args_sig_file = talloc_strdup(cod->bd, opt->args_sig_file);
>  
>  	/* This disconnects items array from menu. */
>  	result = set_menu_items(cui->main->ncm, NULL);
> @@ -566,6 +567,7 @@ static int cui_boot_option_add(struct device *dev, struct boot_option *opt,
>  	pb_log("   image  '%s'\n", cod->bd->image);
>  	pb_log("   initrd '%s'\n", cod->bd->initrd);
>  	pb_log("   args   '%s'\n", cod->bd->args);
> +	pb_log("   argsig '%s'\n", cod->bd->args_sig_file);
>  
>  	/* Re-attach the items array. */
>  	result = set_menu_items(cui->main->ncm, cui->main->items);
Timothy Pearson Aug. 18, 2016, 9:48 a.m. UTC | #5
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/16/2016 10:46 PM, Samuel Mendoza-Jonas wrote:

> Since we pass boot_task here, is there any need to define the const
> chars above? gpg_validate_boot_files can just use boot_task->local_image
> etc, and then local_image/etc can be normal char pointers that are
> allocated by gpg_validate_boot_files.

Fixed in patch V7.

> nitpick - indentation

Fixed in patch V7.

> boot_task should probably get freed here.

Fixed in patch V7.

> This block is the same for each image/initrd/dtb, could it be worth it
> to add a function to gpg.c like
> 
> 			image_sig = gpg_set_signature(ctx, image) ?

Fixed in patch V7.

> Probably best to bail out straight away here.

Fixed in patch V7.

> talloc() this instead?

Fixed in patch V7.

> Change these last few debug errors to -1

Fixed in patch V7.

> Why not just break; here? It looks like even if there's an error we'll
> continue to read the source file until finished.

Fixed in patch V7.

> Debug error

Not entirely sure what this means, but the retcode was change to -1 in
patch V7.

> const char *variable

Fixed in patch V7.

> nitpick - put these before the if (signature..) check

Fixed in patch V7.

> If I'm reading this right, could you instead do
> 	if (strncmp(auth_sig_line, verfication_signatures->fpr,
> 		    	strlen(verification_signatures->fpr) == 0)
> and skip the while loop?

No, this would allow essentially a substring match and could create a
security hole.  Truncating only newline / carriage return characters
from the end of the line does not have this side-effect.

> first instead to save yourself an indent of the above block?

Fixed in patch V7.

> Don't need this else since we return above

Right.  Fixed in patch V7.

> Assume I nitpick the rest of these too :)

Should all be fixed in patch V7, though I imagine some may have still
snuck through.

> Since the else case here is do nothing, it looks neater to just do
> 
> 	if (!boot_task->verify_signature)
> 		return result;
> 
> And save an indent on the rest of the function.

Fixed in patch V7.

> Can just do
> 	return KEXEC_LOAD_SIG_SETUP_INVALID;

Fixed in patch V7.

> I wonder if it might not be nicer to just pass a pointer and have
> copy_file_to_destination() talloc_strdup() the resulting filename. Then
> we don't need to allocate 8*3K for all the names and can drop
> MAX_FILENAME_SIZE. Thoughts?

Agreed.  Fixed in patch V7.

> If gpg_validate_boot_files() fails we'll call
> gpg_validate_boot_files_cleanup() afterwards, so I think we're safe to
> skip unlink()-ing local_image and local_initrd at this point.

I'm somewhat ambivalent on this, so it's been changed in patch V7 to
behave as described.

> (especially if we change to if (!verify_signature) as above), declare
> these variables at the start of the function.

Fixed in patch V7.

> style nitpick - use /* blah */ instead of // blah

Yeah, that snuck in accidentally.  Fixed in patch V7.

> Related to a comment below, neater if these aren't const chars and
> the casts can be dropped.

Fixed in patch V7.

> I'm unsure about the changes in user-event. On changing the prototype,
> would we be better off adding a new event param rather than adding
> gen_boot_args_sigfile.
> Since the security model as it stands appears to depend on the user not
> accessing the shell, does it even make sense to handle signatures in
> user events?

I did this to maintain consistency with the rest of the boot options as
handled elsewhere in petitboot.  Even if there is no legitimate use case
at this time, eventually a limited console might be desirable, in which
case this becomes usable again.

As an aside, Gerrit and/or GitHub really offer a nicer mechanism for
these sorts of complex reviews.  This is especially true when modified
patches keep getting caught in mailing list moderation.

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEbBAEBAgAGBQJXtYRlAAoJEK+E3vEXDOFbuvQH+Kj6D2zqWwhqEOVdIQM6B0JY
aENPYGKrg+9fud4WGaf6kQg3UfKRYPefcVOVVMRUsRyzgXZDN+hI15hUDBBPR/aO
GgD3FNPjtGM8X/Kkar+wa27+/MTtVtz1oPlsHlDDbEnWOTET1Y/wUoMgCPTT0w5J
sm6LRv7J21kqBf1WieX65T8zFAG4/BCxtz2i+b1WgYhf7JFT/HgmbPXWWSLYl27B
yviCuM+XAKm8g9+CfCl/diBOodo0N9j54IzBRasGPJW0xCEBoKf3ZktNd92109aM
hnCVLGfcDRRhSxJ+48tcLtq8d3jT4b9MiJC3lTmoezGN5lsl/MmoqkkBjQL66g==
=B8V9
-----END PGP SIGNATURE-----
Samuel Mendoza-Jonas Aug. 19, 2016, 1:32 a.m. UTC | #6
On Thu, 2016-08-18 at 04:48 -0500, Timothy Pearson wrote:
> On 08/16/2016 10:46 PM, Samuel Mendoza-Jonas wrote:
> 
> > 
> > Since we pass boot_task here, is there any need to define the const
> > chars above? gpg_validate_boot_files can just use boot_task->local_image
> > etc, and then local_image/etc can be normal char pointers that are
> > allocated by gpg_validate_boot_files.
> 
> Fixed in patch V7.
> 
> > 
> > nitpick - indentation
> 
> Fixed in patch V7.
> 
> > 
> > boot_task should probably get freed here.
> 
> Fixed in patch V7.
> 
> > 
> > This block is the same for each image/initrd/dtb, could it be worth it
> > to add a function to gpg.c like
> > 
> > 			image_sig = gpg_set_signature(ctx, image) ?
> 
> Fixed in patch V7.
> 
> > 
> > Probably best to bail out straight away here.
> 
> Fixed in patch V7.
> 
> > 
> > talloc() this instead?
> 
> Fixed in patch V7.
> 
> > 
> > Change these last few debug errors to -1
> 
> Fixed in patch V7.
> 
> > 
> > Why not just break; here? It looks like even if there's an error we'll
> > continue to read the source file until finished.
> 
> Fixed in patch V7.
> 
> > 
> > Debug error
> 
> Not entirely sure what this means, but the retcode was change to -1 in
> patch V7.
> 
> > 
> > const char *variable
> 
> Fixed in patch V7.
> 
> > 
> > nitpick - put these before the if (signature..) check
> 
> Fixed in patch V7.
> 
> > 
> > If I'm reading this right, could you instead do
> > 	if (strncmp(auth_sig_line, verfication_signatures->fpr,
> > 		    	strlen(verification_signatures->fpr) == 0)
> > and skip the while loop?
> 
> No, this would allow essentially a substring match and could create a
> security hole.  Truncating only newline / carriage return characters
> from the end of the line does not have this side-effect.

Sure, it makes sense to err on the side of caution here. The other case
where you check for the string "ENCRYPTED" would be a case for strncmp
thought I imagine, since the length of the string is static.

> 
> > 
> > first instead to save yourself an indent of the above block?
> 
> Fixed in patch V7.
> 
> > 
> > Don't need this else since we return above
> 
> Right.  Fixed in patch V7.
> 
> > 
> > Assume I nitpick the rest of these too :)
> 
> Should all be fixed in patch V7, though I imagine some may have still
> snuck through.
> 
> > 
> > Since the else case here is do nothing, it looks neater to just do
> > 
> > 	if (!boot_task->verify_signature)
> > 		return result;
> > 
> > And save an indent on the rest of the function.
> 
> Fixed in patch V7.
> 
> > 
> > Can just do
> > 	return KEXEC_LOAD_SIG_SETUP_INVALID;
> 
> Fixed in patch V7.
> 
> > 
> > I wonder if it might not be nicer to just pass a pointer and have
> > copy_file_to_destination() talloc_strdup() the resulting filename. Then
> > we don't need to allocate 8*3K for all the names and can drop
> > MAX_FILENAME_SIZE. Thoughts?
> 
> Agreed.  Fixed in patch V7.
> 
> > 
> > If gpg_validate_boot_files() fails we'll call
> > gpg_validate_boot_files_cleanup() afterwards, so I think we're safe to
> > skip unlink()-ing local_image and local_initrd at this point.
> 
> I'm somewhat ambivalent on this, so it's been changed in patch V7 to
> behave as described.
> 
> > 
> > (especially if we change to if (!verify_signature) as above), declare
> > these variables at the start of the function.
> 
> Fixed in patch V7.
> 
> > 
> > style nitpick - use /* blah */ instead of // blah
> 
> Yeah, that snuck in accidentally.  Fixed in patch V7.
> 
> > 
> > Related to a comment below, neater if these aren't const chars and
> > the casts can be dropped.
> 
> Fixed in patch V7.
> 
> > 
> > I'm unsure about the changes in user-event. On changing the prototype,
> > would we be better off adding a new event param rather than adding
> > gen_boot_args_sigfile.
> > Since the security model as it stands appears to depend on the user not
> > accessing the shell, does it even make sense to handle signatures in
> > user events?
> 
> I did this to maintain consistency with the rest of the boot options as
> handled elsewhere in petitboot.  Even if there is no legitimate use case
> at this time, eventually a limited console might be desirable, in which
> case this becomes usable again.
> 
> As an aside, Gerrit and/or GitHub really offer a nicer mechanism for
> these sorts of complex reviews.  This is especially true when modified
> patches keep getting caught in mailing list moderation.

Changing the review process is fairly low on my TODO list, but I have
cracked and changed the list's maximum file size limit to save everyone's
sanity :)

> 
> _______________________________________________
> Petitboot mailing list
> Petitboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot
Timothy Pearson Aug. 19, 2016, 8:38 p.m. UTC | #7
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/18/2016 08:32 PM, Samuel Mendoza-Jonas wrote:
> Sure, it makes sense to err on the side of caution here. The other case
> where you check for the string "ENCRYPTED" would be a case for strncmp
> thought I imagine, since the length of the string is static.

It's possible, though in general I'd prefer the parsing to remain
consistent from line to line.

If this is the only nit to be picked can this series be merged? :-)

Thanks!

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXt25BAAoJEK+E3vEXDOFbakcH/R0msehoYQUmd+0avHU6t9O/
ojimES1+ejwB4hK1RrCT63mVfuszWmwUxhuh7CFfTfcTgN/DNlIJ5lsWPMzofbbc
ikTGL2ywC8rmGOMdsySp9IvarkmKYrKlX4wQnlUEqmn4MtUbLUMsbMKKD33Xnw1Q
GjL6ZOVPF9Xl7VVG1kKbBi+9TMXM8stu9xVzoLmP/Q3q1iF7ESGHhVMABwVh9WuF
qJJyTR++J8UAOf2ncvR4l/CbADBLTwcmksA58TMyC3XsH8wjyyyQFYywbmtfy2GV
+78PYpNRaHNo1JKjC2PVVKOXaKQsB+2SpCL2fTsVbU55IB+uwRajpYgAJHPTebI=
=b5gq
-----END PGP SIGNATURE-----
Timothy Pearson Aug. 23, 2016, 10 p.m. UTC | #8
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/19/2016 03:38 PM, Timothy Pearson wrote:
> On 08/18/2016 08:32 PM, Samuel Mendoza-Jonas wrote:
>> Sure, it makes sense to err on the side of caution here. The other case
>> where you check for the string "ENCRYPTED" would be a case for strncmp
>> thought I imagine, since the length of the string is static.
> 
> It's possible, though in general I'd prefer the parsing to remain
> consistent from line to line.
> 
> If this is the only nit to be picked can this series be merged? :-)
> 
> Thanks!
> 

I just wanted to follow up on this patch series.

Thanks!

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXvMd3AAoJEK+E3vEXDOFbFDQH/RpRHeUuW1nw3JDuOfijuUBO
dO+PS6FcXWKPBUZDEIjVDhNcDW2EqP5eMilFX9rVyOSFg4L9+dPcQmcuqxry2017
34bBVyHFN46fssGkqekyCKMOFJeGZH6WkrmzFZMrQ9IOF72JrJE/KUdAtoTtJZst
Aga5kPQzRuuvbDRQBhtr5dtXDmwYT6EwckhSDlK2L7zCrwLvoZ+rCN31ekpsT3bP
lYIEAVOngjkZxGE/d9ar35G5fyyWfyjb8cpH9rxxRdwGnHdmjQ+Qn761tcgIPNja
s+R8gab3uC6vaX2kd6Y71M3vXG26r9X4YTYOTT6esYTpZ56yYaCD8qqTtVgdboo=
=TH2L
-----END PGP SIGNATURE-----
Samuel Mendoza-Jonas Aug. 24, 2016, 12:40 a.m. UTC | #9
On Tue, 2016-08-23 at 17:00 -0500, Timothy Pearson wrote:
> On 08/19/2016 03:38 PM, Timothy Pearson wrote:
> > 
> > On 08/18/2016 08:32 PM, Samuel Mendoza-Jonas wrote:
> > > 
> > > Sure, it makes sense to err on the side of caution here. The other case
> > > where you check for the string "ENCRYPTED" would be a case for strncmp
> > > thought I imagine, since the length of the string is static.
> > 
> > It's possible, though in general I'd prefer the parsing to remain
> > consistent from line to line.
> > 
> > If this is the only nit to be picked can this series be merged? :-)
> > 
> > Thanks!
> > 
> 
> I just wanted to follow up on this patch series.
> 
> Thanks!

Hi! Excuse me not following up, got caught up putting out some other
fires :)

I'm pretty happy to merge this I think. I've tested with buildroot with
only a moderate amount of hair-pulling, and everything acts as expected.
One, maybe two nitpicks, but I can handle those myself when I merge,
being:
- I'll change --with-signed-boot to default=no
- I might add a comment to a file or commit message to stress that it's
probably best to hold off using the word 'secure' too much unless this
is used in conjunction with a proper trusted-boot implementation so that
you can trust the integrity of the initramfs. If you like I can send
that to you to bikeshed as well :)

Cheers,
Sam
Timothy Pearson Aug. 24, 2016, 3:54 p.m. UTC | #10
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/23/2016 07:40 PM, Samuel Mendoza-Jonas wrote:
> Hi! Excuse me not following up, got caught up putting out some other
> fires :)

No problem, it happens.

> I'm pretty happy to merge this I think. I've tested with buildroot with
> only a moderate amount of hair-pulling, and everything acts as expected.
> One, maybe two nitpicks, but I can handle those myself when I merge,
> being:
> - I'll change --with-signed-boot to default=no

OK.

> - I might add a comment to a file or commit message to stress that it's
> probably best to hold off using the word 'secure' too much unless this
> is used in conjunction with a proper trusted-boot implementation so that
> you can trust the integrity of the initramfs. If you like I can send
> that to you to bikeshed as well :)

Yeah, this is probably a true statement.  Essentially the entire
contents of NOR flash, including petitboot, become the CRTM.  While this
is definitely more secure than the existing boot methods (and in line
with some commercial x86 offerings) I wouldn't call it complete until
the TPM is being used and the CRTM is as small as possible, preferably
in the first 4K of Flash (write protected) or, even better, in the
on-die nonvolatile memory for OpenPOWER systems.  Not sure if that's
even possible at this time, but it would definitely be interesting.

Thanks!

- -- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJXvcMwAAoJEK+E3vEXDOFbnKAH/0b8Dw/nl27Y1eYZLBrGSlVw
txIxOILDrFXyMqyr+xOm/mWH2VpGqeg6uk0sUMFZaKMW7iKTzh9fT1J4Sbq7gZiC
ZuinV5wWgoWkhcUx9Jgp7VonJx4c9bBK3tmRKs97KyO2nXH7Wx06v8TGZ+Cv4wfq
TE6M3/xBm/64nFsSWloloM5iJwFtLFnJQs3/FtfAi6vHfY7tlLX4C0rJBUY1MvFf
cIJyOl0a6E1Po1rIFeWBRU6Spm3LvFy9RkbATaxRRtXGVSM2HDjUW5rUmnx7e7N+
sdblUU6K4QVQXq/HZkInOLAULbxhUsyuXWcJQV5GJUrQyYY7oUeB3gFhdGve5yw=
=Z7Xz
-----END PGP SIGNATURE-----
Samuel Mendoza-Jonas Aug. 26, 2016, 4:10 a.m. UTC | #11
On Wed, 2016-08-24 at 10:54 -0500, Timothy Pearson wrote:
> On 08/23/2016 07:40 PM, Samuel Mendoza-Jonas wrote:
> > 
> > Hi! Excuse me not following up, got caught up putting out some other
> > fires :)
> 
> No problem, it happens.
> 
> > 
> > I'm pretty happy to merge this I think. I've tested with buildroot with
> > only a moderate amount of hair-pulling, and everything acts as expected.
> > One, maybe two nitpicks, but I can handle those myself when I merge,
> > being:
> > - I'll change --with-signed-boot to default=no
> 
> OK.
> 
> > 
> > - I might add a comment to a file or commit message to stress that it's
> > probably best to hold off using the word 'secure' too much unless this
> > is used in conjunction with a proper trusted-boot implementation so that
> > you can trust the integrity of the initramfs. If you like I can send
> > that to you to bikeshed as well :)
> 
> Yeah, this is probably a true statement.  Essentially the entire
> contents of NOR flash, including petitboot, become the CRTM.  While this
> is definitely more secure than the existing boot methods (and in line
> with some commercial x86 offerings) I wouldn't call it complete until
> the TPM is being used and the CRTM is as small as possible, preferably
> in the first 4K of Flash (write protected) or, even better, in the
> on-die nonvolatile memory for OpenPOWER systems.  Not sure if that's
> even possible at this time, but it would definitely be interesting.
> 
> Thanks!
> 

With some build-related fixes and the comment mentioned above, series
merged as ccb478ac. Thanks!

Patch
diff mbox

diff --git a/configure.ac b/configure.ac
index 00a6113..370511b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -170,6 +170,70 @@  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"
+	]
+)
+
+AM_CONDITIONAL([WITH_GPGME], [test "x$with_signed_boot" = "xyes"])
+
+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 899c9a6..4a74da3 100644
--- a/discover/Makefile.am
+++ b/discover/Makefile.am
@@ -15,6 +15,12 @@ 
 sbin_PROGRAMS += discover/pb-discover
 noinst_PROGRAMS += discover/platform.ro
 
+if WITH_GPGME
+gpg_int_SOURCES = discover/gpg.c
+else
+gpg_int_SOURCES =
+endif
+
 discover_pb_discover_SOURCES = \
 	discover/boot.c \
 	discover/boot.h \
@@ -52,13 +58,15 @@  discover_pb_discover_SOURCES = \
 	discover/user-event.h \
 	discover/kboot-parser.c \
 	discover/yaboot-parser.c \
-	discover/pxe-parser.c
+	discover/pxe-parser.c \
+	$(gpg_int_SOURCES)
 
 discover_pb_discover_LDADD = \
 	discover/grub2/grub2-parser.ro \
 	discover/platform.ro \
 	$(core_lib) \
-	$(UDEV_LIBS)
+	$(UDEV_LIBS) \
+	$(GPGME_LIBS)
 
 discover_pb_discover_LDFLAGS = \
 	$(AM_LDFLAGS) \
diff --git a/discover/boot.c b/discover/boot.c
index ba6ce25..38be536 100644
--- a/discover/boot.c
+++ b/discover/boot.c
@@ -25,6 +25,7 @@ 
 #include "paths.h"
 #include "resource.h"
 #include "platform.h"
+#include "gpg.h"
 
 static const char *boot_hook_dir = PKG_SYSCONF_DIR "/boot.d";
 enum {
@@ -32,21 +33,6 @@  enum {
 	BOOT_HOOK_EXIT_UPDATE	= 2,
 };
 
-struct boot_task {
-	struct load_url_result *image;
-	struct load_url_result *initrd;
-	struct load_url_result *dtb;
-	const char *local_image;
-	const char *local_initrd;
-	const char *local_dtb;
-	const char *args;
-	const char *boot_tty;
-	boot_status_fn status_fn;
-	void *status_arg;
-	bool dry_run;
-	bool cancelled;
-};
-
 /**
  * kexec_load - kexec load helper.
  */
@@ -59,20 +45,33 @@  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 ((result = gpg_validate_boot_files(boot_task, &local_initrd,
+		&local_dtb, &local_image))) {
+		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
+			pb_log("%s: Aborting kexec due to"
+				" signature verification failure\n", __func__);
+			goto abort_kexec;
+		}
+	}
+
 	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 */
 	}
@@ -84,7 +83,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);
@@ -92,6 +91,10 @@  static int kexec_load(struct boot_task *boot_task)
 	if (result)
 		pb_log("%s: failed: (%d)\n", __func__, result);
 
+abort_kexec:
+	gpg_validate_boot_files_cleanup(boot_task, &local_initrd, &local_dtb,
+					&local_image);
+
 	return result;
 }
 
@@ -378,23 +381,69 @@  static void boot_process(struct load_url_result *result, void *data)
 			check_load(task, "dtb", task->dtb))
 		goto no_load;
 
+	if (task->verify_signature) {
+		if (load_pending(task->image_signature) ||
+				load_pending(task->initrd_signature) ||
+				load_pending(task->dtb_signature) ||
+				load_pending(task->cmdline_signature))
+			return;
+
+		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) ||
+				check_load(task, "command line signature",
+					task->cmdline_signature))
+			goto no_sig_load;
+	}
+
 	/* 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 (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;
+		task->local_cmdline_signature = task->cmdline_signature ?
+			task->cmdline_signature->local : NULL;
+	}
+
 	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, _("kexec load failed"));
+				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);
+	cleanup_load(task->cmdline_signature);
+
 no_load:
 	cleanup_load(task->image);
 	cleanup_load(task->initrd);
@@ -435,6 +484,8 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 		boot_status_fn status_fn, void *status_arg)
 {
 	struct pb_url *image = NULL, *initrd = NULL, *dtb = NULL;
+	struct pb_url *image_sig = NULL, *initrd_sig = NULL, *dtb_sig = NULL,
+		*cmdline_sig = NULL;
 	const struct config *config;
 	struct boot_task *boot_task;
 	const char *boot_desc;
@@ -478,6 +529,8 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 	boot_task->status_fn = status_fn;
 	boot_task->status_arg = status_arg;
 
+	boot_task->verify_signature = (lockdown_status() == PB_LOCKDOWN_SIGN);
+
 	if (cmd && cmd->boot_args) {
 		boot_task->args = talloc_strdup(boot_task, cmd->boot_args);
 	} else if (opt && opt->option->boot_args) {
@@ -494,11 +547,69 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 		boot_task->boot_tty = config ? config->boot_tty : NULL;
 	}
 
+	if (boot_task->verify_signature) {
+		if (cmd && cmd->args_sig_file) {
+			cmdline_sig = pb_url_parse(opt, cmd->args_sig_file);
+		} else if (opt && opt->args_sig_file) {
+			cmdline_sig = opt->args_sig_file->url;
+		} else {
+			pb_log("%s: no command line signature file"
+				" specified\n", __func__);
+			update_status(status_fn, status_arg, BOOT_STATUS_INFO,
+					_("Boot failed: no command line"
+						" signature file specified"));
+			return NULL;
+		}
+	}
+
 	/* start async loads for boot resources */
 	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 (boot_task->verify_signature) {
+		/* Generate names of associated signature files and load */
+		if (image) {
+			image_sig = pb_url_copy(ctx, image);
+			talloc_free(image_sig->file);
+			image_sig->file = talloc_asprintf(image_sig,
+				"%s.sig", image->file);
+			talloc_free(image_sig->path);
+			image_sig->path = talloc_asprintf(image_sig,
+				"%s.sig", image->path);
+			rc |= start_url_load(boot_task,
+				"kernel image signature", image_sig,
+				&boot_task->image_signature);
+		}
+		if (initrd) {
+			initrd_sig = pb_url_copy(ctx, initrd);
+			talloc_free(initrd_sig->file);
+			initrd_sig->file = talloc_asprintf(initrd_sig,
+				"%s.sig", initrd->file);
+			talloc_free(initrd_sig->path);
+			initrd_sig->path = talloc_asprintf(initrd_sig,
+				"%s.sig", initrd->path);
+			rc |= start_url_load(boot_task, "initrd signature",
+				initrd_sig, &boot_task->initrd_signature);
+		}
+		if (dtb) {
+			dtb_sig = pb_url_copy(ctx, dtb);
+			talloc_free(dtb_sig->file);
+			dtb_sig->file = talloc_asprintf(dtb_sig,
+				"%s.sig", dtb->file);
+			talloc_free(dtb_sig->path);
+			dtb_sig->path = talloc_asprintf(dtb_sig,
+				"%s.sig", dtb->path);
+			rc |= start_url_load(boot_task, "dtb signature",
+				dtb_sig, &boot_task->dtb_signature);
+		}
+
+		rc |= start_url_load(boot_task,
+			"kernel command line signature", cmdline_sig,
+			&boot_task->cmdline_signature);
+	}
+
+	/* If all URLs are local, we may be done. */
 	if (rc) {
 		/* Don't call boot_cancel() to preserve the status update */
 		boot_task->cancelled = true;
diff --git a/discover/boot.h b/discover/boot.h
index ec61703..5f6e874 100644
--- a/discover/boot.h
+++ b/discover/boot.h
@@ -11,4 +11,34 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 		boot_status_fn status_fn, void *status_arg);
 
 void boot_cancel(struct boot_task *task);
+
+struct boot_task {
+	struct load_url_result *image;
+	struct load_url_result *initrd;
+	struct load_url_result *dtb;
+	const char *local_image;
+	const char *local_initrd;
+	const char *local_dtb;
+	const char *args;
+	const char *boot_tty;
+	boot_status_fn status_fn;
+	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;
+};
+
+enum {
+	KEXEC_LOAD_SIG_SETUP_INVALID = 253,
+	KEXEC_LOAD_SIGNATURE_FAILURE = 254,
+};
+
 #endif /* _BOOT_H */
diff --git a/discover/device-handler.c b/discover/device-handler.c
index c31fcea..f6b6d22 100644
--- a/discover/device-handler.c
+++ b/discover/device-handler.c
@@ -613,6 +613,7 @@  static bool __attribute__((used)) boot_option_is_resolved(
 	return resource_is_resolved(opt->boot_image) &&
 		resource_is_resolved(opt->initrd) &&
 		resource_is_resolved(opt->dtb) &&
+		resource_is_resolved(opt->args_sig_file) &&
 		resource_is_resolved(opt->icon);
 }
 
@@ -638,6 +639,8 @@  static bool boot_option_resolve(struct discover_boot_option *opt,
 	return resource_resolve(opt->boot_image, "boot_image", opt, handler) &&
 		resource_resolve(opt->initrd, "initrd", opt, handler) &&
 		resource_resolve(opt->dtb, "dtb", opt, handler) &&
+		resource_resolve(opt->args_sig_file, "args_sig_file", opt,
+			handler) &&
 		resource_resolve(opt->icon, "icon", opt, handler);
 }
 
@@ -652,6 +655,7 @@  static void boot_option_finalise(struct device_handler *handler,
 	assert(!opt->option->dtb_file);
 	assert(!opt->option->icon_file);
 	assert(!opt->option->device_id);
+	assert(!opt->option->args_sig_file);
 
 	if (opt->boot_image)
 		opt->option->boot_image_file = opt->boot_image->url->full;
@@ -661,6 +665,8 @@  static void boot_option_finalise(struct device_handler *handler,
 		opt->option->dtb_file = opt->dtb->url->full;
 	if (opt->icon)
 		opt->option->icon_file = opt->icon->url->full;
+	if (opt->args_sig_file)
+		opt->option->args_sig_file = opt->args_sig_file->url->full;
 
 	opt->option->device_id = opt->device->device->id;
 
diff --git a/discover/device-handler.h b/discover/device-handler.h
index b6f9fd5..f785ccc 100644
--- a/discover/device-handler.h
+++ b/discover/device-handler.h
@@ -48,6 +48,7 @@  struct discover_boot_option {
 	struct resource		*boot_image;
 	struct resource		*initrd;
 	struct resource		*dtb;
+	struct resource		*args_sig_file;
 	struct resource		*icon;
 };
 
diff --git a/discover/gpg.c b/discover/gpg.c
new file mode 100644
index 0000000..e1c1d04
--- /dev/null
+++ b/discover/gpg.c
@@ -0,0 +1,381 @@ 
+/*
+ *  Copyright (C) 2016 Raptor Engineering, LLC
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#if defined(HAVE_CONFIG_H)
+#include "config.h"
+#endif
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <assert.h>
+#include <dirent.h>
+#include <string.h>
+#include <fcntl.h>
+#include <sys/types.h>
+
+#include <log/log.h>
+#include <talloc/talloc.h>
+#include <url/url.h>
+#include <util/util.h>
+#include <i18n/i18n.h>
+
+#include "gpg.h"
+
+int copy_file_to_destination(const char * source_file,
+	char * destination_file, int max_dest_filename_size) {
+	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
+		pb_log("%s: failed: unable to open source file '%s'\n",
+			__func__, source_file);
+		result = 1;
+	}
+
+	size_t l1;
+	unsigned char buffer[FILE_XFER_BUFFER_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';
+	}
+	else {
+		ssize_t r;
+		char readlink_buffer[MAX_FILENAME_SIZE];
+		snprintf(readlink_buffer, MAX_FILENAME_SIZE, "/proc/self/fd/%d",
+			destination_fd);
+		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;
+}
+
+int verify_file_signature(const char * plaintext_filename,
+	const char * signature_filename, FILE * authorized_signatures_handle,
+	const char * keyring_path)
+{
+	int valid = 0;
+
+	if (signature_filename == NULL)
+		return -1;
+
+	gpgme_signature_t verification_signatures;
+	gpgme_verify_result_t verification_result;
+	gpgme_data_t plaintext_data;
+	gpgme_data_t signature_data;
+	gpgme_engine_info_t enginfo;
+	gpgme_ctx_t gpg_context;
+	gpgme_error_t err;
+
+	/* 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__);
+		return -1;
+	}
+	err = gpgme_get_engine_info(&enginfo);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: GPG engine failed to initialize\n", __func__);
+		return -1;
+	}
+	err = gpgme_new(&gpg_context);
+	if (err != GPG_ERR_NO_ERROR) {
+		pb_log("%s: GPG context could not be created\n", __func__);
+		return -1;
+	}
+	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__);
+		return -1;
+	}
+	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__);
+		return -1;
+	}
+	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);
+		return -1;
+	}
+	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);
+		return -1;
+	}
+
+	/* 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 file using GPG signature '%s'\n",
+			__func__, signature_filename);
+		return -1;
+	}
+	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__);
+		return -1;
+	}
+	else {
+		pb_log("%s: GPG signature '%s' for file '%s' verified\n",
+			__func__, signature_filename, plaintext_filename);
+	}
+
+	return 0;
+}
+
+int gpg_validate_boot_files(struct boot_task *boot_task,
+	const char** local_initrd, const char** local_dtb,
+	const char** local_image) {
+	int result = 0;
+
+	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;
+
+	if (boot_task->verify_signature) {
+		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 = talloc_strdup(boot_task,
+			kernel_filename);
+		if (boot_task->local_initrd)
+			*local_initrd = talloc_strdup(boot_task,
+				initrd_filename);
+		if (boot_task->local_dtb)
+			*local_dtb = talloc_strdup(boot_task,
+				dtb_filename);
+
+		/* Write command line to temporary file for verification */
+		char cmdline_template[21] = "/tmp/petitbootXXXXXX";
+		int cmdline_fd = mkstemp(cmdline_template);
+		FILE *cmdline_handle = NULL;
+		if (cmdline_fd < 0) {
+			// handle mkstemp error
+			pb_log("%s: failed: unable to create command line"
+				" temporary file for verification\n",
+				__func__);
+			result = -1;
+		}
+		else {
+			cmdline_handle = fdopen(cmdline_fd, "w");
+		}
+		if (!cmdline_handle) {
+			// handle open error
+			pb_log("%s: failed: unable to write command line"
+				" temporary file for verification\n",
+				__func__);
+			result = -1;
+		}
+		else {
+			fwrite(boot_task->args, sizeof(char),
+				strlen(boot_task->args), cmdline_handle);
+			fflush(cmdline_handle);
+		}
+
+		/* Check signatures */
+		if (verify_file_signature(kernel_filename,
+			local_image_signature,
+			authorized_signatures_handle, "/etc/gpg"))
+			result = KEXEC_LOAD_SIGNATURE_FAILURE;
+		if (verify_file_signature(cmdline_template,
+			local_cmdline_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;
+
+		/* Clean up */
+		if (cmdline_handle) {
+			fclose(cmdline_handle);
+			unlink(cmdline_template);
+		}
+		fclose(authorized_signatures_handle);
+	}
+
+	return result;
+}
+
+void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
+	const char** local_initrd, const char** local_dtb,
+	const char** local_image) {
+	if (boot_task->verify_signature) {
+		unlink(*local_image);
+		if (*local_initrd)
+			unlink(*local_initrd);
+		if (*local_dtb)
+			unlink(*local_dtb);
+
+		talloc_free((char*)*local_image);
+		if (*local_initrd)
+			talloc_free((char*)*local_initrd);
+		if (*local_dtb)
+			talloc_free((char*)*local_dtb);
+	}
+}
+
+int lockdown_status() {
+	if (access(LOCKDOWN_FILE, F_OK) == -1)
+		return PB_LOCKDOWN_NONE;
+	else
+		return PB_LOCKDOWN_SIGN;
+}
\ No newline at end of file
diff --git a/discover/gpg.h b/discover/gpg.h
new file mode 100644
index 0000000..9b452b8
--- /dev/null
+++ b/discover/gpg.h
@@ -0,0 +1,73 @@ 
+/*
+ *  Copyright (C) 2016 Raptor Engineering, LLC
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef _PB_GPG_H
+#define _PB_GPG_H
+
+#define MAX_FILENAME_SIZE	8192
+#define FILE_XFER_BUFFER_SIZE	8192
+
+#include "boot.h"
+
+enum {
+	PB_LOCKDOWN_NONE	= 0,
+	PB_LOCKDOWN_SIGN	= 1,
+};
+
+#if defined(HAVE_LIBGPGME)
+#include <gpgme.h>
+#endif /* HAVE_LIBGPGME */
+
+int lockdown_status(void);
+
+int copy_file_to_destination(const char * source_file,
+	char * destination_file, int max_dest_filename_size);
+
+int verify_file_signature(const char * plaintext_filename,
+	const char * signature_filename, FILE * authorized_signatures_handle,
+	const char * keyring_path);
+
+int gpg_validate_boot_files(struct boot_task *boot_task,
+	const char** local_initrd, const char** local_dtb,
+	const char** local_image);
+
+void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
+	const char** local_initrd, const char** local_dtb,
+	const char** local_image);
+
+#if !defined(HAVE_LIBGPGME)
+
+int lockdown_status(void) { return PB_LOCKDOWN_NONE; }
+
+int copy_file_to_destination(const char * source_file,
+	char * destination_file, int max_dest_filename_size) { return -1; }
+
+int verify_file_signature(const char * plaintext_filename,
+	const char * signature_filename, FILE * authorized_signatures_handle,
+	const char * keyring_path) { return -1; }
+
+int gpg_validate_boot_files(struct boot_task *boot_task,
+	const char** local_initrd, const char** local_dtb,
+	const char** local_image) { return 0; }
+
+void gpg_validate_boot_files_cleanup(struct boot_task *boot_task,
+	const char** local_initrd, const char** local_dtb,
+	const char** local_image) { }
+
+#endif /* HAVE_LIBGPGME */
+
+#endif /* _PB_GPG_H */
\ No newline at end of file
diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c
index 8bff732..c16b639 100644
--- a/discover/grub2/builtins.c
+++ b/discover/grub2/builtins.c
@@ -6,7 +6,9 @@ 
 #include <types/types.h>
 #include <talloc/talloc.h>
 #include <util/util.h>
+#include <url/url.h>
 
+#include "discover/resource.h"
 #include "discover/parser.h"
 #include "grub2.h"
 
@@ -69,6 +71,12 @@  static int builtin_linux(struct grub2_script *script,
 		opt->option->boot_args = talloc_asprintf_append(
 						opt->option->boot_args,
 						" %s", argv[i]);
+
+	char* args_sigfile_default = talloc_asprintf(opt,
+		"%s.cmdline.sig", argv[1]);
+	opt->args_sig_file = create_grub2_resource(opt, script->ctx->device,
+						root, args_sigfile_default);
+	talloc_free(args_sigfile_default);
 	return 0;
 }
 
diff --git a/discover/kboot-parser.c b/discover/kboot-parser.c
index cebe787..f7f75e0 100644
--- a/discover/kboot-parser.c
+++ b/discover/kboot-parser.c
@@ -96,6 +96,12 @@  out_add:
 	d_opt->boot_image = create_devpath_resource(d_opt,
 				conf->dc->device, value);
 
+	char* args_sigfile_default = talloc_asprintf(d_opt,
+		"%s.cmdline.sig", value);
+	d_opt->args_sig_file = create_devpath_resource(d_opt,
+				conf->dc->device, args_sigfile_default);
+	talloc_free(args_sigfile_default);
+
 	if (root) {
 		opt->boot_args = talloc_asprintf(opt, "root=%s %s", root, args);
 		talloc_free(args);
diff --git a/discover/pxe-parser.c b/discover/pxe-parser.c
index 4481e5d..4aae8b1 100644
--- a/discover/pxe-parser.c
+++ b/discover/pxe-parser.c
@@ -166,6 +166,13 @@  static void pxe_process_pair(struct conf_context *ctx,
 		url = pxe_url_join(ctx->dc, ctx->dc->conf_url, value);
 		opt->boot_image = create_url_resource(opt, url);
 
+		char* args_sigfile_default = talloc_asprintf(opt,
+			"%s.cmdline.sig", value);
+		url = pxe_url_join(ctx->dc, ctx->dc->conf_url,
+			args_sigfile_default);
+		opt->args_sig_file = create_url_resource(opt, url);
+		talloc_free(args_sigfile_default);
+
 	} else if (streq(name, "INITRD")) {
 		url = pxe_url_join(ctx->dc, ctx->dc->conf_url, value);
 		opt->initrd = create_url_resource(opt, url);
diff --git a/discover/user-event.c b/discover/user-event.c
index 7350b6c..6ea754f 100644
--- a/discover/user-event.c
+++ b/discover/user-event.c
@@ -82,7 +82,7 @@  static void user_event_print_event(struct event __attribute__((unused)) *event)
 }
 
 static struct resource *user_event_resource(struct discover_boot_option *opt,
-		struct event *event)
+		struct event *event, bool gen_boot_args_sigfile)
 {
 	const char *siaddr, *boot_file;
 	struct resource *res;
@@ -101,7 +101,16 @@  static struct resource *user_event_resource(struct discover_boot_option *opt,
 		return NULL;
 	}
 
-	url_str = talloc_asprintf(opt, "%s%s/%s", "tftp://", siaddr, boot_file);
+	if (gen_boot_args_sigfile) {
+		char* args_sigfile_default = talloc_asprintf(opt,
+			"%s.cmdline.sig", boot_file);
+		url_str = talloc_asprintf(opt, "%s%s/%s", "tftp://", siaddr,
+			args_sigfile_default);
+		talloc_free(args_sigfile_default);
+	}
+	else
+		url_str = talloc_asprintf(opt, "%s%s/%s", "tftp://", siaddr,
+			boot_file);
 	url = pb_url_parse(opt, url_str);
 	talloc_free(url_str);
 
@@ -143,12 +152,13 @@  static int parse_user_event(struct discover_context *ctx, struct event *event)
 	opt->id = talloc_asprintf(opt, "%s#%s", dev->id, val);
 	opt->name = talloc_strdup(opt, val);
 
-	d_opt->boot_image = user_event_resource(d_opt, event);
+	d_opt->boot_image = user_event_resource(d_opt, event, false);
 	if (!d_opt->boot_image) {
 		pb_log("%s: no boot image found for %s!\n", __func__,
 				opt->name);
 		goto fail_opt;
 	}
+	d_opt->args_sig_file = user_event_resource(d_opt, event, true);
 
 	val = event_get_param(event, "rootpath");
 	if (val) {
diff --git a/discover/yaboot-parser.c b/discover/yaboot-parser.c
index aa99392..b62f39d 100644
--- a/discover/yaboot-parser.c
+++ b/discover/yaboot-parser.c
@@ -114,6 +114,13 @@  static void yaboot_finish(struct conf_context *conf)
 	/* populate the boot option from state data */
 	state->opt->boot_image = create_yaboot_devpath_resource(state,
 				conf, state->boot_image);
+
+	char* args_sigfile_default = talloc_asprintf(opt,
+		"%s.cmdline.sig", state->boot_image);
+	state->opt->args_sig_file = create_yaboot_devpath_resource(state,
+				conf, args_sigfile_default);
+	talloc_free(args_sigfile_default);
+
 	if (state->initrd) {
 		state->opt->initrd = create_yaboot_devpath_resource(state,
 				conf, state->initrd);
diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
index 7887fb0..ad7798e 100644
--- a/lib/pb-protocol/pb-protocol.c
+++ b/lib/pb-protocol/pb-protocol.c
@@ -37,6 +37,7 @@ 
  *    4-byte len, initrd_file
  *    4-byte len, dtb_file
  *    4-byte len, boot_args
+ *    4-byte len, args_sig_file
  *
  * action = 0x2: device remove message
  *  payload:
@@ -49,6 +50,7 @@ 
  *   4-byte len, initrd_file
  *   4-byte len, dtb_file
  *   4-byte len, boot_args
+ *   4-byte len, args_sig_file
  *
  */
 
@@ -72,6 +74,7 @@  void pb_protocol_dump_device(const struct device *dev, const char *text,
 		fprintf(stream, "%s\t\tinit: %s\n", text, opt->initrd_file);
 		fprintf(stream, "%s\t\tdtb:  %s\n", text, opt->dtb_file);
 		fprintf(stream, "%s\t\targs: %s\n", text, opt->boot_args);
+		fprintf(stream, "%s\t\tasig: %s\n", text, opt->args_sig_file);
 	}
 }
 
@@ -197,6 +200,7 @@  int pb_protocol_boot_option_len(const struct boot_option *opt)
 		4 + optional_strlen(opt->initrd_file) +
 		4 + optional_strlen(opt->dtb_file) +
 		4 + optional_strlen(opt->boot_args) +
+		4 + optional_strlen(opt->args_sig_file) +
 		sizeof(opt->is_default);
 }
 
@@ -207,6 +211,7 @@  int pb_protocol_boot_len(const struct boot_command *boot)
 		4 + optional_strlen(boot->initrd_file) +
 		4 + optional_strlen(boot->dtb_file) +
 		4 + optional_strlen(boot->boot_args) +
+		4 + optional_strlen(boot->args_sig_file) + 
 		4 + optional_strlen(boot->tty);
 }
 
@@ -360,6 +365,7 @@  int pb_protocol_serialise_boot_option(const struct boot_option *opt,
 	pos += pb_protocol_serialise_string(pos, opt->initrd_file);
 	pos += pb_protocol_serialise_string(pos, opt->dtb_file);
 	pos += pb_protocol_serialise_string(pos, opt->boot_args);
+	pos += pb_protocol_serialise_string(pos, opt->args_sig_file);
 
 	*(bool *)pos = opt->is_default;
 	pos += sizeof(bool);
@@ -380,6 +386,7 @@  int pb_protocol_serialise_boot_command(const struct boot_command *boot,
 	pos += pb_protocol_serialise_string(pos, boot->initrd_file);
 	pos += pb_protocol_serialise_string(pos, boot->dtb_file);
 	pos += pb_protocol_serialise_string(pos, boot->boot_args);
+	pos += pb_protocol_serialise_string(pos, boot->args_sig_file);
 	pos += pb_protocol_serialise_string(pos, boot->tty);
 
 	assert(pos <= buf + buf_len);
@@ -750,6 +757,9 @@  int pb_protocol_deserialise_boot_option(struct boot_option *opt,
 	if (read_string(opt, &pos, &len, &opt->boot_args))
 		goto out;
 
+	if (read_string(opt, &pos, &len, &opt->args_sig_file))
+		goto out;
+
 	if (len < sizeof(bool))
 		goto out;
 	opt->is_default = *(bool *)(pos);
@@ -785,6 +795,9 @@  int pb_protocol_deserialise_boot_command(struct boot_command *cmd,
 	if (read_string(cmd, &pos, &len, &cmd->boot_args))
 		goto out;
 
+	if (read_string(cmd, &pos, &len, &cmd->args_sig_file))
+		goto out;
+
 	if (read_string(cmd, &pos, &len, &cmd->tty))
 		goto out;
 
diff --git a/lib/types/types.h b/lib/types/types.h
index 5c5f6ed..6b607cd 100644
--- a/lib/types/types.h
+++ b/lib/types/types.h
@@ -52,6 +52,7 @@  struct boot_option {
 	char		*initrd_file;
 	char		*dtb_file;
 	char		*boot_args;
+	char		*args_sig_file;
 	bool		is_default;
 
 	struct list_item	list;
@@ -65,6 +66,7 @@  struct boot_command {
 	char *initrd_file;
 	char *dtb_file;
 	char *boot_args;
+	char *args_sig_file;
 	char *tty;
 };
 
diff --git a/ui/common/discover-client.c b/ui/common/discover-client.c
index 6247dd0..5dbd99b 100644
--- a/ui/common/discover-client.c
+++ b/ui/common/discover-client.c
@@ -312,6 +312,7 @@  static void create_boot_command(struct boot_command *command,
 	command->initrd_file = data->initrd;
 	command->dtb_file = data->dtb;
 	command->boot_args = data->args;
+	command->args_sig_file = data->args_sig_file;
 	command->tty = ttyname(STDIN_FILENO);
 }
 
diff --git a/ui/common/discover-client.h b/ui/common/discover-client.h
index 542a275..59d2df9 100644
--- a/ui/common/discover-client.h
+++ b/ui/common/discover-client.h
@@ -11,6 +11,7 @@  struct pb_boot_data {
 	char *initrd;
 	char *dtb;
 	char *args;
+	char *args_sig_file;
 };
 
 /**
diff --git a/ui/ncurses/nc-boot-editor.c b/ui/ncurses/nc-boot-editor.c
index 4012ec5..9788fef 100644
--- a/ui/ncurses/nc-boot-editor.c
+++ b/ui/ncurses/nc-boot-editor.c
@@ -63,6 +63,8 @@  struct boot_editor {
 		struct nc_widget_textbox	*dtb_f;
 		struct nc_widget_label		*args_l;
 		struct nc_widget_textbox	*args_f;
+		struct nc_widget_label		*args_sig_file_l;
+		struct nc_widget_textbox	*args_sig_file_f;
 		struct nc_widget_button		*ok_b;
 		struct nc_widget_button		*help_b;
 		struct nc_widget_button		*cancel_b;
@@ -73,6 +75,9 @@  struct boot_editor {
 	char			*initrd;
 	char			*dtb;
 	char			*args;
+	char			*args_sig_file;
+
+	bool			use_signature_files;
 };
 
 extern const struct help_text boot_editor_help_text;
@@ -198,6 +203,12 @@  static struct pb_boot_data *boot_editor_prepare_data(
 	s = widget_textbox_get_value(boot_editor->widgets.args_f);
 	bd->args = *s ? talloc_strdup(bd, s) : NULL;
 
+	if (boot_editor->use_signature_files) {
+		s = widget_textbox_get_value(
+			boot_editor->widgets.args_sig_file_f);
+		bd->args_sig_file = conditional_prefix(bd, prefix, s);
+	}
+
 	return bd;
 }
 
@@ -323,6 +334,12 @@  static void boot_editor_layout_widgets(struct boot_editor *boot_editor)
 	y += layout_pair(boot_editor, y, boot_editor->widgets.args_l,
 					 boot_editor->widgets.args_f);
 
+	if (boot_editor->use_signature_files) {
+		y += layout_pair(boot_editor, y,
+					boot_editor->widgets.args_sig_file_l,
+					boot_editor->widgets.args_sig_file_f);
+	}
+
 
 	y++;
 	widget_move(widget_button_base(boot_editor->widgets.ok_b), y,
@@ -445,6 +462,11 @@  static void boot_editor_find_device(struct boot_editor *boot_editor,
 	if (bd->dtb && !path_on_device(bd_info, bd->dtb))
 		return;
 
+	if (boot_editor->use_signature_files)
+		if (bd->args_sig_file && !path_on_device(bd_info,
+			bd->args_sig_file))
+			return;
+
 	/* ok, we match; preselect the device option, and remove the common
 	 * prefix */
 	boot_editor->selected_device = bd_info->name;
@@ -454,6 +476,9 @@  static void boot_editor_find_device(struct boot_editor *boot_editor,
 		boot_editor->initrd += len;
 	if (boot_editor->dtb)
 		boot_editor->dtb += len;
+	if (boot_editor->use_signature_files)
+		if (boot_editor->args_sig_file)
+			boot_editor->args_sig_file += len;
 }
 
 static void boot_editor_setup_widgets(struct boot_editor *boot_editor,
@@ -501,6 +526,13 @@  static void boot_editor_setup_widgets(struct boot_editor *boot_editor,
 	boot_editor->widgets.args_f = widget_new_textbox(set, 0, 0,
 					field_size, boot_editor->args);
 
+	if (boot_editor->use_signature_files) {
+		boot_editor->widgets.args_sig_file_l = widget_new_label(set,
+				0, 0, _("Argument signature file:"));
+		boot_editor->widgets.args_sig_file_f = widget_new_textbox(set,
+				0, 0, field_size, boot_editor->args_sig_file);
+	}
+
 	boot_editor->widgets.ok_b = widget_new_button(set, 0, 0, 10,
 					_("OK"), ok_click, boot_editor);
 	boot_editor->widgets.help_b = widget_new_button(set, 0, 0, 10,
@@ -553,6 +585,15 @@  struct boot_editor *boot_editor_init(struct cui *cui,
 	if (!boot_editor)
 		return NULL;
 
+#if defined(HAVE_LIBGPGME)
+	if (access(LOCKDOWN_FILE, F_OK) == -1)
+		boot_editor->use_signature_files = false;
+	else
+		boot_editor->use_signature_files = true;
+#else
+	boot_editor->use_signature_files = false;
+#endif
+
 	talloc_set_destructor(boot_editor, boot_editor_destructor);
 	boot_editor->cui = cui;
 	boot_editor->item = item;
@@ -563,9 +604,10 @@  struct boot_editor *boot_editor_init(struct cui *cui,
 
 	int ncols1 = strncols(_("Device tree:"));
 	int ncols2 = strncols(_("Boot arguments:"));
+	int ncols3 = strncols(_("Argument signature file:"));
 
 	boot_editor->label_x = 1;
-	boot_editor->field_x = 2 + max(ncols1, ncols2);
+	boot_editor->field_x = 2 + max(max(ncols1, ncols2), ncols3);
 
 	nc_scr_init(&boot_editor->scr, pb_boot_editor_sig, 0,
 			cui, boot_editor_process_key,
@@ -584,10 +626,15 @@  struct boot_editor *boot_editor_init(struct cui *cui,
 		boot_editor->initrd = bd->initrd;
 		boot_editor->dtb = bd->dtb;
 		boot_editor->args = bd->args;
+		if (boot_editor->use_signature_files)
+			boot_editor->args_sig_file = bd->args_sig_file;
+		else
+			boot_editor->args_sig_file = "";
 		boot_editor_find_device(boot_editor, bd, sysinfo);
 	} else {
 		boot_editor->image = boot_editor->initrd =
-			boot_editor->dtb = boot_editor->args = "";
+			boot_editor->dtb = boot_editor->args =
+			boot_editor->args_sig_file = "";
 	}
 
 	boot_editor->pad = newpad(
diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index 0c355cc..09b63b0 100644
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -543,6 +543,7 @@  static int cui_boot_option_add(struct device *dev, struct boot_option *opt,
 	cod->bd->initrd = talloc_strdup(cod->bd, opt->initrd_file);
 	cod->bd->dtb = talloc_strdup(cod->bd, opt->dtb_file);
 	cod->bd->args = talloc_strdup(cod->bd, opt->boot_args);
+	cod->bd->args_sig_file = talloc_strdup(cod->bd, opt->args_sig_file);
 
 	/* This disconnects items array from menu. */
 	result = set_menu_items(cui->main->ncm, NULL);
@@ -566,6 +567,7 @@  static int cui_boot_option_add(struct device *dev, struct boot_option *opt,
 	pb_log("   image  '%s'\n", cod->bd->image);
 	pb_log("   initrd '%s'\n", cod->bd->initrd);
 	pb_log("   args   '%s'\n", cod->bd->args);
+	pb_log("   argsig '%s'\n", cod->bd->args_sig_file);
 
 	/* Re-attach the items array. */
 	result = set_menu_items(cui->main->ncm, cui->main->items);