[PATCH 1/3] [V5] Add support for GPG signature enforcement on booted
Samuel Mendoza-Jonas
sam at mendozajonas.com
Wed Aug 17 13:46:42 AEST 2016
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 at 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);
More information about the Petitboot
mailing list