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

Jon McCune jonmccune at google.com
Tue Aug 2 15:33:54 AEST 2016


Hi!

On Mon, Aug 1, 2016 at 9:00 PM, Samuel Mendoza-Jonas <sam at ozlabs.au.ibm.com>
wrote:

> On Mon, 2016-08-01 at 12:09 -0500, Timothy Pearson wrote:
> >  kernels and related blobs
> >
> > This can be used to implement a form of organization-controlled secure
> boot,
> > whereby kernels may be loaded from a variety of sources but they will
> only
> > boot if a valid signature file is found for each component, and only if
> the
> > signature is listed in the /etc/pb-lockdown file.
>
> Cool! This all looks pretty interesting - I have some general comments
> lower down, but in particular:
>
> - Some work with Secure Boot is also being done by Nayna Jain, but with
> a different approach. I don't think either version steps on the other's
> toes but something to look at.
> (http://patchwork.ozlabs.org/patch/629157/ etc)
>

I think it's also worth noting that GRUB2 has some OpenPGP signature
verification support:
https://github.com/jonmccune/grub2/blob/master/grub-core/commands/verify.c
https://github.com/jonmccune/grub2/blob/master/grub-core/commands/loadenv.c
https://github.com/jonmccune/grub2/blob/master/docs/grub.texi (search for
'detached')
(I used a github.com link b/c the http://git.savannah.gnu.org/ was
reporting errors.)

The implementation in these patches doesn't seem to consider the mechanism
by which Petitboot discovers its boot options (which is actually pretty
cute), but that seems to restrict use to kernels with their entire kernel
command-line compiled in (or allowing perhaps unexpected things like
init=/bin/bash). Have you considered integrating with any bootloader
configuration file format?

I wasn't aware of Nayna Jain's efforts, and haven't looked at them yet, but
I might be interested in following up on work like the present patch series
by updating the Petitboot parser for GRUB-style configuration to support
some of the related commands that GRUB2 offers.

Thanks!
-Jon



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

I don't know the author's intention but copying to a ramdisk helps to
address a time-of-check, time-of-use attack if the disk firmware (or
network mount...) is malicious, and feeds one set of files for signature
verification and a distinct set for the actual kexec.


