[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v3 0/4] Refactor/improve cryptomount data passing to crypto modul
From: |
Glenn Washburn |
Subject: |
[PATCH v3 0/4] Refactor/improve cryptomount data passing to crypto modules |
Date: |
Tue, 12 Oct 2021 18:26:25 -0500 |
Updates since v2:
* Add space after function name in function calls
* Add comments for members of struct grub_cryptomount_args
* Correct commit message for patch #4
---
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).
The infrastructure is implemented in patch #1 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 #2 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 #3 gets rid of some long time globals in cryptodisk, moving them
into the passed struct.
Patch #4 removes the found_uuid flag from the cargs struct, which is not
needed because the same information can be obtained from the return value
of grub_device_iterate.
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 (4):
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: Remove unneeded found_uuid from cryptomount args
grub-core/disk/cryptodisk.c | 108 ++++++++++++++++++++++++------------
grub-core/disk/geli.c | 35 ++++--------
grub-core/disk/luks.c | 37 ++++--------
grub-core/disk/luks2.c | 33 ++++-------
include/grub/cryptodisk.h | 19 ++++++-
5 files changed, 120 insertions(+), 112 deletions(-)
Range-diff against v2:
1: 475bf7e9e ! 1: 83112518f cryptodisk: Add infrastructure to pass data from
cryptomount to cryptodisk modules
@@ grub-core/disk/cryptodisk.c: static grub_err_t
+ if (state[3].set) /* password */
+ {
+ cargs.key_data = (grub_uint8_t *) state[3].arg;
-+ cargs.key_len = grub_strlen(state[3].arg);
++ cargs.key_len = grub_strlen (state[3].arg);
+ }
+
have_it = 0;
2: a0c0694fc ! 2: 65a18c5e8 cryptodisk: Refactor password input out of crypto
dev modules into cryptodisk
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
+ goto error;
+ }
-+ cargs->key_len = grub_strlen((char *) cargs->key_data);
++ cargs->key_len = grub_strlen ((char *) cargs->key_data);
+ }
+
+ ret = cr->recover_key (source, dev, cargs);
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
+ if (askpass)
+ {
+ cargs->key_len = 0;
-+ grub_free(cargs->key_data);
++ grub_free (cargs->key_data);
+ }
+ return ret;
}
3: 419f114d8 ! 3: fed38b3dc cryptodisk: Move global variables into
grub_cryptomount_args struct
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device (const char
*name,
static grub_err_t
@@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount
(grub_extcmd_context_t ctxt, int argc, char **args)
- cargs.key_len = grub_strlen(state[3].arg);
+ cargs.key_len = grub_strlen (state[3].arg);
}
- have_it = 0;
@@ include/grub/cryptodisk.h: typedef gcry_err_code_t
struct grub_cryptomount_args
{
++ /* scan: Flag to indicate that only bootable volumes should be
decrypted */
+ grub_uint32_t check_boot : 1;
+ grub_uint32_t found_uuid : 1;
++ /* scan: Only volumes matching this UUID should be decrpyted */
+ char *search_uuid;
++ /* recover_key: Key data used to decrypt voume */
grub_uint8_t *key_data;
++ /* recover_key: Length of key_data */
grub_size_t key_len;
};
+ typedef struct grub_cryptomount_args *grub_cryptomount_args_t;
@@ include/grub/cryptodisk.h: struct grub_cryptodisk_dev
struct grub_cryptodisk_dev *next;
struct grub_cryptodisk_dev **prev;
4: e6e1e8bc3 ! 4: 854709929 cryptodisk: Remove unneeded found_uuid from
cryptomount args
@@ Commit message
iff grub_cryptodisk_scan_device returns 1. And
grub_cryptodisk_scan_device
will only return 1 if a search_uuid has been specified and a
cryptodisk was
successfully setup by a crypto-backend. So the return value of
- grub_cryptodisk_scan_device is equivalent to found_uuid.
+ grub_cryptodisk_scan_device is almost equivalent to found_uuid, with
the
+ exception of the case where a mount is requested or an already opened
+ crypto device.
+
+ With this change grub_device_iterate will return 1 when
+ grub_cryptodisk_scan_device when a crypto device is successfully
decrypted
+ or when the source device has already been successfully opened. Prior
to
+ this change, trying to mount an already successfully opened device
would
+ trigger an error with the message "no such cryptodisk found", which is
at
+ best misleading. The mount should silently succeed in this case, which
is
+ what happens with this patch.
## grub-core/disk/cryptodisk.c ##
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const
char *name,
@@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t
ctxt, i
}
## include/grub/cryptodisk.h ##
-@@ include/grub/cryptodisk.h: typedef gcry_err_code_t
- struct grub_cryptomount_args
+@@ include/grub/cryptodisk.h: struct grub_cryptomount_args
{
+ /* scan: Flag to indicate that only bootable volumes should be
decrypted */
grub_uint32_t check_boot : 1;
- grub_uint32_t found_uuid : 1;
+ /* scan: Only volumes matching this UUID should be decrpyted */
char *search_uuid;
- grub_uint8_t *key_data;
- grub_size_t key_len;
+ /* recover_key: Key data used to decrypt voume */
--
2.27.0
- [PATCH v3 0/4] Refactor/improve cryptomount data passing to crypto modules,
Glenn Washburn <=