From: Patrick Steinhardt <ps@pks.im>
To: The development of GNU GRUB <grub-devel@gnu.org>
Cc: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>,
John Lane <john@lane.uk.net>
Subject: Re: [PATCH 1/2] Cryptomount support LUKS detached header
Date: Tue, 25 Feb 2020 22:46:22 +0100 [thread overview]
Message-ID: <20200225214622.GA4264@xps> (raw)
In-Reply-To: <20200224111237.jqtefw7ms2ldpw66@tomti.i.net-space.pl>
[-- Attachment #1: Type: text/plain, Size: 10038 bytes --]
On Mon, Feb 24, 2020 at 12:12:37PM +0100, Daniel Kiper wrote:
> Adding Patrick...
>
> On Fri, Feb 21, 2020 at 10:03:48PM +0100, Denis 'GNUtoo' Carikli wrote:
> > From: John Lane <john@lane.uk.net>
>
> Both patches require explanation what they do and more importantly why
> they are needed... And it would be nice to have cover letter.
>
> ...and please CC Patrick next time...
>
> Daniel
I'll have a look towards the end of this week.
Patrick
> > Signed-off-by: John Lane <john@lane.uk.net>
> > GNUtoo@cyberdimension.org: rebase
> > Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
> > ---
> > grub-core/disk/cryptodisk.c | 22 ++++++++++++++----
> > grub-core/disk/geli.c | 7 ++++--
> > grub-core/disk/luks.c | 45 ++++++++++++++++++++++++++++++-------
> > include/grub/cryptodisk.h | 5 +++--
> > include/grub/file.h | 2 ++
> > 5 files changed, 65 insertions(+), 16 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index 1897acc4b..6d4befc6f 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =
> > /* TRANSLATORS: It's still restricted to cryptodisks only. */
> > {"all", 'a', 0, N_("Mount all."), 0, 0},
> > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
> > + {"header", 'H', 0, N_("Read LUKS header from file"), 0, ARG_TYPE_STRING},
> > {0, 0, 0, 0, 0, 0}
> > };
> >
> > @@ -970,6 +971,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
> >
> > static int check_boot, have_it;
> > static char *search_uuid;
> > +static grub_file_t hdr;
> >
> > static void
> > cryptodisk_close (grub_cryptodisk_t dev)
> > @@ -994,13 +996,13 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
> >
> > FOR_CRYPTODISK_DEVS (cr)
> > {
> > - dev = cr->scan (source, search_uuid, check_boot);
> > + dev = cr->scan (source, search_uuid, check_boot, hdr);
> > if (grub_errno)
> > return grub_errno;
> > if (!dev)
> > continue;
> >
> > - err = cr->recover_key (source, dev);
> > + err = cr->recover_key (source, dev, hdr);
> > if (err)
> > {
> > cryptodisk_close (dev);
> > @@ -1041,7 +1043,7 @@ grub_cryptodisk_cheat_mount (const char *sourcedev, const char *cheat)
> >
> > FOR_CRYPTODISK_DEVS (cr)
> > {
> > - dev = cr->scan (source, search_uuid, check_boot);
> > + dev = cr->scan (source, search_uuid, check_boot,0);
> > if (grub_errno)
> > return grub_errno;
> > if (!dev)
> > @@ -1095,6 +1097,18 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
> > if (argc < 1 && !state[1].set && !state[2].set)
> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
> >
> > + if (state[3].set) /* LUKS detached header */
> > + {
> > + if (state[0].set) /* Cannot use UUID lookup with detached header */
> > + return GRUB_ERR_BAD_ARGUMENT;
> > +
> > + hdr = grub_file_open (state[3].arg, GRUB_FILE_TYPE_LUKS_DETACHED_HEADER);
> > + if (!hdr)
> > + return grub_errno;
> > + }
> > + else
> > + hdr = NULL;
> > +
> > have_it = 0;
> > if (state[0].set)
> > {
> > @@ -1302,7 +1316,7 @@ GRUB_MOD_INIT (cryptodisk)
> > {
> > grub_disk_dev_register (&grub_cryptodisk_dev);
> > cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
> > - N_("SOURCE|-u UUID|-a|-b"),
> > + N_("SOURCE|-u UUID|-a|-b|-H file"),
> > N_("Mount a crypto device."), options);
> > grub_procfs_register ("luks_script", &luks_script);
> > }
> > diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
> > index e9d23299a..f4394eb42 100644
> > --- a/grub-core/disk/geli.c
> > +++ b/grub-core/disk/geli.c
> > @@ -52,6 +52,7 @@
> > #include <grub/dl.h>
> > #include <grub/err.h>
> > #include <grub/disk.h>
> > +#include <grub/file.h>
> > #include <grub/crypto.h>
> > #include <grub/partition.h>
> > #include <grub/i18n.h>
> > @@ -243,7 +244,8 @@ grub_util_get_geli_uuid (const char *dev)
> >
> > static grub_cryptodisk_t
> > configure_ciphers (grub_disk_t disk, const char *check_uuid,
> > - int boot_only)
> > + int boot_only,
> > + grub_file_t hdr __attribute__ ((unused)) )
> > {
> > grub_cryptodisk_t newdev;
> > struct grub_geli_phdr header;
> > @@ -398,7 +400,8 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid,
> > }
> >
> > static grub_err_t
> > -recover_key (grub_disk_t source, grub_cryptodisk_t dev)
> > +recover_key (grub_disk_t source, grub_cryptodisk_t dev,
> > + grub_file_t hdr __attribute__ ((unused)) )
> > {
> > grub_size_t keysize;
> > grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN];
> > diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> > index 410cd6f84..950e89237 100644
> > --- a/grub-core/disk/luks.c
> > +++ b/grub-core/disk/luks.c
> > @@ -23,6 +23,7 @@
> > #include <grub/dl.h>
> > #include <grub/err.h>
> > #include <grub/disk.h>
> > +#include <grub/file.h>
> > #include <grub/crypto.h>
> > #include <grub/partition.h>
> > #include <grub/i18n.h>
> > @@ -66,7 +67,7 @@ gcry_err_code_t AF_merge (const gcry_md_spec_t * hash, grub_uint8_t * src,
> >
> > static grub_cryptodisk_t
> > configure_ciphers (grub_disk_t disk, const char *check_uuid,
> > - int check_boot)
> > + int check_boot, grub_file_t hdr)
> > {
> > grub_cryptodisk_t newdev;
> > const char *iptr;
> > @@ -78,11 +79,21 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid,
> > char hashspec[sizeof (header.hashSpec) + 1];
> > grub_err_t err;
> >
> > + err = GRUB_ERR_NONE;
> > +
> > if (check_boot)
> > return NULL;
> >
> > /* Read the LUKS header. */
> > - err = grub_disk_read (disk, 0, 0, sizeof (header), &header);
> > + if (hdr)
> > + {
> > + grub_file_seek (hdr, 0);
> > + if (grub_file_read (hdr, &header, sizeof (header)) != sizeof (header))
> > + err = GRUB_ERR_READ_ERROR;
> > + }
> > + else
> > + err = grub_disk_read (disk, 0, 0, sizeof (header), &header);
> > +
> > if (err)
> > {
> > if (err == GRUB_ERR_OUT_OF_RANGE)
> > @@ -146,12 +157,14 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid,
> > }
> >
> > COMPILE_TIME_ASSERT (sizeof (newdev->uuid) >= sizeof (uuid));
> > +
> > return newdev;
> > }
> >
> > static grub_err_t
> > luks_recover_key (grub_disk_t source,
> > - grub_cryptodisk_t dev)
> > + grub_cryptodisk_t dev,
> > + grub_file_t hdr)
> > {
> > struct grub_luks_phdr header;
> > grub_size_t keysize;
> > @@ -163,8 +176,19 @@ luks_recover_key (grub_disk_t source,
> > grub_err_t err;
> > grub_size_t max_stripes = 1;
> > char *tmp;
> > + grub_uint32_t sector;
> > +
> > + err = GRUB_ERR_NONE;
> > +
> > + if (hdr)
> > + {
> > + grub_file_seek (hdr, 0);
> > + if (grub_file_read (hdr, &header, sizeof (header)) != sizeof (header))
> > + err = GRUB_ERR_READ_ERROR;
> > + }
> > + else
> > + err = grub_disk_read (source, 0, 0, sizeof (header), &header);
> >
> > - err = grub_disk_read (source, 0, 0, sizeof (header), &header);
> > if (err)
> > return err;
> >
> > @@ -233,13 +257,18 @@ luks_recover_key (grub_disk_t source,
> > return grub_crypto_gcry_error (gcry_err);
> > }
> >
> > + sector = grub_be_to_cpu32 (header.keyblock[i].keyMaterialOffset);
> > length = (keysize * grub_be_to_cpu32 (header.keyblock[i].stripes));
> >
> > /* Read and decrypt the key material from the disk. */
> > - err = grub_disk_read (source,
> > - grub_be_to_cpu32 (header.keyblock
> > - [i].keyMaterialOffset), 0,
> > - length, split_key);
> > + if (hdr)
> > + {
> > + grub_file_seek (hdr, sector * 512);
> > + if (grub_file_read (hdr, split_key, length) != (grub_ssize_t)length)
> > + err = GRUB_ERR_READ_ERROR;
> > + }
> > + else
> > + err = grub_disk_read (source, sector, 0, length, split_key);
> > if (err)
> > {
> > grub_free (split_key);
> > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> > index e1b21e785..ccbe7ce93 100644
> > --- a/include/grub/cryptodisk.h
> > +++ b/include/grub/cryptodisk.h
> > @@ -20,6 +20,7 @@
> > #define GRUB_CRYPTODISK_HEADER 1
> >
> > #include <grub/disk.h>
> > +#include <grub/file.h>
> > #include <grub/crypto.h>
> > #include <grub/list.h>
> > #ifdef GRUB_UTIL
> > @@ -107,8 +108,8 @@ struct grub_cryptodisk_dev
> > struct grub_cryptodisk_dev **prev;
> >
> > grub_cryptodisk_t (*scan) (grub_disk_t disk, const char *check_uuid,
> > - int boot_only);
> > - grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev);
> > + int boot_only, grub_file_t hdr);
> > + grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev, grub_file_t hdr);
> > };
> > typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
> >
> > diff --git a/include/grub/file.h b/include/grub/file.h
> > index 31567483c..49428bf8d 100644
> > --- a/include/grub/file.h
> > +++ b/include/grub/file.h
> > @@ -90,6 +90,8 @@ enum grub_file_type
> > GRUB_FILE_TYPE_FONT,
> > /* File holding encryption key for encrypted ZFS. */
> > GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY,
> > + /* LUKS detached (separated) metadata header */
> > + GRUB_FILE_TYPE_LUKS_DETACHED_HEADER,
> > /* File we open n grub-fstest. */
> > GRUB_FILE_TYPE_FSTEST,
> > /* File we open n grub-mount. */
> > --
> > 2.25.1
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-02-25 21:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-21 21:03 [PATCH 1/2] Cryptomount support LUKS detached header Denis 'GNUtoo' Carikli
2020-02-21 21:03 ` [PATCH 2/2] Cryptomount support key files Denis 'GNUtoo' Carikli
2020-03-01 8:39 ` Patrick Steinhardt
2020-02-24 11:12 ` [PATCH 1/2] Cryptomount support LUKS detached header Daniel Kiper
2020-02-25 21:46 ` Patrick Steinhardt [this message]
2020-03-01 8:26 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200225214622.GA4264@xps \
--to=ps@pks.im \
--cc=GNUtoo@cyberdimension.org \
--cc=grub-devel@gnu.org \
--cc=john@lane.uk.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.