>
> > +     int result = 0;
> > +     char template[21] = "/tmp/petitbootXXXXXX";
> > +     FILE *source_handle = fopen(source_file, "r");
> > +     int destination_fd = mkstemp(template);
> > +     FILE *destination_handle = fdopen(destination_fd, "w");
> > +     if(!source_handle || !(destination_handle)) {
> > +             // handle open error
> > +             result = 1;
>
> Do you mean to return here?
>
> > +             pb_log("%s: failed: unable to open source file '%s'\n",
> __func__, source_file);
> > +     }
> > +
> > +     size_t l1;
> > +     unsigned char buffer[8192];
>
> Any particular reason for this size?
>
> > +
> > +     /* Copy data */
> > +     while ((l1 = fread(buffer, 1, sizeof buffer, source_handle)) > 0) {
> > +             size_t l2 = fwrite(buffer, 1, l1, destination_handle);
> > +             if (l2 < l1) {
> > +                     if (ferror(destination_handle)) {
> > +                             /* General error */
> > +                             result = 1;
> > +                             pb_log("%s: failed: unknown fault\n",
> __func__);
> > +                     }
> > +                     else {
> > +                             /* No space on destination device */
> > +                             result = 2;
> > +                             pb_log("%s: failed: temporary storage
> full\n", __func__);
> > +                     }
> > +             }
> > +     }
> > +
> > +     if (result) {
> > +             destination_file[0] = 0;
>
> '0' or '\0'?
>
> > +     }
> > +     else {
> > +             ssize_t r;
> > +             char readlink_buffer[8192];
> > +             snprintf(readlink_buffer, 8192, "/proc/self/fd/%d",
> destination_fd);
>
> Style stuff - unless there's a particular reason for it we generally
> prefer that all variables are defined at the start of a function.
> Additionally you could probably flip this around and have
>         if (result) {
>                 ...
>                 return result;
>         }
> to lose the indent for this block.
>
> > +             r = readlink(readlink_buffer, destination_file,
> max_dest_filename_size);
> > +             if (r < 0) {
> > +                     /* readlink failed */
> > +                     result = 3;
> > +                     pb_log("%s: failed: unable to obtain temporary
> filename\n", __func__);
> > +             }
> > +             destination_file[r] = 0;
> > +     }
> > +
> > +     fclose(source_handle);
> > +     fclose(destination_handle);
> > +
> > +     return result;
> > +}
> > +
> > +#if defined(HAVE_LIBGPGME)
>
> I think there's officially enough code depending on libgpgme that you
> could justify moving it into it's own .c file instead of interspersing
> it within boot.c. Then instead from wrapping everything in #if defines,
> you could decide somewhere further up whether to boot normally or with
> signatures (eg. by stat-ing /etc/pb-lockdown). Thoughts?
>
> > +static int verify_file_signature(const char * plaintext_filename, const
> char * signature_filename, FILE * authorized_signatures_handle, const char
> * keyring_path)
> > +{
> > +     int result = 0;
> > +     int valid = 0;
> > +
> > +     if (signature_filename == NULL)
> > +             return 10;
>
> I noticed in a few places increasing numbers are returned for errors, but
> it doesn't look like any callers check for specific numbers - do these
> have any particular meaning?
>
> > +
> > +     gpgme_ctx_t gpg_context;
> > +     gpgme_error_t err;
> > +     gpgme_data_t plaintext_data;
> > +     gpgme_data_t signature_data;
> > +     gpgme_engine_info_t enginfo;
> > +     gpgme_verify_result_t verification_result;
> > +     gpgme_signature_t verification_signatures;
>
> Style - we order variable declarations in decreasing size for clarity, eg:
>
>         gpgme_signature_t verification_signatures;
>         gpgme_verify_result_t verification_result;
>         gpgme_engine_info_t enginfo;
>         etc...
>
> > +
> > +     /* Initialize gpgme */
> > +     setlocale (LC_ALL, "");
> > +     gpgme_check_version(NULL);
> > +     gpgme_set_locale(NULL, LC_CTYPE, setlocale (LC_CTYPE, NULL));
> > +     err = gpgme_engine_check_version(GPGME_PROTOCOL_OpenPGP);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: OpenPGP support not available\n", __func__);
> > +             result = 1;
> > +             return result;
>
> return 1; ?
>
> > +     }
> > +     err = gpgme_get_engine_info(&enginfo);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: GPG engine failed to initialize\n", __func__);
> > +             result = 2;
> > +             return result;
> > +     }
> > +     err = gpgme_new(&gpg_context);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: GPG context could not be created\n", __func__);
> > +             result = 3;
> > +             return result;
> > +     }
> > +     err = gpgme_set_protocol(gpg_context, GPGME_PROTOCOL_OpenPGP);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: GPG protocol could not be set\n", __func__);
> > +             result = 4;
> > +             return result;
> > +     }
> > +     if (keyring_path)
> > +             err = gpgme_ctx_set_engine_info (gpg_context,
> GPGME_PROTOCOL_OpenPGP, enginfo->file_name, keyring_path);
> > +     else
> > +             err = gpgme_ctx_set_engine_info (gpg_context,
> GPGME_PROTOCOL_OpenPGP, enginfo->file_name, enginfo->home_dir);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: Could not set GPG engine information\n",
> __func__);
> > +             result = 5;
> > +             return result;
> > +     }
> > +     err = gpgme_data_new_from_file(&plaintext_data,
> plaintext_filename, 1);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: Could not create GPG plaintext data buffer
> from file '%s'\n", __func__, plaintext_filename);
> > +             result = 6;
> > +             return result;
> > +     }
> > +     err = gpgme_data_new_from_file(&signature_data,
> signature_filename, 1);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: Could not create GPG signature data buffer
> from file '%s'\n", __func__, signature_filename);
> > +             result = 7;
> > +             return result;
> > +     }
> > +
> > +     /* Check signature */
> > +     err = gpgme_op_verify(gpg_context, signature_data, plaintext_data,
> NULL);
> > +     if (err != GPG_ERR_NO_ERROR) {
> > +             pb_log("%s: Could not verify GPG signature\n", __func__);
> > +             result = 8;
> > +             return result;
> > +     }
> > +     verification_result = gpgme_op_verify_result(gpg_context);
> > +     verification_signatures = verification_result->signatures;
> > +     while (verification_signatures) {
> > +             if (verification_signatures->status == GPG_ERR_NO_ERROR) {
> > +                     pb_log("%s: Good signature for key ID '%s'
> ('%s')\n", __func__, verification_signatures->fpr, signature_filename);
> > +                     /* Verify fingerprint is present in authorized
> signatures file */
> > +                     char *auth_sig_line = NULL;
> > +                     size_t auth_sig_len = 0;
> > +                     ssize_t auth_sig_read;
> > +                     rewind(authorized_signatures_handle);
> > +                     while ((auth_sig_read = getline(&auth_sig_line,
> &auth_sig_len, authorized_signatures_handle)) != -1) {
> > +                             auth_sig_len = strlen(auth_sig_line);
> > +                             while ((auth_sig_line[auth_sig_len-1] ==
> '\n') || (auth_sig_line[auth_sig_len-1] == '\r'))
> > +                                     auth_sig_len--;
> > +                             auth_sig_line[auth_sig_len] = 0;
> > +                             if (strcmp(auth_sig_line,
> verification_signatures->fpr) == 0)
> > +                                     valid = 1;
> > +                     }
> > +                     free(auth_sig_line);
> > +             }
> > +             else {
> > +                     pb_log("%s: Signature for key ID '%s' ('%s')
> invalid.  Status: %08x\n", __func__, verification_signatures->fpr,
> signature_filename, verification_signatures->status);
> > +             }
> > +             verification_signatures = verification_signatures->next;
> > +     }
> > +
> > +     /* Clean up */
> > +     gpgme_data_release(plaintext_data);
> > +     gpgme_data_release(signature_data);
> > +     gpgme_release(gpg_context);
> > +
> > +     if (!valid) {
> > +             pb_log("%s: Incorrect GPG signature\n", __func__);
> > +             result = 9;
> > +             return result;
> > +     }
> > +     else {
> > +             pb_log("%s: GPG signature '%s' for file '%s' verified\n",
> __func__, signature_filename, plaintext_filename);
> > +     }
> > +
> > +     return result;
> > +}
> > +#endif
> > +
> >  /**
> >   * kexec_load - kexec load helper.
> >   */
> > @@ -57,20 +247,98 @@ static int kexec_load(struct boot_task *boot_task)
> >       char *s_dtb = NULL;
> >       char *s_args = NULL;
> >
> > +     const char* local_initrd = boot_task->local_initrd;
> > +     const char* local_dtb = boot_task->local_dtb;
> > +     const char* local_image = boot_task->local_image;
> > +
> > +#if defined(HAVE_LIBGPGME)
> > +     const char* local_initrd_signature = (boot_task->verify_signature)
> ? boot_task->local_initrd_signature : NULL;
> > +     const char* local_dtb_signature = (boot_task->verify_signature) ?
> boot_task->local_dtb_signature : NULL;
> > +     const char* local_image_signature = (boot_task->verify_signature)
> ? boot_task->local_image_signature : NULL;
> > +
> > +     if (boot_task->verify_signature) {
> > +             int max_filename_size = 8192;
> > +             char kernel_filename[max_filename_size];
> > +             char initrd_filename[max_filename_size];
> > +             char dtb_filename[max_filename_size];
> > +
> > +             kernel_filename[0] = 0;
> > +             initrd_filename[0] = 0;
> > +             dtb_filename[0] = 0;
> > +
> > +             FILE *authorized_signatures_handle = NULL;
> > +
> > +             /* Load authorized signatures file */
> > +             authorized_signatures_handle = fopen(LOCKDOWN_FILE, "r");
> > +             if (!authorized_signatures_handle) {
> > +                     pb_log("%s: unable to read lockdown file\n",
> __func__);
> > +                     result = KEXEC_LOAD_SIG_SETUP_INVALID;
> > +                     return result;
> > +             }
> > +
> > +             /* Copy files to temporary directory for verification /
> boot */
> > +             result = copy_file_to_destination(boot_task->local_image,
> kernel_filename, max_filename_size);
> > +             if (result) {
> > +                     pb_log("%s: image copy failed: (%d)\n", __func__,
> result);
> > +                     return result;
> > +             }
> > +             if (boot_task->local_initrd) {
> > +                     result =
> copy_file_to_destination(boot_task->local_initrd, initrd_filename,
> max_filename_size);
> > +                     if (result) {
> > +                             pb_log("%s: initrd copy failed: (%d)\n",
> __func__, result);
> > +                             unlink(local_image);
> > +                             return result;
> > +                     }
> > +             }
> > +             if (boot_task->local_dtb) {
> > +                     result =
> copy_file_to_destination(boot_task->local_dtb, dtb_filename,
> max_filename_size);
> > +                     if (result) {
> > +                             pb_log("%s: dtb copy failed: (%d)\n",
> __func__, result);
> > +                             unlink(local_image);
> > +                             if (local_initrd)
> > +                                     unlink(local_initrd);
> > +                             return result;
> > +                     }
> > +             }
> > +             local_image = strdup(kernel_filename);
>
> Use talloc_strndup() instead of strdup()
>
> > +             if (boot_task->local_initrd)
> > +                     local_initrd = strdup(initrd_filename);
> > +             if (boot_task->local_dtb)
> > +                     local_dtb = strdup(dtb_filename);
> > +
> > +             /* Check signatures */
> > +             if (verify_file_signature(kernel_filename,
> local_image_signature, authorized_signatures_handle, "/etc/gpg"))
> > +                     result = KEXEC_LOAD_SIGNATURE_FAILURE;
> > +             if (boot_task->local_initrd_signature)
> > +                     if (verify_file_signature(initrd_filename,
> local_initrd_signature, authorized_signatures_handle, "/etc/gpg"))
> > +                             result = KEXEC_LOAD_SIGNATURE_FAILURE;
> > +             if (boot_task->local_dtb_signature)
> > +                     if (verify_file_signature(dtb_filename,
> local_dtb_signature, authorized_signatures_handle, "/etc/gpg"))
> > +                             result = KEXEC_LOAD_SIGNATURE_FAILURE;
> > +
> > +             fclose(authorized_signatures_handle);
> > +
> > +             if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> > +                     pb_log("%s: Aborting kexec due to signature
> verification failure\n", __func__);
> > +                     goto abort_kexec;
> > +             }
> > +     }
> > +#endif
> > +
> >       p = argv;
> >       *p++ = pb_system_apps.kexec;    /* 1 */
> >       *p++ = "-l";                    /* 2 */
> >
> > -     if (boot_task->local_initrd) {
> > +     if (local_initrd) {
> >               s_initrd = talloc_asprintf(boot_task, "--initrd=%s",
> > -                             boot_task->local_initrd);
> > +                             local_initrd);
> >               assert(s_initrd);
> >               *p++ = s_initrd;         /* 3 */
> >       }
> >
> > -     if (boot_task->local_dtb) {
> > +     if (local_dtb) {
> >               s_dtb = talloc_asprintf(boot_task, "--dtb=%s",
> > -                                             boot_task->local_dtb);
> > +                                             local_dtb);
> >               assert(s_dtb);
> >               *p++ = s_dtb;            /* 4 */
> >       }
> > @@ -82,7 +350,7 @@ static int kexec_load(struct boot_task *boot_task)
> >               *p++ = s_args;          /* 5 */
> >       }
> >
> > -     *p++ = boot_task->local_image;  /* 6 */
> > +     *p++ = local_image;     /* 6 */
> >       *p++ = NULL;                    /* 7 */
> >
> >       result = process_run_simple_argv(boot_task, argv);
> > @@ -90,6 +358,23 @@ static int kexec_load(struct boot_task *boot_task)
> >       if (result)
> >               pb_log("%s: failed: (%d)\n", __func__, result);
> >
> > +#if defined(HAVE_LIBGPGME)
> > +abort_kexec:
> > +     if (boot_task->verify_signature) {
> > +             unlink(local_image);
> > +             if (local_initrd)
> > +                     unlink(local_initrd);
> > +             if (local_dtb)
> > +                     unlink(local_dtb);
> > +
> > +             free((char*)local_image);
> > +             if (local_initrd)
> > +                     free((char*)local_initrd);
> > +             if (local_dtb)
> > +                     free((char*)local_dtb);
> > +     }
> > +#endif
> > +
> >       return result;
> >  }
> >
> > @@ -373,23 +658,53 @@ static void boot_process(struct load_url_result
> *result, void *data)
> >                       check_load(task, "dtb", task->dtb))
> >               goto no_load;
> >
> > +#if defined(HAVE_LIBGPGME)
> > +     if (task->verify_signature) {
> > +             if (check_load(task, "kernel image signature",
> task->image_signature) ||
> > +                             check_load(task, "initrd signature",
> task->initrd_signature) ||
> > +                             check_load(task, "dtb signature",
> task->dtb_signature))
> > +                     goto no_sig_load;
> > +     }
> > +#endif
> > +
> >       /* we make a copy of the local paths, as the boot hooks might
> update
> >        * and/or create these */
> >       task->local_image = task->image ? task->image->local : NULL;
> >       task->local_initrd = task->initrd ? task->initrd->local : NULL;
> >       task->local_dtb = task->dtb ? task->dtb->local : NULL;
> >
> > +#if defined(HAVE_LIBGPGME)
> > +     if (task->verify_signature) {
> > +             task->local_image_signature = task->image_signature ?
> task->image_signature->local : NULL;
> > +             task->local_initrd_signature = task->initrd_signature ?
> task->initrd_signature->local : NULL;
> > +             task->local_dtb_signature = task->dtb_signature ?
> task->dtb_signature->local : NULL;
> > +     }
> > +#endif
> > +
> >       run_boot_hooks(task);
> >
> >       update_status(task->status_fn, task->status_arg, BOOT_STATUS_INFO,
> >                       _("performing kexec_load"));
> >
> >       rc = kexec_load(task);
> > -     if (rc) {
> > +     if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
> > +             update_status(task->status_fn, task->status_arg,
> > +                             BOOT_STATUS_ERROR, _("signature
> verification failed"));
> > +     }
> > +     else if (rc == KEXEC_LOAD_SIG_SETUP_INVALID) {
> > +             update_status(task->status_fn, task->status_arg,
> > +                             BOOT_STATUS_ERROR, _("invalid signature
> configuration"));
> > +     }
> > +     else if (rc) {
> >               update_status(task->status_fn, task->status_arg,
> >                               BOOT_STATUS_ERROR, _("kexec load failed"));
> >       }
> >
> > +no_sig_load:
> > +     cleanup_load(task->image_signature);
> > +     cleanup_load(task->initrd_signature);
> > +     cleanup_load(task->dtb_signature);
> > +
> >  no_load:
> >       cleanup_load(task->image);
> >       cleanup_load(task->initrd);
> > @@ -434,6 +749,19 @@ struct boot_task *boot(void *ctx, struct
> discover_boot_option *opt,
> >       const char *boot_desc;
> >       int rc;
> >
> > +#if defined(HAVE_LIBGPGME)
> > +     struct pb_url *image_sig = NULL, *initrd_sig = NULL, *dtb_sig =
> NULL;
> > +
> > +     int max_filename_size = 8192;
> > +     char kernel_signature_filename[max_filename_size];
> > +     char initrd_signature_filename[max_filename_size];
> > +     char dtb_signature_filename[max_filename_size];
> > +
> > +     kernel_signature_filename[0] = 0;
> > +     initrd_signature_filename[0] = 0;
> > +     dtb_signature_filename[0] = 0;
> > +#endif
> > +
> >       if (opt && opt->option->name)
> >               boot_desc = opt->option->name;
> >       else if (cmd && cmd->boot_image_file)
> > @@ -471,6 +799,12 @@ struct boot_task *boot(void *ctx, struct
> discover_boot_option *opt,
> >       boot_task->dry_run = dry_run;
> >       boot_task->status_fn = status_fn;
> >       boot_task->status_arg = status_arg;
> > +#if defined(HAVE_LIBGPGME)
> > +     if (access(LOCKDOWN_FILE, F_OK) == -1)
> > +             boot_task->verify_signature = false;
> > +     else
> > +             boot_task->verify_signature = true;
> > +#endif
> >
> >       if (cmd && cmd->boot_args) {
> >               boot_task->args = talloc_strdup(boot_task, cmd->boot_args);
> > @@ -485,6 +819,41 @@ struct boot_task *boot(void *ctx, struct
> discover_boot_option *opt,
> >       rc = start_url_load(boot_task, "kernel image", image,
> &boot_task->image)
> >         || start_url_load(boot_task, "initrd", initrd,
> &boot_task->initrd)
> >         || start_url_load(boot_task, "dtb", dtb, &boot_task->dtb);
> > +#if defined(HAVE_LIBGPGME)
> > +     if (boot_task->verify_signature) {
> > +             /* Generate names of associated signature files and load */
> > +             if (image) {
> > +                     snprintf(kernel_signature_filename,
> max_filename_size, "%s%s", image->file, ".sig");
> > +                     image_sig = pb_url_copy(ctx, image);
> > +                     talloc_free(image_sig->file);
> > +                     image_sig->file = talloc_strdup(image_sig,
> kernel_signature_filename);
> > +                     snprintf(kernel_signature_filename,
> max_filename_size, "%s%s", image->path, ".sig");
> > +                     talloc_free(image_sig->path);
> > +                     image_sig->path = talloc_strdup(image_sig,
> kernel_signature_filename);
> > +                     rc |= start_url_load(boot_task, "kernel image
> signature", image_sig, &boot_task->image_signature);
>
> In other spots too, but keep to an 80-character limit
>
> > +             }
> > +             if (initrd) {
> > +                     snprintf(initrd_signature_filename,
> max_filename_size, "%s%s", initrd->file, ".sig");
> > +                     initrd_sig = pb_url_copy(ctx, initrd);
> > +                     talloc_free(initrd_sig->file);
> > +                     initrd_sig->file = talloc_strdup(initrd_sig,
> initrd_signature_filename);
> > +                     snprintf(initrd_signature_filename,
> max_filename_size, "%s%s", initrd->path, ".sig");
> > +                     talloc_free(initrd_sig->path);
> > +                     initrd_sig->path = talloc_strdup(initrd_sig,
> initrd_signature_filename);
> > +                     rc |= start_url_load(boot_task, "initrd
> signature", initrd_sig, &boot_task->initrd_signature);
> > +             }
> > +             if (dtb) {
> > +                     snprintf(dtb_signature_filename,
> max_filename_size, "%s%s", dtb->file, ".sig");
> > +                     dtb_sig = pb_url_copy(ctx, dtb);
> > +                     talloc_free(dtb_sig->file);
> > +                     dtb_sig->file = talloc_strdup(dtb_sig,
> dtb_signature_filename);
> > +                     snprintf(dtb_signature_filename,
> max_filename_size, "%s%s", dtb->path, ".sig");
> > +                     talloc_free(dtb_sig->path);
> > +                     dtb_sig->path = talloc_strdup(dtb_sig,
> dtb_signature_filename);
> > +                     rc |= start_url_load(boot_task, "dtb signature",
> dtb_sig, &boot_task->dtb_signature);
> > +             }
> > +     }
> > +#endif
> >
> >       /* If all URLs are local, we may be done. */
> >       if (rc) {
> > diff --git a/lib/url/url.c b/lib/url/url.c
> > index 7202f49..6eeced3 100644
> > --- a/lib/url/url.c
> > +++ b/lib/url/url.c
> > @@ -246,7 +246,7 @@ static void pb_url_update_full(struct pb_url *url)
> >       url->full = pb_url_to_string(url);
> >  }
> >
> > -static struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url)
> > +struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url)
> >  {
> >       struct pb_url *new_url;
> >
> > diff --git a/lib/url/url.h b/lib/url/url.h
> > index 25e1ad8..9043615 100644
> > --- a/lib/url/url.h
> > +++ b/lib/url/url.h
> > @@ -62,6 +62,7 @@ struct pb_url {
> >
> >  bool is_url(const char *str);
> >  struct pb_url *pb_url_parse(void *ctx, const char *url_str);
> > +struct pb_url *pb_url_copy(void *ctx, const struct pb_url *url);
> >  struct pb_url *pb_url_join(void *ctx, const struct pb_url *url, const
> char *s);
> >  char *pb_url_to_string(struct pb_url *url);
> >
> > diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
> > index 3f1e0a2..0d027a8 100644
> > --- a/ui/ncurses/nc-cui.c
> > +++ b/ui/ncurses/nc-cui.c
> > @@ -746,6 +746,9 @@ static struct pmenu *main_menu_init(struct cui *cui)
> >       struct pmenu_item *i;
> >       struct pmenu *m;
> >       int result;
> > +     bool lockdown = false;
> > +     if (access(LOCKDOWN_FILE, F_OK) != -1)
> > +             lockdown = true;
> >
> >       m = pmenu_init(cui, 7, cui_on_exit);
> >       if (!m) {
> > @@ -789,9 +792,16 @@ static struct pmenu *main_menu_init(struct cui *cui)
> >       i->on_execute = menu_add_url_execute;
> >       pmenu_item_insert(m, i, 5);
> >
> > -     i = pmenu_item_create(m, _("Exit to shell"));
> > -     i->on_execute = pmenu_exit_cb;
> > -     pmenu_item_insert(m, i, 6);
> > +     if (lockdown) {
> > +             i = pmenu_item_create(m, _("Reboot"));
> > +             i->on_execute = pmenu_exit_cb;
> > +             pmenu_item_insert(m, i, 6);
> > +     }
> > +     else {
> > +             i = pmenu_item_create(m, _("Exit to shell"));
> > +             i->on_execute = pmenu_exit_cb;
> > +             pmenu_item_insert(m, i, 6);
> > +     }
>
> The problem here is that while the "Exit to shell" button is replaced,
> users can still exit with 'x' or potentially some other key combination
> that causes ncurses to exit.
> Additionally it looks like the "Reboot" button just calls the exit
> callback anyway :)
>
> >
> >       result = pmenu_setup(m);
> >
>
> _______________________________________________
> Petitboot mailing list
> Petitboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/petitboot/attachments/20160801/19c016c0/attachment-0001.html>


More information about the Petitboot mailing list