[PATCH v2 0/5] Cryptomount keyfile support

* [PATCH v2 0/5] Cryptomount keyfile support
@ 2022-05-13 17:00 Glenn Washburn
  2022-05-13 17:00 ` [PATCH v2 1/5] cryptodisk: luks: Unify grub_cryptodisk_dev function names Glenn Washburn
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Glenn Washburn @ 2022-05-13 17:00 UTC (permalink / raw)
  To: grub-devel, Daniel Kiper
  Cc: Denis 'GNUtoo' Carikli, Patrick Steinhardt, John Lane,
	Glenn Washburn

Updates from v1:
 * Make some changes suggested by Daniel
 * Improve error message for grub_strtoull() failures
 * Add patch to use enum constants to index parsed option array

Glenn

Denis 'GNUtoo' Carikli (2):
  cryptodisk: luks: Unify grub_cryptodisk_dev function names
  cryptodisk: geli: Unify grub_cryptodisk_dev function names

Glenn Washburn (2):
  cryptodisk: Use enum constants as indexes into cryptomount option
    array
  docs: Add documentation on keyfile option to cryptomount

John Lane (1):
  cryptodisk: Add options to cryptomount to support keyfiles

 docs/grub.texi              |  14 ++--
 grub-core/disk/cryptodisk.c | 133 +++++++++++++++++++++++++++++++++---
 grub-core/disk/geli.c       |   8 +--
 grub-core/disk/luks.c       |   4 +-
 include/grub/cryptodisk.h   |   2 +
 include/grub/file.h         |   2 +
 6 files changed, 142 insertions(+), 21 deletions(-)

Range-diff against v1:
1:  b19b567a6 = 1:  b19b567a6 cryptodisk: luks: Unify grub_cryptodisk_dev function names
2:  80a284dbe = 2:  80a284dbe cryptodisk: geli: Unify grub_cryptodisk_dev function names
3:  8c2cd5ce6 ! 3:  34816c265 cryptodisk: Add options to cryptomount to support keyfiles
    @@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t ctxt, i
      
     +  if (state[4].set) /* keyfile */
     +    {
    ++      char tmp_errmsg[GRUB_MAX_ERRMSG];
     +      const char *p = NULL;
     +      grub_file_t keyfile;
     +      unsigned long long keyfile_offset = 0, keyfile_size = 0;
     +
     +      if (state[5].set) /* keyfile-offset */
     +	{
    ++	  grub_errno = GRUB_ERR_NONE;
     +	  keyfile_offset = grub_strtoull (state[5].arg, &p, 0);
     +
    -+	  if (grub_errno != GRUB_ERR_NONE)
    -+	    return grub_errno;
    -+
     +	  if (state[5].arg[0] == '\0' || *p != '\0')
    -+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
    -+			       N_("non-numeric or invalid keyfile offset `%s'"),
    -+			       state[5].arg);
    ++	    {
    ++	      if (grub_errno != GRUB_ERR_NONE)
    ++		{
    ++		  grub_strncpy (tmp_errmsg, grub_errmsg, GRUB_MAX_ERRMSG);
    ++		  return grub_error (grub_errno,
    ++				     N_("non-numeric or invalid keyfile offset `%s': %s"),
    ++				     state[5].arg, tmp_errmsg);
    ++		}
    ++	      else
    ++		return grub_error (GRUB_ERR_BAD_ARGUMENT,
    ++				   N_("invalid keyfile offset `%s': non-numeric"
    ++				      " characters at end of number"),
    ++				   state[5].arg);
    ++	    }
     +	}
     +
     +      if (state[6].set) /* keyfile-size */
     +	{
    -+	  keyfile_size = grub_strtoul (state[6].arg, &p, 0);
    ++	  grub_errno = GRUB_ERR_NONE;
    ++	  keyfile_size = grub_strtoull (state[6].arg, &p, 0);
     +
     +	  if (state[6].arg[0] == '\0' || *p != '\0')
    -+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
    -+			       N_("non-numeric or invalid keyfile size `%s'"),
    -+			       state[6].arg);
    -+
    -+	  if (grub_errno != GRUB_ERR_NONE)
    -+	    return grub_errno;
    ++	    {
    ++	      if (grub_errno != GRUB_ERR_NONE)
    ++		{
    ++		  grub_strncpy (tmp_errmsg, grub_errmsg, GRUB_MAX_ERRMSG);
    ++		  return grub_error (grub_errno,
    ++				     N_("non-numeric or invalid keyfile offset `%s': %s"),
    ++				     state[5].arg, tmp_errmsg);
    ++		}
    ++	      else
    ++		return grub_error (GRUB_ERR_BAD_ARGUMENT,
    ++				   N_("invalid keyfile offset `%s': non-numeric"
    ++				      " characters at end of number"),
    ++				   state[6].arg);
    ++	    }
     +
     +	  if (keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE)
     +	    return grub_error (GRUB_ERR_OUT_OF_RANGE,
    @@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t ctxt, i
     +	return grub_errno;
     +
     +      if (keyfile_offset > keyfile->size)
    -+	{
    -+	  keyfile_offset = keyfile->size;
    -+	  grub_dprintf ("cryptodisk","Keyfile offset, %llu, is greater than"
    -+				     "keyfile size, %" PRIuGRUB_UINT64_T "\n",
    -+				     keyfile_offset, keyfile->size);
    -+	}
    ++	return grub_error (GRUB_ERR_OUT_OF_RANGE,
    ++			   N_("Keyfile offset, %llu, is greater than"
    ++			      "keyfile size, %" PRIuGRUB_UINT64_T),
    ++			   keyfile_offset, keyfile->size);
     +
     +      if (grub_file_seek (keyfile, (grub_off_t) keyfile_offset) == (grub_off_t) -1)
     +	return grub_errno;
     +
    -+      if (keyfile_size > 0)
    ++      if (keyfile_size != 0)
     +	{
     +	  if (keyfile_size > (keyfile->size - keyfile_offset))
     +	    return grub_error (GRUB_ERR_FILE_READ_ERROR,
     +			       N_("keyfile is too small: requested %llu bytes,"
     +				  " but the file only has %" PRIuGRUB_UINT64_T
    -+				  " bytes"),
    ++				  " bytes left at offset %llu"),
     +			       keyfile_size,
    -+			       keyfile->size);
    ++			       (grub_size_t) (keyfile->size - keyfile_offset),
    ++			       keyfile_offset);
     +
     +	  cargs.key_len = keyfile_size;
     +	}
    @@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t ctxt, i
     +	return GRUB_ERR_OUT_OF_MEMORY;
     +
     +      if (grub_file_read (keyfile, cargs.key_data, cargs.key_len) != (grub_ssize_t) cargs.key_len)
    -+	return grub_error (GRUB_ERR_FILE_READ_ERROR, (N_("reading key file")));
    ++	return grub_error (GRUB_ERR_FILE_READ_ERROR, (N_("failed to read key file")));
     +    }
     +
        if (state[0].set) /* uuid */
-:  --------- > 4:  e7fd2c8cb cryptodisk: Use enum constants as indexes into cryptomount option array
4:  459a61800 = 5:  dccdd7e03 docs: Add documentation on keyfile option to cryptomount
-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread