All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrei Borzenkov <arvidjaar@gmail.com>
To: Alexandre Oliva <oliva@gnu.org>
Cc: grub-devel@gnu.org, John Lane <grub@jelmail.com>
Subject: Re: enable cryptomount to read passphrase from file
Date: Sun, 18 Jan 2015 17:57:19 +0300	[thread overview]
Message-ID: <20150118175719.5ddabee5@opensuse.site> (raw)
In-Reply-To: <orfvb8kbpy.fsf@livre.home>

В Sun, 18 Jan 2015 01:29:45 -0200
Alexandre Oliva <oliva@gnu.org> пишет:

> Here's a compile-tested patch that attempts to add '-p FILE' support to
> cryptomount, so that the passphrase can be read from a usb key or
> somesuch.
> 

I suggest you cooperate with John for this; he has a set of patches to
support it as well. See also http://grub.johnlane.ie/.

> I was unsure about how much to massage or verify the file, say, dropping
> a trailing newline or truncating the passphrase at the first newline or
> whatever.  I ended up deciding to keep it simple and just use the file
> contents unchanged in any way (though the passphrase users will use it
> as a C string, so any NUL in the file will effectively truncate the
> passphrase).  Thoughts?
> 

Just pass in passphrase+len. Do not expect anything about content of
passphrase file at all.

> 
> I've only checked that it builds, but I haven't really tested it yet
> I'm not sure yet how to go about that without risking rendering my
> boxes unbootable :-)  Suggestions are welcome.
> 

You can always unlock encrypted filesystem manually, right?

