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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel