Hello John!
I see your commit 81b2f625f54cb670e36739e3a599daafd34bc44a, about
adding key-file support. This is great! I've been waiting for grub
official support for removable key-file support for a long time.
I suppose grub key-file meant to keep key files on a separate drive
with fast removal feature (aka USB), not on the same drive? Basically
using USB as removable, cheap, TPM device. Right? Great!
If so. Then why does your allow users to remove a removable key?
Because your code, strictly requiring for key file to exist and be
available to read, and if grub fails to read the key then cryptomount
function will fail.
As we know grub rescue shell is very limited, and dosn't even have a
'if' statement. Initial script can only have few commands like 'search'
or 'cryptomount'. Here is no option for user to write a script which
can check if key file exists and readable before calling 'cryptomount'
func. Then if we want to support removable keys, then code should allow
to fail when reading keys.
Here is my patch on top of your work, attached.
Allow grub to continue after key reading fail. Usefull when USB drive (with a key) unmounted and we can continue nomral decyption procedure (with a password).
We could try:
if search.fs_uuid $USBUUID u; then
cryptomount -k (\$u)/$KEY -u ${UUID//-/}
else
cryptomount -u ${UUID//-/}
fi
But grub rescue shell has no 'if' or '||' commands.
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 2246af5..58c04c6 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -1289,9 +1289,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
keyfile_offset = grub_strtoull (state[OPTION_KEYFILE_OFFSET].arg, &p, 0);
if (state[OPTION_KEYFILE_OFFSET].arg[0] == '\0' || *p != '\0')
- return grub_error (grub_errno,
- N_("non-numeric or invalid keyfile offset `%s'"),
- state[OPTION_KEYFILE_OFFSET].arg);
+ goto next;
}
if (state[OPTION_KEYFILE_SIZE].set) /* keyfile-size */
@@ -1300,43 +1298,30 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
keyfile_size = grub_strtoull (state[OPTION_KEYFILE_SIZE].arg, &p, 0);
if (state[OPTION_KEYFILE_SIZE].arg[0] == '\0' || *p != '\0')
- return grub_error (grub_errno,
- N_("non-numeric or invalid keyfile size `%s'"),
- state[OPTION_KEYFILE_SIZE].arg);
+ goto next;
if (keyfile_size == 0)
- return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("key file size is 0"));
+ goto next;
if (keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE)
- return grub_error (GRUB_ERR_OUT_OF_RANGE,
- N_("key file size exceeds maximum (%d)"),
- GRUB_CRYPTODISK_MAX_KEYFILE_SIZE);
+ goto next;
}
keyfile = grub_file_open (state[OPTION_KEYFILE].arg,
GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY);
if (keyfile == NULL)
- return grub_errno;
+ goto next;
if (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);
+ goto next;
if (grub_file_seek (keyfile, (grub_off_t) keyfile_offset) == (grub_off_t) -1)
- return grub_errno;
+ goto next;
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 left at offset %llu"),
- keyfile_size,
- (grub_off_t) (keyfile->size - keyfile_offset),
- keyfile_offset);
+ goto next;
cargs.key_len = keyfile_size;
}
@@ -1345,12 +1330,16 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
cargs.key_data = grub_malloc (cargs.key_len);
if (cargs.key_data == NULL)
- return GRUB_ERR_OUT_OF_MEMORY;
+ goto next;
- 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_("failed to read key file")));
+ if (grub_file_read (keyfile, cargs.key_data, cargs.key_len) != (grub_ssize_t) cargs.key_len) {
+ cargs.key_data = NULL;
+ cargs.key_len = 0;
+ goto next;
+ }
}
+next:
if (state[OPTION_HEADER].set) /* header */
{
if (state[OPTION_UUID].set)
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