> Thanks,
> 
> Index: docs/grub.texi
> --- grub.orig/docs/grub.texi	2014-10-14 23:30:59.000000000 -0300
> +++ grub/docs/grub.texi	2015-01-18 01:26:49.499924206 -0200
> @@ -4073,9 +4073,12 @@ Alias for @code{hashsum --hash crc32 arg
>  @node cryptomount
>  @subsection cryptomount
>  
> -@deffn Command cryptomount device|@option{-u} uuid|@option{-a}|@option{-b}
> -Setup access to encrypted device. If necessary, passphrase
> -is requested interactively. Option @var{device} configures specific grub device
> +@deffn Command cryptomount [@option{-p} file] device|@option{-u} uuid|@option{-a}|@option{-b}
> +Setup access to encrypted device.  The passphrase will be asked
> +interactively, unless @option{-p} @var{file} is specified and the
> +passphrase read from the file succeeds.  The file should contain
> +@emph{only} the passphrase, not even a trailing line break.  Option
> +@var{device} configures specific grub device

Hmm ... I'm not sure whether we should fallback to asking user. The whole
point of using keyfile is to avoid user interaction in the first place,
right? May be it makes more sense to simply fail boot in this case?
This allows you to continue with nexct boot option.

>  (@pxref{Naming convention}); option @option{-u} @var{uuid} configures device
>  with specified @var{uuid}; option @option{-a} configures all detected encrypted
>  devices; option @option{-b} configures all geli containers that have boot flag set.

> Index: grub-core/disk/cryptodisk.c
> --- grub.orig/grub-core/disk/cryptodisk.c	2014-10-14 23:30:57.000000000 -0300
> +++ grub/grub-core/disk/cryptodisk.c	2015-01-18 01:08:27.506796052 -0200
>    
> @@ -916,10 +920,49 @@ static grub_err_t
>  grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>  {
>    struct grub_arg_list *state = ctxt->state;
> +  char *key = NULL;
> +  char buf[GRUB_CRYPTODISK_MAX_PASSPHRASE];
>  
>    if (argc < 1 && !state[1].set && !state[2].set)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
>  
> +  /* Read key from file.  */
> +  if (ctxt->state[3].set)
> +    {
> +      char *fname = ctxt->state[3].arg;
> +      grub_file_t file;
> +      file = grub_file_open (fname);
> +      if (!file)
> +	grub_printf_ (N_("Failed to open passphrase file\n"));
> +      else if (file)

Comparison is redundant; it was already checked to be !NULL.

> +	{
> +	  grub_ssize_t size;
> +	  size = grub_file_read (file, buf, sizeof (buf));
> +	  if (size < 0)
> +	    grub_printf_ (N_("Error reading from passphrase file\n"));
> +	  else if (size == 0)
> +	    grub_printf_ (N_("Passphrase file is empty\n"));
> +	  else if ((grub_size_t)size >= sizeof (buf))

Can it be larger than sizeof(buf)?

> +	    grub_printf_ (N_("Passphrase file is too big\n"));

With my non-native English I find "too long" sounds better.

> +	  else
> +	    {
> +#if 0
> +	      int count;
> +	      for (count = 0; count < size; count++)
> +		if (buf[count] == 0
> +		    || buf[count] == '\n'
> +		    || buf[count] == '\r')
> +		  break;
> +	      if (count != size)
> +		grub_printf_ (N_("Extraneous byte in passphrase file!\n"));
> +#endif

Is it prohibited to have new line or carriage return in passphrase file?

> +	      grub_memset (buf + size, 0, sizeof (buf) - size);
> +	      key = buf;
> +	    }
> +	}
> +      grub_file_close (file);
> +    }
> +
>    have_it = 0;
>    if (state[0].set)
>      {
> @@ -935,7 +978,7 @@ grub_cmd_cryptomount (grub_extcmd_contex
>  
>        check_boot = state[2].set;
>        search_uuid = args[0];
> -      grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> +      grub_device_iterate (&grub_cryptodisk_scan_device, key);
>        search_uuid = NULL;
>  
>        if (!have_it)
> @@ -946,7 +989,7 @@ grub_cmd_cryptomount (grub_extcmd_contex
>      {
>        search_uuid = NULL;
>        check_boot = state[2].set;
> -      grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> +      grub_device_iterate (&grub_cryptodisk_scan_device, key);
>        search_uuid = NULL;
>        return GRUB_ERR_NONE;
>      }
> @@ -980,7 +1023,7 @@ grub_cmd_cryptomount (grub_extcmd_contex
>  	  return GRUB_ERR_NONE;
>  	}
>  
> -      err = grub_cryptodisk_scan_device_real (args[0], disk);
> +      err = grub_cryptodisk_scan_device_real (args[0], disk, key);
>  
>        grub_disk_close (disk);
>  
> @@ -1117,7 +1160,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_("[-p FILE] SOURCE|-u UUID|-a|-b"),
>  			      N_("Mount a crypto device."), options);
>    grub_procfs_register ("luks_script", &luks_script);
>  }
> Index: include/grub/cryptodisk.h
> --- grub.orig/include/grub/cryptodisk.h	2014-10-14 23:31:00.000000000 -0300
> +++ grub/include/grub/cryptodisk.h	2015-01-18 00:14:22.572709178 -0200
> @@ -54,6 +54,10 @@ typedef enum
>  #define GRUB_CRYPTODISK_GF_BYTES (1U << GRUB_CRYPTODISK_GF_LOG_BYTES)
>  #define GRUB_CRYPTODISK_MAX_KEYLEN 128
>  
> +/* Backends may limit passphrases to smaller sizes, but this should be
> +   the max among them all.  */
> +#define GRUB_CRYPTODISK_MAX_PASSPHRASE 256
> +
>  struct grub_cryptodisk;
>  
>  typedef gcry_err_code_t
> @@ -107,7 +111,8 @@ struct grub_cryptodisk_dev
>  
>    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);
> +  grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
> +			     const char *key);
>  };
>  typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
>  
> Index: grub-core/disk/geli.c
> --- grub.orig/grub-core/disk/geli.c	2014-10-14 23:30:57.000000000 -0300
> +++ grub/grub-core/disk/geli.c	2015-01-18 01:06:31.422050391 -0200
> @@ -135,7 +135,7 @@ const char *algorithms[] = {
>    [0x16] = "aes"
>  };
>  
> -#define MAX_PASSPHRASE 256
> +#define MAX_PASSPHRASE GRUB_CRYPTODISK_MAX_PASSPHRASE
>  
>  static gcry_err_code_t
>  geli_rekey (struct grub_cryptodisk *dev, grub_uint64_t zoneno)
> @@ -384,7 +384,7 @@ configure_ciphers (grub_disk_t disk, con
>  }
>  
>  static grub_err_t
> -recover_key (grub_disk_t source, grub_cryptodisk_t dev)
> +recover_key (grub_disk_t source, grub_cryptodisk_t dev, const char *key)
>  {
>    grub_size_t keysize;
>    grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN];
> @@ -419,16 +419,31 @@ recover_key (grub_disk_t source, grub_cr
>  
>    grub_puts_ (N_("Attempting to decrypt master key..."));
>  
> -  /* Get the passphrase from the user.  */
> + retry:
>    tmp = NULL;
> -  if (source->partition)
> -    tmp = grub_partition_get_name (source->partition);
> -  grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> -		source->partition ? "," : "", tmp ? : "",
> -		dev->uuid);
> -  grub_free (tmp);
> -  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> -    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
> +  if (key && grub_strlen (key) >= MAX_PASSPHRASE)
> +    {
> +      grub_printf_ (N_("Discarding too-long supplied passphrase\n"));

passphrase file?

> +      key = NULL;
> +    }
> +  if (key)
> +    {
> +      grub_size_t len = grub_strlen (key);
> +      grub_memcpy (passphrase, key, len);
> +      grub_memset (passphrase + len, 0, sizeof (passphrase) - len);
> +    }
> +  else
> +    {
> +      /* Get the passphrase from the user.  */
> +      if (source->partition)
> +	tmp = grub_partition_get_name (source->partition);
> +      grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> +		    source->partition ? "," : "", tmp ? : "",
> +		    dev->uuid);
> +      grub_free (tmp);
> +      if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +	return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
> +    }
>  
>    /* Calculate the PBKDF2 of the user supplied passphrase.  */
>    if (grub_le_to_cpu32 (header.niter) != 0)
> @@ -548,6 +563,13 @@ recover_key (grub_disk_t source, grub_cr
>        return GRUB_ERR_NONE;
>      }
>  
> +  if (key)
> +    {
> +      grub_printf_ (N_("Supplied passphrase failed to unlock key\n"));

"Passphrase file failed to unlock key"?

> +      key = NULL;
> +      goto retry;
> +    }
> +
>    return GRUB_ACCESS_DENIED;
>  }
>  
> Index: grub-core/disk/luks.c
> --- grub.orig/grub-core/disk/luks.c	2014-10-14 23:30:57.000000000 -0300
> +++ grub/grub-core/disk/luks.c	2015-01-18 01:06:26.336192968 -0200
> @@ -29,7 +29,7 @@
>  
>  GRUB_MOD_LICENSE ("GPLv3+");
>  
> -#define MAX_PASSPHRASE 256
> +#define MAX_PASSPHRASE GRUB_CRYPTODISK_MAX_PASSPHRASE
>  
>  #define LUKS_KEY_ENABLED  0x00AC71F3
>  
> @@ -297,7 +297,8 @@ configure_ciphers (grub_disk_t disk, con
>  
>  static grub_err_t
>  luks_recover_key (grub_disk_t source,
> -		  grub_cryptodisk_t dev)
> +		  grub_cryptodisk_t dev,
> +		  const char *key)
>  {
>    struct grub_luks_phdr header;
>    grub_size_t keysize;
> @@ -328,18 +329,33 @@ luks_recover_key (grub_disk_t source,
>    if (!split_key)
>      return grub_errno;
>  
> -  /* Get the passphrase from the user.  */
> + retry:
>    tmp = NULL;
> -  if (source->partition)
> -    tmp = grub_partition_get_name (source->partition);
> -  grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> -	       source->partition ? "," : "", tmp ? : "",
> -	       dev->uuid);
> -  grub_free (tmp);
> -  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +  if (key && grub_strlen (key) >= MAX_PASSPHRASE)
>      {
> -      grub_free (split_key);
> -      return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
> +      grub_printf_ (N_("Discarding too-long supplied passphrase\n"));

passphrase file?

> +      key = NULL;
> +    }
> +  if (key)
> +    {
> +      grub_size_t len = grub_strlen (key);
> +      grub_memcpy (passphrase, key, len);
> +      grub_memset (passphrase + len, 0, sizeof (passphrase) - len);
> +    }
> +  else
> +    {
> +      /* Get the passphrase from the user.  */
> +      if (source->partition)
> +	tmp = grub_partition_get_name (source->partition);
> +      grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> +		    source->partition ? "," : "", tmp ? : "",
> +		    dev->uuid);
> +      grub_free (tmp);
> +      if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +	{
> +	  grub_free (split_key);
> +	  return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
> +	}
>      }
>  
>    /* Try to recover master key from each active keyslot.  */
> @@ -451,6 +467,13 @@ luks_recover_key (grub_disk_t source,
>        return GRUB_ERR_NONE;
>      }
>  
> +  if (key)
> +    {
> +      grub_printf_ (N_("Supplied passphrase failed to unlock key\n"));

passhprase file

> +      key = NULL;
> +      goto retry;
> +    }
> +
>    return GRUB_ACCESS_DENIED;
>  }
>  
> 
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


  reply	other threads:[~2015-01-18 14:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-18  3:29 enable cryptomount to read passphrase from file Alexandre Oliva
2015-01-18 14:57 ` Andrei Borzenkov [this message]
2015-01-19  6:33   ` Alexandre Oliva

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=20150118175719.5ddabee5@opensuse.site \
    --to=arvidjaar@gmail.com \
    --cc=grub-devel@gnu.org \
    --cc=grub@jelmail.com \
    --cc=oliva@gnu.org \
    /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.