[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v5 0/9] Refactor/improve cryptomount data passing to crypto modul
From: |
Glenn Washburn |
Subject: |
[PATCH v5 0/9] Refactor/improve cryptomount data passing to crypto modules |
Date: |
Thu, 9 Dec 2021 11:14:49 -0600 |
Updates since v4:
* Rework patch #2 to (hopefully) be easier to understand
* Add more commentary to patch #2 commit message
* Split previous patch #3 into three separate patches
---
This patch series refactors the way cryptomount passes data to the crypto
modules. Currently, the method has been by global variable and function call
argument, neither of which are ideal. This method passes data via a
grub_cryptomount_args struct, which can be added to over time as opposed to
continually adding arguments to the cryptodisk recover_key (as is being
proposed in the keyfile and detached header patches).
To make thing simpler and easier to understand, the "have_it" global variable
is gotten rid of first in patch #2. Taking advantage of this change, patches
#3-5 improve some long standing issues in cryptomount error messaging.
Then, the infrastructure for passing argument data to cryptodisk backends is
implemented in patch #6 along with adding a new -p parameter to cryptomount
partly as an example to show how a password would be passed to the crypto
module backends. The backends do nothing with this data in this patch, but
print a message saying that sending a password is unimplemented.
Patch #7 takes advantage of this new data passing mechanism to refactor the
essentially duplicated code in each crypto backend module for inputting the
password and puts that functionality in the cryptodisk code. Conceptually,
the crypto backends should not be getting user input anyway.
Patch #8 gets rid of some long time globals in cryptodisk, moving them
into the passed struct.
Patch #9 improves handling of partition name in cryptomount password prompt.
My intention is for this patch series to lay the foundation for an improved
patch series providing detached header and keyfile support (I already have
the series updated and ready to send once this is accepted). I also believe
tha this will somewhat simplify the patch series by James Bottomley in
passing secrets to the crypto backends.
Glenn
Glenn Washburn (9):
luks2: Add debug message to align with luks and geli modules
cryptodisk: Refactor to discard have_it global
cryptodisk: Return failure in cryptomount when no cryptodisk modules
are loaded
cryptodisk: Improve error messaging in cryptomount invocations
cryptodisk: Improve cryptomount -u error message
cryptodisk: Add infrastructure to pass data from cryptomount to
cryptodisk modules
cryptodisk: Refactor password input out of crypto dev modules into
cryptodisk
cryptodisk: Move global variables into grub_cryptomount_args struct
cryptodisk: Improve handling of partition name in cryptomount password
prompt
docs/grub.texi | 9 +-
grub-core/disk/cryptodisk.c | 164 +++++++++++++++++++++++++-----------
grub-core/disk/geli.c | 35 +++-----
grub-core/disk/luks.c | 37 +++-----
grub-core/disk/luks2.c | 38 ++++-----
include/grub/cryptodisk.h | 19 ++++-
6 files changed, 175 insertions(+), 127 deletions(-)
Range-diff against v4:
1: 0ae554743 < -: --------- cryptodisk: Refactor to discard have_it global
-: --------- > 1: 5056163ca cryptodisk: Refactor to discard have_it global
-: --------- > 2: 224d7f9bc cryptodisk: Return failure in cryptomount when no
cryptodisk modules are loaded
2: d5040065e ! 3: a0dfaeb5a cryptodisk: Improve error messaging in
cryptomount invocations
@@ Commit message
when an invalid passphrase is given and the most relevant error message
will be displayed.
- Improve error message which is displayed when a UUID is specified, but
no
- cryptodisk backends find a disk with that UUID.
-
- Also, make cryptomount return failure when no cryptodisk modules are
loaded,
- which allows an error to be displayed notifying the user that they'll
want
- to load a backend module to make cryptomount useful.
-
## grub-core/disk/cryptodisk.c ##
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device (const char
*name,
+ if (grub_errno == GRUB_ERR_BAD_MODULE)
+ grub_error_pop ();
- grub_disk_close (source);
-
-- /*
-- * Do not print error when err is GRUB_ERR_BAD_MODULE to avoid many
unhelpful
-- * error messages.
-- */
-- if (err != GRUB_ERR_NONE && err != GRUB_ERR_EXISTS && err !=
GRUB_ERR_BAD_MODULE)
-+ if (err == GRUB_ERR_NONE || err == GRUB_ERR_EXISTS)
-+ ; /* Success, skip the error handling */
-+ else if (err == GRUB_ERR_BAD_MODULE)
-+ /* Do nothing to avoid printing unhelpful error messages */
-+ grub_errno = GRUB_ERR_NONE;
-+ else if (cargs->search_uuid != NULL)
+- if (grub_errno != GRUB_ERR_NONE)
++ if (search_uuid != NULL)
+ /* Push error onto stack to save for cryptomount */
-+ grub_error_push();
++ grub_error_push ();
+ else
grub_print_error ();
- return (err == GRUB_ERR_NONE && search_uuid != NULL);
- }
-@@ grub-core/disk/cryptodisk.c: 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 (grub_cryptodisk_list == NULL)
-+ return grub_error (GRUB_ERR_BAD_MODULE, "no cryptodisk modules
loaded");
-+
- if (state[0].set)
- {
- int found_uuid;
+ cleanup:
@@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount
(grub_extcmd_context_t ctxt, int argc, char **args)
found_uuid = grub_device_iterate (&grub_cryptodisk_scan_device,
NULL);
search_uuid = NULL;
@@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t
ctxt, i
+ * Try to pop the next error on the stack. If there is not one, then
+ * no device matched the given UUID.
+ */
-+ grub_error_pop();
++ grub_error_pop ();
+ if (grub_errno == GRUB_ERR_NONE)
-+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk
found, perhaps a needed disk or cryptodisk module is not loaded");
++ return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk
found");
+ }
+ return grub_errno;
}
-: --------- > 4: 53a2d29b4 cryptodisk: Improve cryptomount -u error message
3: 4e7a9f135 ! 5: 9ab7601bd cryptodisk: Add infrastructure to pass data from
cryptomount to cryptodisk modules
@@ grub-core/disk/cryptodisk.c: static const struct grub_arg_option
options[] =
@@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev)
}
- static grub_err_t
+ static grub_cryptodisk_t
-grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
+grub_cryptodisk_scan_device_real (const char *name,
+ grub_disk_t source,
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_cheat_mount (const char
*sourcedev,
- void *data __attribute__ ((unused)))
+ void *data)
{
- grub_err_t err;
+ int ret = 0;
grub_disk_t source;
+ grub_cryptodisk_t dev;
+ grub_cryptomount_args_t cargs = data;
+ grub_errno = GRUB_ERR_NONE;
/* Try to open disk. */
- source = grub_disk_open (name);
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device (const char
*name,
return 0;
}
-- err = grub_cryptodisk_scan_device_real (name, source);
-+ err = grub_cryptodisk_scan_device_real (name, source, cargs);
-
- grub_disk_close (source);
-
+- dev = grub_cryptodisk_scan_device_real (name, source);
++ dev = grub_cryptodisk_scan_device_real (name, source, cargs);
+ if (dev)
+ {
+ ret = (search_uuid != NULL && grub_strcasecmp (search_uuid,
dev->uuid) == 0);
@@ grub-core/disk/cryptodisk.c: static grub_err_t
grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
{
@@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t
ctxt, i
return GRUB_ERR_NONE;
}
-- err = grub_cryptodisk_scan_device_real (diskname, disk);
-+ err = grub_cryptodisk_scan_device_real (diskname, disk, &cargs);
+- dev = grub_cryptodisk_scan_device_real (diskname, disk);
++ dev = grub_cryptodisk_scan_device_real (diskname, disk, &cargs);
grub_disk_close (disk);
if (disklast)
4: 4149dcb56 ! 6: 9db80950c cryptodisk: Refactor password input out of crypto
dev modules into cryptodisk
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
dev = grub_cryptodisk_get_by_source_disk (source);
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
- return grub_errno;
+ return NULL;
if (!dev)
continue;
-
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
- if (err)
- {
- cryptodisk_close (dev);
-- return err;
+- return NULL;
- }
+
+ if (!cargs->key_len)
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
+
+ cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
+ if (cargs->key_data == NULL)
-+ return grub_errno;
++ return NULL;
+
+ if (!grub_password_get ((char *) cargs->key_data,
GRUB_CRYPTODISK_MAX_PASSPHRASE))
+ {
-+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
++ grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
+ goto error;
+ }
+ cargs->key_len = grub_strlen ((char *) cargs->key_data);
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
grub_cryptodisk_insert (dev, name, source);
-- return GRUB_ERR_NONE;
+- return dev;
+ goto cleanup;
}
-- return grub_error (GRUB_ERR_BAD_MODULE, "no cryptodisk module can
handle this device");
-+ ret = grub_error (GRUB_ERR_BAD_MODULE, "no cryptodisk module can handle
this device");
+-
+ grub_error (GRUB_ERR_BAD_MODULE, "no cryptodisk module can handle this
device");
+- return NULL;
+ goto cleanup;
+
+ error:
+ cryptodisk_close (dev);
++ dev = NULL;
+
+ cleanup:
+ if (askpass)
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
+ cargs->key_len = 0;
+ grub_free (cargs->key_data);
+ }
-+ return ret;
++ return dev;
}
#ifdef GRUB_UTIL
5: d0cfd1b18 ! 7: 33385f215 cryptodisk: Move global variables into
grub_cryptomount_args struct
@@ grub-core/disk/cryptodisk.c: grub_util_cryptodisk_get_uuid (grub_disk_t
disk)
{
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
- if (dev)
- {
-- if (grub_strcasecmp (search_uuid, dev->uuid) == 0)
-+ if (grub_strcasecmp (cargs->search_uuid, dev->uuid) == 0)
- return GRUB_ERR_NONE;
- else
- return GRUB_ERR_EXISTS;
-@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
-
FOR_CRYPTODISK_DEVS (cr)
{
- dev = cr->scan (source, search_uuid, check_boot);
+ dev = cr->scan (source, cargs);
if (grub_errno)
- return grub_errno;
+ return NULL;
if (!dev)
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_cheat_mount (const char
*sourcedev, const char *cheat)
grub_cryptodisk_t dev;
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_cheat_mount (const char
*sourcedev,
return grub_errno;
if (!dev)
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device (const char
*name,
- grub_error_push();
- else
- grub_print_error ();
-- return (err == GRUB_ERR_NONE && search_uuid != NULL);
-+ return (err == GRUB_ERR_NONE && cargs->search_uuid != NULL);
- }
+ dev = grub_cryptodisk_scan_device_real (name, source, cargs);
+ if (dev)
+ {
+- ret = (search_uuid != NULL && grub_strcasecmp (search_uuid,
dev->uuid) == 0);
++ ret = (cargs->search_uuid != NULL && grub_strcasecmp
(cargs->search_uuid, dev->uuid) == 0);
+ goto cleanup;
+ }
- static grub_err_t
+@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device (const char
*name,
+ if (grub_errno == GRUB_ERR_BAD_MODULE)
+ grub_error_pop ();
+
+- if (search_uuid != NULL)
++ if (cargs->search_uuid != NULL)
+ /* Push error onto stack to save for cryptomount */
+ grub_error_push ();
+ else
@@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount
(grub_extcmd_context_t ctxt, int argc, char **args)
return GRUB_ERR_NONE;
}
6: 1e6641f0f = 8: 6835aa866 cryptodisk: Improve handling of partition name in
cryptomount password prompt
--
2.27.0
- [PATCH v5 0/9] Refactor/improve cryptomount data passing to crypto modules,
Glenn Washburn <=
- [PATCH v5 1/9] luks2: Add debug message to align with luks and geli modules, Glenn Washburn, 2021/12/09
- [PATCH v5 2/9] cryptodisk: Refactor to discard have_it global, Glenn Washburn, 2021/12/09
- [PATCH v5 4/9] cryptodisk: Improve error messaging in cryptomount invocations, Glenn Washburn, 2021/12/09
- [PATCH v5 3/9] cryptodisk: Return failure in cryptomount when no cryptodisk modules are loaded, Glenn Washburn, 2021/12/09
- [PATCH v5 5/9] cryptodisk: Improve cryptomount -u error message, Glenn Washburn, 2021/12/09